ChangeSet 1.1557.49.5, 2004/02/17 16:20:54-08:00, stern@rowland.harvard.edu
[PATCH] USB: Remove unneeded and error-provoking variable in UHCI
This patch removes an unneeded "status" field from the UHCI driver's
URB-private data structure. The driver had been storing the status of
completed URBs there rather than in the URBs themselves. This not only is
wasteful of space but is also a cause of bugs, since the USB core relies
on urb->status for proper synchronization with the driver.
The patch also takes care of a number of small things that have been
bugging me for ages:
Close a small loophole by allowing an URB to be unlinked even
before the uhci_urb_enqueue() procedure has started.
Remove some fossil code from back when interrupt URBs were
automagically resubmitted.
Giveback unlinked URBs in the order they were unlinked.
Don't set urb->status for dequeued URBs; the core has already
set it for us.
Rename uhci_remove_pending_qhs to uhci_remove_pending_urbps.
(That has _really_ bothered me!)
drivers/usb/host/uhci-debug.c | 4 +--
drivers/usb/host/uhci-hcd.c | 50 ++++++++++++------------------------------
drivers/usb/host/uhci-hcd.h | 2 -
3 files changed, 17 insertions(+), 39 deletions(-)
diff -Nru a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
--- a/drivers/usb/host/uhci-debug.c Thu Feb 19 17:22:42 2004
+++ b/drivers/usb/host/uhci-debug.c Thu Feb 19 17:22:42 2004
@@ -321,8 +321,8 @@
out += sprintf(out, "%s", (urbp->fsbr ? "FSBR " : ""));
out += sprintf(out, "%s", (urbp->fsbr_timeout ? "FSBR_TO " : ""));
- if (urbp->status != -EINPROGRESS)
- out += sprintf(out, "Status=%d ", urbp->status);
+ if (urbp->urb->status != -EINPROGRESS)
+ out += sprintf(out, "Status=%d ", urbp->urb->status);
//out += sprintf(out, "Inserttime=%lx ",urbp->inserttime);
//out += sprintf(out, "FSBRtime=%lx ",urbp->fsbrtime);
diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
--- a/drivers/usb/host/uhci-hcd.c Thu Feb 19 17:22:42 2004
+++ b/drivers/usb/host/uhci-hcd.c Thu Feb 19 17:22:42 2004
@@ -1462,11 +1462,14 @@
spin_lock_irqsave(&uhci->urb_list_lock, flags);
+ if (urb->status != -EINPROGRESS) /* URB already unlinked! */
+ goto out;
+
eurb = uhci_find_urb_ep(uhci, urb);
if (!uhci_alloc_urb_priv(uhci, urb)) {
- spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out;
}
switch (usb_pipetype(urb->pipe)) {
@@ -1514,10 +1517,11 @@
return ret;
}
+ ret = 0;
+out:
spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
-
- return 0;
+ return ret;
}
/*
@@ -1527,7 +1531,7 @@
*/
static void uhci_transfer_result(struct uhci_hcd *uhci, struct urb *urb)
{
- int ret = -EINVAL;
+ int ret = -EINPROGRESS;
unsigned long flags;
struct urb_priv *urbp;
@@ -1535,10 +1539,8 @@
urbp = (struct urb_priv *)urb->hcpriv;
- if (urb->status != -EINPROGRESS) {
- info("uhci_transfer_result: called for URB %p not in flight?", urb);
+ if (urb->status != -EINPROGRESS) /* URB already dequeued */
goto out;
- }
switch (usb_pipetype(urb->pipe)) {
case PIPE_CONTROL:
@@ -1555,10 +1557,9 @@
break;
}
- urbp->status = ret;
-
if (ret == -EINPROGRESS)
goto out;
+ urb->status = ret;
switch (usb_pipetype(urb->pipe)) {
case PIPE_CONTROL:
@@ -1607,10 +1608,6 @@
struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
int prevactive = 1;
- /* We can get called when urbp allocation fails, so check */
- if (!urbp)
- return;
-
uhci_dec_fsbr(uhci, urb); /* Safe since it checks */
/*
@@ -1660,13 +1657,6 @@
unsigned long flags;
struct urb_priv *urbp = urb->hcpriv;
- /* If this is an interrupt URB that is being killed in urb->complete, */
- /* then just set its status and return */
- if (!urbp) {
- urb->status = -ECONNRESET;
- return 0;
- }
-
spin_lock_irqsave(&uhci->urb_list_lock, flags);
list_del_init(&urbp->urb_list);
@@ -1678,7 +1668,7 @@
/* If we're the first, set the next interrupt bit */
if (list_empty(&uhci->urb_remove_list))
uhci_set_next_interrupt(uhci);
- list_add(&urbp->urb_list, &uhci->urb_remove_list);
+ list_add_tail(&urbp->urb_list, &uhci->urb_remove_list);
spin_unlock(&uhci->urb_remove_list_lock);
spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
@@ -1844,17 +1834,11 @@
static void uhci_finish_urb(struct usb_hcd *hcd, struct urb *urb, struct pt_regs *regs)
{
- struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
- int status;
unsigned long flags;
spin_lock_irqsave(&urb->lock, flags);
- status = urbp->status;
uhci_destroy_urb_priv(uhci, urb);
-
- if (urb->status != -ENOENT && urb->status != -ECONNRESET)
- urb->status = status;
spin_unlock_irqrestore(&urb->lock, flags);
usb_hcd_giveback_urb(hcd, urb, regs);
@@ -1885,7 +1869,7 @@
spin_unlock_irqrestore(&uhci->complete_list_lock, flags);
}
-static void uhci_remove_pending_qhs(struct uhci_hcd *uhci)
+static void uhci_remove_pending_urbps(struct uhci_hcd *uhci)
{
struct list_head *tmp, *head;
unsigned long flags;
@@ -1898,11 +1882,7 @@
struct urb *urb = urbp->urb;
tmp = tmp->next;
-
list_del_init(&urbp->urb_list);
-
- urbp->status = urb->status = -ECONNRESET;
-
uhci_add_complete(uhci, urb);
}
spin_unlock_irqrestore(&uhci->urb_remove_list_lock, flags);
@@ -1942,7 +1922,7 @@
uhci_free_pending_tds(uhci);
- uhci_remove_pending_qhs(uhci);
+ uhci_remove_pending_urbps(uhci);
uhci_clear_next_interrupt(uhci);
@@ -2467,7 +2447,7 @@
*/
uhci_free_pending_qhs(uhci);
uhci_free_pending_tds(uhci);
- uhci_remove_pending_qhs(uhci);
+ uhci_remove_pending_urbps(uhci);
reset_hc(uhci);
diff -Nru a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
--- a/drivers/usb/host/uhci-hcd.h Thu Feb 19 17:22:42 2004
+++ b/drivers/usb/host/uhci-hcd.h Thu Feb 19 17:22:42 2004
@@ -383,8 +383,6 @@
/* a control transfer, retrigger */
/* the status phase */
- int status; /* Final status */
-
unsigned long inserttime; /* In jiffies */
unsigned long fsbrtime; /* In jiffies */