On 02/23/2018 12:02 AM, Marek Vasut wrote:
On 02/22/2018 09:53 PM, Heinrich Schuchardt wrote:
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
So this is only usable if you have display connected to the board ?
Why? We are talking about input.
Neither local nor remote display is needed to hit the F10 key on a USB
keyboard.
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.
Ha
- 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.
But there is no xterm ?
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
So vt100 , OK.
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.g...@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.
At least the part which looks like ad-hoc implementation of memcpy() ,
any other cleanups are welcome.
Do you really want that ugly monster below for typically copying a
single byte?
As said it is slower, has more lines, and gives a larger executable.
int remaining;
/* Copy to buffer */
remaining = data->usb_in_pointer + count - USB_KBD_BUFFER_LEN;
if (remaining < 0)
remaining = 0;
memcpy(&usb_kbd_buffer[data->usb_in_pointer], buf, count - remaining);
if (remaining)
memcpy(usb_kbd_buffer, buf + count - remaining, remaining);
data->usb_in_pointer += count;
if (data->usb_in_pointer >= USB_KBD_BUFFER_LEN)
data->usb_in_pointer -= USB_KBD_BUFFER_LEN;
Best regards
Heinrich
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
