Hi,

Here's a patch against 2.5.3-pre1 for the USB hcd driver that removes a
workaround when unlinking periodic urbs in their completion handlers.
This patch was written by David Brownell.

thanks,

greg k-h


diff -Nru a/drivers/usb/hcd.c b/drivers/usb/hcd.c
--- a/drivers/usb/hcd.c Wed Jan 16 09:57:47 2002
+++ b/drivers/usb/hcd.c Wed Jan 16 09:57:47 2002
@@ -1081,20 +1081,20 @@
        if (!urb)
                return -EINVAL;
 
-       // FIXME:  add some explicit records to flag the
-       // state where the URB is "in periodic completion".
-       // Workaround is for driver to set the urb status
-       // to "-EINPROGRESS", so it can get through here
-       // and unlink from the completion handler.
-
        /*
         * we contend for urb->status with the hcd core,
         * which changes it while returning the urb.
+        *
+        * Caller guaranteed that the urb pointer hasn't been freed, and
+        * that it was submitted.  But as a rule it can't know whether or
+        * not it's already been unlinked ... so we respect the reversed
+        * lock sequence needed for the usb_hcd_giveback_urb() code paths
+        * (urb lock, then hcd_data_lock) in case some other CPU is now
+        * unlinking it.
         */
        spin_lock_irqsave (&urb->lock, flags);
-       if (!urb->hcpriv
-                       || urb->status != -EINPROGRESS
-                       || urb->transfer_flags & USB_TIMEOUT_KILLED) {
+       spin_lock (&hcd_data_lock);
+       if (!urb->hcpriv || urb->transfer_flags & USB_TIMEOUT_KILLED) {
                retval = -EINVAL;
                goto done;
        }
@@ -1103,6 +1103,8 @@
                retval = -ENODEV;
                goto done;
        }
+
+       /* giveback clears dev; non-null means it's linked at this level */
        dev = urb->dev->hcpriv;
        hcd = urb->dev->bus->hcpriv;
        if (!dev || !hcd) {
@@ -1110,6 +1112,27 @@
                goto done;
        }
 
+       /* For non-periodic transfers, any status except -EINPROGRESS means
+        * the HCD has already started to unlink this URB from the hardware.
+        * In that case, there's no more work to do.
+        *
+        * For periodic transfers, this is the only way to trigger unlinking
+        * from the hardware.  Since we (currently) overload urb->status to
+        * tell the driver to unlink, error status might get clobbered ...
+        * unless that transfer hasn't yet restarted.  One such case is when
+        * the URB gets unlinked from its completion handler.
+        *
+        * FIXME use an URB_UNLINKED flag to match URB_TIMEOUT_KILLED
+        */
+       switch (usb_pipetype (urb->pipe)) {
+       case PIPE_CONTROL:
+       case PIPE_BULK:
+               if (urb->status != -EINPROGRESS) {
+                       retval = 0;
+                       goto done;
+               }
+       }
+
        /* maybe set up to block on completion notification */
        if ((urb->transfer_flags & USB_TIMEOUT_KILLED))
                urb->status = -ETIMEDOUT;
@@ -1130,6 +1153,7 @@
                /* asynchronous unlink */
                urb->status = -ECONNRESET;
        }
+       spin_unlock (&hcd_data_lock);
        spin_unlock_irqrestore (&urb->lock, flags);
 
        if (urb == (struct urb *) hcd->rh_timer.data) {
@@ -1154,6 +1178,7 @@
        }
        goto bye;
 done:
+       spin_unlock (&hcd_data_lock);
        spin_unlock_irqrestore (&urb->lock, flags);
 bye:
        if (retval)


_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to