On Wed, Oct 09, 2013 at 05:19:28PM +0200, Hans de Goede wrote:
> This is a preparation patch for adding support for bulk streams.
This patch seems to have the (unintended?) side effect of enabling
scatter gather for interrupt transfers over 16384 bytes (USB_SG_SIZE):
switch(uurb->type) {
...
case USBDEVFS_URB_TYPE_BULK:
switch (usb_endpoint_type(&ep->desc)) {
case USB_ENDPOINT_XFER_CONTROL:
case USB_ENDPOINT_XFER_ISOC:
return -EINVAL;
case USB_ENDPOINT_XFER_INT:
/* allow single-shot interrupt transfers */
uurb->type = USBDEVFS_URB_TYPE_INTERRUPT;
- goto interrupt_urb;
+ break;
}
- uurb->number_of_packets = 0;
num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE);
if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize)
num_sgs = 0;
break;
case USBDEVFS_URB_TYPE_INTERRUPT:
if (!usb_endpoint_xfer_int(&ep->desc))
return -EINVAL;
- interrupt_urb:
- uurb->number_of_packets = 0;
break;
...
}
...
u += sizeof(struct async) + sizeof(struct urb) + uurb->buffer_length +
num_sgs * sizeof(struct scatterlist);
ret = usbfs_increase_memory_usage(u);
if (ret)
goto error;
as->mem_usage = u;
if (num_sgs) {
....
The code breaks out of the inner switch statement, and falls through to
setting num_sgs, which will later cause a scatter gather list to be
allocated in the struct urb for the interrupt transfer.
Scatter gather is actually broken under xHCI right now for everything
except mass storage, which hands down entries in nice page-sized chunks.
We have a quick fix for bulk transfers under review, but fixing
interrupt transfers is going to be a lot more complex. I'd rather not
enable interrupt scatter-gather through usbfs if it doesn't currently
work.
Besides, the patch shouldn't change behavior when you're just trying to
change the code to use a local variable.
Sarah Sharp
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/usb/core/devio.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index e1ebbd1..539e632 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -1192,6 +1192,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
> struct usb_ctrlrequest *dr = NULL;
> unsigned int u, totlen, isofrmlen;
> int i, ret, is_in, num_sgs = 0, ifnum = -1;
> + int number_of_packets = 0;
> void *buf;
>
> if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
> @@ -1245,7 +1246,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
> le16_to_cpup(&dr->wIndex));
> if (ret)
> goto error;
> - uurb->number_of_packets = 0;
> uurb->buffer_length = le16_to_cpup(&dr->wLength);
> uurb->buffer += 8;
> if ((dr->bRequestType & USB_DIR_IN) && uurb->buffer_length) {
> @@ -1273,9 +1273,8 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
> case USB_ENDPOINT_XFER_INT:
> /* allow single-shot interrupt transfers */
> uurb->type = USBDEVFS_URB_TYPE_INTERRUPT;
> - goto interrupt_urb;
> + break;
> }
> - uurb->number_of_packets = 0;
> num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE);
> if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize)
> num_sgs = 0;
> @@ -1284,8 +1283,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
> case USBDEVFS_URB_TYPE_INTERRUPT:
> if (!usb_endpoint_xfer_int(&ep->desc))
> return -EINVAL;
> - interrupt_urb:
> - uurb->number_of_packets = 0;
> break;
>
> case USBDEVFS_URB_TYPE_ISO:
> @@ -1295,15 +1292,16 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
> return -EINVAL;
> if (!usb_endpoint_xfer_isoc(&ep->desc))
> return -EINVAL;
> + number_of_packets = uurb->number_of_packets;
> isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
> - uurb->number_of_packets;
> + number_of_packets;
> if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL)))
> return -ENOMEM;
> if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
> ret = -EFAULT;
> goto error;
> }
> - for (totlen = u = 0; u < uurb->number_of_packets; u++) {
> + for (totlen = u = 0; u < number_of_packets; u++) {
> /*
> * arbitrary limit need for USB 3.0
> * bMaxBurst (0~15 allowed, 1~16 packets)
> @@ -1334,7 +1332,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
> ret = -EFAULT;
> goto error;
> }
> - as = alloc_async(uurb->number_of_packets);
> + as = alloc_async(number_of_packets);
> if (!as) {
> ret = -ENOMEM;
> goto error;
> @@ -1428,7 +1426,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
> as->urb->setup_packet = (unsigned char *)dr;
> dr = NULL;
> as->urb->start_frame = uurb->start_frame;
> - as->urb->number_of_packets = uurb->number_of_packets;
> + as->urb->number_of_packets = number_of_packets;
> if (uurb->type == USBDEVFS_URB_TYPE_ISO ||
> ps->dev->speed == USB_SPEED_HIGH)
> as->urb->interval = 1 << min(15, ep->desc.bInterval - 1);
> @@ -1436,7 +1434,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb,
> as->urb->interval = ep->desc.bInterval;
> as->urb->context = as;
> as->urb->complete = async_completed;
> - for (totlen = u = 0; u < uurb->number_of_packets; u++) {
> + for (totlen = u = 0; u < number_of_packets; u++) {
> as->urb->iso_frame_desc[u].offset = totlen;
> as->urb->iso_frame_desc[u].length = isopkt[u].length;
> totlen += isopkt[u].length;
> --
> 1.8.3.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
