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; }