Patchwork Update usb-linux to handle maximum of 16k transfers.

login
register
mail settings
Submitter David S. Ahern
Date April 22, 2010, 5:22 p.m.
Message ID <1271956976-31355-1-git-send-email-daahern@cisco.com>
Download mbox | patch
Permalink /patch/50754/
State New
Headers show

Comments

David S. Ahern - April 22, 2010, 5:22 p.m.
Update usb-linux to handle maximum of 16k transfers. The 16k limit
is imposed by USBFS. EHCI allows up to 20k per request.

USBFS cannot be increased to 20k due to constraints on memory use (wants
contiguous memory in allocations that are powers of 2). This change
breaks large requests from a host controller into 2 URB submissions
to the host devices.

Signed-off-by: David Ahern <daahern@cisco.com>
---
 usb-linux.c |  139 +++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 83 insertions(+), 56 deletions(-)
Jan Kiszka - April 23, 2010, 7:50 a.m.
David Ahern wrote:
> Update usb-linux to handle maximum of 16k transfers. The 16k limit
> is imposed by USBFS. EHCI allows up to 20k per request.
> 
> USBFS cannot be increased to 20k due to constraints on memory use (wants
> contiguous memory in allocations that are powers of 2). This change
> breaks large requests from a host controller into 2 URB submissions
> to the host devices.
> 
> Signed-off-by: David Ahern <daahern@cisco.com>
> ---
>  usb-linux.c |  139 +++++++++++++++++++++++++++++++++++------------------------
>  1 files changed, 83 insertions(+), 56 deletions(-)
> 
> diff --git a/usb-linux.c b/usb-linux.c
> index d0d7cff..688f45e 100644
> --- a/usb-linux.c
> +++ b/usb-linux.c
> @@ -182,6 +182,8 @@ typedef struct AsyncURB
>  
>      USBPacket     *packet;
>      USBHostDevice *hdev;
> +
> +    int more;     /* packet required multiple URBs */
>  } AsyncURB;
>  
>  static AsyncURB *async_alloc(void)
> @@ -220,9 +222,9 @@ static void async_complete(void *opaque)
>      AsyncURB *aurb;
>  
>      while (1) {
> -    	USBPacket *p;
> +        USBPacket *p;
>  
> -	int r = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb);
> +        int r = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb);
>          if (r < 0) {
>              if (errno == EAGAIN)
>                  return;
> @@ -238,31 +240,33 @@ static void async_complete(void *opaque)
>              return;
>          }
>  
> -        p = aurb->packet;
> -
> -	DPRINTF("husb: async completed. aurb %p status %d alen %d\n", 
> +        DPRINTF("husb: async completed. aurb %p status %d alen %d\n", 
>                  aurb, aurb->urb.status, aurb->urb.actual_length);
>  
> -	if (p) {
> +        p = aurb->packet;
> +	    if (p) {

There is still a tab in the line above.

Some hunks below is another inherited style issue. But I would recommend
to avoid style fixes in this patch. Focus on the functional change of
the split-up and do those "cosmetic" fixes separately. This is not the
orthogonal ehci module we need to clean up any, but preexisting code.

>              switch (aurb->urb.status) {
>              case 0:
> -                p->len = aurb->urb.actual_length;
> +                p->len += aurb->urb.actual_length;
>                  if (aurb->urb.type == USBDEVFS_URB_TYPE_CONTROL)
>                      async_complete_ctrl(s, p);
>                  break;
>  
>              case -EPIPE:
>                  set_halt(s, p->devep);
> -		p->len = USB_RET_STALL;
> -		break;
> +                p->len = USB_RET_STALL;
> +                break;
>  
>              default:
>                  p->len = USB_RET_NAK;
>                  break;
>              }
>  
> -            usb_packet_complete(p);
> -	}
> +            if (!aurb->more) {
> +                DPRINTF("invoking packet_complete. plen = %d\n", p->len);
> +                usb_packet_complete(p);
> +            }
> +        }
>  
>          async_free(aurb);
>      }
> @@ -404,67 +408,90 @@ static void usb_host_handle_destroy(USBDevice *dev)
>  
>  static int usb_linux_update_endp_table(USBHostDevice *s);
>  
> +/* devio.c limits single requests to 16k */
> +#define MAX_USBFS_BUFFER_SIZE 16384
> +
>  static int usb_host_handle_data(USBHostDevice *s, USBPacket *p)
>  {
>      struct usbdevfs_urb *urb;
>      AsyncURB *aurb;
> -    int ret;
> +    int ret, len, rem;
> +    uint8_t *pbuf;
>  
> -    aurb = async_alloc();
> -    aurb->hdev   = s;
> -    aurb->packet = p;
> +    rem = p->len;
> +    pbuf = p->data;
>  
> -    urb = &aurb->urb;
> +    p->len = 0;
> +    while (rem) {
> +        aurb = async_alloc();
> +        aurb->hdev   = s;
> +        aurb->packet = p;
> + 
> +        urb = &aurb->urb;
>  
> -    if (p->pid == USB_TOKEN_IN)
> -    	urb->endpoint = p->devep | 0x80;
> -    else
> -    	urb->endpoint = p->devep;
> +        if (p->pid == USB_TOKEN_IN)
> +    	    urb->endpoint = p->devep | 0x80;
> +        else
> +    	    urb->endpoint = p->devep;

Missing braces and improper indention.

> +
> +        if (is_halted(s, p->devep)) {
> +            ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
> +            if (ret < 0) {
> +                DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n", 
> +                        urb->endpoint, errno);
> +                return USB_RET_NAK;
> +            }
> +            clear_halt(s, p->devep);
> +        }
>  
> -    if (is_halted(s, p->devep)) {
> -	ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
> -        if (ret < 0) {
> -            DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n", 
> -                   urb->endpoint, errno);
> -            return USB_RET_NAK;
> +        if (is_isoc(s, p->devep)) {
> +            /* Setup ISOC transfer */
> +            urb->type     = USBDEVFS_URB_TYPE_ISO;
> +            urb->flags    = USBDEVFS_URB_ISO_ASAP;
> +            urb->number_of_packets = 1;
> +            urb->iso_frame_desc[0].length = p->len;
> +        } else {
> +            /* Setup bulk transfer */
> +            urb->type     = USBDEVFS_URB_TYPE_BULK;
>          }
> -        clear_halt(s, p->devep);
> -    }
>  
> -    urb->buffer        = p->data;
> -    urb->buffer_length = p->len;
> +        urb->usercontext = s;
>  
> -    if (is_isoc(s, p->devep)) {
> -        /* Setup ISOC transfer */
> -        urb->type     = USBDEVFS_URB_TYPE_ISO;
> -        urb->flags    = USBDEVFS_URB_ISO_ASAP;
> -        urb->number_of_packets = 1;
> -        urb->iso_frame_desc[0].length = p->len;
> -    } else {
> -        /* Setup bulk transfer */
> -        urb->type     = USBDEVFS_URB_TYPE_BULK;
> -    }
> +        /* USBFS limits max request size to 16k */
> +        if (rem > MAX_USBFS_BUFFER_SIZE) {
> +            len = MAX_USBFS_BUFFER_SIZE;
> +            aurb->more = 1;
> +        } else {
> +            len = rem;
> +            aurb->more = 0;
> +        }
> +        urb->buffer_length = len;
> +        urb->buffer = pbuf;
>  
> -    urb->usercontext = s;
> +        ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
>  
> -    ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
> +        DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n",
> +                urb->endpoint, len, aurb);
>  
> -    DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n", urb->endpoint, p->len, aurb);
> +        if (ret < 0) {
> +            DPRINTF("husb: submit failed. errno %d\n", errno);
>  
> -    if (ret < 0) {
> -        DPRINTF("husb: submit failed. errno %d\n", errno);
> -        async_free(aurb);
> +            async_free(aurb);
>  
> -        switch(errno) {
> -        case ETIMEDOUT:
> -            return USB_RET_NAK;
> -        case EPIPE:
> -        default:
> -            return USB_RET_STALL;
> +            switch(errno) {
> +            case ETIMEDOUT:
> +                return USB_RET_NAK;
> +            case EPIPE:
> +            default:
> +                return USB_RET_STALL;
> +            }
>          }
> +        usb_defer_packet(p, async_cancel, aurb);
> +
> +        pbuf += len;
> +        rem -= len;
>      }
>  
> -    usb_defer_packet(p, async_cancel, aurb);
>      return USB_RET_ASYNC;
>  }
>  
> @@ -577,7 +604,7 @@ static int usb_host_handle_control(USBHostDevice *s, USBPacket *p)
>      urb->buffer_length = buffer_len;
>  
>      urb->usercontext = s;
> -
> +    p->len = 0;
>      ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
>  
>      DPRINTF("husb: submit ctrl. len %u aurb %p\n", urb->buffer_length, aurb);

Again, please submit as separate functional change + style cleanup
patches, maybe the latter directly for merge in upstream, and the former
on top of it for the ehci branch.

Jan

Patch

diff --git a/usb-linux.c b/usb-linux.c
index d0d7cff..688f45e 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -182,6 +182,8 @@  typedef struct AsyncURB
 
     USBPacket     *packet;
     USBHostDevice *hdev;
+
+    int more;     /* packet required multiple URBs */
 } AsyncURB;
 
 static AsyncURB *async_alloc(void)
@@ -220,9 +222,9 @@  static void async_complete(void *opaque)
     AsyncURB *aurb;
 
     while (1) {
-    	USBPacket *p;
+        USBPacket *p;
 
-	int r = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb);
+        int r = ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb);
         if (r < 0) {
             if (errno == EAGAIN)
                 return;
@@ -238,31 +240,33 @@  static void async_complete(void *opaque)
             return;
         }
 
-        p = aurb->packet;
-
-	DPRINTF("husb: async completed. aurb %p status %d alen %d\n", 
+        DPRINTF("husb: async completed. aurb %p status %d alen %d\n", 
                 aurb, aurb->urb.status, aurb->urb.actual_length);
 
-	if (p) {
+        p = aurb->packet;
+	    if (p) {
             switch (aurb->urb.status) {
             case 0:
-                p->len = aurb->urb.actual_length;
+                p->len += aurb->urb.actual_length;
                 if (aurb->urb.type == USBDEVFS_URB_TYPE_CONTROL)
                     async_complete_ctrl(s, p);
                 break;
 
             case -EPIPE:
                 set_halt(s, p->devep);
-		p->len = USB_RET_STALL;
-		break;
+                p->len = USB_RET_STALL;
+                break;
 
             default:
                 p->len = USB_RET_NAK;
                 break;
             }
 
-            usb_packet_complete(p);
-	}
+            if (!aurb->more) {
+                DPRINTF("invoking packet_complete. plen = %d\n", p->len);
+                usb_packet_complete(p);
+            }
+        }
 
         async_free(aurb);
     }
@@ -404,67 +408,90 @@  static void usb_host_handle_destroy(USBDevice *dev)
 
 static int usb_linux_update_endp_table(USBHostDevice *s);
 
+/* devio.c limits single requests to 16k */
+#define MAX_USBFS_BUFFER_SIZE 16384
+
 static int usb_host_handle_data(USBHostDevice *s, USBPacket *p)
 {
     struct usbdevfs_urb *urb;
     AsyncURB *aurb;
-    int ret;
+    int ret, len, rem;
+    uint8_t *pbuf;
 
-    aurb = async_alloc();
-    aurb->hdev   = s;
-    aurb->packet = p;
+    rem = p->len;
+    pbuf = p->data;
 
-    urb = &aurb->urb;
+    p->len = 0;
+    while (rem) {
+        aurb = async_alloc();
+        aurb->hdev   = s;
+        aurb->packet = p;
+ 
+        urb = &aurb->urb;
 
-    if (p->pid == USB_TOKEN_IN)
-    	urb->endpoint = p->devep | 0x80;
-    else
-    	urb->endpoint = p->devep;
+        if (p->pid == USB_TOKEN_IN)
+    	    urb->endpoint = p->devep | 0x80;
+        else
+    	    urb->endpoint = p->devep;
+
+        if (is_halted(s, p->devep)) {
+            ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
+            if (ret < 0) {
+                DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n", 
+                        urb->endpoint, errno);
+                return USB_RET_NAK;
+            }
+            clear_halt(s, p->devep);
+        }
 
-    if (is_halted(s, p->devep)) {
-	ret = ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);
-        if (ret < 0) {
-            DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\n", 
-                   urb->endpoint, errno);
-            return USB_RET_NAK;
+        if (is_isoc(s, p->devep)) {
+            /* Setup ISOC transfer */
+            urb->type     = USBDEVFS_URB_TYPE_ISO;
+            urb->flags    = USBDEVFS_URB_ISO_ASAP;
+            urb->number_of_packets = 1;
+            urb->iso_frame_desc[0].length = p->len;
+        } else {
+            /* Setup bulk transfer */
+            urb->type     = USBDEVFS_URB_TYPE_BULK;
         }
-        clear_halt(s, p->devep);
-    }
 
-    urb->buffer        = p->data;
-    urb->buffer_length = p->len;
+        urb->usercontext = s;
 
-    if (is_isoc(s, p->devep)) {
-        /* Setup ISOC transfer */
-        urb->type     = USBDEVFS_URB_TYPE_ISO;
-        urb->flags    = USBDEVFS_URB_ISO_ASAP;
-        urb->number_of_packets = 1;
-        urb->iso_frame_desc[0].length = p->len;
-    } else {
-        /* Setup bulk transfer */
-        urb->type     = USBDEVFS_URB_TYPE_BULK;
-    }
+        /* USBFS limits max request size to 16k */
+        if (rem > MAX_USBFS_BUFFER_SIZE) {
+            len = MAX_USBFS_BUFFER_SIZE;
+            aurb->more = 1;
+        } else {
+            len = rem;
+            aurb->more = 0;
+        }
+        urb->buffer_length = len;
+        urb->buffer = pbuf;
 
-    urb->usercontext = s;
+        ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
 
-    ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
+        DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n",
+                urb->endpoint, len, aurb);
 
-    DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n", urb->endpoint, p->len, aurb);
+        if (ret < 0) {
+            DPRINTF("husb: submit failed. errno %d\n", errno);
 
-    if (ret < 0) {
-        DPRINTF("husb: submit failed. errno %d\n", errno);
-        async_free(aurb);
+            async_free(aurb);
 
-        switch(errno) {
-        case ETIMEDOUT:
-            return USB_RET_NAK;
-        case EPIPE:
-        default:
-            return USB_RET_STALL;
+            switch(errno) {
+            case ETIMEDOUT:
+                return USB_RET_NAK;
+            case EPIPE:
+            default:
+                return USB_RET_STALL;
+            }
         }
+        usb_defer_packet(p, async_cancel, aurb);
+
+        pbuf += len;
+        rem -= len;
     }
 
-    usb_defer_packet(p, async_cancel, aurb);
     return USB_RET_ASYNC;
 }
 
@@ -577,7 +604,7 @@  static int usb_host_handle_control(USBHostDevice *s, USBPacket *p)
     urb->buffer_length = buffer_len;
 
     urb->usercontext = s;
-
+    p->len = 0;
     ret = ioctl(s->fd, USBDEVFS_SUBMITURB, urb);
 
     DPRINTF("husb: submit ctrl. len %u aurb %p\n", urb->buffer_length, aurb);