From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O5Dfh-0001Wi-PK for qemu-devel@nongnu.org; Fri, 23 Apr 2010 03:51:34 -0400 Received: from [140.186.70.92] (port=33854 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O5DfG-0001OZ-IG for qemu-devel@nongnu.org; Fri, 23 Apr 2010 03:51:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O5Df2-0001S8-3j for qemu-devel@nongnu.org; Fri, 23 Apr 2010 03:51:06 -0400 Received: from fmmailgate03.web.de ([217.72.192.234]:42718) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O5Deu-0001Q8-Nx for qemu-devel@nongnu.org; Fri, 23 Apr 2010 03:50:48 -0400 Message-ID: <4BD15151.4090006@web.de> Date: Fri, 23 Apr 2010 09:50:41 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1271956976-31355-1-git-send-email-daahern@cisco.com> In-Reply-To: <1271956976-31355-1-git-send-email-daahern@cisco.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig8E22F43E3CBBB6D6B56EF3C0" Sender: jan.kiszka@web.de Subject: [Qemu-devel] Re: [PATCH] Update usb-linux to handle maximum of 16k transfers. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Ahern Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig8E22F43E3CBBB6D6B56EF3C0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable 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. >=20 > USBFS cannot be increased to 20k due to constraints on memory use (want= s > 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. >=20 > Signed-off-by: David Ahern > --- > usb-linux.c | 139 +++++++++++++++++++++++++++++++++++----------------= -------- > 1 files changed, 83 insertions(+), 56 deletions(-) >=20 > 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 > =20 > USBPacket *packet; > USBHostDevice *hdev; > + > + int more; /* packet required multiple URBs */ > } AsyncURB; > =20 > static AsyncURB *async_alloc(void) > @@ -220,9 +222,9 @@ static void async_complete(void *opaque) > AsyncURB *aurb; > =20 > while (1) { > - USBPacket *p; > + USBPacket *p; > =20 > - int r =3D ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb); > + int r =3D ioctl(s->fd, USBDEVFS_REAPURBNDELAY, &aurb); > if (r < 0) { > if (errno =3D=3D EAGAIN) > return; > @@ -238,31 +240,33 @@ static void async_complete(void *opaque) > return; > } > =20 > - p =3D aurb->packet; > - > - DPRINTF("husb: async completed. aurb %p status %d alen %d\n",=20 > + DPRINTF("husb: async completed. aurb %p status %d alen %d\n", = > aurb, aurb->urb.status, aurb->urb.actual_length); > =20 > - if (p) { > + p =3D 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 =3D aurb->urb.actual_length; > + p->len +=3D aurb->urb.actual_length; > if (aurb->urb.type =3D=3D USBDEVFS_URB_TYPE_CONTROL) > async_complete_ctrl(s, p); > break; > =20 > case -EPIPE: > set_halt(s, p->devep); > - p->len =3D USB_RET_STALL; > - break; > + p->len =3D USB_RET_STALL; > + break; > =20 > default: > p->len =3D USB_RET_NAK; > break; > } > =20 > - usb_packet_complete(p); > - } > + if (!aurb->more) { > + DPRINTF("invoking packet_complete. plen =3D %d\n", p->= len); > + usb_packet_complete(p); > + } > + } > =20 > async_free(aurb); > } > @@ -404,67 +408,90 @@ static void usb_host_handle_destroy(USBDevice *de= v) > =20 > static int usb_linux_update_endp_table(USBHostDevice *s); > =20 > +/* 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; > =20 > - aurb =3D async_alloc(); > - aurb->hdev =3D s; > - aurb->packet =3D p; > + rem =3D p->len; > + pbuf =3D p->data; > =20 > - urb =3D &aurb->urb; > + p->len =3D 0; > + while (rem) { > + aurb =3D async_alloc(); > + aurb->hdev =3D s; > + aurb->packet =3D p; > +=20 > + urb =3D &aurb->urb; > =20 > - if (p->pid =3D=3D USB_TOKEN_IN) > - urb->endpoint =3D p->devep | 0x80; > - else > - urb->endpoint =3D p->devep; > + if (p->pid =3D=3D USB_TOKEN_IN) > + urb->endpoint =3D p->devep | 0x80; > + else > + urb->endpoint =3D p->devep; Missing braces and improper indention. > + > + if (is_halted(s, p->devep)) { > + ret =3D ioctl(s->fd, USBDEVFS_CLEAR_HALT, &urb->endpoint);= > + if (ret < 0) { > + DPRINTF("husb: failed to clear halt. ep 0x%x errno %d\= n",=20 > + urb->endpoint, errno); > + return USB_RET_NAK; > + } > + clear_halt(s, p->devep); > + } > =20 > - if (is_halted(s, p->devep)) { > - ret =3D 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 =3D USBDEVFS_URB_TYPE_ISO; > + urb->flags =3D USBDEVFS_URB_ISO_ASAP; > + urb->number_of_packets =3D 1; > + urb->iso_frame_desc[0].length =3D p->len; > + } else { > + /* Setup bulk transfer */ > + urb->type =3D USBDEVFS_URB_TYPE_BULK; > } > - clear_halt(s, p->devep); > - } > =20 > - urb->buffer =3D p->data; > - urb->buffer_length =3D p->len; > + urb->usercontext =3D s; > =20 > - if (is_isoc(s, p->devep)) { > - /* Setup ISOC transfer */ > - urb->type =3D USBDEVFS_URB_TYPE_ISO; > - urb->flags =3D USBDEVFS_URB_ISO_ASAP; > - urb->number_of_packets =3D 1; > - urb->iso_frame_desc[0].length =3D p->len; > - } else { > - /* Setup bulk transfer */ > - urb->type =3D USBDEVFS_URB_TYPE_BULK; > - } > + /* USBFS limits max request size to 16k */ > + if (rem > MAX_USBFS_BUFFER_SIZE) { > + len =3D MAX_USBFS_BUFFER_SIZE; > + aurb->more =3D 1; > + } else { > + len =3D rem; > + aurb->more =3D 0; > + } > + urb->buffer_length =3D len; > + urb->buffer =3D pbuf; > =20 > - urb->usercontext =3D s; > + ret =3D ioctl(s->fd, USBDEVFS_SUBMITURB, urb); > =20 > - ret =3D ioctl(s->fd, USBDEVFS_SUBMITURB, urb); > + DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n", > + urb->endpoint, len, aurb); > =20 > - DPRINTF("husb: data submit. ep 0x%x len %u aurb %p\n", urb->endpoi= nt, p->len, aurb); > + if (ret < 0) { > + DPRINTF("husb: submit failed. errno %d\n", errno); > =20 > - if (ret < 0) { > - DPRINTF("husb: submit failed. errno %d\n", errno); > - async_free(aurb); > + async_free(aurb); > =20 > - 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 +=3D len; > + rem -=3D len; > } > =20 > - usb_defer_packet(p, async_cancel, aurb); > return USB_RET_ASYNC; > } > =20 > @@ -577,7 +604,7 @@ static int usb_host_handle_control(USBHostDevice *s= , USBPacket *p) > urb->buffer_length =3D buffer_len; > =20 > urb->usercontext =3D s; > - > + p->len =3D 0; > ret =3D ioctl(s->fd, USBDEVFS_SUBMITURB, urb); > =20 > 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 --------------enig8E22F43E3CBBB6D6B56EF3C0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkvRUVEACgkQitSsb3rl5xRzGwCfZO7UmrK1P13m5RQxa/BPy2K5 OhsAnj5dURnKxP2cqHsYt9Nr1SvMHGi3 =Qbzy -----END PGP SIGNATURE----- --------------enig8E22F43E3CBBB6D6B56EF3C0--