Hello Felix Singer, Matt Delco, Patrick Georgi, Furquan Shaikh, Julius Werner, Angel Pons, Patrick Rudolph, EricR Lai,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/47088
to review the following change.
Change subject: libpayload/keyboard: Implement self-test ......................................................................
libpayload/keyboard: Implement self-test
The keyboard self-test is required for some devices. At least one device (integrated keyboard in a ThinkPad X201) actually starts the test automatically leading to spurious output and no response for the first seconds.
We wait up to 5s for the self-test result. On failure or timeout, the command will be repeated indefinitely. This happens all in the background of the UI polling loop.
TODO: Figure out if we should make this optional.
Change-Id: Ie07b31e74d06e116ac81e76309621eed39a19b49 Signed-off-by: Nico Huber nico.h@gmx.de --- M payloads/libpayload/drivers/i8042/keyboard.c 1 file changed, 46 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/47088/1
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index a30bdaf..a9b2e61 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -244,6 +244,7 @@
static enum keyboard_state { STATE_INIT = 0, + STATE_SELF_TEST, STATE_CONFIGURE, STATE_ENABLE_SCAN, STATE_RUNNING, @@ -251,6 +252,16 @@
uint64_t state_time;
+static void start_self_test(void) +{ + if (!keyboard_cmd(I8042_KBCMD_RESET)) + printf("ERROR: Keyboard self-test couldn't be started.\n"); + /* We ignore errors and always move to the self-test state + which will simply try again if necessary. */ + keyboard_state = STATE_SELF_TEST; + state_time = timer_us(0); +} + static void keyboard_poll(void) { enum keyboard_state next_state = keyboard_state; @@ -261,6 +272,34 @@ /* Wait until keyboard_init() has been called. */ break;
+ case STATE_SELF_TEST: + if (!i8042_data_ready_ps2()) { + if (timer_us(state_time) > 5*1000*1000) { + start_self_test(); + return; + } + break; + } + + const uint8_t self_test_result = i8042_read_data_ps2(); + switch (self_test_result) { + case 0xaa: + /* Success! */ + break; + case 0xfc: + case 0xfd: + /* Failure. Try again. */ + start_self_test(); + return; + default: + printf("WARNING: Keyboard sent spurious 0x%02x\n", + self_test_result); + return; + } + + next_state = STATE_CONFIGURE; + break; + case STATE_CONFIGURE: /* * We only support scancode set 1. The controller translation @@ -269,8 +308,10 @@ * o set 1 if the controller doesn't translate, and * o set 2 if the controller does. */ - if (!set_scancode_set(controller_translates() ? 2 : 1)) - break; + if (!set_scancode_set(controller_translates() ? 2 : 1)) { + start_self_test(); + return; + }
next_state = STATE_ENABLE_SCAN; break; @@ -279,8 +320,8 @@ /* Enable scanning */ if (!keyboard_cmd(I8042_KBCMD_EN)) { printf("ERROR: Keyboard enable scanning failed!\n"); - keyboard_state = STATE_CONFIGURE; - break; + start_self_test(); + return; }
next_state = STATE_RUNNING; @@ -451,8 +492,7 @@ keyboard_cmd(I8042_KBCMD_DEFAULT_DIS); keyboard_drain_input();
- keyboard_state = STATE_CONFIGURE; - state_time = timer_us(0); + start_self_test();
console_add_input_driver(&cons); }