Patchwork [1/4] Make initializing PS2 keyboard optional.

login
register
about
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

Kevin O'Connor - 2010-09-06 23:23:42
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(-)
Patrick Georgi - 2010-09-07 06:55:58
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
Kevin O'Connor - 2010-09-08 00:25:34
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
ron minnich - 2010-09-08 00:34:38
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
Patrick Georgi - 2010-09-08 19:44:01
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
Kevin O'Connor - 2010-09-09 00:43:39
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
Patrick Georgi - 2010-09-09 08:34:34
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 */