Hi Bin, On 8 May 2015 at 22:26, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Sat, May 9, 2015 at 5:44 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Bin, >> >> On 6 May 2015 at 03:34, Bin Meng <bmeng...@gmail.com> wrote: >>> There are two places in the cfb_console driver that test whether >>> CONFIG_VGA_AS_SINGLE_DEVICE is defined or not, but actually it is >>> unnecessary, hence clean it up. >>> >>> Signed-off-by: Bin Meng <bmeng...@gmail.com> >>> --- >>> >>> drivers/video/cfb_console.c | 4 ---- >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c >>> index f4231b8..fcaaa7f 100644 >>> --- a/drivers/video/cfb_console.c >>> +++ b/drivers/video/cfb_console.c >>> @@ -2274,9 +2274,7 @@ int drv_video_init(void) >>> #endif >>> if (have_keyboard) { >>> debug("KBD: Keyboard init ...\n"); >>> -#if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) >>> skip_dev_init |= (VIDEO_KBD_INIT_FCT == -1); >>> -#endif >>> } >>> if (skip_dev_init) >>> return 0; >>> @@ -2289,14 +2287,12 @@ int drv_video_init(void) >>> console_dev.putc = video_putc; /* 'putc' function */ >>> console_dev.puts = video_puts; /* 'puts' function */ >>> >>> -#if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) >>> if (have_keyboard) { >>> /* Also init console device */ >>> console_dev.flags |= DEV_FLAGS_INPUT; >>> console_dev.tstc = VIDEO_TSTC_FCT; /* 'tstc' function >>> */ >>> console_dev.getc = VIDEO_GETC_FCT; /* 'getc' function >>> */ >>> } >>> -#endif >>> >>> if (stdio_register(&console_dev) != 0) >>> return 0; >>> -- >>> 1.8.2.1 >>> >> >> This is needed by some boards, e.g. (just a sample): >> >> 02: video: cfb_console: Remove the unnecessary CONFIG_VGA_AS_SINGLE_DEVICE >> wraps >> arm: + wandboard_quad wandboard_dl wandboard_solo cm_fx6 >> +../drivers/video/cfb_console.c: In function ‘drv_video_init’: >> +../drivers/video/cfb_console.c:2277:21: error: ‘VIDEO_KBD_INIT_FCT’ >> undeclared (first use in this function) >> +../drivers/video/cfb_console.c:2277:21: note: each undeclared >> identifier is reported only once for each function it appears in >> +../drivers/video/cfb_console.c:2293:22: error: ‘VIDEO_TSTC_FCT’ >> undeclared (first use in this function) >> +../drivers/video/cfb_console.c:2294:22: error: ‘VIDEO_GETC_FCT’ >> undeclared (first use in this function) >> +make[3]: *** [drivers/video/cfb_console.o] Error 1 >> +make[2]: *** [drivers/video] Error 2 >> +make[1]: *** [drivers] Error 2 >> +make: *** [sub-make] Error 2 >> > > Sorry, I failed to check this CONFIG_VGA_AS_SINGLE_DEVICE is actually > defined by so many boards. But I still see the logic is not straight > forward, due to the variable 'have_keyboard' is equal to > '!defined(CONFIG_VGA_AS_SINGLE_DEVICE)'. Can we do it like this:
Well the presence of a keyboard is also controlled by device tree. If this moved to Kconfig then we could use if() instead of #ifdef which would be better. > > diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c > index f4231b8..640f0a8 100644 > --- a/drivers/video/cfb_console.c > +++ b/drivers/video/cfb_console.c > @@ -2252,7 +2252,6 @@ int drv_video_init(void) > { > int skip_dev_init; > struct stdio_dev console_dev; > - bool have_keyboard; > > /* Check if video initialization should be skipped */ > if (board_video_skip()) > @@ -2264,20 +2263,10 @@ int drv_video_init(void) > if (board_cfb_skip()) > return 0; > > -#if defined(CONFIG_VGA_AS_SINGLE_DEVICE) > - have_keyboard = false; > -#elif defined(CONFIG_OF_CONTROL) > - have_keyboard = !fdtdec_get_config_bool(gd->fdt_blob, > - "u-boot,no-keyboard"); > -#else > - have_keyboard = true; > -#endif > - if (have_keyboard) { > - debug("KBD: Keyboard init ...\n"); > #if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) > - skip_dev_init |= (VIDEO_KBD_INIT_FCT == -1); > + debug("KBD: Keyboard init ...\n"); > + skip_dev_init |= (VIDEO_KBD_INIT_FCT == -1); > #endif > - } > if (skip_dev_init) > return 0; > > @@ -2290,12 +2279,10 @@ int drv_video_init(void) > console_dev.puts = video_puts; /* 'puts' function */ > > #if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) > - if (have_keyboard) { > - /* Also init console device */ > - console_dev.flags |= DEV_FLAGS_INPUT; > - console_dev.tstc = VIDEO_TSTC_FCT; /* 'tstc' function */ > - console_dev.getc = VIDEO_GETC_FCT; /* 'getc' function */ > - } > + /* Also init console device */ > + console_dev.flags |= DEV_FLAGS_INPUT; > + console_dev.tstc = VIDEO_TSTC_FCT; /* 'tstc' function */ > + console_dev.getc = VIDEO_GETC_FCT; /* 'getc' function */ > #endif > > if (stdio_register(&console_dev) != 0) > > > Also I checked include/configs/nokia_rx51.h, it has the following 3 > defined but not actually hooked into the cfb_console.c > > #define VIDEO_KBD_INIT_FCT rx51_kp_init() > #define VIDEO_TSTC_FCT rx51_kp_tstc > #define VIDEO_GETC_FCT rx51_kp_getc > > I believe this is broken? i don't have this board but it does seem to build OK and these functions are in the image. What is broken? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot