This fixes various minor problems:

- re-orders some tests so that "(no bus?)" diagnostic should
   no longer be appearing (and making folk worry needlessly)

- removes one unreachable test for URB_TIMEOUT_KILLED

- removes the reachable test, since it's never an error on the
   part of the device driver to unlink something the HCD is already
   unlinking.

- gets rid of some comments and code that expected automagic resubmits
   for interrupts (no more!),

- resolves a FIXME for a rather unlikely situation (HCD can't
   perform the unlink, it reports an error)

It also starts to use dev_dbg() macros, which give more concise
(lately) and useful (they have both driver name and device id)
diagnostics than the previous usb-only dbg() macros.  To do this,
DEBUG had to be #defined before <linux/driver.h> is included, but
it can't be #undeffed before <linux/kernel.h> is included.

- Dave

--- ./drivers/usb-dist/core/hcd.c       Mon Nov 11 06:37:46 2002
+++ ./drivers/usb/core/hcd.c    Tue Nov 12 12:49:05 2002
@@ -27,4 +27,11 @@
 #include <linux/version.h>
 #include <linux/kernel.h>
+
+#ifdef CONFIG_USB_DEBUG
+#      define DEBUG
+#else
+#      undef DEBUG
+#endif
+
 #include <linux/slab.h>
 #include <linux/completion.h>
@@ -33,11 +40,4 @@
 #include <asm/byteorder.h>
 
-
-#ifdef CONFIG_USB_DEBUG
-       #define DEBUG
-#else
-       #undef DEBUG
-#endif
-
 #include <linux/usb.h>
 #include "hcd.h"
@@ -1091,4 +1078,5 @@
        struct hcd_dev                  *dev;
        struct usb_hcd                  *hcd = 0;
+       struct device                   *sys = 0;
        unsigned long                   flags;
        struct completion_splice        splice;
@@ -1111,8 +1099,4 @@
        spin_lock_irqsave (&urb->lock, flags);
        spin_lock (&hcd_data_lock);
-       if (!urb->hcpriv || urb->transfer_flags & URB_TIMEOUT_KILLED) {
-               retval = -EINVAL;
-               goto done;
-       }
 
        if (!urb->dev || !urb->dev->bus) {
@@ -1121,6 +1105,6 @@
        }
 
-       /* giveback clears dev; non-null means it's linked at this level */
        dev = urb->dev->hcpriv;
+       sys = &urb->dev->dev;
        hcd = urb->dev->bus->hcpriv;
        if (!dev || !hcd) {
@@ -1129,18 +1113,15 @@
        }
 
-       /* Except for interrupt transfers, any status except -EINPROGRESS
-        * means the HCD already started to unlink this URB from the hardware.
-        * So there's no more work to do.
-        *
-        * For interrupt 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.
+       if (!urb->hcpriv) {
+               retval = -EINVAL;
+               goto done;
+       }
+
+       /* Any status except -EINPROGRESS means something already started to
+        * unlink this URB from the hardware.  So there's no more work to do.
         *
-        * FIXME use an URB_UNLINKED flag to match URB_TIMEOUT_KILLED
+        * FIXME use better explicit urb state
         */
-       if (urb->status != -EINPROGRESS
-                       && usb_pipetype (urb->pipe) != PIPE_INTERRUPT) {
+       if (urb->status != -EINPROGRESS) {
                retval = -EINVAL;
                goto done;
@@ -1151,7 +1132,5 @@
         * updates; an intercepted completion unblocks us.
         */
-       if ((urb->transfer_flags & URB_TIMEOUT_KILLED))
-               urb->status = -ETIMEDOUT;
-       else if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
+       if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
                if (in_interrupt ()) {
                        dbg ("non-async unlink in_interrupt");
@@ -1178,27 +1157,32 @@
        } else {
                retval = hcd->driver->urb_dequeue (hcd, urb);
-// FIXME:  if retval and we tried to splice, whoa!!
-if (retval && urb->status == -ENOENT) err ("whoa! retval %d", retval);
+
+               /* hcds shouldn't really fail these calls, but... */
+               if (retval) {
+                       dev_dbg (*sys, "dequeue %p --> %d\n", urb, retval);
+                       if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
+                               spin_lock_irqsave (&urb->lock, flags);
+                               urb->complete = splice.complete;
+                               urb->context = splice.context;
+                               spin_unlock_irqrestore (&urb->lock, flags);
+                       }
+                       goto bye;
+               }
        }
 
        /* block till giveback, if needed */
-       if (!(urb->transfer_flags & (URB_ASYNC_UNLINK|URB_TIMEOUT_KILLED))
-                       && HCD_IS_RUNNING (hcd->state)
-                       && !retval) {
-               dbg ("%s: wait for giveback urb %p",
-                       hcd->self.bus_name, urb);
-               wait_for_completion (&splice.done);
-       } else if ((urb->transfer_flags & URB_ASYNC_UNLINK) && retval == 0) {
+       if (urb->transfer_flags & URB_ASYNC_UNLINK)
                return -EINPROGRESS;
-       }
-       goto bye;
+
+       dev_dbg (*sys, "wait for giveback urb %p\n", urb);
+       wait_for_completion (&splice.done);
+       return 0;
+
 done:
        spin_unlock (&hcd_data_lock);
        spin_unlock_irqrestore (&urb->lock, flags);
 bye:
-       if (retval)
-               dbg ("%s: hcd_unlink_urb fail %d",
-                   hcd ? hcd->self.bus_name : "(no bus?)",
-                   retval);
+       if (retval && sys)
+               dev_dbg (*sys, "hcd_unlink_urb %p fail %d\n", urb, retval);
        return retval;
 }

Reply via email to