Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32951
Change subject: libpayload: Reset PS/2 keyboard ......................................................................
libpayload: Reset PS/2 keyboard
Loading a libpayload based payload like coreinfo or FILO from SeaBIOS pressing keys does not give the expected results.
For example, pressing F1 gives the character 24 translated to scan code 6a. ESC for example 43 (111).
The problem is not reproducible using the payload directly, that means without SeaBIOS. The problem seems to be, that SeaBIOS already initializes the PS/2 controller and AT keyboard.
Comparing it with coreboot’s PS/2 keyboard code, the keyboard needs to be reset. That seems to fix the issue, when the keyboard was initialized before.
TEST=Build coreboot for QEMU Q35 with SeaBIOS, and coreinfo as secondary payload. Run
qemu-system-i386 -M q35 -L /dev/shm -bios build/coreboot.rom -serial stdio
press 3 to select the coreinfo payload, and verify that the keys F1 and F2 are working.
Change-Id: I2732292ac316d4bc0029ecb5c95fa7d1e7d68947 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de --- M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 2 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/32951/1
diff --git a/payloads/libpayload/drivers/i8042/i8042.h b/payloads/libpayload/drivers/i8042/i8042.h index 643167e..e864ac9 100644 --- a/payloads/libpayload/drivers/i8042/i8042.h +++ b/payloads/libpayload/drivers/i8042/i8042.h @@ -63,6 +63,7 @@ #define I8042_KBCMD_EN 0xf4 #define I8042_KBCMD_DEFAULT_DIS 0xf5 #define I8042_KBCMD_SET_DEFAULT 0xf6 +#define I8042_KBCMD_ACK 0xfa #define I8042_KBCMD_RESEND 0xfe #define I8042_KBCMD_RESET 0xff
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index cded638..fea9e71 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -317,6 +317,13 @@ /* Enable first PS/2 port */ i8042_cmd(I8042_CMD_EN_KB);
+ /* Reset keyboard and self test (keyboard side) */ + ret = keyboard_cmd(I8042_KBCMD_RESET); + if (ret != I8042_KBCMD_ACK) { + printf("ERROR: Keyboard reset failed ACK: 0x%x\n", ret); + return; + } + /* Set scancode set 1 */ ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE))
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32951 )
Change subject: libpayload: Reset PS/2 keyboard ......................................................................
Patch Set 1:
Nico, could you please test if this fixes your FILO issue with the board you have?
Also, I am not firm with the terminology PS/2 controller, (AT) keyboard. Feel free to correct the commit message.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32951 )
Change subject: libpayload: Reset PS/2 keyboard ......................................................................
Patch Set 1:
Same problem happened with loading coreinfo.elf from GRUB using the command `chainloader`. No it is fixed.
I wonder if the warning message should be shown or not.
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32951
to look at the new patch set (#2).
Change subject: libpayload: Reset PS/2 keyboard ......................................................................
libpayload: Reset PS/2 keyboard
Loading a libpayload based payload like coreinfo or FILO from SeaBIOS or GRUB pressing keys does not give the expected results.
For example, pressing F1 gives the character 24 translated to scan code 6a. ESC for example 43 (111) in coreinfo loaded from SeaBIOS on QEMU Q35.
The problem is not reproducible using the payload directly, that means without SeaBIOS or GRUB. The problem seems to be, that those have already initialized the PS/2 controller and AT keyboard.
Comparing it with coreboot’s PS/2 keyboard code, the keyboard needs to be reset. That seems to fix the issue, when the keyboard was initialized before.
TEST=Build coreboot for QEMU Q35 with SeaBIOS, and coreinfo as secondary payload. Run
qemu-system-i386 -M q35 -L /dev/shm -bios build/coreboot.rom -serial stdio
press 3 to select the coreinfo payload, and verify that the keys F1 and F2 are working.
Same with coreinfo loaded from GRUB on the ASRock E350M1.
Change-Id: I2732292ac316d4bc0029ecb5c95fa7d1e7d68947 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de --- M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 2 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/32951/2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32951 )
Change subject: libpayload: Reset PS/2 keyboard ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32951 )
Change subject: libpayload: Reset PS/2 keyboard ......................................................................
libpayload: Reset PS/2 keyboard
Loading a libpayload based payload like coreinfo or FILO from SeaBIOS or GRUB pressing keys does not give the expected results.
For example, pressing F1 gives the character 24 translated to scan code 6a. ESC for example 43 (111) in coreinfo loaded from SeaBIOS on QEMU Q35.
The problem is not reproducible using the payload directly, that means without SeaBIOS or GRUB. The problem seems to be, that those have already initialized the PS/2 controller and AT keyboard.
Comparing it with coreboot’s PS/2 keyboard code, the keyboard needs to be reset. That seems to fix the issue, when the keyboard was initialized before.
TEST=Build coreboot for QEMU Q35 with SeaBIOS, and coreinfo as secondary payload. Run
qemu-system-i386 -M q35 -L /dev/shm -bios build/coreboot.rom -serial stdio
press 3 to select the coreinfo payload, and verify that the keys F1 and F2 are working.
Same with coreinfo loaded from GRUB on the ASRock E350M1.
Change-Id: I2732292ac316d4bc0029ecb5c95fa7d1e7d68947 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/32951 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com --- M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 2 files changed, 8 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/i8042/i8042.h b/payloads/libpayload/drivers/i8042/i8042.h index 643167e..e864ac9 100644 --- a/payloads/libpayload/drivers/i8042/i8042.h +++ b/payloads/libpayload/drivers/i8042/i8042.h @@ -63,6 +63,7 @@ #define I8042_KBCMD_EN 0xf4 #define I8042_KBCMD_DEFAULT_DIS 0xf5 #define I8042_KBCMD_SET_DEFAULT 0xf6 +#define I8042_KBCMD_ACK 0xfa #define I8042_KBCMD_RESEND 0xfe #define I8042_KBCMD_RESET 0xff
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index cded638..fea9e71 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -317,6 +317,13 @@ /* Enable first PS/2 port */ i8042_cmd(I8042_CMD_EN_KB);
+ /* Reset keyboard and self test (keyboard side) */ + ret = keyboard_cmd(I8042_KBCMD_RESET); + if (ret != I8042_KBCMD_ACK) { + printf("ERROR: Keyboard reset failed ACK: 0x%x\n", ret); + return; + } + /* Set scancode set 1 */ ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE))
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32951 )
Change subject: libpayload: Reset PS/2 keyboard ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/32951/3/payloads/libpayload/drivers/i8042/ke... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/#/c/32951/3/payloads/libpayload/drivers/i8042/ke... PS3, Line 322: if (ret != I8042_KBCMD_ACK) { Did you mean to use i8042_cmd() instead of keyboard_cmd(), or "ret != 1"?
keyboard_cmd() basically returns a bool instead of I8042_KBCMD_ACK: return i8042_wait_read_ps2() == I8042_KBCMD_ACK;
so this fails with:
ERROR: Keyboard reset failed ACK: 0x1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32951 )
Change subject: libpayload: Reset PS/2 keyboard ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/32951/3/payloads/libpayload/drivers/i8042/ke... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/#/c/32951/3/payloads/libpayload/drivers/i8042/ke... PS3, Line 322: if (ret != I8042_KBCMD_ACK) {
Did you mean to use i8042_cmd() instead of keyboard_cmd(), or "ret != 1"? […]
Yes, that doesn't look right. I have pushed a fix here: https://review.coreboot.org/c/coreboot/+/33185
-
Furquan Shaikh (Code Review) -
Matt Delco (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)