| Submitter | Kevin O'Connor |
|---|---|
| Date | 2010-09-06 23:23:42 |
| Message ID | <53241126486840e63685efc05e61ee3b9dac2b9b.1283815188.git.kevin@koconnor.net> |
| Download | mbox | patch |
| Permalink | /patch/1875/ |
| State | Accepted |
| Headers | show |
Comments
Am 07.09.2010 01:23, schrieb Kevin O'Connor: > + Enable this option to initialize PS2 keyboards found connected > + to the PS2 port. Some payloads (eg, filo) require this > + option. Other payloads (eg, SeaBIOS, Linux) do not require If that's only libpayload that's lacking this kind of init, maybe we should reimplement it there, and drop this init altogether? Patrick
On Tue, Sep 07, 2010 at 08:55:58AM +0200, Patrick Georgi wrote: > Am 07.09.2010 01:23, schrieb Kevin O'Connor: > > + Enable this option to initialize PS2 keyboards found connected > > + to the PS2 port. Some payloads (eg, filo) require this > > + option. Other payloads (eg, SeaBIOS, Linux) do not require > If that's only libpayload that's lacking this kind of init, maybe we > should reimplement it there, and drop this init altogether? I think libpayload should be updated. I think adding the option to coreboot is a good intermediate step. It will give folks time to update their payloads. I'm also not sure if plan9 (or other less common payloads) will have the proper support. -Kevin
On Tue, Sep 7, 2010 at 5:25 PM, Kevin O'Connor <kevin@koconnor.net> wrote: > I think libpayload should be updated. I think adding the option to > coreboot is a good intermediate step. It will give folks time to > update their payloads. I'm also not sure if plan9 (or other less > common payloads) will have the proper support. > There's more and more movement in Plan 9 toward ARM, but that said, if this kind of thing has to go into Plan 9 I think we can make it happen. ron
Am 08.09.2010 02:25, schrieb Kevin O'Connor: > I think libpayload should be updated. I think adding the option to > coreboot is a good intermediate step. It will give folks time to > update their payloads. I'm also not sure if plan9 (or other less > common payloads) will have the proper support. Seems like Marc has found the reason in the other thread ("timeout during PS/2 keyboard init") - should we get that fixed, I'd prefer to keep the full init in coreboot unconditionally, even if it's 10-20ms. Could you try with "[PATCH] timeout during PS/2 keyboard init" (http://patchwork.coreboot.org/patch/1891/), if that gives an acceptable boot time for you? (unless I misunderstand and your gripe isn't with the overly long timeout we suffer on some chipsets, but with the time the init takes at all, however tiny) Thanks, Patrick
On Wed, Sep 08, 2010 at 09:44:01PM +0200, Patrick Georgi wrote: > Am 08.09.2010 02:25, schrieb Kevin O'Connor: > > I think libpayload should be updated. I think adding the option to > > coreboot is a good intermediate step. It will give folks time to > > update their payloads. I'm also not sure if plan9 (or other less > > common payloads) will have the proper support. > Seems like Marc has found the reason in the other thread ("timeout > during PS/2 keyboard init") - should we get that fixed, I'd prefer to > keep the full init in coreboot unconditionally, even if it's 10-20ms. > > Could you try with "[PATCH] timeout during PS/2 keyboard init" > (http://patchwork.coreboot.org/patch/1891/), if that gives an acceptable > boot time for you? (unless I misunderstand and your gripe isn't with the > overly long timeout we suffer on some chipsets, but with the time the > init takes at all, however tiny) That patch is separate from the underlying issue. Talking to the PS2 keyboard is just plain slow. Here is my boot log (epia-cn) with no ps2 keyboard attached (Marc's patch doesn't impact this): 00.556: Primary IDE interface enabled 00.556: Secondary IDE interface enabled 00.704: No PS/2 keyboard detected. 00.705: CBFS: Could not find file pci1106,3344.rom Here is with a ps2 keyboard attached and Marc's fix not applied: 00.558: Primary IDE interface enabled 00.558: Secondary IDE interface enabled 01.367: Keyboard controller output buffer result timeout 01.368: CBFS: Could not find file pci1106,3344.rom Here it is with ps2 keyboard attached and Marc's fix applied: 00.559: Primary IDE interface enabled 00.559: Secondary IDE interface enabled 00.966: CBFS: Could not find file pci1106,3344.rom Here it is with CONFIG_DRIVERS_PS2_KEYBOARD set to 0: 00.562: Primary IDE interface enabled 00.563: Secondary IDE interface enabled 00.563: CBFS: Could not find file pci1106,3344.rom It's easy to get timing numbers on your own boards. Just grab the file tools/readserial.py from the seabios repo, and run: readserial.py /dev/ttyS0 . The tool measures data read from the serial port using the host clock; it takes into account the time it takes the target machine to write to the serial port. I've found the numbers to be very reproducible. -Kevin
Am 09.09.2010 02:43, schrieb Kevin O'Connor: > That patch is separate from the underlying issue. Talking to the PS2 > keyboard is just plain slow. Here is my boot log (epia-cn) with no > ps2 keyboard attached (Marc's patch doesn't impact this): I added this patch, but moved your new config option to src/Kconfig.deprecated_options with some comment on when it's going to be gone (once payloads do ps/2 init themselves, and the code can be removed completely) Acked-by: Patrick Georgi <patrick.georgi@coresystems.de> and committed in r5790- Thanks, Patrick
Patch
diff --git a/src/Kconfig b/src/Kconfig index ec3a13b..ebbb14a 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -120,6 +120,20 @@ endmenu menu "Generic Drivers" source src/drivers/Kconfig + +config DRIVERS_PS2_KEYBOARD + bool "PS2 Keyboard init" + default y + help + Enable this option to initialize PS2 keyboards found connected + to the PS2 port. Some payloads (eg, filo) require this + option. Other payloads (eg, SeaBIOS, Linux) do not require + it. Initializing a PS2 keyboard can take several hundred + milliseconds. + + If you know you will only use a payload which does not require + this option, then you can say "n" here to speed up boot time. + Otherwise say "y". endmenu config PCI_BUS_SEGN_BITS diff --git a/src/pc80/keyboard.c b/src/pc80/keyboard.c index dee6279..9dadf0e 100644 --- a/src/pc80/keyboard.c +++ b/src/pc80/keyboard.c @@ -162,6 +162,8 @@ static u8 send_keyboard(u8 command) void pc_keyboard_init(struct pc_keyboard *keyboard) { u8 regval; + if (!CONFIG_DRIVERS_PS2_KEYBOARD) + return; printk(BIOS_DEBUG, "Keyboard init...\n"); /* Run a keyboard controller self-test */
Add a DRIVERS_PS2_KEYBOARD option which controls the PS2 keyboard initialization. Not all payloads require it and some keyboards take a long time to init. Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/Kconfig | 14 ++++++++++++++ src/pc80/keyboard.c | 2 ++ 2 files changed, 16 insertions(+), 0 deletions(-)