[U-Boot] [PATCH 1/2] usb: kbd: allow multibyte sequences to be put into ring buffer

[Marek Vasut]

On 02/23/2018 06:50 AM, Heinrich Schuchardt wrote:
[...]
>>> 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 confused me was the constant usage of xterm control sequences to
describe terminal control sequences. Now it makes sense.

[...]

>>>>>>> +    /* 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;

Can the code be tweaked so it doesn't use the circular buffer, but a
linear one , thus two memcpy()s are not needed ?

The whole usb keyboard code is not nice and could use some cleanup.

-- 
Best regards,
Marek Vasut