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

Bin Meng bmeng.cn at gmail.com
Mon May 11 01:15:48 CEST 2015


Hi Simon,

On Sun, May 10, 2015 at 9:53 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 8 May 2015 at 22:26, Bin Meng <bmeng.cn at gmail.com> wrote:
>> 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:
>
> 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.

OK, I will drop this patch in v2.

>>
>> 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?
>

Sorry I was not clear. I mean these functions are defined elsewhere
than the cfb_console.c while 8042 keyboard routines are defined inside
cfb_console.c. Inconsistent?

#ifdef  CONFIG_I8042_KBD
#include <i8042.h>

#define VIDEO_KBD_INIT_FCT      i8042_kbd_init()
#define VIDEO_TSTC_FCT          i8042_tstc
#define VIDEO_GETC_FCT          i8042_getc
#endif

Seems this rx51 board is the only one that provides keyboard hooks
into the cfb_console driver.

Regards,
Bin


More information about the U-Boot mailing list