Some keyboards take pretty long to respond to a reset command, some even delay the ACK to the command. To make the keyboard driver more robust, increase the timeout for this special command. Also do an interface test after the self-test to ensure the keyboard is functioning properly.
Another point is to reenable the keyboard *after* the scancode was set, not before. We also set the system bit when enabling the keyboard because this seems to be what older operating systems do expect.
One of the problematic keyboards, which will work with this patch applied, is the DELL RT7D20. Without the patch an overly optimistic operating system, read Linux 2.4, will not recognise the keyboard because coreboot didn't fully initialize it.
Signed-off-by: Mathias Krause mathias.krause@secunet.com --- Since I was unable to download the git tree (why is it HTTP only, btw?) a patch against a tarball snapshot.
diff -urp coreboot-403d2d6/src/pc80/keyboard.c coreboot-kbd_fix/src/pc80/keyboard.c --- coreboot-403d2d6/src/pc80/keyboard.c 2011-12-08 22:42:04.000000000 +0100 +++ coreboot-kbd_fix/src/pc80/keyboard.c 2011-12-12 10:16:04.000000000 +0100 @@ -36,6 +36,7 @@ #define KBC_CMD_READ_COMMAND 0x20 // Read command byte #define KBC_CMD_WRITE_COMMAND 0x60 // Write command byte #define KBC_CMD_SELF_TEST 0xAA // Controller self-test +#define KBC_CMD_KBD_TEST 0xAB // Keyboard Interface test
/* The Keyboard controller command byte * BIT | Description @@ -136,6 +137,26 @@ static int kbc_self_test(void) return 0; }
+ /* ensure the buffers are empty */ + kbc_cleanup_buffers(); + + /* keyboard interface test */ + outb(KBC_CMD_KBD_TEST, KBD_COMMAND); + + if (!kbc_output_buffer_full()) { + printk(BIOS_ERR, "Keyboard Interface test timed out.\n"); + return 0; + } + + /* read test result, 0x00 should be returned in case of no failures */ + self_test = inb(KBD_DATA); + + if (self_test != 0x00) { + printk(BIOS_ERR, "Keyboard Interface test failed: 0x%x\n", + self_test); + return 0; + } + return 1; }
@@ -147,6 +168,14 @@ static u8 send_keyboard(u8 command) do { if (!kbc_input_buffer_empty()) return 0; outb(command, KBD_DATA); + /* the reset command takes much longer then normal commands and + * even worse, some keyboards do send the ACK _after_ doing the + * reset */ + if (command == 0xFF) { + u8 retries; + for (retries = 9; retries && !kbc_output_buffer_full(); retries--) + ; + } if (!kbc_output_buffer_full()) { printk(BIOS_ERR, "Could not send keyboard command %02x\n", command); @@ -161,6 +190,7 @@ static u8 send_keyboard(u8 command)
void pc_keyboard_init(struct pc_keyboard *keyboard) { + u8 retries; u8 regval; if (!CONFIG_DRIVERS_PS2_KEYBOARD) return; @@ -192,10 +222,14 @@ void pc_keyboard_init(struct pc_keyboard }
if (regval != KBD_REPLY_ACK) { - printk(BIOS_ERR, "Keyboard selftest failed ACK: 0x%x\n", regval); + printk(BIOS_ERR, "Keyboard reset failed ACK: 0x%x\n", regval); return; }
+ /* the reset command takes some time, so wait a little longer */ + for (retries = 9; retries && !kbc_output_buffer_full(); retries--) + ; + if (!kbc_output_buffer_full()) { printk(BIOS_ERR, "Timeout waiting for keyboard after reset.\n"); return; @@ -203,7 +237,7 @@ void pc_keyboard_init(struct pc_keyboard
regval = inb(KBD_DATA); if (regval != 0xAA) { - printk(BIOS_ERR, "Keyboard selftest failed: 0x%x\n", regval); + printk(BIOS_ERR, "Keyboard reset selftest failed: 0x%x\n", regval); return; }
@@ -232,20 +266,20 @@ void pc_keyboard_init(struct pc_keyboard return; }
- /* enable the keyboard */ - regval = send_keyboard(0xF4); - if (regval != KBD_REPLY_ACK) { - printk(BIOS_ERR, "Keyboard enable failed ACK: 0x%x\n", regval); - return; - } - /* All is well - enable keyboard interface */ if (!kbc_input_buffer_empty()) return; outb(0x60, KBD_COMMAND); if (!kbc_input_buffer_empty()) return; - outb(0x61, KBD_DATA); /* send cmd: enable keyboard and IRQ 1 */ + outb(0x65, KBD_DATA); /* send cmd: enable keyboard and IRQ 1 */ if (!kbc_input_buffer_empty()) { - printk(BIOS_ERR, "Timeout during final keyboard enable\n"); + printk(BIOS_ERR, "Timeout during keyboard enable\n"); + return; + } + + /* enable the keyboard */ + regval = send_keyboard(0xF4); + if (regval != KBD_REPLY_ACK) { + printk(BIOS_ERR, "Keyboard enable failed ACK: 0x%x\n", regval); return; } }
Hi,
Mathias Krause wrote:
Some keyboards take pretty long to respond to a reset command,
I've pushed this to Gerrit.
Since I was unable to download the git tree (why is it HTTP only, btw?)
You can also access the Git repo using SSH. Please do that. You'll need to create a user in Gerrit via the web interface, and upload your public SSH key. Then you can push directly to Gerrit. You can't count on someone else doing it for you, and there is a high risk that patches get overlooked. :\
a patch against a tarball snapshot.
Please always make a git commit and not only a patch. You can send an email with the patch easily using git send-email, or you can format a patch for attachment (as text/plain) using git format-patch, after creating the commit.
I suggest using one branch per independent change, to not have any dependencies in Gerrit, even though they are really only advisory.
The patch looks generally fine, but what about changing the 0x61 port to 0x65? That was an error all along?
//Peter
Hi Peter,
On 14.12.2011 08:45, Peter Stuge wrote:
Hi,
Mathias Krause wrote:
Some keyboards take pretty long to respond to a reset command,
I've pushed this to Gerrit.
thank you! I wasn't aware of that, but maybe the wiki[1] should be updated to reflect that this is the preferred way to submit patches and no longer the mailing list? It's even mentioning the usage of subversion for the coreboot source, which is now obsolete, I guess.
Since I was unable to download the git tree (why is it HTTP only, btw?)
You can also access the Git repo using SSH. Please do that. You'll need to create a user in Gerrit via the web interface, and upload your public SSH key. Then you can push directly to Gerrit. You can't count on someone else doing it for you, and there is a high risk that patches get overlooked. :\
I'll try to set it up for the next time. I already talked to Patrick why the HTTP repo failed for me. Looks like my git version is to old and that for lacks smart http support.
a patch against a tarball snapshot.
Please always make a git commit and not only a patch. You can send an email with the patch easily using git send-email, or you can format a patch for attachment (as text/plain) using git format-patch, after creating the commit.
Due to the lack of a git repo I was unable to do so. But, in fact, I used git send-email to deliver the message. And the email should have almost looked like as git format-patch generates them -- meaning it has a subject describing the change, a commit message and the SOB signature. Anything else missing?
I suggest using one branch per independent change, to not have any dependencies in Gerrit, even though they are really only advisory.
The patch looks generally fine, but what about changing the 0x61 port to 0x65? That was an error all along?
No, it was not. In fact, it's not the port that changed but the value written. The additional bit is the "also set the system bit when enabling the keyboard" part of the commit message.
Thanks, Mathias
[1] http://www.coreboot.org/Development_Guidelines#How_to_contribute
-
Mathias Krause -
Peter Stuge