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