Change in coreboot[master]: libpayload/drivers/i8042: Add AT translated Keyboard support
EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
libpayload/drivers/i8042: Add AT translated Keyboard support
Wilco device use the AT translated keyboard and don't need to set scancode set. Remove the ignore flag and put into translation mode instead.
BUG=b:145130110 TEST=Draillion keyboard is usable on every boot.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ie1053e24e44c5bad28b56cc92d091e24f3d9b6fd --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 3 files changed, 38 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/37594/1
diff --git a/payloads/libpayload/Kconfig b/payloads/libpayload/Kconfig index d216f61..f7501e3 100644 --- a/payloads/libpayload/Kconfig +++ b/payloads/libpayload/Kconfig @@ -343,8 +343,8 @@ 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" +config PC_KEYBOARD_AT_TRANSLATED + bool "AT Translation keyboard device" default n
config PC_KEYBOARD_LAYOUT_US diff --git a/payloads/libpayload/drivers/i8042/i8042.h b/payloads/libpayload/drivers/i8042/i8042.h index 643167e..f039569 100644 --- a/payloads/libpayload/drivers/i8042/i8042.h +++ b/payloads/libpayload/drivers/i8042/i8042.h @@ -33,6 +33,7 @@ /* Port 0x64 commands */ #define I8042_CMD_RD_CMD_BYTE 0x20 #define I8042_CMD_WR_CMD_BYTE 0x60 +#define I8042_CMD_BYTE_XLATE (1 << 6) #define I8042_CMD_DIS_AUX 0xa7 #define I8042_CMD_EN_AUX 0xa8 #define I8042_CMD_AUX_TEST 0xa9 diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index f9932ed..31ef384 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -328,34 +328,44 @@ /* Enable first PS/2 port */ i8042_cmd(I8042_CMD_EN_KB);
- /* Set scancode set 1 */ - ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); - if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) { - printf("ERROR: Keyboard set scancode failed!\n"); - return; - } + /* Enable keyboard translated */ + if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) { + /* Put keyboard into translated mode for the OS */ + 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 { /* Set scancode set 1 */ + ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); + if (!ret) { + printf("ERROR: Keyboard set scancode failed!\n"); + return; + }
- ret = keyboard_cmd(I8042_SCANCODE_SET_1); - if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) { - printf("ERROR: Keyboard scancode set#1 failed!\n"); - return; - } + ret = keyboard_cmd(I8042_SCANCODE_SET_1); + if (!ret) { + printf("ERROR: Keyboard scancode set#1 failed!\n"); + return; + }
- /* - * Set default parameters. - * Fix for broken QEMU ps/2 make scancodes. - */ - ret = keyboard_cmd(0xf6); - if (!ret) { - printf("ERROR: Keyboard set default params failed!\n"); - return; - } + /* + * Set default parameters. + * Fix for broken QEMU ps/2 make scancodes. + */ + ret = keyboard_cmd(0xf6); + if (!ret) { + printf("ERROR: Keyboard set default params failed!\n"); + return; + }
- /* Enable scanning */ - ret = keyboard_cmd(I8042_KBCMD_EN); - if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) { - printf("ERROR: Keyboard enable scanning failed!\n"); - return; + /* Enable scanning */ + ret = keyboard_cmd(I8042_KBCMD_EN); + if (!ret) { + printf("ERROR: Keyboard enable scanning failed!\n"); + return; + } }
console_add_input_driver(&cons);
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 1:
Please review this and give some comment on it if any. thanks.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37594/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37594/1//COMMIT_MSG@9 PS1, Line 9: don't doesn’t
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... PS1, Line 332: if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) { How is the Linux kernel able to detect that itself, or initialize all PS/2 keyboards?
Hello Mathew King, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37594
to look at the new patch set (#2).
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
libpayload/drivers/i8042: Add AT translated Keyboard support
Wilco device use the AT translated keyboard and doesn't need to set scancode set. Remove the ignore flag and put into translation mode instead.
BUG=b:145130110 TEST=Draillion keyboard is usable on every boot.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ie1053e24e44c5bad28b56cc92d091e24f3d9b6fd --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 3 files changed, 38 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/37594/2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37594/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37594/1//COMMIT_MSG@9 PS1, Line 9: don't
doesn’t
Done
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... PS1, Line 332: if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) {
How is the Linux kernel able to detect that itself, or initialize all PS/2 keyboards?
Acturally we enable this before enter OS again. I think kernel doesn't detect this. I guess you must binding the particular KB driver. But we use common driver here.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... PS1, Line 332: if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) {
Acturally we enable this before enter OS again. I think kernel doesn't detect this. […]
Sorry, I do not understand your answer. Could you please rephrase it?
I mean, the Linux kernel is able to initialize the device on all Chromebook devices, isn’t it?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... PS1, Line 332: if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) {
Sorry, I do not understand your answer. Could you please rephrase it? […]
This EC is not chrome EC. So this need special case for it. But in the kernel we use the same driver. Maybe Duncan can help answer this. @Duncan, can we talk wilco EC in public? I am not sure about it.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... PS1, Line 332: if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) {
This EC is not chrome EC. So this need special case for it. […]
@Paul, please look mirror here. https://github.com/coreboot/depthcharge/commit/0227447af1df0da8adf0d048a1829...
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/1/payloads/libpayload/drivers... PS1, Line 332: if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) {
@Paul, please look mirror here. […]
Ah, thanks. I thought would be able to handle that.
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 2:
(1 comment)
I am not opposed to this change, however we need to be careful to not break Chromium ToT by removing the IGNORE_INIT_FAILURE flag. We should add the new flag and migrate to it before removing the old flag. Also have you verified that all keys work as expected in depthcharge including F12 and the arrow keys?
https://review.coreboot.org/c/coreboot/+/37594/2/payloads/libpayload/Kconfig File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/37594/2/payloads/libpayload/Kconfig... PS2, Line 346: PC_KEYBOARD_IGNORE_INIT_FAILURE We should keep this in place until we can remove uses of it from the Chromium codebase.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
I am not opposed to this change, however we need to be careful to not break Chromium ToT by removing the IGNORE_INIT_FAILURE flag. We should add the new flag and migrate to it before removing the old flag. Also have you verified that all keys work as expected in depthcharge including F12 and the arrow keys?
I can test it with Asset ID and diag mode.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/2/payloads/libpayload/Kconfig File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/37594/2/payloads/libpayload/Kconfig... PS2, Line 346: PC_KEYBOARD_IGNORE_INIT_FAILURE
We should keep this in place until we can remove uses of it from the Chromium codebase.
okay.
Hello Mathew King, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37594
to look at the new patch set (#3).
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
libpayload/drivers/i8042: Add AT translated Keyboard support
Wilco device use the AT translated keyboard and doesn't need to set scancode set. Remove the ignore flag and put into translation mode instead.
BUG=b:145130110 TEST=Draillion keyboard is usable on every boot.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ie1053e24e44c5bad28b56cc92d091e24f3d9b6fd --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 3 files changed, 39 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/37594/3
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/2/payloads/libpayload/Kconfig File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/37594/2/payloads/libpayload/Kconfig... PS2, Line 346: PC_KEYBOARD_IGNORE_INIT_FAILURE
okay.
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37594/3//COMMIT_MSG@9 PS3, Line 9: use uses
Hello Mathew King, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37594
to look at the new patch set (#4).
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
libpayload/drivers/i8042: Add AT translated Keyboard support
Wilco device uses the AT translated keyboard and doesn't need to set scancode set. Remove the ignore flag and put into translation mode instead.
BUG=b:145130110 TEST=Draillion keyboard is usable on every boot.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ie1053e24e44c5bad28b56cc92d091e24f3d9b6fd --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 3 files changed, 39 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/37594/4
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37594/3//COMMIT_MSG@9 PS3, Line 9: use
uses
Done
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/4/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/4/payloads/libpayload/drivers... PS4, Line 332: /* Enable keyboard translated */ : 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); : } maybe pull this into a separate function.
If the first i8042_cmd(I8042_CD_RD_CMD_BYTE) fails should it fall down the "set scancode set 1" path? Or at least print an error message...
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37594/4/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/4/payloads/libpayload/drivers... PS4, Line 332: /* Enable keyboard translated */ : 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); : }
maybe pull this into a separate function. […]
oh, okay. I'll separate this. Maybe separate set scan code set as well?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 5:
(1 comment)
I tested with recovery mode and OS. KB is work as expect.
https://review.coreboot.org/c/coreboot/+/37594/4/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/4/payloads/libpayload/drivers... PS4, Line 332: /* Enable keyboard translated */ : 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); : }
oh, okay. I'll separate this. […]
Done
Hello Mathew King, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37594
to look at the new patch set (#6).
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
libpayload/drivers/i8042: Add AT translated Keyboard support
Wilco device uses the AT translated keyboard and doesn't need to set scancode set. Remove the ignore flag and put into translation mode instead.
BUG=b:145130110 TEST=Draillion keyboard is usable on every boot.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ie1053e24e44c5bad28b56cc92d091e24f3d9b6fd --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 3 files changed, 62 insertions(+), 29 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/37594/6
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37594/6/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/6/payloads/libpayload/drivers... PS6, Line 382: if(!enable_translated()) space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/37594/6/payloads/libpayload/drivers... PS6, Line 385: if(!set_scancode_set()) space required before the open parenthesis '('
Hello Mathew King, Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37594
to look at the new patch set (#7).
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
libpayload/drivers/i8042: Add AT translated Keyboard support
Wilco device uses the AT translated keyboard and doesn't need to set scancode set. Remove the ignore flag and put into translation mode instead.
BUG=b:145130110 TEST=Draillion keyboard is usable on every boot.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ie1053e24e44c5bad28b56cc92d091e24f3d9b6fd --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 3 files changed, 62 insertions(+), 29 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/37594/7
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37594/6/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/6/payloads/libpayload/drivers... PS6, Line 382: if(!enable_translated())
space required before the open parenthesis '('
Done
https://review.coreboot.org/c/coreboot/+/37594/6/payloads/libpayload/drivers... PS6, Line 385: if(!set_scancode_set())
space required before the open parenthesis '('
Done
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 7: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
libpayload/drivers/i8042: Add AT translated Keyboard support
Wilco device uses the AT translated keyboard and doesn't need to set scancode set. Remove the ignore flag and put into translation mode instead.
BUG=b:145130110 TEST=Draillion keyboard is usable on every boot.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ie1053e24e44c5bad28b56cc92d091e24f3d9b6fd Reviewed-on: https://review.coreboot.org/c/coreboot/+/37594 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Mathew King mathewk@chromium.org --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 3 files changed, 62 insertions(+), 29 deletions(-)
Approvals: build bot (Jenkins): Verified Mathew King: Looks good to me, approved
diff --git a/payloads/libpayload/Kconfig b/payloads/libpayload/Kconfig index d216f61..347ccac 100644 --- a/payloads/libpayload/Kconfig +++ b/payloads/libpayload/Kconfig @@ -347,6 +347,10 @@ bool "Ignore keyboard failures during init and always add input device" 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/i8042.h b/payloads/libpayload/drivers/i8042/i8042.h index 643167e..f039569 100644 --- a/payloads/libpayload/drivers/i8042/i8042.h +++ b/payloads/libpayload/drivers/i8042/i8042.h @@ -33,6 +33,7 @@ /* Port 0x64 commands */ #define I8042_CMD_RD_CMD_BYTE 0x20 #define I8042_CMD_WR_CMD_BYTE 0x60 +#define I8042_CMD_BYTE_XLATE (1 << 6) #define I8042_CMD_DIS_AUX 0xa7 #define I8042_CMD_EN_AUX 0xa8 #define I8042_CMD_AUX_TEST 0xa9 diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index f9932ed..4b4a569 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -312,9 +312,59 @@ .input_type = CONSOLE_INPUT_TYPE_EC, };
-void keyboard_init(void) +/* Enable keyboard translated */ +static int enable_translated(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: Keyboard i8042_cmd failed!\n"); + return 0; + } + return 1; +} + +/* Set scancode set 1 */ +static int set_scancode_set(void) { unsigned int 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); + 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"); + return ret; + } + + return ret; +} + +void keyboard_init(void) +{ map = &keyboard_layouts[0];
/* Initialized keyboard controller. */ @@ -328,34 +378,12 @@ /* Enable first PS/2 port */ i8042_cmd(I8042_CMD_EN_KB);
- /* Set scancode set 1 */ - ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); - if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) { - printf("ERROR: Keyboard set scancode failed!\n"); - return; - } - - ret = keyboard_cmd(I8042_SCANCODE_SET_1); - if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) { - printf("ERROR: Keyboard scancode set#1 failed!\n"); - return; - } - - /* - * Set default parameters. - * Fix for broken QEMU ps/2 make scancodes. - */ - ret = keyboard_cmd(0xf6); - if (!ret) { - printf("ERROR: Keyboard set default params failed!\n"); - return; - } - - /* Enable scanning */ - ret = keyboard_cmd(I8042_KBCMD_EN); - if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) { - printf("ERROR: Keyboard enable scanning failed!\n"); - return; + if (CONFIG(LP_PC_KEYBOARD_AT_TRANSLATED)) { + if (!enable_translated()) + return; + } else { + if (!set_scancode_set()) + return; }
console_add_input_driver(&cons);
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37594/7/payloads/libpayload/Kconfig File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/37594/7/payloads/libpayload/Kconfig... PS7, Line 346: config PC_KEYBOARD_IGNORE_INIT_FAILURE : bool "Ignore keyboard failures during init and always add input device" : default n Isn't this config effectively obsolete now?
https://review.coreboot.org/c/coreboot/+/37594/7/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/7/payloads/libpayload/drivers... PS7, Line 321: if (!i8042_cmd(I8042_CMD_WR_CMD_BYTE)) What if this fails? Don't you want to return 0 in that case?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37594 )
Change subject: libpayload/drivers/i8042: Add AT translated Keyboard support ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37594/7/payloads/libpayload/Kconfig File payloads/libpayload/Kconfig:
https://review.coreboot.org/c/coreboot/+/37594/7/payloads/libpayload/Kconfig... PS7, Line 346: config PC_KEYBOARD_IGNORE_INIT_FAILURE : bool "Ignore keyboard failures during init and always add input device" : default n
Isn't this config effectively obsolete now?
I will remove this after overlay CL is merged.
https://review.coreboot.org/c/coreboot/+/37594/7/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/37594/7/payloads/libpayload/drivers... PS7, Line 321: if (!i8042_cmd(I8042_CMD_WR_CMD_BYTE))
What if this fails? Don't you want to return 0 in that case?
I am not sure. I think it's okay for check. In i8042 it check all the command as well. I will patch it.
-
build bot (Jenkins) (Code Review) -
Duncan Laurie (Code Review)
-
EricR Lai (Code Review)
-
Furquan Shaikh (Code Review)
-
Mathew King (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)