[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] Update usb-linux to handle maximum of 16k trans
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH] Update usb-linux to handle maximum of 16k transfers. |
Date: |
Fri, 23 Apr 2010 09:50:41 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
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 <address@hidden>
> ---
> 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
signature.asc
Description: OpenPGP digital signature