Commit 8959bd88 authored by David Brownell's avatar David Brownell Committed by Greg Kroah-Hartman

[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.
parent 859f1bb3
...@@ -26,19 +26,19 @@ ...@@ -26,19 +26,19 @@
#include <linux/module.h> #include <linux/module.h>
#include <linux/version.h> #include <linux/version.h>
#include <linux/kernel.h> #include <linux/kernel.h>
#ifdef CONFIG_USB_DEBUG
# define DEBUG
#else
# undef DEBUG
#endif
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/completion.h> #include <linux/completion.h>
#include <linux/uts.h> /* for UTS_SYSNAME */ #include <linux/uts.h> /* for UTS_SYSNAME */
#include <linux/pci.h> /* for hcd->pdev and dma addressing */ #include <linux/pci.h> /* for hcd->pdev and dma addressing */
#include <asm/byteorder.h> #include <asm/byteorder.h>
#ifdef CONFIG_USB_DEBUG
#define DEBUG
#else
#undef DEBUG
#endif
#include <linux/usb.h> #include <linux/usb.h>
#include "hcd.h" #include "hcd.h"
...@@ -1090,6 +1090,7 @@ static int hcd_unlink_urb (struct urb *urb) ...@@ -1090,6 +1090,7 @@ static int hcd_unlink_urb (struct urb *urb)
{ {
struct hcd_dev *dev; struct hcd_dev *dev;
struct usb_hcd *hcd = 0; struct usb_hcd *hcd = 0;
struct device *sys = 0;
unsigned long flags; unsigned long flags;
struct completion_splice splice; struct completion_splice splice;
int retval; int retval;
...@@ -1110,38 +1111,31 @@ static int hcd_unlink_urb (struct urb *urb) ...@@ -1110,38 +1111,31 @@ static int hcd_unlink_urb (struct urb *urb)
*/ */
spin_lock_irqsave (&urb->lock, flags); spin_lock_irqsave (&urb->lock, flags);
spin_lock (&hcd_data_lock); spin_lock (&hcd_data_lock);
if (!urb->hcpriv || urb->transfer_flags & URB_TIMEOUT_KILLED) {
retval = -EINVAL;
goto done;
}
if (!urb->dev || !urb->dev->bus) { if (!urb->dev || !urb->dev->bus) {
retval = -ENODEV; retval = -ENODEV;
goto done; goto done;
} }
/* giveback clears dev; non-null means it's linked at this level */
dev = urb->dev->hcpriv; dev = urb->dev->hcpriv;
sys = &urb->dev->dev;
hcd = urb->dev->bus->hcpriv; hcd = urb->dev->bus->hcpriv;
if (!dev || !hcd) { if (!dev || !hcd) {
retval = -ENODEV; retval = -ENODEV;
goto done; goto done;
} }
/* Except for interrupt transfers, any status except -EINPROGRESS if (!urb->hcpriv) {
* means the HCD already started to unlink this URB from the hardware. retval = -EINVAL;
* So there's no more work to do. goto done;
* }
* For interrupt transfers, this is the only way to trigger unlinking
* from the hardware. Since we (currently) overload urb->status to /* Any status except -EINPROGRESS means something already started to
* tell the driver to unlink, error status might get clobbered ... * unlink this URB from the hardware. So there's no more work to do.
* 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 * FIXME use better explicit urb state
*/ */
if (urb->status != -EINPROGRESS if (urb->status != -EINPROGRESS) {
&& usb_pipetype (urb->pipe) != PIPE_INTERRUPT) {
retval = -EINVAL; retval = -EINVAL;
goto done; goto done;
} }
...@@ -1150,9 +1144,7 @@ static int hcd_unlink_urb (struct urb *urb) ...@@ -1150,9 +1144,7 @@ static int hcd_unlink_urb (struct urb *urb)
* lower level hcd code is always async, locking on urb->status * lower level hcd code is always async, locking on urb->status
* updates; an intercepted completion unblocks us. * updates; an intercepted completion unblocks us.
*/ */
if ((urb->transfer_flags & URB_TIMEOUT_KILLED)) if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
urb->status = -ETIMEDOUT;
else if (!(urb->transfer_flags & URB_ASYNC_UNLINK)) {
if (in_interrupt ()) { if (in_interrupt ()) {
dbg ("non-async unlink in_interrupt"); dbg ("non-async unlink in_interrupt");
retval = -EWOULDBLOCK; retval = -EWOULDBLOCK;
...@@ -1177,29 +1169,34 @@ static int hcd_unlink_urb (struct urb *urb) ...@@ -1177,29 +1169,34 @@ static int hcd_unlink_urb (struct urb *urb)
retval = 0; retval = 0;
} else { } else {
retval = hcd->driver->urb_dequeue (hcd, urb); 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 */ /* block till giveback, if needed */
if (!(urb->transfer_flags & (URB_ASYNC_UNLINK|URB_TIMEOUT_KILLED)) if (urb->transfer_flags & URB_ASYNC_UNLINK)
&& 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) {
return -EINPROGRESS; return -EINPROGRESS;
}
goto bye; dev_dbg (*sys, "wait for giveback urb %p\n", urb);
wait_for_completion (&splice.done);
return 0;
done: done:
spin_unlock (&hcd_data_lock); spin_unlock (&hcd_data_lock);
spin_unlock_irqrestore (&urb->lock, flags); spin_unlock_irqrestore (&urb->lock, flags);
bye: bye:
if (retval) if (retval && sys)
dbg ("%s: hcd_unlink_urb fail %d", dev_dbg (*sys, "hcd_unlink_urb %p fail %d\n", urb, retval);
hcd ? hcd->self.bus_name : "(no bus?)",
retval);
return retval; return retval;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment