| Message ID | BLU436-SMTP18898A4378245446E46D763BF790@phx.gbl |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
+Anatolij Hi Bin, On 17 August 2015 at 04:45, Bin Meng <bmeng.cn@gmail.com> wrote: > So far if CONFIG_VGA_AS_SINGLE_DEVICE is not defined, the VGA device > will try to initialize a keyboard device (for x86, it is i8042). But > if i8042 controller initialization fails (eg: there is no keyboard > connected to the PS/2 port), drv_video_init() just simply returns. > This kills the opportunity of using a usb keyboard later with the vga > concole, as the vga initialization part is actually ok, only keyboard console > part fails. Change the code logic to allow this. This is a change in behaviour. It seems OK to me though. In fact this problem annoyed me quite recently. > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > --- > In the long term, we should remove CONFIG_VGA_AS_SINGLE_DEVICE and > treat the cfb_concole an output device only. The keyboard part should > be moved out of cfb_console driver and be as a input device driver > separately. Agreed. Actually we should implement driver model for vga/lcd and input. > > drivers/video/cfb_console.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> > > diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c > index 30e0317..aa7ca86 100644 > --- a/drivers/video/cfb_console.c > +++ b/drivers/video/cfb_console.c > @@ -2247,16 +2247,17 @@ __weak int board_video_skip(void) > > int drv_video_init(void) > { > - int skip_dev_init; > struct stdio_dev console_dev; > bool have_keyboard; > + bool __maybe_unused keyboard_ok = false; > > /* Check if video initialization should be skipped */ > if (board_video_skip()) > return 0; > > /* Init video chip - returns with framebuffer cleared */ > - skip_dev_init = (video_init() == -1); > + if (video_init() == -1) > + return 0; > > if (board_cfb_skip()) > return 0; > @@ -2272,11 +2273,9 @@ int drv_video_init(void) > if (have_keyboard) { > debug("KBD: Keyboard init ...\n"); > #if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) > - skip_dev_init |= (VIDEO_KBD_INIT_FCT == -1); > + keyboard_ok = !(VIDEO_KBD_INIT_FCT == -1); > #endif > } > - if (skip_dev_init) > - return 0; > > /* Init vga device */ > memset(&console_dev, 0, sizeof(console_dev)); > @@ -2287,7 +2286,7 @@ int drv_video_init(void) > console_dev.puts = video_puts; /* 'puts' function */ > > #if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) > - if (have_keyboard) { > + if (have_keyboard && keyboard_ok) { > /* Also init console device */ > console_dev.flags |= DEV_FLAGS_INPUT; > console_dev.tstc = VIDEO_TSTC_FCT; /* 'tstc' function */ > -- > 1.8.2.1 > Regards, Simon
On Tue, Aug 18, 2015 at 10:00 AM, Simon Glass <sjg@chromium.org> wrote: > +Anatolij > > Hi Bin, > > On 17 August 2015 at 04:45, Bin Meng <bmeng.cn@gmail.com> wrote: >> So far if CONFIG_VGA_AS_SINGLE_DEVICE is not defined, the VGA device >> will try to initialize a keyboard device (for x86, it is i8042). But >> if i8042 controller initialization fails (eg: there is no keyboard >> connected to the PS/2 port), drv_video_init() just simply returns. >> This kills the opportunity of using a usb keyboard later with the vga >> concole, as the vga initialization part is actually ok, only keyboard > > console Will fix in v2. > >> part fails. Change the code logic to allow this. > > This is a change in behaviour. It seems OK to me though. In fact this > problem annoyed me quite recently. > >> >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> >> --- >> In the long term, we should remove CONFIG_VGA_AS_SINGLE_DEVICE and >> treat the cfb_concole an output device only. The keyboard part should >> be moved out of cfb_console driver and be as a input device driver >> separately. > > Agreed. Actually we should implement driver model for vga/lcd and input. Yes, I was planning to do. But that's a lot work so I decided not to change it in this series. > >> >> drivers/video/cfb_console.c | 11 +++++------ >> 1 file changed, 5 insertions(+), 6 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > >> >> diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c >> index 30e0317..aa7ca86 100644 >> --- a/drivers/video/cfb_console.c >> +++ b/drivers/video/cfb_console.c >> @@ -2247,16 +2247,17 @@ __weak int board_video_skip(void) >> >> int drv_video_init(void) >> { >> - int skip_dev_init; >> struct stdio_dev console_dev; >> bool have_keyboard; >> + bool __maybe_unused keyboard_ok = false; >> >> /* Check if video initialization should be skipped */ >> if (board_video_skip()) >> return 0; >> >> /* Init video chip - returns with framebuffer cleared */ >> - skip_dev_init = (video_init() == -1); >> + if (video_init() == -1) >> + return 0; >> >> if (board_cfb_skip()) >> return 0; >> @@ -2272,11 +2273,9 @@ int drv_video_init(void) >> if (have_keyboard) { >> debug("KBD: Keyboard init ...\n"); >> #if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) >> - skip_dev_init |= (VIDEO_KBD_INIT_FCT == -1); >> + keyboard_ok = !(VIDEO_KBD_INIT_FCT == -1); >> #endif >> } >> - if (skip_dev_init) >> - return 0; >> >> /* Init vga device */ >> memset(&console_dev, 0, sizeof(console_dev)); >> @@ -2287,7 +2286,7 @@ int drv_video_init(void) >> console_dev.puts = video_puts; /* 'puts' function */ >> >> #if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) >> - if (have_keyboard) { >> + if (have_keyboard && keyboard_ok) { >> /* Also init console device */ >> console_dev.flags |= DEV_FLAGS_INPUT; >> console_dev.tstc = VIDEO_TSTC_FCT; /* 'tstc' function */ >> -- Regards, Bin
diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c index 30e0317..aa7ca86 100644 --- a/drivers/video/cfb_console.c +++ b/drivers/video/cfb_console.c @@ -2247,16 +2247,17 @@ __weak int board_video_skip(void) int drv_video_init(void) { - int skip_dev_init; struct stdio_dev console_dev; bool have_keyboard; + bool __maybe_unused keyboard_ok = false; /* Check if video initialization should be skipped */ if (board_video_skip()) return 0; /* Init video chip - returns with framebuffer cleared */ - skip_dev_init = (video_init() == -1); + if (video_init() == -1) + return 0; if (board_cfb_skip()) return 0; @@ -2272,11 +2273,9 @@ int drv_video_init(void) if (have_keyboard) { debug("KBD: Keyboard init ...\n"); #if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) - skip_dev_init |= (VIDEO_KBD_INIT_FCT == -1); + keyboard_ok = !(VIDEO_KBD_INIT_FCT == -1); #endif } - if (skip_dev_init) - return 0; /* Init vga device */ memset(&console_dev, 0, sizeof(console_dev)); @@ -2287,7 +2286,7 @@ int drv_video_init(void) console_dev.puts = video_puts; /* 'puts' function */ #if !defined(CONFIG_VGA_AS_SINGLE_DEVICE) - if (have_keyboard) { + if (have_keyboard && keyboard_ok) { /* Also init console device */ console_dev.flags |= DEV_FLAGS_INPUT; console_dev.tstc = VIDEO_TSTC_FCT; /* 'tstc' function */
So far if CONFIG_VGA_AS_SINGLE_DEVICE is not defined, the VGA device will try to initialize a keyboard device (for x86, it is i8042). But if i8042 controller initialization fails (eg: there is no keyboard connected to the PS/2 port), drv_video_init() just simply returns. This kills the opportunity of using a usb keyboard later with the vga concole, as the vga initialization part is actually ok, only keyboard part fails. Change the code logic to allow this. Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- In the long term, we should remove CONFIG_VGA_AS_SINGLE_DEVICE and treat the cfb_concole an output device only. The keyboard part should be moved out of cfb_console driver and be as a input device driver separately. drivers/video/cfb_console.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)