[U-Boot] [PATCH v4 0/7] usb: kbd: implement special keys

GRUB uses function keys. So we should support these with an USB keyboard. Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. Simplify the code beforehand.
Enhance the keyboard unit test.
In total I could not see any increase of u-boot.img on the TBS2910 but as the special keys are only needed in the context of the UEFI subsystem it makes sense to save several hundred bytes on other boards.
size of u-boot.bin for tbs2910_defconfig 390080 with patch usb: kbd: implement special keys 389968 with all my patches 389968 tbs_defconfig, function keys enabled 390080
v4: reduce code size by avoiding string constants and introducing a configuration switch CONFIG_USB_KEYBOARD_FN_KEYS v3: rebase on current git HEAD v2: enhance the keyboard unit test
Heinrich Schuchardt (7): usb: kbd: fix typo usb: kbd: signature of usb_kbd_put_queue() usb: kbd: simplify coding for arrow keys usb: kbd: implement special keys usb: kbd: move USB_KBD_BOOT_REPORT_SIZE to usb.h dm: test: usb: rework keyboard test sandbox: enable USB_KEYBOARD_FN_KEYS
common/usb_kbd.c | 97 +++++----- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + drivers/usb/Kconfig | 6 + drivers/usb/emul/sandbox_keyb.c | 27 ++- include/usb.h | 6 + test/dm/usb.c | 284 ++++++++++++++++++++++++++++- 9 files changed, 364 insertions(+), 60 deletions(-)
-- 2.24.0

%s/a interrupt/an interrupt/
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Simon Glass sjg@chromium.org --- v4: no change v3: no change v2: new patch --- common/usb_kbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index d178af248a..8c09e61f45 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -339,7 +339,7 @@ static inline void usb_kbd_poll_for_event(struct usb_device *dev) #if defined(CONFIG_SYS_USB_EVENT_POLL) struct usb_kbd_pdata *data = dev->privptr;
- /* Submit a interrupt transfer request */ + /* Submit an interrupt transfer request */ if (usb_int_msg(dev, data->intpipe, &data->new[0], data->intpktsize, data->intinterval, true) >= 0) usb_kbd_irq_worker(dev); -- 2.24.0

usb_kbd_buffer is defined as u8[]. So let usb_kbd_put_queue() use u8 as type of the parameter for the new byte.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v4: new patch --- common/usb_kbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 8c09e61f45..e4711eb655 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -127,7 +127,7 @@ extern int __maybe_unused net_busy_flag; static unsigned long __maybe_unused kbd_testc_tms;
/* Puts character in the queue and sets up the in and out pointer. */ -static void usb_kbd_put_queue(struct usb_kbd_pdata *data, char c) +static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c) { if (data->usb_in_pointer == USB_KBD_BUFFER_LEN - 1) { /* Check for buffer full. */ -- 2.24.0

Avoid duplicate translation of arrow key codes.
Reduce code size by avoiding strings and eliminating usb_kbd_put_sequence().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v4: Reduce code size by avoiding strings. v3: no change v2: no change --- common/usb_kbd.c | 47 ++++++++++++----------------------------------- 1 file changed, 12 insertions(+), 35 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index e4711eb655..d177b97d67 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -75,13 +75,8 @@ static const unsigned char usb_kbd_num_keypad[] = { '.', 0, 0, 0, '=' };
-/* - * map arrow keys to ^F/^B ^N/^P, can't really use the proper - * ANSI sequence for arrow keys because the queuing code breaks - * when a single keypress expands to 3 queue elements - */ -static const unsigned char usb_kbd_arrow[] = { - 0x6, 0x2, 0xe, 0x10 +static const u8 usb_special_keys[] = { + 'C', 'D', 'B', 'A' };
/* @@ -146,12 +141,6 @@ static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c) data->usb_kbd_buffer[data->usb_in_pointer] = c; }
-static void usb_kbd_put_sequence(struct usb_kbd_pdata *data, char *s) -{ - for (; *s; s++) - usb_kbd_put_queue(data, *s); -} - /* * Set the LEDs. Since this is used in the irq routine, the control job is * issued with a timeout of 0. This means, that the job is queued without @@ -214,10 +203,6 @@ static int usb_kbd_translate(struct usb_kbd_pdata *data, unsigned char scancode, keycode = usb_kbd_numkey[scancode - 0x1e]; }
- /* Arrow keys */ - if ((scancode >= 0x4f) && (scancode <= 0x52)) - keycode = usb_kbd_arrow[scancode - 0x4f]; - /* Numeric keypad */ if ((scancode >= 0x54) && (scancode <= 0x67)) keycode = usb_kbd_num_keypad[scancode - 0x54]; @@ -242,28 +227,20 @@ static int usb_kbd_translate(struct usb_kbd_pdata *data, unsigned char scancode, }
/* Report keycode if any */ - if (keycode) + if (keycode) { debug("%c", keycode); - - switch (keycode) { - case 0x0e: /* Down arrow key */ - usb_kbd_put_sequence(data, "\e[B"); - break; - case 0x10: /* Up arrow key */ - usb_kbd_put_sequence(data, "\e[A"); - break; - case 0x06: /* Right arrow key */ - usb_kbd_put_sequence(data, "\e[C"); - break; - case 0x02: /* Left arrow key */ - usb_kbd_put_sequence(data, "\e[D"); - break; - default: usb_kbd_put_queue(data, keycode); - break; + return 0; }
- return 0; + /* Left, Right, Up, Down */ + if (scancode > 0x4e && scancode < 0x53) { + usb_kbd_put_queue(data, 0x1b); + usb_kbd_put_queue(data, '['); + usb_kbd_put_queue(data, usb_special_keys[scancode - 0x4f]); + return 0; + } + return 1; }
static uint32_t usb_kbd_service_key(struct usb_device *dev, int i, int up) -- 2.24.0

Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down.
As this leads to a size increase provide a customizing setting CONFIG_USB_KEYBOARD_FN_KEYS.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v4: reduce code size by avoiding string constants v3: no change v2: no change --- common/usb_kbd.c | 42 ++++++++++++++++++++++++++++++++++++++++++ drivers/usb/Kconfig | 6 ++++++ 2 files changed, 48 insertions(+)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index d177b97d67..d6b340bbe7 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -76,7 +76,11 @@ static const unsigned char usb_kbd_num_keypad[] = { };
static const u8 usb_special_keys[] = { +#ifdef CONFIG_USB_KEYBOARD_FN_KEYS + '2', 'H', '5', '3', 'F', '6', 'C', 'D', 'B', 'A' +#else 'C', 'D', 'B', 'A' +#endif };
/* @@ -233,6 +237,43 @@ static int usb_kbd_translate(struct usb_kbd_pdata *data, unsigned char scancode, return 0; }
+#ifdef CONFIG_USB_KEYBOARD_FN_KEYS + if (scancode < 0x3a || scancode > 0x52 || + scancode == 0x46 || scancode == 0x47) + return 1; + + usb_kbd_put_queue(data, 0x1b); + if (scancode < 0x3e) { + /* F1 - F4 */ + usb_kbd_put_queue(data, 0x4f); + usb_kbd_put_queue(data, scancode - 0x3a + 'P'); + return 0; + } + usb_kbd_put_queue(data, '['); + if (scancode < 0x42) { + /* F5 - F8 */ + usb_kbd_put_queue(data, '1'); + if (scancode == 0x3e) + --scancode; + keycode = scancode - 0x3f + '7'; + } else if (scancode < 0x49) { + /* F9 - F12 */ + usb_kbd_put_queue(data, '2'); + if (scancode > 0x43) + ++scancode; + keycode = scancode - 0x42 + '0'; + } else { + /* + * INSERT, HOME, PAGE UP, DELETE, END, PAGE DOWN, + * RIGHT, LEFT, DOWN, UP + */ + keycode = usb_special_keys[scancode - 0x49]; + } + usb_kbd_put_queue(data, keycode); + if (scancode < 0x4f && scancode != 0x4a && scancode != 0x4d) + usb_kbd_put_queue(data, '~'); + return 0; +#else /* Left, Right, Up, Down */ if (scancode > 0x4e && scancode < 0x53) { usb_kbd_put_queue(data, 0x1b); @@ -241,6 +282,7 @@ static int usb_kbd_translate(struct usb_kbd_pdata *data, unsigned char scancode, return 0; } return 1; +#endif /* CONFIG_USB_KEYBOARD_FN_KEYS */ }
static uint32_t usb_kbd_service_key(struct usb_device *dev, int i, int up) diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig index 9af78e8822..bea4a92b61 100644 --- a/drivers/usb/Kconfig +++ b/drivers/usb/Kconfig @@ -100,6 +100,12 @@ config USB_KEYBOARD
if USB_KEYBOARD
+config USB_KEYBOARD_FN_KEYS + bool "USB keyboard function key support" + help + Say Y here if you want support for keys F1 - F12, INS, HOME, DELETE, + END, PAGE UP, and PAGE DOWN. + choice prompt "USB keyboard polling" default SYS_USB_EVENT_POLL -- 2.24.0

Move constant USB_KBD_BOOT_REPORT_SIZE. This allows us to reuse it.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Simon Glass sjg@chromium.org --- v4: no change v3: no change v2: new patch --- common/usb_kbd.c | 6 ------ include/usb.h | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index d6b340bbe7..a6221ef716 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -95,12 +95,6 @@ static const u8 usb_special_keys[] = { #define USB_KBD_LEDMASK \ (USB_KBD_NUMLOCK | USB_KBD_CAPSLOCK | USB_KBD_SCROLLLOCK)
-/* - * USB Keyboard reports are 8 bytes in boot protocol. - * Appendix B of HID Device Class Definition 1.11 - */ -#define USB_KBD_BOOT_REPORT_SIZE 8 - struct usb_kbd_pdata { unsigned long intpipe; int intpktsize; diff --git a/include/usb.h b/include/usb.h index bcad552f85..efb67ea33f 100644 --- a/include/usb.h +++ b/include/usb.h @@ -242,6 +242,12 @@ int usb_host_eth_scan(int mode);
#ifdef CONFIG_USB_KEYBOARD
+/* + * USB Keyboard reports are 8 bytes in boot protocol. + * Appendix B of HID Device Class Definition 1.11 + */ +#define USB_KBD_BOOT_REPORT_SIZE 8 + int drv_usb_kbd_init(void); int usb_kbd_deregister(int force);
-- 2.24.0

Allow the unit test to pass full 8 byte scan code sequences to the USB keyboard emulation driver and to parse multi-byte escape sequences.
The following features are not yet tested:
* LED status * caps-lock * num-lock * numerical pad keys
The following features are not yet implemented by the USB keyboard driver and therefore not tested:
* modifiers for non-alpha-numeric keys, e.g. <SHIFT><TAB> and <ALT><F4> * some special keys, e.g. <PRINT> * some modifiers, e.g. <ALT> and <META> * alternative keyboard layouts
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v4: adjust test scope to CONFIG_USB_KEYBOARD_FN_KEYS v3: no change v2: new patch --- drivers/usb/emul/sandbox_keyb.c | 27 +-- test/dm/usb.c | 284 +++++++++++++++++++++++++++++++- 2 files changed, 293 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/emul/sandbox_keyb.c b/drivers/usb/emul/sandbox_keyb.c index dc43880d27..32bc9a1698 100644 --- a/drivers/usb/emul/sandbox_keyb.c +++ b/drivers/usb/emul/sandbox_keyb.c @@ -155,14 +155,20 @@ static void *keyb_desc_list[] = { NULL, };
-int sandbox_usb_keyb_add_string(struct udevice *dev, const char *str) +/** + * sandbox_usb_keyb_add_string() - provide a USB scancode buffer + * + * @dev: the keyboard emulation device + * @scancode: scancode buffer with USB_KBD_BOOT_REPORT_SIZE bytes + */ +int sandbox_usb_keyb_add_string(struct udevice *dev, + const char scancode[USB_KBD_BOOT_REPORT_SIZE]) { struct sandbox_keyb_priv *priv = dev_get_priv(dev); - int len, ret; + int ret;
- len = strlen(str); - ret = membuff_put(&priv->in, str, len); - if (ret != len) + ret = membuff_put(&priv->in, scancode, USB_KBD_BOOT_REPORT_SIZE); + if (ret != USB_KBD_BOOT_REPORT_SIZE) return -ENOSPC;
return 0; @@ -183,12 +189,12 @@ static int sandbox_keyb_interrupt(struct udevice *dev, struct usb_device *udev, { struct sandbox_keyb_priv *priv = dev_get_priv(dev); uint8_t *data = buffer; - int ch;
memset(data, '\0', length); - ch = membuff_getbyte(&priv->in); - if (ch != -1) - data[2] = 4 + ch - 'a'; + if (length < USB_KBD_BOOT_REPORT_SIZE) + return 0; + + membuff_get(&priv->in, buffer, USB_KBD_BOOT_REPORT_SIZE);
return 0; } @@ -213,7 +219,8 @@ static int sandbox_keyb_probe(struct udevice *dev) { struct sandbox_keyb_priv *priv = dev_get_priv(dev);
- return membuff_new(&priv->in, 256); + /* Provide an 80 character keyboard buffer */ + return membuff_new(&priv->in, 80 * USB_KBD_BOOT_REPORT_SIZE); }
static const struct dm_usb_ops sandbox_usb_keyb_ops = { diff --git a/test/dm/usb.c b/test/dm/usb.c index ef454b0ae5..e396c2a0ea 100644 --- a/test/dm/usb.c +++ b/test/dm/usb.c @@ -15,6 +15,12 @@ #include <dm/uclass-internal.h> #include <test/ut.h>
+struct keyboard_test_data { + const char modifiers; + const char scancode; + const char result[6]; +}; + /* Test that sandbox USB works correctly */ static int dm_test_usb_base(struct unit_test_state *uts) { @@ -115,9 +121,263 @@ static int dm_test_usb_stop(struct unit_test_state *uts) } DM_TEST(dm_test_usb_stop, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+/** + * dm_test_usb_keyb() - test USB keyboard driver + * + * This test copies USB keyboard scan codes into the key buffer of the USB + * keyboard emulation driver. These are picked up during emulated interrupts + * by the USB keyboard driver and converted to characters and escape sequences. + * The test then reads and verifies these characters and escape sequences from + * the standard input. + * + * TODO: The following features are not yet tested: + * + * * LED status + * * caps-lock + * * num-lock + * * numerical pad keys + * + * TODO: The following features are not yet implemented by the USB keyboard + * driver and therefore not tested: + * + * * modifiers for non-alpha-numeric keys, e.g. <SHIFT><TAB> and <ALT><F4> + * * some special keys, e.g. <PRINT> + * * some modifiers, e.g. <ALT> and <META> + * * alternative keyboard layouts + * + * @uts: unit test state + * Return: 0 on success + */ static int dm_test_usb_keyb(struct unit_test_state *uts) { struct udevice *dev; + const struct keyboard_test_data *pos; + const struct keyboard_test_data kbd_test_data[] = { + /* <A> */ + {0x00, 0x04, "a"}, + /* <B> */ + {0x00, 0x05, "b"}, + /* <C> */ + {0x00, 0x06, "c"}, + /* <D> */ + {0x00, 0x07, "d"}, + /* <E> */ + {0x00, 0x08, "e"}, + /* <F> */ + {0x00, 0x09, "f"}, + /* <G> */ + {0x00, 0x0a, "g"}, + /* <H> */ + {0x00, 0x0b, "h"}, + /* <I> */ + {0x00, 0x0c, "i"}, + /* <J> */ + {0x00, 0x0d, "j"}, + /* <K> */ + {0x00, 0x0e, "k"}, + /* <L> */ + {0x00, 0x0f, "l"}, + /* <M> */ + {0x00, 0x10, "m"}, + /* <N> */ + {0x00, 0x11, "n"}, + /* <O> */ + {0x00, 0x12, "o"}, + /* <P> */ + {0x00, 0x13, "p"}, + /* <Q> */ + {0x00, 0x14, "q"}, + /* <R> */ + {0x00, 0x15, "r"}, + /* <S> */ + {0x00, 0x16, "s"}, + /* <T> */ + {0x00, 0x17, "t"}, + /* <U> */ + {0x00, 0x18, "u"}, + /* <V> */ + {0x00, 0x19, "v"}, + /* <W> */ + {0x00, 0x1a, "w"}, + /* <X> */ + {0x00, 0x1b, "x"}, + /* <Y> */ + {0x00, 0x1c, "y"}, + /* <Z> */ + {0x00, 0x1d, "z"}, + + /* <LEFT-SHIFT><A> */ + {0x02, 0x04, "A"}, + /* <RIGHT-SHIFT><Z> */ + {0x20, 0x1d, "Z"}, + + /* <LEFT-CONTROL><A> */ + {0x01, 0x04, "\x01"}, + /* <RIGHT-CONTROL><Z> */ + {0x10, 0x1d, "\x1a"}, + + /* <1> */ + {0x00, 0x1e, "1"}, + /* <2> */ + {0x00, 0x1f, "2"}, + /* <3> */ + {0x00, 0x20, "3"}, + /* <4> */ + {0x00, 0x21, "4"}, + /* <5> */ + {0x00, 0x22, "5"}, + /* <6> */ + {0x00, 0x23, "6"}, + /* <7> */ + {0x00, 0x24, "7"}, + /* <8> */ + {0x00, 0x25, "8"}, + /* <9> */ + {0x00, 0x26, "9"}, + /* <0> */ + {0x00, 0x27, "0"}, + + /* <LEFT-SHIFT><1> */ + {0x02, 0x1e, "!"}, + /* <RIGHT-SHIFT><2> */ + {0x20, 0x1f, "@"}, + /* <LEFT-SHIFT><3> */ + {0x02, 0x20, "#"}, + /* <RIGHT-SHIFT><4> */ + {0x20, 0x21, "$"}, + /* <LEFT-SHIFT><5> */ + {0x02, 0x22, "%"}, + /* <RIGHT-SHIFT><6> */ + {0x20, 0x23, "^"}, + /* <LEFT-SHIFT><7> */ + {0x02, 0x24, "&"}, + /* <RIGHT-SHIFT><8> */ + {0x20, 0x25, "*"}, + /* <LEFT-SHIFT><9> */ + {0x02, 0x26, "("}, + /* <RIGHT-SHIFT><0> */ + {0x20, 0x27, ")"}, + + /* <ENTER> */ + {0x00, 0x28, "\r"}, + /* <ESCAPE> */ + {0x00, 0x29, "\x1b"}, + /* <BACKSPACE> */ + {0x00, 0x2a, "\x08"}, + /* <TAB> */ + {0x00, 0x2b, "\x09"}, + /* <SPACE> */ + {0x00, 0x2c, " "}, + /* <MINUS> */ + {0x00, 0x2d, "-"}, + /* <EQUAL> */ + {0x00, 0x2e, "="}, + /* <LEFT BRACE> */ + {0x00, 0x2f, "["}, + /* <RIGHT BRACE> */ + {0x00, 0x30, "]"}, + /* <BACKSLASH> */ + {0x00, 0x31, "\"}, + /* <HASH-TILDE> */ + {0x00, 0x32, "#"}, + /* <SEMICOLON> */ + {0x00, 0x33, ";"}, + /* <APOSTROPHE> */ + {0x00, 0x34, "'"}, + /* <GRAVE> */ + {0x00, 0x35, "`"}, + /* <COMMA> */ + {0x00, 0x36, ","}, + /* <DOT> */ + {0x00, 0x37, "."}, + /* <SLASH> */ + {0x00, 0x38, "/"}, + + /* <LEFT-SHIFT><ENTER> */ + {0x02, 0x28, "\r"}, + /* <RIGHT-SHIFT><ESCAPE> */ + {0x20, 0x29, "\x1b"}, + /* <LEFT-SHIFT><BACKSPACE> */ + {0x02, 0x2a, "\x08"}, + /* <RIGHT-SHIFT><TAB> */ + {0x20, 0x2b, "\x09"}, + /* <LEFT-SHIFT><SPACE> */ + {0x02, 0x2c, " "}, + /* <MINUS> */ + {0x20, 0x2d, "_"}, + /* <LEFT-SHIFT><EQUAL> */ + {0x02, 0x2e, "+"}, + /* <RIGHT-SHIFT><LEFT BRACE> */ + {0x20, 0x2f, "{"}, + /* <LEFT-SHIFT><RIGHT BRACE> */ + {0x02, 0x30, "}"}, + /* <RIGHT-SHIFT><BACKSLASH> */ + {0x20, 0x31, "|"}, + /* <LEFT-SHIFT><HASH-TILDE> */ + {0x02, 0x32, "~"}, + /* <RIGHT-SHIFT><SEMICOLON> */ + {0x20, 0x33, ":"}, + /* <LEFT-SHIFT><APOSTROPHE> */ + {0x02, 0x34, """}, + /* <RIGHT-SHIFT><GRAVE> */ + {0x20, 0x35, "~"}, + /* <LEFT-SHIFT><COMMA> */ + {0x02, 0x36, "<"}, + /* <RIGHT-SHIFT><DOT> */ + {0x20, 0x37, ">"}, + /* <LEFT-SHIFT><SLASH> */ + {0x02, 0x38, "?"}, +#ifdef CONFIG_USB_KEYBOARD_FN_KEYS + /* <F1> */ + {0x00, 0x3a, "\x1bOP"}, + /* <F2> */ + {0x00, 0x3b, "\x1bOQ"}, + /* <F3> */ + {0x00, 0x3c, "\x1bOR"}, + /* <F4> */ + {0x00, 0x3d, "\x1bOS"}, + /* <F5> */ + {0x00, 0x3e, "\x1b[15~"}, + /* <F6> */ + {0x00, 0x3f, "\x1b[17~"}, + /* <F7> */ + {0x00, 0x40, "\x1b[18~"}, + /* <F8> */ + {0x00, 0x41, "\x1b[19~"}, + /* <F9> */ + {0x00, 0x42, "\x1b[20~"}, + /* <F10> */ + {0x00, 0x43, "\x1b[21~"}, + /* <F11> */ + {0x00, 0x44, "\x1b[23~"}, + /* <F12> */ + {0x00, 0x45, "\x1b[24~"}, + /* <INSERT> */ + {0x00, 0x49, "\x1b[2~"}, + /* <HOME> */ + {0x00, 0x4a, "\x1b[H"}, + /* <PAGE UP> */ + {0x00, 0x4b, "\x1b[5~"}, + /* <DELETE> */ + {0x00, 0x4c, "\x1b[3~"}, + /* <END> */ + {0x00, 0x4d, "\x1b[F"}, + /* <PAGE DOWN> */ + {0x00, 0x4e, "\x1b[6~"}, + /* <RIGHT> */ + {0x00, 0x4f, "\x1b[C"}, + /* <LEFT> */ + {0x00, 0x50, "\x1b[D"}, + /* <DOWN> */ + {0x00, 0x51, "\x1b[B"}, + /* <UP> */ + {0x00, 0x52, "\x1b[A"}, +#endif /* CONFIG_USB_KEYBOARD_FN_KEYS */ + + /* End of list */ + {0x00, 0x00, "\0"} + }; +
state_set_skip_delays(true); ut_assertok(usb_init()); @@ -129,16 +389,24 @@ static int dm_test_usb_keyb(struct unit_test_state *uts) &dev));
/* - * Add a string to the USB keyboard buffer - it should appear in - * stdin + * Add scan codes to the USB keyboard buffer. They should appear as + * corresponding characters and escape sequences in stdin. */ - ut_assertok(sandbox_usb_keyb_add_string(dev, "ab")); - ut_asserteq(1, tstc()); - ut_asserteq('a', getc()); - ut_asserteq(1, tstc()); - ut_asserteq('b', getc()); - ut_asserteq(0, tstc()); + for (pos = kbd_test_data; pos->scancode; ++pos) { + const char *c; + char scancodes[USB_KBD_BOOT_REPORT_SIZE] = {0}; + + scancodes[0] = pos->modifiers; + scancodes[2] = pos->scancode;
+ ut_assertok(sandbox_usb_keyb_add_string(dev, scancodes)); + + for (c = pos->result; *c; ++c) { + ut_asserteq(1, tstc()); + ut_asserteq(*c, getc()); + } + ut_asserteq(0, tstc()); + } ut_assertok(usb_stop());
return 0; -- 2.24.0

Enable the support of function keys on the USB keyboard. This is necessary to test the USB keyboard driver.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v4: new patch --- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 716096abc5..be1e103cf4 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -181,6 +181,7 @@ CONFIG_USB=y CONFIG_DM_USB=y CONFIG_USB_EMUL=y CONFIG_USB_KEYBOARD=y +CONFIG_USB_KEYBOARD_FN_KEYS=y CONFIG_DM_VIDEO=y CONFIG_CONSOLE_ROTATION=y CONFIG_CONSOLE_TRUETYPE=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index a8144436eb..eda595fca9 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -203,6 +203,7 @@ CONFIG_USB=y CONFIG_DM_USB=y CONFIG_USB_EMUL=y CONFIG_USB_KEYBOARD=y +CONFIG_USB_KEYBOARD_FN_KEYS=y CONFIG_DM_VIDEO=y CONFIG_CONSOLE_ROTATION=y CONFIG_CONSOLE_TRUETYPE=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 774c278bce..02969f95f1 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -163,6 +163,7 @@ CONFIG_USB=y CONFIG_DM_USB=y CONFIG_USB_EMUL=y CONFIG_USB_KEYBOARD=y +CONFIG_USB_KEYBOARD_FN_KEYS=y CONFIG_DM_VIDEO=y CONFIG_CONSOLE_ROTATION=y CONFIG_CONSOLE_TRUETYPE=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 02702fa7a5..3b0f15de88 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -183,6 +183,7 @@ CONFIG_USB=y CONFIG_DM_USB=y CONFIG_USB_EMUL=y CONFIG_USB_KEYBOARD=y +CONFIG_USB_KEYBOARD_FN_KEYS=y CONFIG_DM_VIDEO=y CONFIG_CONSOLE_ROTATION=y CONFIG_CONSOLE_TRUETYPE=y -- 2.24.0

On 11/23/19 6:15 PM, Heinrich Schuchardt wrote:
GRUB uses function keys. So we should support these with an USB keyboard. Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. Simplify the code beforehand.
Enhance the keyboard unit test.
In total I could not see any increase of u-boot.img on the TBS2910 but as the special keys are only needed in the context of the UEFI subsystem it makes sense to save several hundred bytes on other boards.
Applied all to usb/next, thanks, so let's see what CI has to say.

Hi,
On Sat, 23 Nov 2019 at 13:05, Marek Vasut marex@denx.de wrote:
On 11/23/19 6:15 PM, Heinrich Schuchardt wrote:
GRUB uses function keys. So we should support these with an USB keyboard. Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. Simplify the code beforehand.
Enhance the keyboard unit test.
In total I could not see any increase of u-boot.img on the TBS2910 but as the special keys are only needed in the context of the UEFI subsystem it makes sense to save several hundred bytes on other boards.
Applied all to usb/next, thanks, so let's see what CI has to say.
I notice that pressing F1 at the prompt now shows P and then pressing backspace a few times makes a bit of a mess. If U-Boot itself doesn't understand these keys, could they be ignored?
Regards, Simon

On 12/27/19 5:41 PM, Simon Glass wrote:
Hi,
On Sat, 23 Nov 2019 at 13:05, Marek Vasut marex@denx.de wrote:
On 11/23/19 6:15 PM, Heinrich Schuchardt wrote:
GRUB uses function keys. So we should support these with an USB keyboard. Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. Simplify the code beforehand.
Enhance the keyboard unit test.
In total I could not see any increase of u-boot.img on the TBS2910 but as the special keys are only needed in the context of the UEFI subsystem it makes sense to save several hundred bytes on other boards.
Applied all to usb/next, thanks, so let's see what CI has to say.
I notice that pressing F1 at the prompt now shows P and then pressing backspace a few times makes a bit of a mess. If U-Boot itself doesn't understand these keys, could they be ignored?
Hello Simon,
Thanks for reporting your test results. Could you, please, describe your scenario in detail.
Was USB_KEYBOARD_FN_KEYS enabled? Which output device did you use?
Best regards
Heinrich

Hi Heinrich,
On Fri, 27 Dec 2019 at 11:21, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/27/19 5:41 PM, Simon Glass wrote:
Hi,
On Sat, 23 Nov 2019 at 13:05, Marek Vasut marex@denx.de wrote:
On 11/23/19 6:15 PM, Heinrich Schuchardt wrote:
GRUB uses function keys. So we should support these with an USB keyboard. Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. Simplify the code beforehand.
Enhance the keyboard unit test.
In total I could not see any increase of u-boot.img on the TBS2910 but as the special keys are only needed in the context of the UEFI subsystem it makes sense to save several hundred bytes on other boards.
Applied all to usb/next, thanks, so let's see what CI has to say.
I notice that pressing F1 at the prompt now shows P and then pressing backspace a few times makes a bit of a mess. If U-Boot itself doesn't understand these keys, could they be ignored?
Hello Simon,
Thanks for reporting your test results. Could you, please, describe your scenario in detail.
Was USB_KEYBOARD_FN_KEYS enabled? Which output device did you use?
I ran U-Boot sandbox with -D and then pressed F1, followed by a few backspaces, on the command line. I did not change any options.
Regards, Simon

On 12/28/19 3:26 AM, Simon Glass wrote:
Hi Heinrich,
On Fri, 27 Dec 2019 at 11:21, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/27/19 5:41 PM, Simon Glass wrote:
Hi,
On Sat, 23 Nov 2019 at 13:05, Marek Vasut marex@denx.de wrote:
On 11/23/19 6:15 PM, Heinrich Schuchardt wrote:
GRUB uses function keys. So we should support these with an USB keyboard. Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. Simplify the code beforehand.
Enhance the keyboard unit test.
In total I could not see any increase of u-boot.img on the TBS2910 but as the special keys are only needed in the context of the UEFI subsystem it makes sense to save several hundred bytes on other boards.
Applied all to usb/next, thanks, so let's see what CI has to say.
I notice that pressing F1 at the prompt now shows P and then pressing backspace a few times makes a bit of a mess. If U-Boot itself doesn't understand these keys, could they be ignored?
Hello Simon,
Thanks for reporting your test results. Could you, please, describe your scenario in detail.
Was USB_KEYBOARD_FN_KEYS enabled? Which output device did you use?
I ran U-Boot sandbox with -D and then pressed F1, followed by a few backspaces, on the command line. I did not change any options.
The same behavior can be seen with v2019.10.
`./u-boot -D` uses sandbox_serial_getc() and not the USB driver as you can verify by putting a breakpoint here and into usb_kbd_put_queue(). So your observation seems to be unrelated to the patch series.
If you want to test the USB driver, you have to emulate USB keyboard strokes.
Best regards
Heinrich

Hi Heinrich,
On Fri, 27 Dec 2019 at 21:54, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/28/19 3:26 AM, Simon Glass wrote:
Hi Heinrich,
On Fri, 27 Dec 2019 at 11:21, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 12/27/19 5:41 PM, Simon Glass wrote:
Hi,
On Sat, 23 Nov 2019 at 13:05, Marek Vasut marex@denx.de wrote:
On 11/23/19 6:15 PM, Heinrich Schuchardt wrote:
GRUB uses function keys. So we should support these with an USB keyboard. Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. Simplify the code beforehand.
Enhance the keyboard unit test.
In total I could not see any increase of u-boot.img on the TBS2910 but as the special keys are only needed in the context of the UEFI subsystem it makes sense to save several hundred bytes on other boards.
Applied all to usb/next, thanks, so let's see what CI has to say.
I notice that pressing F1 at the prompt now shows P and then pressing backspace a few times makes a bit of a mess. If U-Boot itself doesn't understand these keys, could they be ignored?
Hello Simon,
Thanks for reporting your test results. Could you, please, describe your scenario in detail.
Was USB_KEYBOARD_FN_KEYS enabled? Which output device did you use?
I ran U-Boot sandbox with -D and then pressed F1, followed by a few backspaces, on the command line. I did not change any options.
The same behavior can be seen with v2019.10.
`./u-boot -D` uses sandbox_serial_getc() and not the USB driver as you can verify by putting a breakpoint here and into usb_kbd_put_queue(). So your observation seems to be unrelated to the patch series.
If you want to test the USB driver, you have to emulate USB keyboard strokes.
OK I see, thanks. I misunderstood the effect of this series I think.
Regards, Simon
participants (3)
-
Heinrich Schuchardt
-
Marek Vasut
-
Simon Glass