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 hdegoede@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