| Message ID | 20180222120418.12832-3-xypron.glpk@gmx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | usb: kbd: implement special keys | expand |
Hi Heinrich, On 22 February 2018 at 05:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > Correct support for arrow keys: use the standard xterm escape sequences. > > Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > common/usb_kbd.c | 121 +++++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 90 insertions(+), 31 deletions(-) > Is there any way this code could be shared with input.c? It has translation tables. Regards, Simon
On 02/23/2018 09:59 PM, Simon Glass wrote: > Hi Heinrich, > > On 22 February 2018 at 05:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> Correct support for arrow keys: use the standard xterm escape sequences. >> >> Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> common/usb_kbd.c | 121 +++++++++++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 90 insertions(+), 31 deletions(-) >> > > Is there any way this code could be shared with input.c? It has > translation tables. Hello Simon, input.c seems to be another incomplete implementation of a keyboard driver. Yes it would make sense to avoid duplicating code. But unfortunately input.c lacks any documentation. As maintainer I hope you can answer the following: When configuring a German keyboard and typing Right-Alt + M the output is 0xE6. This is mju (ยต) in code page 437. Wouldn't we always use UTF-8 in U-Boot? The scan codes for PS/2 serial keyboards and USB keyboards differ (cf. "USB HID to PS/2 Scan Code Translation Table" http://download.microsoft.com/download/1/6/1/161ba512-40e2-4cc9-843a-923143f3456c/translate.pdf). Is input.c meant to handle PS/2 scan codes or USB scan codes? Regards Heinrich
On 03/08/2018 09:30 PM, Simon Glass wrote: > Hi Heinrich, > > input.c uses PS/2 scan codes at present. However these are somewhat > internal. You should see docs in input.h > > I am not sure of the best approach, but one would be to convert USB > scan codes to PS/2 codes. > > Regards, > Simon > Hello Simon, hello Marek, thanks for the clarification. Between input.c and usb_kbd.c we have the following differences: - Most keycodes are different. - The way ALT, CTRL, SHIFT, META are transferred differ. - Input.c has implemented support for different keyboard layouts (EN, DE) which is not easily expandable to other layouts. So if we wanted to have a common layer this would require a complete rewrite of both input.c and usb_kbd.c. I do not have access to any device using input.c. So I am unable to test any of it. So I would prefer if we could just patch usb_kbd.c to provide the missing special keys and keep the code separate. Having usb_kbd.c in directory common/ looks like a misplacement. It could be put under drivers/input/ or under drivers/usb/hid/. I think in future we should have also mouse and touchscreen as hid drivers. Otherwise using an EFI menu on a tablet will not be possible. So I would prefer moving usb_kbd to drivers/usb/hid/. Best regards Heinrich
+Tom for comment Hi Heinrich, On 8 March 2018 at 15:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > On 03/08/2018 09:30 PM, Simon Glass wrote: >> Hi Heinrich, >> >> input.c uses PS/2 scan codes at present. However these are somewhat >> internal. You should see docs in input.h >> >> I am not sure of the best approach, but one would be to convert USB >> scan codes to PS/2 codes. >> >> Regards, >> Simon >> > > Hello Simon, hello Marek, > > thanks for the clarification. > > Between input.c and usb_kbd.c we have the following differences: > > - Most keycodes are different. Yes, although if you call input_add_table() you can supply your own. > - The way ALT, CTRL, SHIFT, META are transferred differ. Indeed > - Input.c has implemented support for different keyboard layouts > (EN, DE) which is not easily expandable to other layouts. That's just the tables though, right? > > So if we wanted to have a common layer this would require a complete rewrite > of both input.c and usb_kbd.c. I hope that in fact only the latter would need a rewrite. But I admit I have not done it. > > I do not have access to any device using input.c. So I am unable to test any > of it. > > So I would prefer if we could just patch usb_kbd.c to provide the missing > special keys and keep the code separate. Actually sandbox has tests for this. Try running with -l and you should get a keyboard on the simulated LCD screen, which uses the input layer. > > Having usb_kbd.c in directory common/ looks like a misplacement. > It could be put under drivers/input/ or under drivers/usb/hid/. > I think in future we should have also mouse and touchscreen as hid drivers. > Otherwise using an EFI menu on a tablet will not be possible. > So I would prefer moving usb_kbd to drivers/usb/hid/. Well yes it would be better out of common. I think it would be possible to use the input_send_keycodes() interface from the USB keyboard driver, given a suitable keymap. There is quite a lot of code that looks much the same, and unifying them was always my intent. But I understand that it is not a trivial undertaking, so I suppose it might be a bridge too far. Regards, Simon
On Mon, Mar 19, 2018 at 11:59:45AM -0600, Simon Glass wrote: > +Tom for comment > > Hi Heinrich, > > On 8 March 2018 at 15:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 03/08/2018 09:30 PM, Simon Glass wrote: > >> Hi Heinrich, > >> > >> input.c uses PS/2 scan codes at present. However these are somewhat > >> internal. You should see docs in input.h > >> > >> I am not sure of the best approach, but one would be to convert USB > >> scan codes to PS/2 codes. > >> > >> Regards, > >> Simon > >> > > > > Hello Simon, hello Marek, > > > > thanks for the clarification. > > > > Between input.c and usb_kbd.c we have the following differences: > > > > - Most keycodes are different. > > Yes, although if you call input_add_table() you can supply your own. > > > - The way ALT, CTRL, SHIFT, META are transferred differ. > > Indeed > > > - Input.c has implemented support for different keyboard layouts > > (EN, DE) which is not easily expandable to other layouts. > > That's just the tables though, right? > > > > > So if we wanted to have a common layer this would require a complete rewrite > > of both input.c and usb_kbd.c. > > I hope that in fact only the latter would need a rewrite. But I admit > I have not done it. > > > > > I do not have access to any device using input.c. So I am unable to test any > > of it. > > > > So I would prefer if we could just patch usb_kbd.c to provide the missing > > special keys and keep the code separate. > > Actually sandbox has tests for this. Try running with -l and you > should get a keyboard on the simulated LCD screen, which uses the > input layer. > > > > Having usb_kbd.c in directory common/ looks like a misplacement. > > It could be put under drivers/input/ or under drivers/usb/hid/. > > I think in future we should have also mouse and touchscreen as hid drivers. > > Otherwise using an EFI menu on a tablet will not be possible. > > So I would prefer moving usb_kbd to drivers/usb/hid/. > > Well yes it would be better out of common. > > I think it would be possible to use the input_send_keycodes() > interface from the USB keyboard driver, given a suitable keymap. There > is quite a lot of code that looks much the same, and unifying them was > always my intent. But I understand that it is not a trivial > undertaking, so I suppose it might be a bridge too far. Yes, we should probably try and get that code unified, or at least have an idea of how long it would take to do so.
diff --git a/common/usb_kbd.c b/common/usb_kbd.c index 706cc350a6..e8adccf219 100644 --- a/common/usb_kbd.c +++ b/common/usb_kbd.c @@ -75,12 +75,13 @@ static const unsigned char usb_kbd_num_keypad[] = { }; /* - * 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 + * map arrow keys to CSI A..D */ static const unsigned char usb_kbd_arrow[] = { - 0x6, 0x2, 0xe, 0x10 + 'C', /* 0x4f, right */ + 'D', /* 0x50, left */ + 'B', /* 0x51, down */ + 'A', /* 0x52, up */ }; /* @@ -175,7 +176,9 @@ static void usb_kbd_setled(struct usb_device *dev) static int usb_kbd_translate(struct usb_kbd_pdata *data, unsigned char scancode, unsigned char modifier, int pressed) { - uint8_t keycode = 0; + uint8_t keycode[8] = {0}; + int i; + int len = 1; /* Key released */ if (pressed == 0) { @@ -191,41 +194,96 @@ static int usb_kbd_translate(struct usb_kbd_pdata *data, unsigned char scancode, data->repeat_delay = REPEAT_DELAY; } - /* Alphanumeric values */ - if ((scancode > 3) && (scancode <= 0x1d)) { - keycode = scancode - 4 + 'a'; + if (data->flags & USB_KBD_CTRL) { + keycode[0] = scancode - 0x3; + } else if ((scancode > 3) && (scancode <= 0x1d)) { + /* Alphanumeric values */ + keycode[0] = scancode - 4 + 'a'; if (data->flags & USB_KBD_CAPSLOCK) - keycode &= ~CAPITAL_MASK; + keycode[0] &= ~CAPITAL_MASK; if (modifier & (LEFT_SHIFT | RIGHT_SHIFT)) { /* Handle CAPSLock + Shift pressed simultaneously */ - if (keycode & CAPITAL_MASK) - keycode &= ~CAPITAL_MASK; + if (keycode[0] & CAPITAL_MASK) + keycode[0] &= ~CAPITAL_MASK; else - keycode |= CAPITAL_MASK; + keycode[0] |= CAPITAL_MASK; } - } - - if ((scancode > 0x1d) && (scancode < 0x39)) { + } else if ((scancode >= 0x1e) && (scancode <= 0x38)) { /* Shift pressed */ if (modifier & (LEFT_SHIFT | RIGHT_SHIFT)) - keycode = usb_kbd_numkey_shifted[scancode - 0x1e]; + keycode[0] = usb_kbd_numkey_shifted[scancode - 0x1e]; else - keycode = usb_kbd_numkey[scancode - 0x1e]; + keycode[0] = usb_kbd_numkey[scancode - 0x1e]; + } else if ((scancode >= 0x3a) && (scancode <= 0x3d)) { + /* F1 - F4 */ + keycode[0] = 0x1b; + keycode[1] = 'O'; + keycode[2] = scancode - 0x3a + 'P'; + len = 3; + } else if ((scancode >= 0x3e) && (scancode <= 0x45)) { + /* F5 - F12 */ + keycode[0] = 0x1b; + keycode[1] = '['; + if (scancode <= 0x41) { + keycode[2] = '1'; + if (scancode == 0x3e) + /* F5 */ + keycode[3] = '5'; + else + /* F6 - F7 */ + keycode[3] = scancode - 0x3f + '7'; + } else { + keycode[2] = '2'; + if (scancode <= 0x43) + /* F9, F10 */ + keycode[3] = scancode - 0x42 + '0'; + else + /* F11, F12 */ + keycode[3] = scancode - 0x44 + '3'; + } + keycode[4] = '~'; + len = 5; + } else if ((scancode >= 0x49) && (scancode <= 0x4e)) { + /* Insert, Home, Page Up, Delete, End, Page Down */ + len = 4; + keycode[0] = 0x1b; + keycode[1] = '['; + keycode[3] = '~'; + switch (scancode) { + case 0x49: /* Insert */ + keycode[2] = '2'; + break; + case 0x4a: /* Home */ + keycode[2] = 'H'; + len = 3; + break; + case 0x4b: /* Page Up */ + keycode[2] = '5'; + break; + case 0x4c: /* Delete */ + keycode[2] = '3'; + break; + case 0x4d: /* End */ + keycode[2] = 'F'; + len = 3; + break; + case 0x4e: /* Page Down */ + keycode[2] = '6'; + break; + } + } else if ((scancode >= 0x4f) && (scancode <= 0x52)) { + /* Arrow keys */ + keycode[0] = 0x1b; + keycode[1] = '['; + keycode[2] = usb_kbd_arrow[scancode - 0x4f]; + len = 3; + } else if ((scancode >= 0x54) && (scancode <= 0x67)) { + /* Numeric keypad */ + keycode[0] = usb_kbd_num_keypad[scancode - 0x54]; } - /* 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]; - - if (data->flags & USB_KBD_CTRL) - keycode = scancode - 0x3; - if (pressed == 1) { if (scancode == NUM_LOCK) { data->flags ^= USB_KBD_NUMLOCK; @@ -243,9 +301,10 @@ static int usb_kbd_translate(struct usb_kbd_pdata *data, unsigned char scancode, } /* Report keycode if any */ - if (keycode) { - debug("%c", keycode); - usb_kbd_put_queue(data, &keycode, 1); + if (keycode[0]) { + for (i = 0; i < len; ++i) + debug("%02x ", (unsigned int)keycode); + usb_kbd_put_queue(data, keycode, len); } return 0;
Correct support for arrow keys: use the standard xterm escape sequences. Provide support for F1-F12, Insert, Delete, Home, End, Page Up, Page Down. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- common/usb_kbd.c | 121 +++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 90 insertions(+), 31 deletions(-)