Change in ...coreboot[master]: libpayload: keyboard: Add option to ignore failures during init
Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31657
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
libpayload: keyboard: Add option to ignore failures during init
If keys are pressed at boot some keyboard controllers will not properly respond with an ACK to commands, which results in the keyboard_init function aborting before it adds the keyboard to the input device list.
This same keyboard controller will manage to properly return keyboard data when keys are pressed later, so it is possible for it to be functional in the payload even if it does not respond properly to every command during initialization.
In order to allow payloads to use the keyboard when this happens a new Kconfig option is added to ignore the keyboard ACK response and always add the keyboard to the input device list. This option is disabled by default and must be enabled by the specific boards that need it.
BUG=b:126633269 TEST=boot on device with this controller and press keys during boot and see that the keyboard is still functional in the payload.
Change-Id: Icc6053f99804f1b57d785cb04235b5c4b8d5426f Signed-off-by: Duncan Laurie dlaurie@google.com --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/keyboard.c 2 files changed, 7 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31657/1
diff --git a/payloads/libpayload/Kconfig b/payloads/libpayload/Kconfig index a79269f..6aa5b61 100644 --- a/payloads/libpayload/Kconfig +++ b/payloads/libpayload/Kconfig @@ -344,6 +344,10 @@ default y if ARCH_X86 # uses IO default n
+config PC_KEYBOARD_IGNORE_FAILURE + bool "Ignore keyboard failures during init and always add input 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 062aec2..918cafa 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -309,16 +309,16 @@
/* Set scancode set 1 */ ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); - if (!ret) + if (!ret && !IS_ENABLED(CONFIG_LP_PC_KEYBOARD_IGNORE_FAILURE)) return;
ret = keyboard_cmd(I8042_SCANCODE_SET_1); - if (!ret) + if (!ret && !IS_ENABLED(CONFIG_LP_PC_KEYBOARD_IGNORE_FAILURE)) return;
/* Enable scanning */ ret = keyboard_cmd(I8042_KBCMD_EN); - if (!ret) + if (!ret && !IS_ENABLED(CONFIG_LP_PC_KEYBOARD_IGNORE_FAILURE)) return;
console_add_input_driver(&cons);
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31657 )
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
Patch Set 1:
This is ugly and I don't like doing it but it is a useful workaround until the underlying issue is hopefully resolved.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31657 )
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
Patch Set 1: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31657 )
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/31657/1/payloads/libpayload/Kconfig File payloads/libpayload/Kconfig:
https://review.coreboot.org/#/c/31657/1/payloads/libpayload/Kconfig@347 PS1, Line 347: F Should this be named PC_KEYBOARD_IGNORE_INIT_FAILURE to make it clear that it is just ignoring the init failures? If it feels like its getting too big, we can go with this too.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31657 )
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31657/1/payloads/libpayload/Kconfig File payloads/libpayload/Kconfig:
https://review.coreboot.org/#/c/31657/1/payloads/libpayload/Kconfig@347 PS1, Line 347: F
Should this be named PC_KEYBOARD_IGNORE_INIT_FAILURE to make it clear that it is just ignoring the i […]
Done
Hello Aaron Durbin, Paul Menzel, build bot (Jenkins), Nico Huber, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31657
to look at the new patch set (#2).
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
libpayload: keyboard: Add option to ignore failures during init
If keys are pressed at boot some keyboard controllers will not properly respond with an ACK to commands, which results in the keyboard_init function aborting before it adds the keyboard to the input device list.
This same keyboard controller will manage to properly return keyboard data when keys are pressed later, so it is possible for it to be functional in the payload even if it does not respond properly to every command during initialization.
In order to allow payloads to use the keyboard when this happens a new Kconfig option is added to ignore the keyboard ACK response and always add the keyboard to the input device list. This option is disabled by default and must be enabled by the specific boards that need it.
BUG=b:126633269 TEST=boot on device with this controller and press keys during boot and see that the keyboard is still functional in the payload.
Change-Id: Icc6053f99804f1b57d785cb04235b5c4b8d5426f Signed-off-by: Duncan Laurie dlaurie@google.com --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/keyboard.c 2 files changed, 7 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/31657/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31657 )
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31657 )
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
Patch Set 2: Code-Review+1
Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31657 )
Change subject: libpayload: keyboard: Add option to ignore failures during init ......................................................................
libpayload: keyboard: Add option to ignore failures during init
If keys are pressed at boot some keyboard controllers will not properly respond with an ACK to commands, which results in the keyboard_init function aborting before it adds the keyboard to the input device list.
This same keyboard controller will manage to properly return keyboard data when keys are pressed later, so it is possible for it to be functional in the payload even if it does not respond properly to every command during initialization.
In order to allow payloads to use the keyboard when this happens a new Kconfig option is added to ignore the keyboard ACK response and always add the keyboard to the input device list. This option is disabled by default and must be enabled by the specific boards that need it.
BUG=b:126633269 TEST=boot on device with this controller and press keys during boot and see that the keyboard is still functional in the payload.
Change-Id: Icc6053f99804f1b57d785cb04235b5c4b8d5426f Signed-off-by: Duncan Laurie dlaurie@google.com Reviewed-on: https://review.coreboot.org/c/31657 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/keyboard.c 2 files changed, 7 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Furquan Shaikh: Looks good to me, approved
diff --git a/payloads/libpayload/Kconfig b/payloads/libpayload/Kconfig index a79269f..fd967c1 100644 --- a/payloads/libpayload/Kconfig +++ b/payloads/libpayload/Kconfig @@ -344,6 +344,10 @@ default y if ARCH_X86 # uses IO default n
+config PC_KEYBOARD_IGNORE_INIT_FAILURE + bool "Ignore keyboard failures during init and always add input 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 062aec2..a0d5b63 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -309,16 +309,16 @@
/* Set scancode set 1 */ ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); - if (!ret) + if (!ret && !IS_ENABLED(CONFIG_LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) return;
ret = keyboard_cmd(I8042_SCANCODE_SET_1); - if (!ret) + if (!ret && !IS_ENABLED(CONFIG_LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) return;
/* Enable scanning */ ret = keyboard_cmd(I8042_KBCMD_EN); - if (!ret) + if (!ret && !IS_ENABLED(CONFIG_LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) return;
console_add_input_driver(&cons);
-
Duncan Laurie (Code Review) -
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)