[U-Boot] [PATCH 1/2] usb: kbd: allow multibyte sequences to be put into ring buffer
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu Feb 22 20:53:49 UTC 2018
On 02/22/2018 08:55 PM, Marek Vasut wrote:
> On 02/22/2018 07:06 PM, Heinrich Schuchardt wrote:
>> On 02/22/2018 03:20 PM, Marek Vasut wrote:
>>> On 02/22/2018 01:04 PM, Heinrich Schuchardt wrote:
>>>> The USB keyboard driver provides a ring buffer for key strokes.
>>>>
>>>> Function keys cannot be encoded as single bytes. Instead xterm control
>>>> sequences have to be put into the ring buffer.
>>>
>>> Does it work without xterm or with any other terminal ?
>> I use two testing environments:
>>
>> - USB keyboard and HDMI monitor. This does not involve xterm.
>
> So how are the "xterm control sequences" interpreted then?
I already pointed you to U-Boot function cread_line().
Here only the sequences for left, right, up, and down are used.
When starting an EFI application the xterm escape sequences are
translated to EFI scan codes. See lib/efi_loader/efi_console.c
> What happens if you use the U-Boot menu, does this work in there too?
Function bootmenu_loop() only looks for
ESC [ A and
ESC [ B
So without the patch series you are not able to navigate inside the
U-Boot menu using a USB keyboard.
>
>> - Connection from Linux via serial cable. Linux screen or minicom
>> transfer xterm escape sequences when typing special keys.
>
> Did you try this ie. on kms console , outside of X11 ?
I never heard of a kms console.
If I boot into the plain Linux console (i.e. without X11), key
pressesare still provided as xterm escape sequences.
>
>> My target is calling iPXE and Grub as EFI applications. For navigation
>> in Grub we need at least up, down, left, right, delete and F10. In
>> lib/efi_loader/efi_console.c xterm escape sequences are translated to
>> EFI scan codes.
>
>> Before the patch series the U-Boot USB keyboard driver signals up, down,
>> left and right as non-standard character codes 0x6, 0x2, 0xe, 0x10.
>> F1-F12, Insert, Delete, Home, End, Page Up, and Page Down are not
>> supported.
>>
>> Function cread_line() (in common/cli_readline.c) interprets both xterm
>> escape sequences and non-standard control characters provided by the USB
>> keyboard driver.
>
> Wait, are those xterm or vt100 sequences ?
VT100 largely overlaps with xterm. See
https://invisible-island.net/xterm/ctlseqs/ctlseqs.pdf
>
>> JinHua Luo's patch
>> Add readline cmdline-editing extension
>> 501090aaa67b6072ebe8b721c8328d32be607660
>> which introduced command line editing unfortunately does not describe
>> why it chose CTRL+B, CTRL+F, CTRL+N, and CTRL+P for navigation. These
>> codes are not documented in any README.
>>
>>>
>>>> This preparatory patch changes function usb_kbd_put_queue() to allow
>>>> adding
>>>> multiple characters at once. If the buffer cannot accommodate the whole
>>>> sequence, it is rejected completely.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>> ---
>>>> common/usb_kbd.c | 42 +++++++++++++++++++++++++-----------------
>>>> 1 file changed, 25 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>>>> index 8cbdba6ac2..706cc350a6 100644
>>>> --- a/common/usb_kbd.c
>>>> +++ b/common/usb_kbd.c
>>>> @@ -125,24 +125,32 @@ extern int __maybe_unused net_busy_flag;
>>>> /* The period of time between two calls of usb_kbd_testc(). */
>>>> 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)
>>>> +/*
>>>> + * Put characters into ring buffer.
>>>> + *
>>>> + * @data: private data
>>>> + * @buf buffer with data to be queued
>>>> + * @count: number of characters to be queued
>>>> + */
>>>> +static void usb_kbd_put_queue(struct usb_kbd_pdata *data,
>>>> + uint8_t *buf, int count)
>>>> {
>>>> - if (data->usb_in_pointer == USB_KBD_BUFFER_LEN - 1) {
>>>> - /* Check for buffer full. */
>>>> - if (data->usb_out_pointer == 0)
>>>> - return;
>>>> -
>>>> - data->usb_in_pointer = 0;
>>>> - } else {
>>>> - /* Check for buffer full. */
>>>> - if (data->usb_in_pointer == data->usb_out_pointer - 1)
>>>> - return;
>>>> -
>>>> - data->usb_in_pointer++;
>>>> + int i, used;
>>>> +
>>>> + /* Check if buffer holds at least 'count' free slots */
>>>> + used = data->usb_in_pointer - data->usb_out_pointer;
>>>> + if (used < 0)
>>>> + used += USB_KBD_BUFFER_LEN;
>>>> + if (used + count >= USB_KBD_BUFFER_LEN)
>>>> + return;
>>>> +
>>>> + /* Copy to buffer */
>>>> + for (i = 0; i < count; ++i) {
>>>> + ++data->usb_in_pointer;
>>>> + if (data->usb_in_pointer == USB_KBD_BUFFER_LEN)
>>>> + data->usb_in_pointer = 0;
>>>> + data->usb_kbd_buffer[data->usb_in_pointer] = buf[i];
>>>
>>> memcpy() ?
>>
>> Typically we copy only one byte. But escape sequences have a maximum
>> length of 8 bytes (e.g. CTRL+F8).
>>
>> We have to consider the case with wrap around. This would require two
>> memcpy() calls.
>>
>> The coding would neither get faster in the average nor less complex
>> using memcpy(). So let's keep it as it is.
>
> I suspect this block of code needs cleanup .
>
Could you, please, give some indication of what you dislike.
Regards
Heinrich
More information about the U-Boot
mailing list