ChangeSet 1.1557.49.5, 2004/02/17 16:20:54-08:00, [EMAIL PROTECTED]

[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 */
 



-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id56&alloc_id438&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to