Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31659
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD
In order to prevent keyboard keys pressed at boot from causing issues in the payload remove the PS2 keyboard driver so it does not get initialized until it is needed in libpayload.
BUG=b:126633269 TEST=boot on sarien while pressing keys and ensure libpayload and/or the kernel does not have any issues initializing the keyboard.
Change-Id: I765e808f0d2589cf23c0349798a07e2706a2a7a4 Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/mainboard/google/sarien/Kconfig 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31659/1
diff --git a/src/mainboard/google/sarien/Kconfig b/src/mainboard/google/sarien/Kconfig index a976d36..ce6b810 100644 --- a/src/mainboard/google/sarien/Kconfig +++ b/src/mainboard/google/sarien/Kconfig @@ -5,7 +5,6 @@ select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_SPI_ACPI - select DRIVERS_PS2_KEYBOARD select DRIVERS_USB_ACPI select EC_GOOGLE_WILCO select GENERIC_SPD_BIN
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31659 )
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
Patch Set 1: Code-Review+1
Any idea why it was added in the first place?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31659 )
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
Patch Set 1:
Patch Set 1: Code-Review+1
Any idea why it was added in the first place?
The keyboard controller does not come up in translated mode, so unless I called keyboard_init() in coreboot it would never work in the kernel because it would come up as an "AT Raw" device instead of an "AT Translated".
In order to keep the keyboard untouched in coreboot (to try and minimize issues with keys pressed at boot) I put the workaround into the payload to put it into translate mode.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31659 )
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review+1
Any idea why it was added in the first place?
The keyboard controller does not come up in translated mode, so unless I called keyboard_init() in coreboot it would never work in the kernel because it would come up as an "AT Raw" device instead of an "AT Translated".
In order to keep the keyboard untouched in coreboot (to try and minimize issues with keys pressed at boot) I put the workaround into the payload to put it into translate mode.
Thank you very much for the detailed answer. Could you please update the commit message accordingly?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31659 )
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
Patch Set 1: Code-Review+2
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31659 )
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1: Code-Review+1
Any idea why it was added in the first place?
The keyboard controller does not come up in translated mode, so unless I called keyboard_init() in coreboot it would never work in the kernel because it would come up as an "AT Raw" device instead of an "AT Translated".
In order to keep the keyboard untouched in coreboot (to try and minimize issues with keys pressed at boot) I put the workaround into the payload to put it into translate mode.
Thank you very much for the detailed answer. Could you please update the commit message accordingly?
Done.
Hello Paul Menzel, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31659
to look at the new patch set (#2).
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD
In order to prevent keyboard keys pressed at boot from causing issues in the payload remove the PS2 keyboard driver so it does not get initialized until it is needed in libpayload.
This was enabled initially because the keyboard controller on this platform does not come up in translated mode, so unless coreboot called keyboard_init() the keyboard would never work properly in the kernel because it would come up as an "AT Raw" device instead of an "AT Translated" device.
Instead of initializing the keyboard in coreboot a workaround is added to the payload to put the keyboard into translated mode.
BUG=b:126633269 TEST=boot on sarien while pressing keys and ensure libpayload and/or the kernel does not have any issues initializing the keyboard.
Change-Id: I765e808f0d2589cf23c0349798a07e2706a2a7a4 Signed-off-by: Duncan Laurie dlaurie@google.com --- M src/mainboard/google/sarien/Kconfig 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/31659/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31659 )
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31659 )
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
Patch Set 2: Code-Review+1
Thank you.
Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31659 )
Change subject: mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD ......................................................................
mb/google/sarien: Remove DRIVERS_PS2_KEYBOARD
In order to prevent keyboard keys pressed at boot from causing issues in the payload remove the PS2 keyboard driver so it does not get initialized until it is needed in libpayload.
This was enabled initially because the keyboard controller on this platform does not come up in translated mode, so unless coreboot called keyboard_init() the keyboard would never work properly in the kernel because it would come up as an "AT Raw" device instead of an "AT Translated" device.
Instead of initializing the keyboard in coreboot a workaround is added to the payload to put the keyboard into translated mode.
BUG=b:126633269 TEST=boot on sarien while pressing keys and ensure libpayload and/or the kernel does not have any issues initializing the keyboard.
Change-Id: I765e808f0d2589cf23c0349798a07e2706a2a7a4 Signed-off-by: Duncan Laurie dlaurie@google.com Reviewed-on: https://review.coreboot.org/c/31659 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 src/mainboard/google/sarien/Kconfig 1 file changed, 0 insertions(+), 1 deletion(-)
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/src/mainboard/google/sarien/Kconfig b/src/mainboard/google/sarien/Kconfig index a976d36..ce6b810 100644 --- a/src/mainboard/google/sarien/Kconfig +++ b/src/mainboard/google/sarien/Kconfig @@ -5,7 +5,6 @@ select DRIVERS_I2C_GENERIC select DRIVERS_I2C_HID select DRIVERS_SPI_ACPI - select DRIVERS_PS2_KEYBOARD select DRIVERS_USB_ACPI select EC_GOOGLE_WILCO select GENERIC_SPD_BIN
-
Duncan Laurie (Code Review) -
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)