[yocto] [psplash][PATCH] Add fbdev option to set the proper /dev/fbX.
Richard Leitner
richard.leitner at skidata.com
Wed May 25 01:00:20 PDT 2016
Hi Julien,
comments on the code are below.
On 05/22/2016 07:46 PM, Julien Gueytat wrote:
> It works exactly the same way than the angle parameter:
> * --angle <-> /etc/rotation file
> * --fbdev <-> /etc/fbdev file
>
> Signed-off-by: Julien Gueytat <contact at jgueytat.fr>
> ---
> psplash-fb.c | 16 ++++++++++------
> psplash-fb.h | 4 ++--
> psplash.c | 57 ++++++++++++++++++++++++++++++++-------------------------
> 3 files changed, 44 insertions(+), 33 deletions(-)
>
> diff --git a/psplash-fb.c b/psplash-fb.c
> index 8daaf6f..d344e5a 100644
> --- a/psplash-fb.c
> +++ b/psplash-fb.c
> @@ -99,18 +99,20 @@ attempt_to_change_pixel_format (PSplashFB *fb,
> }
>
> PSplashFB*
> -psplash_fb_new (int angle)
> +psplash_fb_new (int angle, int fbdev_id)
> {
> struct fb_var_screeninfo fb_var;
> struct fb_fix_screeninfo fb_fix;
> int off;
> - char *fbdev;
> + char fbdev[9] = "/dev/fb0";
>
> PSplashFB *fb = NULL;
>
> - fbdev = getenv("FBDEV");
> - if (fbdev == NULL)
> - fbdev = "/dev/fb0";
> + if (fbdev_id > 0 && fbdev_id < 10)
> + {
> + // Conversion from integer to ascii.
> + fbdev[7] = fbdev_id + 48;
> + }
You removed the possiblity to get the fb device from FBDEV!
Please don't to that as people may rely on that feature!
If you like to add a fbdev commandline option make sure it can co-exist
with the already available methods for setting this option.
And don't forget to add documentation!
>
> if ((fb = malloc (sizeof(PSplashFB))) == NULL)
> {
...
> @@ -205,35 +205,41 @@ int
> main (int argc, char** argv)
> {
> char *tmpdir;
> - int pipe_fd, i = 0, angle = 0, ret = 0;
> + int pipe_fd, i = 0, angle = 0, fbdev_id = 0, ret = 0;
> PSplashFB *fb;
> bool disable_console_switch = FALSE;
> -
> +
> signal(SIGHUP, psplash_exit);
> signal(SIGINT, psplash_exit);
> signal(SIGQUIT, psplash_exit);
>
> - while (++i < argc)
> - {
> - if (!strcmp(argv[i],"-n") || !strcmp(argv[i],"--no-console-switch"))
> - {
> - disable_console_switch = TRUE;
> - continue;
> - }
> + while (++i < argc) {
> + if (!strcmp(argv[i],"-n") || !strcmp(argv[i],"--no-console-switch"))
> + {
> + disable_console_switch = TRUE;
> + continue;
> + }
> +
> + if (!strcmp(argv[i],"-a") || !strcmp(argv[i],"--angle"))
> + {
> + if (++i >= argc) goto fail;
> + angle = atoi(argv[i]);
> + continue;
> + }
> +
> + if (!strcmp(argv[i],"-f") || !strcmp(argv[i],"--fbdev"))
> + {
> + if (++i >= argc) goto fail;
> + fbdev_id = atoi(argv[i]);
> + continue;
> + }
>
> - if (!strcmp(argv[i],"-a") || !strcmp(argv[i],"--angle"))
> - {
> - if (++i >= argc) goto fail;
> - angle = atoi(argv[i]);
> - continue;
> - }
> -
> fail:
> fprintf(stderr,
> - "Usage: %s [-n|--no-console-switch][-a|--angle <0|90|180|270>]\n",
> - argv[0]);
> + "Usage: %s [-n|--no-console-switch][-a|--angle <0|90|180|270>][-f|--fbdev <0..9>]\n",
> + argv[0]);
> exit(-1);
> - }
> + }
Please use the same coding style everywhere in psplash.
Furthermore please avoid/remove trailing whitespaces in new/changed code
>
> tmpdir = getenv("TMPDIR");
>
> @@ -245,10 +251,10 @@ main (int argc, char** argv)
> if (mkfifo(PSPLASH_FIFO, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP))
> {
> if (errno!=EEXIST)
> - {
> - perror("mkfifo");
> - exit(-1);
> - }
> + {
> + perror("mkfifo");
> + exit(-1);
> + }
> }
This change is irrelevant for new fbdev option.
Please send this whitespace fix as a separate patch.
>
> pipe_fd = open (PSPLASH_FIFO,O_RDONLY|O_NONBLOCK);
> @@ -262,10 +268,11 @@ main (int argc, char** argv)
> if (!disable_console_switch)
> psplash_console_switch ();
>
> - if ((fb = psplash_fb_new(angle)) == NULL) {
> + if ((fb = psplash_fb_new(angle,fbdev_id)) == NULL)
> + {
> ret = -1;
> goto fb_fail;
> - }
> + }
>
> /* Clear the background with #ecece1 */
> psplash_fb_draw_rect (fb, 0, 0, fb->width, fb->height,
>
IMHO in this case fixing the whitespace is fine because the if clause
needs to be adopted for the new option.
More information about the yocto
mailing list