Patchwork Keyboard not working on Thinkpad X60/T60

login
register
about
Submitter Sven Schnelle
Date 2011-05-04 08:16:17
Message ID <874o5ag9ta.fsf@begreifnix.stackframe.org>
Download mbox | patch
Permalink /patch/2938/
State Not Applicable
Headers show

Comments

Sven Schnelle - 2011-05-04 08:16:17
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: 
>
> 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)
Kevin O'Connor - 2011-05-07 17:01:59
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
Paul Menzel - 2011-05-07 18:03:25
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
Sven Schnelle - 2011-05-07 18:48:45
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.
Kevin O'Connor - 2011-05-07 19:05:03
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
Stefan Reinauer - 2011-05-07 20:19:23
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'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?
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
Stefan Reinauer - 2011-05-07 20:22:50
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'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.
Maybe we can add some "shoot and run" ps2 initialization code to 
coreboot that runs before most other code, giving the keyboard or kbc 
time to initialize while we do all the other stuff.

Have we considered threading in coreboot? It seems the time we spend in 
ramstage is unsignificant and the effort of multitasking in romstage is 
not worth the trouble.
Kevin O'Connor - 2011-05-07 22:24:08
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'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?
> 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 22:54:11
On Sat, May 07, 2011 at 01:22:50PM -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'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.
> Maybe we can add some "shoot and run" ps2 initialization code to
> coreboot that runs before most other code, giving the keyboard or
> kbc time to initialize while we do all the other stuff.

There's a good chance it just needs more time from power up - starting
the init sooner may have no impact.

I don't think ps2 keyboard init should be in coreboot - it is (in the
general case) an optional external device that is auto-discoverable.
There's nothing really special about a ps2 keyboard that requires it
to be handled in the lowest layer of the firmware.

> Have we considered threading in coreboot? It seems the time we spend
> in ramstage is unsignificant and the effort of multitasking in
> romstage is not worth the trouble.

Are you referring to something like the cooperative multitasking
system in SeaBIOS?  If so, do you have ideas on which device
initialization could be done in parallel?  (SeaBIOS can parallelize
initialization of each disk controller, the ps2 controller, each USB
controller, and each USB device.)

-Kevin

Patch

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;