Hi,
Thanks for the review.
On 11/14/2013 11:16 PM, Sarah Sharp wrote:
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.
Right, my bad. I did not notice there was a nested switch, so my intent
was for the break to break out of the top-level switch, which of course
it does not. I've modified the patch in my local tree to put the
goto back.
Regards,
Hans
--
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
