Discussion:
[SeaBIOS] [coreboot] Keyboard not working on Thinkpad X60/T60
Sven Schnelle
2011-05-04 08:16:17 UTC
Permalink
Hi Setfan,
Can you do a new analysis on where the boot time goes now? It would be
nice to see if there are more optimizations we can do...
Will do. But right now i have the problem that the Keyboard isn't
working on cold boot - seabios is probably started so early that some
hardware parts are not finished with reset or similar things.
Just enabling debug output in coreboot slows down things enough to
make the Keyboard working again.
Does just putting in a delay of some 100ms fix the issue, too? Do you do
keyboard init in coreboot? Did you do it before?
Just want to make sure there are no side effects coming in through
debugging. However, having an EC/SuperIO that needs more than 200ms to boot up
does not sound too unlikely.
I do not initialize the Keyboard in coreboot, i'll leave that to
seabios. (Enabling it in coreboot doesn't help either).
The original Vendor BIOS talks after around ~1s to the Keyboard
controller, so that's quite different to coreboot (coreboot is handing
over to seabios after ~200ms)
Getting through all of coreboot in as little as 200ms? This is totally
awesome!
So i want to figure out first if there's some
'i-finished-reset-you-can-talk-to-me' flag, or if that problem is caused
by another reason.
Does the keyboard init code get any type of timeout?
Well, i've enabled some debugging in seabios, and it's pretty obvious
what's happening here. SeaBIOS sends command 0xff (which is RESET i
think), and SeabIOS gets 0xfe as response (which is RESEND, but seabios
handles that as NAK, and doesn't resend the command).
http://stackframe.org/seriallog-20110503_175245.log
I've just modified seabios to resend commands when 0xfe is received as a
quick hack. It makes my keyboard working again. I'm not sure if SeabIOS
should handle 0xfe as RESEND or not - have not monitored much Keyboards,
and don't know wether this has any side effects.

The boot log can be found here:
http://stackframe.org/seriallog-20110504_100837.log

The diff i've made to seabios is: (beware, it's just an ugly hack just for
testing)

diff --git a/src/ps2port.c b/src/ps2port.c
index d1e6d48..a4cd4de 100644
--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -186,7 +186,8 @@ ps2_recvbyte(int aux, int needack, int timeout)
static int
ps2_sendbyte(int aux, u8 command, int timeout)
{
- dprintf(7, "ps2_sendbyte aux=%d cmd=%x\n", aux, command);
+resend:
+ dprintf(7, "ps2_sendbyte aux=%d cmd=%x\n", aux, command);
int ret;
if (aux)
ret = i8042_aux_write(command);
@@ -199,6 +200,8 @@ ps2_sendbyte(int aux, u8 command, int timeout)
ret = ps2_recvbyte(aux, 1, timeout);
if (ret < 0)
return ret;
+ if (ret == 0xfe)
+ goto resend;
if (ret != PS2_RET_ACK)
return -1;

@@ -232,7 +235,7 @@ __ps2_command(int aux, int command, u8 *param)
ret = i8042_command(I8042_CMD_CTL_WCTR, &newctr);
if (ret)
goto fail;
-
+resend:
if (command == ATKBD_CMD_RESET_BAT) {
// Reset is special wrt timeouts and bytes received.

@@ -243,10 +246,14 @@ __ps2_command(int aux, int command, u8 *param)

// Receive parameters.
ret = ps2_recvbyte(aux, 0, 4000);
+ if (ret == 0xfe)
+ goto resend;
if (ret < 0)
goto fail;
param[0] = ret;
ret = ps2_recvbyte(aux, 0, 100);
+ if (ret == 0xfe)
+ goto resend;
if (ret < 0)
// Some devices only respond with one byte on reset.
ret = 0;
@@ -261,6 +268,8 @@ __ps2_command(int aux, int command, u8 *param)

// Receive parameters.
ret = ps2_recvbyte(aux, 0, 500);
+ if (ret == 0xfe)
+ goto resend;
if (ret < 0)
goto fail;
param[0] = ret;
@@ -268,6 +277,8 @@ __ps2_command(int aux, int command, u8 *param)
|| ret == 0x60 || ret == 0x47) {
// These ids (keyboards) return two bytes.
ret = ps2_recvbyte(aux, 0, 500);
+ if (ret == 0xfe)
+ goto resend;
if (ret < 0)
goto fail;
param[1] = ret;
@@ -291,6 +302,8 @@ __ps2_command(int aux, int command, u8 *param)
// Receive parameters (if any).
for (i = 0; i < receive; i++) {
ret = ps2_recvbyte(aux, 0, 500);
+ if (ret == 0xfe)
+ goto resend;
if (ret < 0)
goto fail;
param[i] = ret;
Kevin O'Connor
2011-05-07 17:01:59 UTC
Permalink
Post by Sven Schnelle
Will do. But right now i have the problem that the Keyboard isn't
working on cold boot - seabios is probably started so early that some
hardware parts are not finished with reset or similar things.
[...]
Post by Sven Schnelle
I've just modified seabios to resend commands when 0xfe is received as a
quick hack. It makes my keyboard working again. I'm not sure if SeabIOS
should handle 0xfe as RESEND or not - have not monitored much Keyboards,
and don't know wether this has any side effects.
Some ps2 ports send NAK (0xfe) when there is no keyboard plugged in.
The detection for NAK was added so that it doesn't take a full second
to recognize that no keyboard is present. The patch you sent would
loop infinitely in this situation.

It's possible to have ps2_send_byte() attempt a resend on each NAK and
use the timer to detect when to stop. However, that will re-introduce
the problem of having to wait a second when there is no keyboard
present.

Do you know how long it takes for your keyboard to become responsive?
(You can pass "-n" to tools/readserial.py to have it report wall times
instead of adjusted times.)

Have you considerd using a USB keyboard? :-)

BTW, if you want to improve boot time, setting
CONFIG_THREAD_OPTIONROMS should help.

-Kevin
Sven Schnelle
2011-05-07 18:48:45 UTC
Permalink
Post by Kevin O'Connor
Post by Sven Schnelle
Will do. But right now i have the problem that the Keyboard isn't
working on cold boot - seabios is probably started so early that some
hardware parts are not finished with reset or similar things.
[...]
Post by Sven Schnelle
I've just modified seabios to resend commands when 0xfe is received as a
quick hack. It makes my keyboard working again. I'm not sure if SeabIOS
should handle 0xfe as RESEND or not - have not monitored much Keyboards,
and don't know wether this has any side effects.
Some ps2 ports send NAK (0xfe) when there is no keyboard plugged in.
The detection for NAK was added so that it doesn't take a full second
to recognize that no keyboard is present. The patch you sent would
loop infinitely in this situation.
Yes, the patch was only for testing purposes. :)

I feared that are controllers out there that behave like this, so i have
to find another solution.
Post by Kevin O'Connor
It's possible to have ps2_send_byte() attempt a resend on each NAK and
use the timer to detect when to stop. However, that will re-introduce
the problem of having to wait a second when there is no keyboard
present.
Yes.
Post by Kevin O'Connor
Do you know how long it takes for your keyboard to become responsive?
(You can pass "-n" to tools/readserial.py to have it report wall times
instead of adjusted times.)
It takes something about 0.8s, according to serial output. I've already
monitored the EC registers, but there seems to be no bit that
can be used as indicator wether EC is ready for Keyboard action or not.
Post by Kevin O'Connor
Have you considerd using a USB keyboard? :-)
Well - it's a Thinkpad. :-)
Post by Kevin O'Connor
BTW, if you want to improve boot time, setting
CONFIG_THREAD_OPTIONROMS should help.
I already have that option set. ;)

Sven.
Kevin O'Connor
2011-05-07 19:05:03 UTC
Permalink
Post by Sven Schnelle
Post by Kevin O'Connor
Do you know how long it takes for your keyboard to become responsive?
(You can pass "-n" to tools/readserial.py to have it report wall times
instead of adjusted times.)
It takes something about 0.8s, according to serial output.
Does adding "msleep(800)" to the top of ps2port.c:keyboard_init()
help?
Post by Sven Schnelle
I've already
monitored the EC registers, but there seems to be no bit that
can be used as indicator wether EC is ready for Keyboard action or not.
The logs you posted show the i8042 controller responsive when SeaBIOS
starts talking to it. So, I wonder if it's the keyboard that is still
powering up (instead of the superIO). (Of course, if the EC contains
both the keyboard and superIO, then this is irrelevant.)
Post by Sven Schnelle
Post by Kevin O'Connor
BTW, if you want to improve boot time, setting
CONFIG_THREAD_OPTIONROMS should help.
I already have that option set. ;)
Oh, okay. The logs you posted show this option disabled.

When this option is enabled, the "init ps2port" message should appear
before the VGA init. It should also eliminate any wait at the boot
prompt, as the boot menu delay should overlap with your hard drive
spin-up.

-Kevin
Stefan Reinauer
2011-05-07 20:19:23 UTC
Permalink
Post by Kevin O'Connor
Post by Sven Schnelle
Post by Kevin O'Connor
Do you know how long it takes for your keyboard to become responsive?
(You can pass "-n" to tools/readserial.py to have it report wall times
instead of adjusted times.)
It takes something about 0.8s, according to serial output.
Does adding "msleep(800)" to the top of ps2port.c:keyboard_init()
help?
Please don't do things like that. Probing the hardware is much better
than a fixed delay.

The various i8042 reimplementations out there are quite ugly and each in
its specific way.

Maybe we should look at the coreboot mainboard type in order to decide
whether to poll the hardware or drop out on the first 0xfe answer. Or at
least make the behavior configurable in Kconfig?
Post by Kevin O'Connor
The logs you posted show the i8042 controller responsive when SeaBIOS
starts talking to it. So, I wonder if it's the keyboard that is still
powering up (instead of the superIO).
Not unlikely. It might also just be some crappy implementation of EC
firmware.

Stefan
Kevin O'Connor
2011-05-07 22:24:08 UTC
Permalink
Post by Stefan Reinauer
Post by Kevin O'Connor
Post by Sven Schnelle
Post by Kevin O'Connor
Do you know how long it takes for your keyboard to become responsive?
(You can pass "-n" to tools/readserial.py to have it report wall times
instead of adjusted times.)
It takes something about 0.8s, according to serial output.
Does adding "msleep(800)" to the top of ps2port.c:keyboard_init()
help?
Please don't do things like that. Probing the hardware is much
better than a fixed delay.
Heh - I was only asking as a test. There's no way I would add an
800ms delay for all users. :-)

-Kevin
Kevin O'Connor
2011-05-07 19:21:30 UTC
Permalink
Post by Sven Schnelle
Post by Kevin O'Connor
Some ps2 ports send NAK (0xfe) when there is no keyboard plugged in.
The detection for NAK was added so that it doesn't take a full second
to recognize that no keyboard is present. The patch you sent would
loop infinitely in this situation.
Yes, the patch was only for testing purposes. :)
I feared that are controllers out there that behave like this, so i have
to find another solution.
How about something like (untested):

--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -438,9 +438,14 @@ keyboard_init(void *data)

/* ------------------- keyboard side ------------------------*/
/* reset keyboard and self test (keyboard side) */
- ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
- if (ret)
- return;
+ u64 end = calc_future_tsc(4000);
+ for (;;) {
+ ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
+ if (!ret)
+ break;
+ if (check_tsc(end))
+ return;
+ }
if (param[0] != 0xaa) {
dprintf(1, "keyboard self test failed (got %x not 0xaa)\n", param[0]);
return;

If it works, the 4000 could be turned into a config option.

-Kevin
Sven Schnelle
2011-05-28 14:23:35 UTC
Permalink
Hi Kevin,
Post by Sven Schnelle
Post by Sven Schnelle
Post by Kevin O'Connor
Some ps2 ports send NAK (0xfe) when there is no keyboard plugged in.
The detection for NAK was added so that it doesn't take a full second
to recognize that no keyboard is present. The patch you sent would
loop infinitely in this situation.
Yes, the patch was only for testing purposes. :)
I feared that are controllers out there that behave like this, so i have
to find another solution.
--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -438,9 +438,14 @@ keyboard_init(void *data)
/* ------------------- keyboard side ------------------------*/
/* reset keyboard and self test (keyboard side) */
- ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
- if (ret)
- return;
+ u64 end = calc_future_tsc(4000);
+ for (;;) {
+ ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
+ if (!ret)
+ break;
+ if (check_tsc(end))
+ return;
+ }
if (param[0] != 0xaa) {
dprintf(1, "keyboard self test failed (got %x not 0xaa)\n", param[0]);
return;
If it works, the 4000 could be turned into a config option.
Yes, that fixes it. I'm using the patch below right now.

Not sure about the default value, but 0 seems to be safe to not
introduce an additional delay for other users ;)

Signed-off-by: Sven Schnelle <***@stackframe.org>

diff --git a/src/Kconfig b/src/Kconfig
index 123db01..21cef19 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -126,6 +126,14 @@ menu "Hardware support"
help
Support PS2 ports (keyboard and mouse).

+ config PS2_RESET_TIMEOUT
+ depends on PS2PORT
+ int "Reset timeout (in ms)"
+ default 0
+ help
+ This option specifies how long we should wait for the Keyboard.
+ Some keyboards are requiring a delay after POR for initialization.
+
config USB
bool "USB"
default y
diff --git a/src/ps2port.c b/src/ps2port.c
index 81d47c9..3df8a8a 100644
--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -438,7 +438,9 @@ keyboard_init(void *data)

/* ------------------- keyboard side ------------------------*/
/* reset keyboard and self test (keyboard side) */
- ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
+ u64 end = calc_future_tsc(CONFIG_PS2_RESET_TIMEOUT);
+ while ((ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param)) && !check_tsc(end));
+
if (ret)
return;
if (param[0] != 0xaa) {
Kevin O'Connor
2011-05-28 15:04:41 UTC
Permalink
Post by Sven Schnelle
Post by Kevin O'Connor
Post by Sven Schnelle
Post by Kevin O'Connor
Some ps2 ports send NAK (0xfe) when there is no keyboard plugged in.
The detection for NAK was added so that it doesn't take a full second
to recognize that no keyboard is present. The patch you sent would
loop infinitely in this situation.
Yes, the patch was only for testing purposes. :)
I feared that are controllers out there that behave like this, so i have
to find another solution.
[...]
Post by Sven Schnelle
Post by Kevin O'Connor
If it works, the 4000 could be turned into a config option.
Yes, that fixes it. I'm using the patch below right now.
Thanks. I committed a similar patch.
Post by Sven Schnelle
Not sure about the default value, but 0 seems to be safe to not
introduce an additional delay for other users ;)
Yeah - using zero as the default should make this a nop for other
users.

-Kevin
Josh Stump
2011-05-31 17:24:40 UTC
Permalink
I know this is off topic, but how did you get corebios flashed to the T60?
Is there a software based way or must I get hardware in order to do it?
Post by Sven Schnelle
Hi Kevin,
Post by Sven Schnelle
Post by Sven Schnelle
Post by Kevin O'Connor
Some ps2 ports send NAK (0xfe) when there is no keyboard plugged in.
The detection for NAK was added so that it doesn't take a full second
to recognize that no keyboard is present. The patch you sent would
loop infinitely in this situation.
Yes, the patch was only for testing purposes. :)
I feared that are controllers out there that behave like this, so i have
to find another solution.
--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -438,9 +438,14 @@ keyboard_init(void *data)
/* ------------------- keyboard side ------------------------*/
/* reset keyboard and self test (keyboard side) */
- ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
- if (ret)
- return;
+ u64 end = calc_future_tsc(4000);
+ for (;;) {
+ ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
+ if (!ret)
+ break;
+ if (check_tsc(end))
+ return;
+ }
if (param[0] != 0xaa) {
dprintf(1, "keyboard self test failed (got %x not 0xaa)\n",
param[0]);
Post by Sven Schnelle
return;
If it works, the 4000 could be turned into a config option.
Yes, that fixes it. I'm using the patch below right now.
Not sure about the default value, but 0 seems to be safe to not
introduce an additional delay for other users ;)
diff --git a/src/Kconfig b/src/Kconfig
index 123db01..21cef19 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -126,6 +126,14 @@ menu "Hardware support"
help
Support PS2 ports (keyboard and mouse).
+ config PS2_RESET_TIMEOUT
+ depends on PS2PORT
+ int "Reset timeout (in ms)"
+ default 0
+ help
+ This option specifies how long we should wait for the Keyboard.
+ Some keyboards are requiring a delay after POR for initialization.
+
config USB
bool "USB"
default y
diff --git a/src/ps2port.c b/src/ps2port.c
index 81d47c9..3df8a8a 100644
--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -438,7 +438,9 @@ keyboard_init(void *data)
/* ------------------- keyboard side ------------------------*/
/* reset keyboard and self test (keyboard side) */
- ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
+ u64 end = calc_future_tsc(CONFIG_PS2_RESET_TIMEOUT);
+ while ((ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param)) && !check_tsc(end));
+
if (ret)
return;
if (param[0] != 0xaa) {
--
http://www.coreboot.org/mailman/listinfo/coreboot
Josh Stump
2011-05-31 19:05:03 UTC
Permalink
I know this is off topic, but how did you get corebios flashed to the T60?
Is there a software based way or must I get hardware in order to do it?
Post by Sven Schnelle
Hi Kevin,
Post by Sven Schnelle
Post by Sven Schnelle
Post by Kevin O'Connor
Some ps2 ports send NAK (0xfe) when there is no keyboard plugged in.
The detection for NAK was added so that it doesn't take a full second
to recognize that no keyboard is present. The patch you sent would
loop infinitely in this situation.
Yes, the patch was only for testing purposes. :)
I feared that are controllers out there that behave like this, so i have
to find another solution.
--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -438,9 +438,14 @@ keyboard_init(void *data)
/* ------------------- keyboard side ------------------------*/
/* reset keyboard and self test (keyboard side) */
- ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
- if (ret)
- return;
+ u64 end = calc_future_tsc(4000);
+ for (;;) {
+ ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
+ if (!ret)
+ break;
+ if (check_tsc(end))
+ return;
+ }
if (param[0] != 0xaa) {
dprintf(1, "keyboard self test failed (got %x not 0xaa)\n",
param[0]);
Post by Sven Schnelle
return;
If it works, the 4000 could be turned into a config option.
Yes, that fixes it. I'm using the patch below right now.
Not sure about the default value, but 0 seems to be safe to not
introduce an additional delay for other users ;)
diff --git a/src/Kconfig b/src/Kconfig
index 123db01..21cef19 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -126,6 +126,14 @@ menu "Hardware support"
help
Support PS2 ports (keyboard and mouse).
+ config PS2_RESET_TIMEOUT
+ depends on PS2PORT
+ int "Reset timeout (in ms)"
+ default 0
+ help
+ This option specifies how long we should wait for the Keyboard.
+ Some keyboards are requiring a delay after POR for initialization.
+
config USB
bool "USB"
default y
diff --git a/src/ps2port.c b/src/ps2port.c
index 81d47c9..3df8a8a 100644
--- a/src/ps2port.c
+++ b/src/ps2port.c
@@ -438,7 +438,9 @@ keyboard_init(void *data)
/* ------------------- keyboard side ------------------------*/
/* reset keyboard and self test (keyboard side) */
- ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param);
+ u64 end = calc_future_tsc(CONFIG_PS2_RESET_TIMEOUT);
+ while ((ret = ps2_kbd_command(ATKBD_CMD_RESET_BAT, param)) && !check_tsc(end));
+
if (ret)
return;
if (param[0] != 0xaa) {
--
http://www.coreboot.org/mailman/listinfo/coreboot
Paul Menzel
2011-05-07 18:03:25 UTC
Permalink
Post by Kevin O'Connor
Post by Sven Schnelle
Will do. But right now i have the problem that the Keyboard isn't
working on cold boot - seabios is probably started so early that some
hardware parts are not finished with reset or similar things.
[...]
Post by Sven Schnelle
I've just modified seabios to resend commands when 0xfe is received as a
quick hack. It makes my keyboard working again. I'm not sure if SeabIOS
should handle 0xfe as RESEND or not - have not monitored much Keyboards,
and don't know wether this has any side effects.
Some ps2 ports send NAK (0xfe) when there is no keyboard plugged in.
The detection for NAK was added so that it doesn't take a full second
to recognize that no keyboard is present. The patch you sent would
loop infinitely in this situation.
It's possible to have ps2_send_byte() attempt a resend on each NAK and
use the timer to detect when to stop. However, that will re-introduce
the problem of having to wait a second when there is no keyboard
present.
Do you know how long it takes for your keyboard to become responsive?
(You can pass "-n" to tools/readserial.py to have it report wall times
instead of adjusted times.)
Have you considerd using a USB keyboard? :-)
I do not know how to interpret your »:-)«, but to clarify Sven is using
coreboot on his laptop so the non-working keyboard is the build in
keyboard.
Post by Kevin O'Connor
BTW, if you want to improve boot time, setting
CONFIG_THREAD_OPTIONROMS should help.
Thanks,

Paul
Loading...