ChangeSet 1.823.3.18, 2002/11/14 11:25:53-08:00, [EMAIL PROTECTED]

[PATCH] cleanup usb hcd unlink code

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.


diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c    Thu Nov 14 14:12:08 2002
+++ b/drivers/usb/core/hcd.c    Thu Nov 14 14:12:08 2002
@@ -26,19 +26,19 @@
 #include <linux/module.h>
 #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>
 #include <linux/uts.h>                 /* for UTS_SYSNAME */
 #include <linux/pci.h>                 /* for hcd->pdev and dma addressing */
 #include <asm/byteorder.h>
 
-
-#ifdef CONFIG_USB_DEBUG
-       #define DEBUG
-#else
-       #undef DEBUG
-#endif
-
 #include <linux/usb.h>
 #include "hcd.h"
 
@@ -1090,6 +1090,7 @@
 {
        struct hcd_dev                  *dev;
        struct usb_hcd                  *hcd = 0;
+       struct device                   *sys = 0;
        unsigned long                   flags;
        struct completion_splice        splice;
        int                             retval;
@@ -1110,38 +1111,31 @@
         */
        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) {
                retval = -ENODEV;
                goto done;
        }
 
-       /* 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) {
                retval = -ENODEV;
                goto done;
        }
 
-       /* 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;
        }
@@ -1150,9 +1144,7 @@
         * lower level hcd code is always async, locking on urb->status
         * 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");
                        retval = -EWOULDBLOCK;
@@ -1177,29 +1169,34 @@
                retval = 0;
        } 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;
 }
 


-------------------------------------------------------
This sf.net email is sponsored by: To learn the basics of securing 
your web site with SSL, click here to get a FREE TRIAL of a Thawte 
Server Certificate: http://www.gothawte.com/rd524.html
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to