Hi,
On 09/20/2014 05:01 PM, Hans de Goede wrote:
> Waiting an interrupt packet to complete in usb_kbd_poll_for_event, causes
> a 40 ms latency for each call to usb_kbd_testc, which is undesirable.
>
> Using control messages leads to lower (but still not 0) latency, but some
> devices do not work well with control messages (e.g. my kvm behaves funny
> with them).
>
> This commit adds support for using the int_queue mechanism which at least
> the ehci-hcd driver supports. This allows polling with 0 latency, while
> using interrupt packets.
>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---
> common/usb_kbd.c | 37 ++++++++++++++++++++++++++++++++++---
> 1 file changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> index 85ee1c8..278937c 100644
> --- a/common/usb_kbd.c
> +++ b/common/usb_kbd.c
> @@ -102,6 +102,7 @@ struct usb_kbd_pdata {
> unsigned long intpipe;
> int intpktsize;
> int intinterval;
> + struct int_queue *intq;
>
> uint32_t repeat_delay;
>
> @@ -324,6 +325,15 @@ static inline void usb_kbd_poll_for_event(struct
> usb_device *dev)
> 1, 0, data->new, USB_KBD_BOOT_REPORT_SIZE);
> if (memcmp(data->old, data->new, USB_KBD_BOOT_REPORT_SIZE))
> usb_kbd_irq_worker(dev);
> +#elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE)
> + struct usb_kbd_pdata *data = dev->privptr;
> + if (poll_int_queue(dev, data->intq)) {
> + usb_kbd_irq_worker(dev);
> + /* We've consumed all queued int packets, create new */
> + destroy_int_queue(dev, data->intq);
> + data->intq = create_int_queue(dev, data->intpipe, 1,
> + USB_KBD_BOOT_REPORT_SIZE, data->new);
> + }
> #endif
> }
>
> @@ -441,8 +451,14 @@ static int usb_kbd_probe(struct usb_device *dev,
> unsigned int ifnum)
> usb_set_idle(dev, iface->desc.bInterfaceNumber, REPEAT_RATE, 0);
>
> debug("USB KBD: enable interrupt pipe...\n");
> +#ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE
> + data->intq = create_int_queue(dev, data->intpipe, 1,
> + USB_KBD_BOOT_REPORT_SIZE, data->new);
> + if (!data->intq) {
> +#else
> if (usb_submit_int_msg(dev, data->intpipe, data->new, data->intpktsize,
> data->intinterval) < 0) {
> +#endif
> printf("Failed to get keyboard state from device %04x:%04x\n",
> dev->descriptor.idVendor, dev->descriptor.idProduct);
> /* Abort, we don't want to use that non-functional keyboard. */
> @@ -515,13 +531,28 @@ int drv_usb_kbd_init(void)
> /* Deregister the keyboard. */
> int usb_kbd_deregister(int force)
> {
> -#ifdef CONFIG_SYS_STDIO_DEREGISTER
> +#if !defined(CONFIG_SYS_STDIO_DEREGISTER)
> + return 1;
> +#elif defined(CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE)
> + struct stdio_dev *dev;
> + struct usb_device *usb_kbd_dev;
> + struct usb_kbd_pdata *data;
> +
> + dev = stdio_get_by_name(DEVNAME);
> + if (dev) {
> + if (stdio_deregister_dev(dev, force) != 0)
> + return 1;
> + usb_kbd_dev = (struct usb_device *)dev->priv;
Hmm I've just realized that this is a use after free for dev.
I've fixed this in my local tree. I'll wait for review of the
other patches in this set before sending a v2.
Note this is not really a use after free, since stdio_deregister_dev
does not free dev, but it reallu should free dev, since stdio_register_dev
does a clone of the passed in dev, so stdio_deregister_dev should free it,
so as to not leak memory. Once that memory leak gets fixed, this will be
a use after free.
Likewise usb_kbd.c should free usb_kbd_pdata and the data->new buffer on
deregister. I'll add a fix plugging the memleak in usb_kbd.c to v2 of this
set.
Regards,
Hans
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
