Daniel and David:
This kernel patch implements the plan we discussed for handling short
bulk transfers. It adds a new URB flag: USBDEVFS_URB_XFER_START.
Here's how it works:
When libusb breaks a bulk-IN transfer up into multiple URBs, it should
set
USBDEVFS_URB_SHORT_NOT_OK | USBDEVFS_URB_XFER_START
in the flags for the first URB, and
USBDEVFS_URB_SHORT_NOT_OK
for all the following ones. If any of these URBs completes with an
error before libusb has finished submitting all of them, it should stop
the submissions (they'll fail anyway with an EREMOTEIO error; perhaps
a different code would be better). It probably already does this.
The kernel changes are transparent for URBs that don't have
USBDEVFS_URB_SHORT_NOT_OK set; they'll just sail on through as before.
Hence versions of libusb that aren't updated should continue to work
with no ill effects.
But for bulk-IN URBs that do have this flag bit set, any fault other
than an unlink will cause the kernel to immediately cancel all the
following URBs for the same endpoint and refuse to accept any new ones.
This state of affairs will persist until an URB that also has
USBDEVFS_URB_XFER_START is encountered, at which point the behavior
reverts to normal. Such an URB would of course mark the beginning of a
new transfer.
The patch behaves correctly in simple tests on my system. We should be
possible to get it incorporated into version 2.6.32 of the kernel if
you want. Do you have a good way to try it out?
Alan Stern
Index: 2.6.30/include/linux/usbdevice_fs.h
===================================================================
--- 2.6.30.orig/include/linux/usbdevice_fs.h
+++ 2.6.30/include/linux/usbdevice_fs.h
@@ -77,6 +77,7 @@ struct usbdevfs_connectinfo {
#define USBDEVFS_URB_SHORT_NOT_OK 0x01
#define USBDEVFS_URB_ISO_ASAP 0x02
+#define USBDEVFS_URB_XFER_START 0x04
#define USBDEVFS_URB_NO_FSBR 0x20
#define USBDEVFS_URB_ZERO_PACKET 0x40
#define USBDEVFS_URB_NO_INTERRUPT 0x80
Index: 2.6.30/drivers/usb/core/devio.c
===================================================================
--- 2.6.30.orig/drivers/usb/core/devio.c
+++ 2.6.30/drivers/usb/core/devio.c
@@ -73,6 +73,7 @@ struct dev_state {
void __user *disccontext;
unsigned long ifclaimed;
u32 secid;
+ u16 disabled_bulk_eps;
};
struct async {
@@ -87,6 +88,8 @@ struct async {
struct urb *urb;
int status;
u32 secid;
+ u16 ep_bit;
+ u8 short_packet_status;
};
static int usbfs_snoop;
@@ -320,6 +323,45 @@ static void snoop_urb(struct urb *urb, v
printk("\n");
}
+#define AS_START 1
+#define AS_UNLINK 2
+
+static void cancel_bulk_urbs(struct dev_state *ps, unsigned ep_bit)
+__releases(ps->lock)
+__acquires(ps->lock)
+{
+ struct async *as;
+
+ /* Mark all the pending URBs that match ep_bit, up to but not
+ * including the first one with AS_START set. If such an URB
+ * is encountered then a new transfer has already started so the
+ * endpoint doesn't need to be disabled; otherwise it does.
+ */
+ list_for_each_entry(as, &ps->async_pending, asynclist) {
+ if (ep_bit == as->ep_bit) {
+ if (as->short_packet_status == AS_START) {
+ ep_bit = 0;
+ break;
+ }
+ as->short_packet_status = AS_UNLINK;
+ as->ep_bit = 0;
+ }
+ }
+ ps->disabled_bulk_eps |= ep_bit;
+
+ /* Now carefully unlink all the marked pending URBs */
+ rescan:
+ list_for_each_entry(as, &ps->async_pending, asynclist) {
+ if (as->short_packet_status == AS_UNLINK) {
+ as->short_packet_status = 0; /* Only once */
+ spin_unlock(&ps->lock); /* Allow completions */
+ usb_unlink_urb(as->urb);
+ spin_lock(&ps->lock);
+ goto rescan;
+ }
+ }
+}
+
static void async_completed(struct urb *urb)
{
struct async *as = urb->context;
@@ -328,6 +370,8 @@ static void async_completed(struct urb *
spin_lock(&ps->lock);
list_move_tail(&as->asynclist, &ps->async_completed);
+ if (urb->status < 0 && urb->status != -ECONNRESET && as->ep_bit)
+ cancel_bulk_urbs(ps, as->ep_bit);
spin_unlock(&ps->lock);
as->status = urb->status;
if (as->signr) {
@@ -978,6 +1022,7 @@ static int proc_do_submiturb(struct dev_
if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
USBDEVFS_URB_SHORT_NOT_OK |
+ USBDEVFS_URB_XFER_START |
USBDEVFS_URB_NO_FSBR |
USBDEVFS_URB_ZERO_PACKET |
USBDEVFS_URB_NO_INTERRUPT))
@@ -1149,6 +1194,8 @@ static int proc_do_submiturb(struct dev_
if (uurb->flags & USBDEVFS_URB_NO_INTERRUPT)
u |= URB_NO_INTERRUPT;
as->urb->transfer_flags = u;
+ if (uurb->flags & USBDEVFS_URB_XFER_START)
+ as->short_packet_status = AS_START;
as->urb->transfer_buffer_length = uurb->buffer_length;
as->urb->setup_packet = (unsigned char *)dr;
@@ -1186,9 +1233,30 @@ static int proc_do_submiturb(struct dev_
return -EFAULT;
}
}
+ if ((uurb->flags & USBDEVFS_URB_SHORT_NOT_OK) &&
+ usb_endpoint_is_bulk_in(&ep->desc))
+ as->ep_bit = 1 << usb_endpoint_num(&ep->desc);
snoop_urb(as->urb, as->userurb);
async_newpending(as);
- if ((ret = usb_submit_urb(as->urb, GFP_KERNEL))) {
+
+ if (as->ep_bit) {
+ /* Don't submit bulk-IN URBs if the endpoint is disabled
+ * because a short packet was received, unless this URB
+ * is the start of a new transfer.
+ */
+ spin_lock_irq(&ps->lock);
+ if (as->short_packet_status == AS_START)
+ ps->disabled_bulk_eps &= ~as->ep_bit;
+ if (ps->disabled_bulk_eps & as->ep_bit)
+ ret = -EREMOTEIO;
+ else
+ ret = usb_submit_urb(as->urb, GFP_ATOMIC);
+ spin_unlock_irq(&ps->lock);
+ } else {
+ ret = usb_submit_urb(as->urb, GFP_KERNEL);
+ }
+
+ if (ret) {
dev_printk(KERN_DEBUG, &ps->dev->dev,
"usbfs: usb_submit_urb returned %d\n", ret);
async_removepending(as);
--
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
