Matt Delco has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32256
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
libpayload: keyboard: Add F11 and F12 support
The firmware is basically ignoring F11 and F12 without this change. This existing source file doesn't see to comply with the automatic formatter, and it wants to make a lot of unrelated line changes so I've disabled it.
BUG=b:130143385 TEST=local compile and flash to device. Confirmed that press of F11 and F12 keys now generates appropriate keypress events (and the same codes that are already generated by these keys on an external USB keyboard).
Signed-off-by: Matt Delco delco@chromium.org Change-Id: Ic43114aa99fc0a1345782c81ed2b90f5569af383 --- M payloads/libpayload/drivers/i8042/keyboard.c 1 file changed, 18 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/32256/1
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index be22497..1035bf2 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -38,7 +38,7 @@
struct layout_maps { const char *country; - const unsigned short map[4][0x57]; + const unsigned short map[4][0x59]; };
static struct layout_maps *map; @@ -58,7 +58,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), }, { /* Shift */ 0x00, 0x1B, 0x21, 0x40, 0x23, 0x24, 0x25, 0x5E, @@ -71,7 +72,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), }, { /* ALT */ 0x00, 0x1B, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, @@ -84,7 +86,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), }, { /* Shift-ALT */ 0x00, 0x1B, 0x21, 0x40, 0x23, 0x24, 0x25, 0x5E, @@ -97,7 +100,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), } }}, #endif @@ -114,7 +118,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x3C + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x3C, KEY_F(11), + KEY_F(12), }, { /* Shift */ 0x00, 0x1B, 0x21, 0x22, 0xA7, 0x24, 0x25, 0x26, @@ -127,7 +132,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x3E + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x3E, KEY_F(11), + KEY_F(12), }, { /* ALT */ 0x00, 0x1B, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, @@ -140,7 +146,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x7C + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x7C, KEY_F(11), + KEY_F(12), }, { /* Shift-ALT */ /* copied from US */ @@ -154,7 +161,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), } }}, #endif @@ -232,7 +240,7 @@
ch = keyboard_get_scancode();
- if (!(ch & 0x80) && ch < 0x57) { + if (!(ch & 0x80) && ch < 0x59) { shift = (modifier & KB_MOD_SHIFT) ^ (modifier & KB_MOD_CAPSLOCK) ? 1 : 0;
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32256 )
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
Patch Set 1:
This change gets F11 and F12 working on PS/2 keyboards (USB already works), though I really only care about F12. From what I've read scan code set 2 is more typical but the PS/2 driver uses scan code set 1:
https://wiki.osdev.org/PS/2_Keyboard#Scan_Code_Set_1
Matt Delco has removed Name of user not set #1000432 from this change. ( https://review.coreboot.org/c/coreboot/+/32256 )
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
Removed reviewer null.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32256 )
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32256 )
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/32256/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32256/1//COMMIT_MSG@10 PS1, Line 10: see seem
https://review.coreboot.org/#/c/32256/1//COMMIT_MSG@10 PS1, Line 10: This existing source file doesn't see to comply with the automatic formatter, : and it wants to make a lot of unrelated line changes so I've disabled : it. Please add a blank line above. Maybe even remove the paragraph.
https://review.coreboot.org/#/c/32256/1/payloads/libpayload/drivers/i8042/ke... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/#/c/32256/1/payloads/libpayload/drivers/i8042/ke... PS1, Line 61: KEY_F(11), : KEY_F(12) Why not add it directly behind `KEY_F(10)`.
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32256 )
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32256/1/payloads/libpayload/drivers/i8042/ke... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/#/c/32256/1/payloads/libpayload/drivers/i8042/ke... PS1, Line 61: KEY_F(11), : KEY_F(12)
Why not add it directly behind `KEY_F(10)`.
The original IBM keyboard only had F1-F10, so in PS/2 scancode the positions after KEY_F(10) are for num lock and scroll lock (example ps/2 doc cited in an earlier comment). When F11 and F12 were added they added it at the end of the codes. I looked through the zeros. Most are undefined (apparently the 10-key is stuck in non-numeric mode, so '5' has no meaning) or for modifier keys (shift, alt, ctrl). Potential keys that could be populated is for offset 0x4a (minus) and 0x4E (plus). To me it seems backwards that the page down key is mapped to KEY_PPAGE and page up key mapped to KEY_NPAGE, but the rest of the code seems to be backwards to cope (often with explanatory comments).
Hello Duncan Laurie, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32256
to look at the new patch set (#2).
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
libpayload: keyboard: Add F11 and F12 support
The firmware is basically ignoring F11 and F12 without this change.
BUG=b:130143385 TEST=local compile and flash to device. Confirmed that press of F11 and F12 keys now generates appropriate keypress events (and the same codes that are already generated by these keys on an external USB keyboard).
Signed-off-by: Matt Delco delco@chromium.org Change-Id: Ic43114aa99fc0a1345782c81ed2b90f5569af383 --- M payloads/libpayload/drivers/i8042/keyboard.c 1 file changed, 18 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/32256/2
Matt Delco has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32256 )
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/32256/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32256/1//COMMIT_MSG@10 PS1, Line 10: see
seem
Ack
https://review.coreboot.org/#/c/32256/1//COMMIT_MSG@10 PS1, Line 10: This existing source file doesn't see to comply with the automatic formatter, : and it wants to make a lot of unrelated line changes so I've disabled : it.
Please add a blank line above. Maybe even remove the paragraph.
Done
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32256 )
Change subject: libpayload: keyboard: Add F11 and F12 support ......................................................................
libpayload: keyboard: Add F11 and F12 support
The firmware is basically ignoring F11 and F12 without this change.
BUG=b:130143385 TEST=local compile and flash to device. Confirmed that press of F11 and F12 keys now generates appropriate keypress events (and the same codes that are already generated by these keys on an external USB keyboard).
Signed-off-by: Matt Delco delco@chromium.org Change-Id: Ic43114aa99fc0a1345782c81ed2b90f5569af383 Reviewed-on: https://review.coreboot.org/c/coreboot/+/32256 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M payloads/libpayload/drivers/i8042/keyboard.c 1 file changed, 18 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index be22497..1035bf2 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -38,7 +38,7 @@
struct layout_maps { const char *country; - const unsigned short map[4][0x57]; + const unsigned short map[4][0x59]; };
static struct layout_maps *map; @@ -58,7 +58,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), }, { /* Shift */ 0x00, 0x1B, 0x21, 0x40, 0x23, 0x24, 0x25, 0x5E, @@ -71,7 +72,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), }, { /* ALT */ 0x00, 0x1B, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, @@ -84,7 +86,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), }, { /* Shift-ALT */ 0x00, 0x1B, 0x21, 0x40, 0x23, 0x24, 0x25, 0x5E, @@ -97,7 +100,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), } }}, #endif @@ -114,7 +118,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x3C + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x3C, KEY_F(11), + KEY_F(12), }, { /* Shift */ 0x00, 0x1B, 0x21, 0x22, 0xA7, 0x24, 0x25, 0x26, @@ -127,7 +132,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x3E + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x3E, KEY_F(11), + KEY_F(12), }, { /* ALT */ 0x00, 0x1B, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, @@ -140,7 +146,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x7C + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x7C, KEY_F(11), + KEY_F(12), }, { /* Shift-ALT */ /* copied from US */ @@ -154,7 +161,8 @@ 0x00, 0x20, 0x00, KEY_F(1), KEY_F(2), KEY_F(3), KEY_F(4), KEY_F(5), KEY_F(6), KEY_F(7), KEY_F(8), KEY_F(9), KEY_F(10), 0x00, 0x00, KEY_HOME, KEY_UP, KEY_NPAGE, 0x00, KEY_LEFT, 0x00, KEY_RIGHT, 0x00, KEY_END, - KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00 + KEY_DOWN, KEY_PPAGE, 0x00, KEY_DC, 0x00, 0x00, 0x00, KEY_F(11), + KEY_F(12), } }}, #endif @@ -232,7 +240,7 @@
ch = keyboard_get_scancode();
- if (!(ch & 0x80) && ch < 0x57) { + if (!(ch & 0x80) && ch < 0x59) { shift = (modifier & KB_MOD_SHIFT) ^ (modifier & KB_MOD_CAPSLOCK) ? 1 : 0;
-
Furquan Shaikh (Code Review) -
Matt Delco (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)