[U-Boot] [PATCH 1/4] video: cfb_console: Remove the unnecessary CONFIG_VGA_AS_SINGLE_DEVICE wraps

[Bin Meng]

Hi Simon,

On Sat, May 9, 2015 at 5:44 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 6 May 2015 at 03:34, Bin Meng <bmeng.cn at 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.cn at 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:

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?

Regards,
Bin