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/+/47086
to review the following change.
Change subject: libpayload/keyboard: Turn init sequence into a state machine ......................................................................
libpayload/keyboard: Turn init sequence into a state machine
We'll process the init sequence as part of the polling loop. This should have several advantages:
* It eases error handling, i.e. we can return to an earlier state. * We don't have to stall initialization when a keyboard takes a little longer. * Generally, these keyboards can be hot-plugged (albeit not by design).
Change-Id: I9cf5cf31eb420b3994bec20e56a72d37f3d2996e Signed-off-by: Nico Huber nico.h@gmx.de --- M payloads/libpayload/drivers/i8042/keyboard.c 1 file changed, 92 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/47086/1
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index efe6bcd..27abd95 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -207,9 +207,96 @@ return false; }
+/** + * Get translation state from the controller. + * + * We want to act on the state left by the firmware, same as + * Linux does. If an error occurs, we assume it's enabled by + * default, as the OSDev wiki suggests. + */ +static bool controller_translates(void) +{ + const int ctrl_cfg = i8042_cmd_with_response(I8042_CMD_RD_CMD_BYTE); + return ctrl_cfg < 0 || (ctrl_cfg & I8042_CMD_BYTE_XLATE); +} + +static bool set_scancode_set(const unsigned char set) +{ + bool ret; + + if (set < 1 || set > 3) + return false; + + ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); + if (!ret) { + printf("ERROR: Keyboard set scancode failed!\n"); + return ret; + } + + ret = keyboard_cmd(set); + if (!ret) { + printf("ERROR: Keyboard scancode set#%u failed!\n", set); + return ret; + } + + return ret; +} + +static enum keyboard_state { + STATE_INIT = 0, + STATE_CONFIGURE, + STATE_ENABLE_SCAN, + STATE_RUNNING, +} keyboard_state; + +static void keyboard_poll(void) +{ + enum keyboard_state next_state = keyboard_state; + + switch (keyboard_state) { + + case STATE_INIT: + /* Wait until keyboard_init() has been called. */ + break; + + case STATE_CONFIGURE: + /* + * We only support scancode set 1. The controller translation + * would translate from set 2 to 1 for us, so we try to configure + * + * 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; + + next_state = STATE_ENABLE_SCAN; + break; + + case STATE_ENABLE_SCAN: + /* Enable scanning */ + if (!keyboard_cmd(I8042_KBCMD_EN)) { + printf("ERROR: Keyboard enable scanning failed!\n"); + keyboard_state = STATE_CONFIGURE; + break; + } + + next_state = STATE_RUNNING; + break; + + case STATE_RUNNING: + /* TODO: Use echo command to detect detach. */ + break; + } + + if (keyboard_state != next_state) + keyboard_state = next_state; +} + bool keyboard_havechar(void) { - return i8042_data_ready_ps2(); + keyboard_poll(); + return keyboard_state == STATE_RUNNING && i8042_data_ready_ps2(); }
unsigned char keyboard_get_scancode(void) @@ -342,43 +429,11 @@ .input_type = CONSOLE_INPUT_TYPE_EC, };
-/** - * Get translation state from the controller. - * - * We want to act on the state left by the firmware, same as - * Linux does. If an error occurs, we assume it's enabled by - * default, as the OSDev wiki suggests. - */ -static bool controller_translates(void) -{ - const int ctrl_cfg = i8042_cmd_with_response(I8042_CMD_RD_CMD_BYTE); - return ctrl_cfg < 0 || (ctrl_cfg & I8042_CMD_BYTE_XLATE); -} - -static bool set_scancode_set(const unsigned char set) -{ - bool ret; - - if (set < 1 || set > 3) - return false; - - ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); - if (!ret) { - printf("ERROR: Keyboard set scancode failed!\n"); - return ret; - } - - ret = keyboard_cmd(set); - if (!ret) { - printf("ERROR: Keyboard scancode set#%u failed!\n", set); - return ret; - } - - return ret; -} - void keyboard_init(void) { + if (keyboard_state != STATE_INIT) + return; + map = &keyboard_layouts[0];
/* Initialized keyboard controller. */ @@ -392,19 +447,7 @@ keyboard_cmd(I8042_KBCMD_DEFAULT_DIS); keyboard_drain_input();
- /* - * We only support scancode set 1. The controller translation - * would translate from set 2 to 1 for us, so we try to configure - * - * o set 1 if the controller doesn't translate, and - * o set 2 if the controller does. - */ - (void)set_scancode_set(controller_translates() ? 2 : 1); - - /* Enable scanning */ - const bool ret = keyboard_cmd(I8042_KBCMD_EN); - if (!ret) - printf("ERROR: Keyboard enable scanning failed!\n"); + keyboard_state = STATE_CONFIGURE;
console_add_input_driver(&cons); }