Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46724 )
Change subject: libpayload/keyboard: Revise scancode set and translation config ......................................................................
libpayload/keyboard: Revise scancode set and translation config
Some background first: The original XT keyboards used what we call scancode set #1 today. The PC/AT keyboards introduced scancode set #2, but for compatibility, its controller translated scancodes back to set #1 by default. Newer keyboards (maybe all we have to deal with) also support switching the scancode set.
This means the translation option in the controller and the scancode set selection in the keyboard have to match. In libpayload, we only support set #1 scancodes. So we either need the controller's trans- lation on and set #2 selected in the keyboard, or the controller's translation off and set #1 selected in the keyboard.
Valid configurations: * SET #1 + XLATE off * SET #2 + XLATE on
The Linux kernel leaves the decision to enable or disable the con- troller's translation to the firmware. We follow that behaviour, because Linux can be a payload too, and ideally want any payload to succeed the same with any given coreboot image.
This way, coreboot (or the controller firmware) can decide how to start best. For instance, if the keyboard is integrated and one knows for sure that it can switch scancode sets, no translation should be necessary.
Currently, coreboot leaves the controller's translation at its default setting, unless DRIVERS_PS2_KEYBOARD is enabled. The latter enables the translation unconditionally. For QEMU this means that the option effectively toggles the translation, as QEMU's controller has it disabled by default. This probably made a lot of earlier testing inconsistent.
Fixes: commit a95a6bf646 (libpayload/drivers/i8402/kbd: Fix qemu) The reset introduced there effectively reverted the scancode selection made before. It's unclear if later changes to the code where only necessary to work around it.
Change-Id: Iad85af516a7b9f9c0269ff9652ed15ee81700057 Signed-off-by: Nico Huber nico.h@gmx.de --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/keyboard.c 2 files changed, 30 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/46724/1
diff --git a/payloads/libpayload/Kconfig b/payloads/libpayload/Kconfig index b5dc9a3..298a7ad 100644 --- a/payloads/libpayload/Kconfig +++ b/payloads/libpayload/Kconfig @@ -367,10 +367,6 @@ default y if ARCH_X86 # uses IO default n
-config PC_KEYBOARD_AT_TRANSLATED - bool "AT Translation keyboard device" - default n - config PC_KEYBOARD_LAYOUT_US bool "English (US) keyboard layout" depends on PC_KEYBOARD diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index 2dec3a3..aa21b9d 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -313,55 +313,36 @@ .input_type = CONSOLE_INPUT_TYPE_EC, };
-/* Enable keyboard translated */ -static bool enable_translated(void) +/** + * 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) { - if (!i8042_cmd(I8042_CMD_RD_CMD_BYTE)) { - int cmd = i8042_read_data_ps2(); - cmd |= I8042_CMD_BYTE_XLATE; - if (!i8042_cmd(I8042_CMD_WR_CMD_BYTE)) { - i8042_write_data(cmd); - } else { - printf("ERROR: i8042_cmd WR_CMD failed!\n"); - return false; - } - } else { - printf("ERROR: i8042_cmd RD_CMD failed!\n"); - return false; - } - return true; + const int ctrl_cfg = i8042_cmd_with_response(I8042_CMD_RD_CMD_BYTE); + return ctrl_cfg < 0 || (ctrl_cfg & I8042_CMD_BYTE_XLATE); }
/* Set scancode set 1 */ -static bool set_scancode_set(void) +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(I8042_SCANCODE_SET_1); + ret = keyboard_cmd(set); if (!ret) { - printf("ERROR: Keyboard scancode set#1 failed!\n"); - return ret; - } - - /* - * Set default parameters. - * Fix for broken QEMU PS/2 make scancodes. - */ - ret = keyboard_cmd(I8042_KBCMD_SET_DEFAULT); - if (!ret) { - printf("ERROR: Keyboard set default params failed!\n"); - return ret; - } - - /* Enable scanning */ - ret = keyboard_cmd(I8042_KBCMD_EN); - if (!ret) { - printf("ERROR: Keyboard enable scanning failed!\n"); + printf("ERROR: Keyboard scancode set#%u failed!\n", set); return ret; }
@@ -383,13 +364,19 @@ /* Enable first PS/2 port */ i8042_cmd(I8042_CMD_EN_KB);
- if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) { - if (!enable_translated()) - return; - } else { - if (!set_scancode_set()) - return; - } + /* + * 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");
console_add_input_driver(&cons); }