Sven Schnelle svens@stackframe.org writes:
Hi Setfan,
Stefan Reinauer stefan.reinauer@coreboot.org writes:
- Sven Schnelle svens@stackframe.org [110503 21:41]:
Stefan Reinauer stefan.reinauer@coreboot.org writes:
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).
You can find the boot log here:
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;
On Wed, May 04, 2011 at 10:16:17AM +0200, Sven Schnelle wrote:
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.
[...]
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
Am Samstag, den 07.05.2011, 13:01 -0400 schrieb Kevin O'Connor:
On Wed, May 04, 2011 at 10:16:17AM +0200, Sven Schnelle wrote:
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.
[...]
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.
BTW, if you want to improve boot time, setting CONFIG_THREAD_OPTIONROMS should help.
Thanks,
Paul
Kevin O'Connor kevin@koconnor.net writes:
On Wed, May 04, 2011 at 10:16:17AM +0200, Sven Schnelle wrote:
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.
[...]
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.
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.
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.
Have you considerd using a USB keyboard? :-)
Well - it's a Thinkpad. :-)
BTW, if you want to improve boot time, setting CONFIG_THREAD_OPTIONROMS should help.
I already have that option set. ;)
Sven.
On Sat, May 07, 2011 at 08:48:45PM +0200, Sven Schnelle wrote:
Kevin O'Connor kevin@koconnor.net writes:
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?
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.)
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
On 5/7/11 12:05 PM, Kevin O'Connor wrote:
On Sat, May 07, 2011 at 08:48:45PM +0200, Sven Schnelle wrote:
Kevin O'Connorkevin@koconnor.net writes:
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?
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
On Sat, May 07, 2011 at 01:19:23PM -0700, Stefan Reinauer wrote:
On 5/7/11 12:05 PM, Kevin O'Connor wrote:
On Sat, May 07, 2011 at 08:48:45PM +0200, Sven Schnelle wrote:
Kevin O'Connorkevin@koconnor.net writes:
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
On Sat, May 07, 2011 at 08:48:45PM +0200, Sven Schnelle wrote:
Kevin O'Connor kevin@koconnor.net writes:
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
Hi Kevin,
Kevin O'Connor kevin@koconnor.net writes:
On Sat, May 07, 2011 at 08:48:45PM +0200, Sven Schnelle wrote:
Kevin O'Connor kevin@koconnor.net writes:
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.
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 svens@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) {
On Sat, May 28, 2011 at 04:23:35PM +0200, Sven Schnelle wrote:
Kevin O'Connor kevin@koconnor.net writes:
On Sat, May 07, 2011 at 08:48:45PM +0200, Sven Schnelle wrote:
Kevin O'Connor kevin@koconnor.net writes:
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):
[...]
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.
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
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?
On Sat, May 28, 2011 at 7:23 AM, Sven Schnelle svens@stackframe.org wrote:
Hi Kevin,
Kevin O'Connor kevin@koconnor.net writes:
On Sat, May 07, 2011 at 08:48:45PM +0200, Sven Schnelle wrote:
Kevin O'Connor kevin@koconnor.net writes:
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.
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 svens@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 PS2PORTint "Reset timeout (in ms)"default 0helpThis option specifies how long we should wait for theKeyboard.
Some keyboards are requiring a delay after POR forinitialization.
- 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) {
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
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?
On Sat, May 28, 2011 at 7:23 AM, Sven Schnelle svens@stackframe.org wrote:
Hi Kevin,
Kevin O'Connor kevin@koconnor.net writes:
On Sat, May 07, 2011 at 08:48:45PM +0200, Sven Schnelle wrote:
Kevin O'Connor kevin@koconnor.net writes:
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.
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 svens@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 PS2PORTint "Reset timeout (in ms)"default 0helpThis option specifies how long we should wait for theKeyboard.
Some keyboards are requiring a delay after POR forinitialization.
- 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) {
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
-
Josh Stump -
Kevin O'Connor -
Paul Menzel -
Stefan Reinauer -
Sven Schnelle