Hi,
By playing with usb-ohci, I've managed to find a bug in
irda-usb. That may explain some of the problem I've seen and that I
was tracking.
I did a quick analysis and came up with the patch attached
(that also clean up some related items). I still need to test it on
usb-uhci, and I will bake it off in the next weeks on my box before
sending it to Linus (unless Dag has comments on my comments).
I welcome you to test the patch and tell me how it works for
you...
Have fun...
Jean
diff -u -p linux/include/net/irda/irda-usb.d9.h linux/include/net/irda/irda-usb.h
--- linux/include/net/irda/irda-usb.d9.h Mon Sep 17 18:26:17 2001
+++ linux/include/net/irda/irda-usb.h Thu Sep 20 14:02:17 2001
@@ -1,7 +1,7 @@
/*****************************************************************************
*
* Filename: irda-usb.h
- * Version: 0.9a
+ * Version: 0.9b
* Description: IrDA-USB Driver
* Status: Experimental
* Author: Dag Brattli <[EMAIL PROTECTED]>
@@ -68,7 +68,7 @@
* ourselves... See also following option. */
#undef IU_BUG_KICK_TX
/* Use the USB_ZERO_PACKET flag instead of sending empty frame (above)
- * Work only with usb-uhci.o so far. Please fix uhic.c and usb-ohci.c */
+ * Work only with usb-uhci.o and usb-ohci.c so far. Please fix uhic.c */
#define IU_USE_USB_ZERO_FLAG
/* Send speed command in case of timeout, just for trying to get things sane */
#define IU_BUG_KICK_TIMEOUT
diff -u -p linux/drivers/net/irda/irda-usb.d9.c linux/drivers/net/irda/irda-usb.c
--- linux/drivers/net/irda/irda-usb.d9.c Fri Sep 14 19:00:31 2001
+++ linux/drivers/net/irda/irda-usb.c Thu Sep 20 14:21:41 2001
@@ -1,7 +1,7 @@
/*****************************************************************************
*
* Filename: irda-usb.c
- * Version: 0.9a
+ * Version: 0.9b
* Description: IrDA-USB Driver
* Status: Experimental
* Author: Dag Brattli <[EMAIL PROTECTED]>
@@ -239,7 +239,7 @@ static void irda_usb_change_speed_xbofs(
frame, IRDA_USB_SPEED_MTU,
speed_bulk_callback, self);
purb->transfer_buffer_length = USB_IRDA_HEADER;
- purb->transfer_flags = USB_QUEUE_BULK;
+ purb->transfer_flags = USB_QUEUE_BULK | USB_ASYNC_UNLINK;
purb->timeout = MSECS_TO_JIFFIES(100);
if ((ret = usb_submit_urb(purb))) {
@@ -275,7 +275,7 @@ static inline void irda_usb_send_empty(s
self->speed_buff, IRDA_USB_SPEED_MTU,
speed_bulk_callback, self);
purb->transfer_buffer_length = 0;
- purb->transfer_flags = USB_QUEUE_BULK;
+ purb->transfer_flags = USB_QUEUE_BULK | USB_ASYNC_UNLINK;
purb->timeout = MSECS_TO_JIFFIES(100);
if ((ret = usb_submit_urb(purb))) {
@@ -397,11 +397,13 @@ static int irda_usb_hard_xmit(struct sk_
skb->data, IRDA_USB_MAX_MTU,
write_bulk_callback, skb);
purb->transfer_buffer_length = skb->len;
- purb->transfer_flags = USB_QUEUE_BULK;
+ /* Note : unlink *must* be Asynchronous because of the code in
+ * irda_usb_net_timeout() -> call in irq - Jean II */
+ purb->transfer_flags = USB_QUEUE_BULK | USB_ASYNC_UNLINK;
#ifdef IU_USE_USB_ZERO_FLAG
/* This flag indicates that what we send is not a continuous stream
* of data but separate frames. In this case, the USB layer will
- * insert empty packet to separate our frames.
+ * insert empty packet as needed to separate our frames.
* This flag was previously called USB_DISABLE_SPD - Jean II */
purb->transfer_flags |= USB_ZERO_PACKET;
#endif IU_USE_USB_ZERO_FLAG
@@ -563,20 +565,19 @@ static void irda_usb_net_timeout(struct
WARNING("%s: Empty change timed out, urb->status=%d,
urb->transfer_flags=0x%04X\n", netdev->name, purb->status, purb->transfer_flags);
switch (purb->status) {
- case -ECONNABORTED: /* -103 */
- case -ECONNRESET: /* -104 */
- case -ENOENT: /* -2 */
- purb->status = USB_ST_NOERROR;
- done = 1;
- break;
case USB_ST_URB_PENDING: /* -EINPROGRESS == -115 */
usb_unlink_urb(purb);
/* Note : above will *NOT* call netif_wake_queue()
* in completion handler - Jean II */
done = 1;
break;
- default:
- /* ??? */
+ case -ECONNABORTED: /* -103 */
+ case -ECONNRESET: /* -104 */
+ case -ETIMEDOUT: /* -110 */
+ case -ENOENT: /* -2 (urb unlinked by us) */
+ default: /* ??? - Play safe */
+ purb->status = USB_ST_NOERROR;
+ done = 1;
break;
}
}
@@ -588,22 +589,22 @@ static void irda_usb_net_timeout(struct
WARNING("%s: Speed change timed out, urb->status=%d,
urb->transfer_flags=0x%04X\n", netdev->name, purb->status, purb->transfer_flags);
switch (purb->status) {
+ case USB_ST_URB_PENDING: /* -EINPROGRESS == -115 */
+ usb_unlink_urb(purb);
+ /* Note : above will *NOT* call netif_wake_queue()
+ * in completion handler, we will come back here.
+ * Jean II */
+ done = 1;
+ break;
case -ECONNABORTED: /* -103 */
case -ECONNRESET: /* -104 */
- case -ENOENT: /* -2 */
+ case -ETIMEDOUT: /* -110 */
+ case -ENOENT: /* -2 (urb unlinked by us) */
+ default: /* ??? - Play safe */
purb->status = USB_ST_NOERROR;
netif_wake_queue(self->netdev);
done = 1;
break;
- case USB_ST_URB_PENDING: /* -EINPROGRESS == -115 */
- usb_unlink_urb(purb);
- /* Note : above will call netif_wake_queue()
- * in completion handler - Jean II */
- done = 1;
- break;
- default:
- /* ??? */
- break;
}
}
@@ -627,9 +628,22 @@ static void irda_usb_net_timeout(struct
#endif IU_BUG_KICK_TIMEOUT
switch (purb->status) {
+ case USB_ST_URB_PENDING: /* -EINPROGRESS == -115 */
+ usb_unlink_urb(purb);
+ /* Note : above will *NOT* call netif_wake_queue()
+ * in completion handler, because purb->status will
+ * be -ENOENT. We will fix that at the next watchdog,
+ * leaving more time to USB to recover...
+ * Also, we are in interrupt, so we need to have
+ * USB_ASYNC_UNLINK to work properly...
+ * Jean II */
+ done = 1;
+ break;
case -ECONNABORTED: /* -103 */
case -ECONNRESET: /* -104 */
- case -ENOENT: /* -2 */
+ case -ETIMEDOUT: /* -110 */
+ case -ENOENT: /* -2 (urb unlinked by us) */
+ default: /* ??? - Play safe */
if(skb != NULL) {
dev_kfree_skb_any(skb);
purb->context = NULL;
@@ -638,15 +652,6 @@ static void irda_usb_net_timeout(struct
netif_wake_queue(self->netdev);
done = 1;
break;
- case USB_ST_URB_PENDING: /* -EINPROGRESS == -115 */
- usb_unlink_urb(purb);
- /* Note : above will call netif_wake_queue()
- * in completion handler - Jean II */
- done = 1;
- break;
- default:
- /* ??? */
- break;
}
}
@@ -740,6 +745,8 @@ static void irda_usb_submit(struct irda_
skb->data, skb->truesize,
irda_usb_receive, skb);
purb->transfer_flags = USB_QUEUE_BULK;
+ /* Note : unlink *must* be synchronous because of the code in
+ * irda_usb_net_close() -> free the skb - Jean II */
purb->status = USB_ST_NOERROR;
purb->next = NULL; /* Don't auto resubmit URBs */
