Hi,
Cleanup of the USB driver + the critical fix of last week.
Jean
diff -u -p linux/include/net/irda/irda-usb.d0.h linux/include/net/irda/irda-usb.h
--- linux/include/net/irda/irda-usb.d0.h Fri Sep 28 11:18:48 2001
+++ linux/include/net/irda/irda-usb.h Fri Sep 28 11:37:10 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]>
@@ -63,13 +63,6 @@
#define IU_MAX_RX_URBS (IU_MAX_ACTIVE_RX_URBS + 1)
/* Various ugly stuff to try to workaround generic problems */
-/* The USB layer should send empty frames at the end of packets multiple
- * of the frame size. As it doesn't do it by default, we need to do it
- * 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 */
-#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
/* Show the USB class descriptor */
@@ -136,9 +129,6 @@ struct irda_usb_cb {
struct urb *idle_rx_urb; /* Pointer to idle URB in Rx path */
struct urb tx_urb; /* URB used to send data frames */
struct urb speed_urb; /* URB used to send speed commands */
-#ifdef IU_BUG_KICK_TX
- struct urb empty_urb; /* URB used to send empty commands */
-#endif IU_BUG_KICK_TX
struct net_device *netdev; /* Yes! we are some kind of netdev. */
struct net_device_stats stats;
diff -u -p linux/drivers/net/irda/irda-usb.d0.c linux/drivers/net/irda/irda-usb.c
--- linux/drivers/net/irda/irda-usb.d0.c Fri Sep 28 11:17:47 2001
+++ linux/drivers/net/irda/irda-usb.c Fri Sep 28 11:52:44 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]>
@@ -26,6 +26,31 @@
*
*****************************************************************************/
+/*
+ * IMPORTANT NOTE
+ * --------------
+ *
+ * As of kernel 2.4.10, this is the state of compliance and testing of
+ * this driver (irda-usb) with regards to the USB low level drivers...
+ *
+ * This driver has been tested SUCCESSFULLY with the following drivers :
+ * o usb-uhci (For Intel/Via USB controllers)
+ * o usb-ohci (For other USB controllers)
+ *
+ * This driver has NOT been tested with the following drivers :
+ * o usb-ehci (USB 2.0 controllers)
+ *
+ * This driver WON'T WORK with the following drivers :
+ * o uhci (Alternate/JE driver for Intel/Via USB controllers)
+ * Amongst the reasons :
+ * o uhci doesn't implement USB_ZERO_PACKET
+ * o uhci non-compliant use of urb->timeout
+ *
+ * Jean II
+ */
+
+/*------------------------------------------------------------------*/
+
#include <linux/module.h>
#include <linux/kernel.h>
@@ -44,6 +69,8 @@
#include <net/irda/irda-usb.h>
+/*------------------------------------------------------------------*/
+
static int qos_mtt_bits = 0;
/* Master instance for each hardware found */
@@ -239,7 +266,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))) {
@@ -248,42 +275,6 @@ static void irda_usb_change_speed_xbofs(
spin_unlock_irqrestore(&self->lock, flags);
}
-#ifdef IU_BUG_KICK_TX
-/*------------------------------------------------------------------*/
-/*
- * Send an empty URB to the dongle
- * The goal there is to try to resynchronise with the dongle. An empty
- * frame signify the end of a Tx frame. Jean II
- */
-static inline void irda_usb_send_empty(struct irda_usb_cb *self)
-{
- purb_t purb;
- int ret;
-
- IRDA_DEBUG(0, __FUNCTION__ "()\n");
-
- /* Grab the empty URB */
- purb = &self->empty_urb;
- if (purb->status != USB_ST_NOERROR) {
- WARNING(__FUNCTION__ "(), Empty URB still in use!\n");
- return;
- }
-
- /* Submit the Empty URB */
- FILL_BULK_URB(purb, self->usbdev,
- usb_sndbulkpipe(self->usbdev, self->bulk_out_ep),
- self->speed_buff, IRDA_USB_SPEED_MTU,
- speed_bulk_callback, self);
- purb->transfer_buffer_length = 0;
- purb->transfer_flags = USB_QUEUE_BULK;
- purb->timeout = MSECS_TO_JIFFIES(100);
-
- if ((ret = usb_submit_urb(purb))) {
- IRDA_DEBUG(0, __FUNCTION__ "(), failed Empty URB\n");
- }
-}
-#endif /* IU_BUG_KICK_TX */
-
/*------------------------------------------------------------------*/
/*
* Note : this function will be called with both speed_urb and empty_urb...
@@ -397,19 +388,21 @@ 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;
-#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.
- * This flag was previously called USB_DISABLE_SPD - Jean II */
+ /* 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;
+ /* This flag (USB_ZERO_PACKET) indicates that what we send is not
+ * a continuous stream of data but separate packets.
+ * In this case, the USB layer will insert an empty USB frame (TD)
+ * after each of our packets that is exact multiple of the frame size.
+ * This is how the dongle will detect the end of packet - Jean II */
purb->transfer_flags |= USB_ZERO_PACKET;
-#endif /* IU_USE_USB_ZERO_FLAG */
+ /* Timeout need to be shorter than NET watchdog timer */
purb->timeout = MSECS_TO_JIFFIES(100);
/* Generate min turn time. FIXME: can we do better than this? */
/* Trying to a turnaround time at this level is trying to measure
- * processor clock cycle with a watch, approximate at best...
+ * processor clock cycle with a wrist-watch, approximate at best...
*
* What we know is the last time we received a frame over USB.
* Due to latency over USB that depend on the USB load, we don't
@@ -443,6 +436,7 @@ static int irda_usb_hard_xmit(struct sk_
}
}
+ /* Ask USB to send the packet */
if ((res = usb_submit_urb(purb))) {
IRDA_DEBUG(0, __FUNCTION__ "(), failed Tx URB\n");
self->stats.tx_errors++;
@@ -454,25 +448,6 @@ static int irda_usb_hard_xmit(struct sk_
self->stats.tx_bytes += skb->len;
netdev->trans_start = jiffies;
-
-#ifdef IU_BUG_KICK_TX
- /* Kick Tx?
- * If the packet is a multiple of 64, the USB layer
- * should send an empty frame (a short packet) to signal
- * the end of frame (that's part of the USB spec).
- * If we enable USB_ZERO_PACKET, the USB layer will just do
- * that (more efficiently) and this code is useless.
- * Better keep this code until USB code clear up this mess...
- *
- * Note : we can't use the speed URB, because the frame
- * might contain a speed change that may be deferred
- * (so we have hard_xmit => tx_urb+empty_urb+speed_urb).
- * Jean II */
- if ((skb->len % self->bulk_out_mtu) == 0) {
- IRDA_DEBUG(2, __FUNCTION__ "(), Kick Tx...\n");
- irda_usb_send_empty(self);
- }
-#endif /* IU_BUG_KICK_TX */
}
spin_unlock_irqrestore(&self->lock, flags);
@@ -556,54 +531,28 @@ static void irda_usb_net_timeout(struct
return;
}
-#ifdef IU_BUG_KICK_TX
- /* Check empty URB */
- purb = &(self->empty_urb);
+ /* Check speed URB */
+ purb = &(self->speed_urb);
if (purb->status != USB_ST_NOERROR) {
- WARNING("%s: Empty change timed out, urb->status=%d,
urb->transfer_flags=0x%04X\n", netdev->name, purb->status, purb->transfer_flags);
+ 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 -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 */
+ /* Note : above will *NOT* call netif_wake_queue()
+ * in completion handler, we will come back here.
+ * Jean II */
done = 1;
break;
- default:
- /* ??? */
- break;
- }
- }
-#endif /* IU_BUG_KICK_TX */
-
- /* Check speed URB */
- purb = &(self->speed_urb);
- if (purb->status != USB_ST_NOERROR) {
- 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 -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 +576,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 +600,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 +693,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 */