From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Thu, 22 Feb 2018 20:55:24 +0100 Subject: [U-Boot] [PATCH 1/2] usb: kbd: allow multibyte sequences to be put into ring buffer In-Reply-To: References: <20180222120418.12832-1-xypron.glpk@gmx.de> <20180222120418.12832-2-xypron.glpk@gmx.de> Message-ID: <3dd144c7-e6d8-d146-70a2-b6093ea09316@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de 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 ? What happens if you use the U-Boot menu, does this work in there too? > - 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 ? > 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 ? > 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 >>> --- >>>   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 maximum > length are 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 . -- Best regards, Marek Vasut