Skip to content

Commit

Permalink
libpayload/keyboard: Revise scancode set and translation config
Browse files Browse the repository at this point in the history
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

Both with and without the PC_KEYBOARD_AT_TRANSLATED option, we were
only configuring one of the two settings, leaving room for invalid
configurations. With this change, we try to select scancode set #2
first, which seems to be the most supported one, and configure the
controller's translation accordingly. We try to fall back to set #1
on failure.

We also keep translation disabled during configuration steps to
ensure that the controller doesn't accidentally translate confi-
guration data.

On the coreboot side, we leave 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 a95a6bf (libpayload/drivers/i8402/kbd: Fix qemu)
       The reset introduced there effectively reverted the scancode
       selection made before (because 2 is the default). It's unclear
       if later changes to the code were only necessary to work
       around it.

Change-Id: Iad85af516a7b9f9c0269ff9652ed15ee81700057
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46724
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
  • Loading branch information
i-c-o-n authored and hungte committed Dec 23, 2020
1 parent 0e1d19b commit a69d682
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 53 deletions.
4 changes: 0 additions & 4 deletions payloads/libpayload/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,6 @@ config PC_KEYBOARD
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
Expand Down
3 changes: 0 additions & 3 deletions payloads/libpayload/drivers/i8042/i8042.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@
#define I8042_MODE_SCROLL_LOCK_ON (1 << 0)
#define I8042_MODE_SCROLL_LOCK_OFF (0 << 0)
#define I8042_KBCMD_SET_SCANCODE 0xf0
#define I8042_SCANCODE_SET_1 (1)
#define I8042_SCANCODE_SET_2 (2)
#define I8042_SCANCODE_SET_3 (3)
#define I8042_KBCMD_SET_TYPEMATIC 0xf3
#define I8042_KBCMD_EN 0xf4
#define I8042_KBCMD_DEFAULT_DIS 0xf5
Expand Down
63 changes: 17 additions & 46 deletions payloads/libpayload/drivers/i8042/keyboard.c
Original file line number Diff line number Diff line change
Expand Up @@ -313,55 +313,22 @@ static struct console_input_driver cons = {
.input_type = CONSOLE_INPUT_TYPE_EC,
};

/* Enable keyboard translated */
static bool enable_translated(void)
static bool set_scancode_set(const unsigned char set)
{
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");
bool ret;

if (set < 1 || set > 3)
return false;
}
return true;
}

/* Set scancode set 1 */
static bool set_scancode_set(void)
{
bool ret;
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);
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);
ret = keyboard_cmd(set);
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;
}

Expand All @@ -383,13 +350,17 @@ void keyboard_init(void)
/* 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;
}
i8042_set_kbd_translation(false);

if (set_scancode_set(2))
i8042_set_kbd_translation(true);
else if (!set_scancode_set(1))
return; /* give up, leave keyboard input disabled */

/* Enable scanning */
const bool ret = keyboard_cmd(I8042_KBCMD_EN);
if (!ret)
printf("ERROR: Keyboard enable scanning failed!\n");

console_add_input_driver(&cons);
}
Expand Down

0 comments on commit a69d682

Please sign in to comment.