USB: Eliminate urb->status usage!
authorAlan Stern <stern@rowland.harvard.edu>
Fri, 24 Aug 2007 19:42:24 +0000 (15:42 -0400)
committerGreg Kroah-Hartman <gregkh@suse.de>
Fri, 12 Oct 2007 21:55:23 +0000 (14:55 -0700)
This patch (as979) removes the last vestiges of urb->status from the
host controller drivers and the root-hub emulator.  Now the field
doesn't get set until just before the URB's completion routine is
called.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: David Brownell <david-b@pacbell.net>
CC: Olav Kongas <ok@artecdesign.ee>
CC: Yoshihiro Shimoda <shimoda.yoshihiro@renesas.com>
CC: Tony Olech <tony.olech@elandigitalsystems.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/usb/core/hcd.c
drivers/usb/core/hcd.h
drivers/usb/gadget/dummy_hcd.c
drivers/usb/host/ehci-q.c
drivers/usb/host/isp116x-hcd.c
drivers/usb/host/ohci-q.c
drivers/usb/host/r8a66597-hcd.c
drivers/usb/host/sl811-hcd.c
drivers/usb/host/u132-hcd.c
drivers/usb/host/uhci-hcd.h
drivers/usb/host/uhci-q.c

index 4ac021e42cd8186cdcf0155e6fbda509be055120..1c5e5d35e08d0f037f69dfbebc286ef19b748236 100644 (file)
@@ -532,7 +532,6 @@ error:
 
        /* any errors get returned through the urb completion */
        spin_lock_irq(&hcd_root_hub_lock);
-       urb->status = status;
        usb_hcd_unlink_urb_from_ep(hcd, urb);
 
        /* This peculiar use of spinlocks echoes what real HC drivers do.
@@ -540,7 +539,7 @@ error:
         * RT-friendly.
         */
        spin_unlock(&hcd_root_hub_lock);
-       usb_hcd_giveback_urb(hcd, urb);
+       usb_hcd_giveback_urb(hcd, urb, status);
        spin_lock(&hcd_root_hub_lock);
 
        spin_unlock_irq(&hcd_root_hub_lock);
@@ -578,13 +577,12 @@ void usb_hcd_poll_rh_status(struct usb_hcd *hcd)
                if (urb) {
                        hcd->poll_pending = 0;
                        hcd->status_urb = NULL;
-                       urb->status = 0;
                        urb->actual_length = length;
                        memcpy(urb->transfer_buffer, buffer, length);
 
                        usb_hcd_unlink_urb_from_ep(hcd, urb);
                        spin_unlock(&hcd_root_hub_lock);
-                       usb_hcd_giveback_urb(hcd, urb);
+                       usb_hcd_giveback_urb(hcd, urb, 0);
                        spin_lock(&hcd_root_hub_lock);
                } else {
                        length = 0;
@@ -677,7 +675,7 @@ static int usb_rh_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
                        usb_hcd_unlink_urb_from_ep(hcd, urb);
 
                        spin_unlock(&hcd_root_hub_lock);
-                       usb_hcd_giveback_urb(hcd, urb);
+                       usb_hcd_giveback_urb(hcd, urb, status);
                        spin_lock(&hcd_root_hub_lock);
                }
        }
@@ -1252,6 +1250,7 @@ int usb_hcd_unlink_urb (struct urb *urb, int status)
  * usb_hcd_giveback_urb - return URB from HCD to device driver
  * @hcd: host controller returning the URB
  * @urb: urb being returned to the USB device driver.
+ * @status: completion status code for the URB.
  * Context: in_interrupt()
  *
  * This hands the URB from HCD to its USB device driver, using its
@@ -1260,25 +1259,26 @@ int usb_hcd_unlink_urb (struct urb *urb, int status)
  * the device driver won't cause problems if it frees, modifies,
  * or resubmits this URB.
  *
- * If @urb was unlinked, the value of @urb->status will be overridden by
+ * If @urb was unlinked, the value of @status will be overridden by
  * @urb->unlinked.  Erroneous short transfers are detected in case
  * the HCD hasn't checked for them.
  */
-void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb)
+void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb, int status)
 {
        urb->hcpriv = NULL;
        if (unlikely(urb->unlinked))
-               urb->status = urb->unlinked;
+               status = urb->unlinked;
        else if (unlikely((urb->transfer_flags & URB_SHORT_NOT_OK) &&
                        urb->actual_length < urb->transfer_buffer_length &&
-                       !urb->status))
-               urb->status = -EREMOTEIO;
+                       !status))
+               status = -EREMOTEIO;
 
        unmap_urb_for_dma(hcd, urb);
-       usbmon_urb_complete(&hcd->self, urb, urb->status);
+       usbmon_urb_complete(&hcd->self, urb, status);
        usb_unanchor_urb(urb);
 
        /* pass ownership to the completion handler */
+       urb->status = status;
        urb->complete (urb);
        atomic_dec (&urb->use_count);
        if (unlikely (urb->reject))
index 729b7a03acd87faccff42de097bf88c51c1c72ca..0fc7b95259f516611ce6f4bad6ace45c54005d59 100644 (file)
@@ -217,7 +217,8 @@ extern void usb_hcd_unlink_urb_from_ep(struct usb_hcd *hcd, struct urb *urb);
 
 extern int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags);
 extern int usb_hcd_unlink_urb (struct urb *urb, int status);
-extern void usb_hcd_giveback_urb (struct usb_hcd *hcd, struct urb *urb);
+extern void usb_hcd_giveback_urb(struct usb_hcd *hcd, struct urb *urb,
+               int status);
 extern void usb_hcd_endpoint_disable (struct usb_device *udev,
                struct usb_host_endpoint *ep);
 extern int usb_hcd_get_frame_number (struct usb_device *udev);
index c1af7bab26f0547e1a0553834249975708d7ed39..58e4d720880519520ca5f98257f0b44fdd475f63 100644 (file)
@@ -1511,8 +1511,7 @@ return_urb:
 
                usb_hcd_unlink_urb_from_ep(dummy_to_hcd(dum), urb);
                spin_unlock (&dum->lock);
-               urb->status = status;
-               usb_hcd_giveback_urb (dummy_to_hcd(dum), urb);
+               usb_hcd_giveback_urb(dummy_to_hcd(dum), urb, status);
                spin_lock (&dum->lock);
 
                goto restart;
index 794d27e07807ce17983ed582430eb042e6b14a05..b10f39c047e944848f3ac2aa4067e820d2f5df90 100644 (file)
@@ -256,8 +256,7 @@ __acquires(ehci->lock)
        /* complete() can reenter this HCD */
        usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
        spin_unlock (&ehci->lock);
-       urb->status = status;
-       usb_hcd_giveback_urb (ehci_to_hcd(ehci), urb);
+       usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status);
        spin_lock (&ehci->lock);
 }
 
index 35b3507ff401f47066258c615c92eddcff3bb512..c27417f5b9d8635320a491bd5fbdad1c8f4b366c 100644 (file)
@@ -277,7 +277,7 @@ static void preproc_atl_queue(struct isp116x *isp116x)
   processed urbs.
 */
 static void finish_request(struct isp116x *isp116x, struct isp116x_ep *ep,
-                          struct urb *urb)
+                          struct urb *urb, int status)
 __releases(isp116x->lock) __acquires(isp116x->lock)
 {
        unsigned i;
@@ -291,7 +291,7 @@ __releases(isp116x->lock) __acquires(isp116x->lock)
 
        usb_hcd_unlink_urb_from_ep(isp116x_to_hcd(isp116x), urb);
        spin_unlock(&isp116x->lock);
-       usb_hcd_giveback_urb(isp116x_to_hcd(isp116x), urb);
+       usb_hcd_giveback_urb(isp116x_to_hcd(isp116x), urb, status);
        spin_lock(&isp116x->lock);
 
        /* take idle endpoints out of the schedule */
@@ -453,13 +453,8 @@ static void postproc_atl_queue(struct isp116x *isp116x)
                }
 
  done:
-               if (status != -EINPROGRESS) {
-                       spin_lock(&urb->lock);
-                       urb->status = status;
-                       spin_unlock(&urb->lock);
-               }
-               if (urb->status != -EINPROGRESS || urb->unlinked)
-                       finish_request(isp116x, ep, urb);
+               if (status != -EINPROGRESS || urb->unlinked)
+                       finish_request(isp116x, ep, urb, status);
        }
 }
 
@@ -853,7 +848,7 @@ static int isp116x_urb_dequeue(struct usb_hcd *hcd, struct urb *urb,
                        }
 
        if (urb)
-               finish_request(isp116x, ep, urb);
+               finish_request(isp116x, ep, urb, status);
  done:
        spin_unlock_irqrestore(&isp116x->lock, flags);
        return rc;
index 13d31edd1a8697c37bdcdba8713cb408876cb286..51817322232b453a5f6c881809d542e9315e0f76 100644 (file)
@@ -62,8 +62,7 @@ __acquires(ohci->lock)
        /* urb->complete() can reenter this HCD */
        usb_hcd_unlink_urb_from_ep(ohci_to_hcd(ohci), urb);
        spin_unlock (&ohci->lock);
-       urb->status = status;
-       usb_hcd_giveback_urb (ohci_to_hcd(ohci), urb);
+       usb_hcd_giveback_urb(ohci_to_hcd(ohci), urb, status);
        spin_lock (&ohci->lock);
 
        /* stop periodic dma if it's not needed */
index fea6036771f6e23debf0a255493d1f46912f6cf4..fd00f1e33fb53a77647c37e8646218313388e6c9 100644 (file)
@@ -782,12 +782,12 @@ static void force_dequeue(struct r8a66597 *r8a66597, u16 pipenum, u16 address)
                kfree(td);
 
                if (urb) {
-                       urb->status = -ENODEV;
                        usb_hcd_unlink_urb_from_ep(r8a66597_to_hcd(r8a66597),
                                        urb);
 
                        spin_unlock(&r8a66597->lock);
-                       usb_hcd_giveback_urb(r8a66597_to_hcd(r8a66597), urb);
+                       usb_hcd_giveback_urb(r8a66597_to_hcd(r8a66597), urb,
+                                       -ENODEV);
                        spin_lock(&r8a66597->lock);
                }
                break;
@@ -1134,10 +1134,8 @@ __releases(r8a66597->lock) __acquires(r8a66597->lock)
                        urb->start_frame = r8a66597_get_frame(hcd);
 
                usb_hcd_unlink_urb_from_ep(r8a66597_to_hcd(r8a66597), urb);
-
-               urb->status = status;
                spin_unlock(&r8a66597->lock);
-               usb_hcd_giveback_urb(hcd, urb);
+               usb_hcd_giveback_urb(hcd, urb, status);
                spin_lock(&r8a66597->lock);
        }
 
index 515152809d3799deb1678aa08bc9c1e3c9a5e3c5..94d859aa73f818bc389dc10db5e3c490b760581a 100644 (file)
@@ -436,9 +436,8 @@ static void finish_request(
                ep->nextpid = USB_PID_SETUP;
 
        usb_hcd_unlink_urb_from_ep(sl811_to_hcd(sl811), urb);
-       urb->status = status;
        spin_unlock(&sl811->lock);
-       usb_hcd_giveback_urb(sl811_to_hcd(sl811), urb);
+       usb_hcd_giveback_urb(sl811_to_hcd(sl811), urb, status);
        spin_lock(&sl811->lock);
 
        /* leave active endpoints in the schedule */
index db800a434b83ee19561b622d69eba587a23d0392..2b379a78d0d5a6feb8fb5fc5f84e24d3187a6943 100644 (file)
@@ -518,7 +518,6 @@ static void u132_hcd_giveback_urb(struct u132 *u132, struct u132_endp *endp,
         unsigned long irqs;
         struct usb_hcd *hcd = u132_to_hcd(u132);
         urb->error_count = 0;
-        urb->status = status;
         spin_lock_irqsave(&endp->queue_lock.slock, irqs);
        usb_hcd_unlink_urb_from_ep(hcd, urb);
         endp->queue_next += 1;
@@ -542,7 +541,7 @@ static void u132_hcd_giveback_urb(struct u132 *u132, struct u132_endp *endp,
         u132_ring_queue_work(u132, ring, 0);
         up(&u132->scheduler_lock);
         u132_endp_put_kref(u132, endp);
-        usb_hcd_giveback_urb(hcd, urb);
+       usb_hcd_giveback_urb(hcd, urb, status);
         return;
 }
 
@@ -558,7 +557,6 @@ static void u132_hcd_abandon_urb(struct u132 *u132, struct u132_endp *endp,
         unsigned long irqs;
         struct usb_hcd *hcd = u132_to_hcd(u132);
         urb->error_count = 0;
-        urb->status = status;
         spin_lock_irqsave(&endp->queue_lock.slock, irqs);
        usb_hcd_unlink_urb_from_ep(hcd, urb);
         endp->queue_next += 1;
@@ -575,7 +573,7 @@ static void u132_hcd_abandon_urb(struct u132 *u132, struct u132_endp *endp,
                 endp->active = 0;
                 spin_unlock_irqrestore(&endp->queue_lock.slock, irqs);
                 kfree(urbq);
-        } usb_hcd_giveback_urb(hcd, urb);
+       } usb_hcd_giveback_urb(hcd, urb, status);
         return;
 }
 
@@ -719,7 +717,7 @@ static void u132_hcd_interrupt_recv(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -771,7 +769,7 @@ static void u132_hcd_bulk_output_sent(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -874,7 +872,7 @@ static void u132_hcd_bulk_input_recv(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -911,7 +909,7 @@ static void u132_hcd_configure_empty_sent(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -983,7 +981,7 @@ static void u132_hcd_configure_input_recv(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -1020,7 +1018,7 @@ static void u132_hcd_configure_empty_recv(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -1080,7 +1078,7 @@ static void u132_hcd_configure_setup_sent(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -1121,7 +1119,7 @@ static void u132_hcd_enumeration_empty_recv(void *data, struct urb *urb,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -1165,7 +1163,7 @@ static void u132_hcd_enumeration_address_sent(void *data, struct urb *urb,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -1202,7 +1200,7 @@ static void u132_hcd_initial_empty_sent(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -1254,7 +1252,7 @@ static void u132_hcd_initial_input_recv(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -1299,7 +1297,7 @@ static void u132_hcd_initial_setup_sent(void *data, struct urb *urb, u8 *buf,
                dev_err(&u132->platform_dev->dev, "CALLBACK called urb=%p "
                                "unlinked=%d\n", urb, urb->unlinked);
                 up(&u132->scheduler_lock);
-                u132_hcd_giveback_urb(u132, endp, urb, urb->status);
+               u132_hcd_giveback_urb(u132, endp, urb, 0);
                 return;
         }
 }
@@ -2428,7 +2426,7 @@ static int dequeue_from_overflow_chain(struct u132 *u132,
                         list_del(scan);
                         endp->queue_size -= 1;
                         urb->error_count = 0;
-                        usb_hcd_giveback_urb(hcd, urb);
+                       usb_hcd_giveback_urb(hcd, urb, 0);
                         return 0;
                 } else
                         continue;
@@ -2472,7 +2470,7 @@ static int u132_endp_urb_dequeue(struct u132 *u132, struct u132_endp *endp,
                         return 0;
                 } else {
                         spin_unlock_irqrestore(&endp->queue_lock.slock, irqs);
-                        u132_hcd_abandon_urb(u132, endp, urb, urb->status);
+                       u132_hcd_abandon_urb(u132, endp, urb, status);
                         return 0;
                 }
         } else {
@@ -2513,7 +2511,7 @@ static int u132_endp_urb_dequeue(struct u132 *u132, struct u132_endp *endp,
                                         irqs);
                                 kfree(urbq);
                         } urb->error_count = 0;
-                        usb_hcd_giveback_urb(hcd, urb);
+                       usb_hcd_giveback_urb(hcd, urb, status);
                         return 0;
                 } else if (list_empty(&endp->urb_more)) {
                         dev_err(&u132->platform_dev->dev, "urb=%p not found in "
index e46d2b0203cb94cfcf6b569f446f279de0f16062..340d6ed3e6e92006c858e80ee8bdcd5f0ef0b179 100644 (file)
@@ -456,21 +456,6 @@ struct urb_priv {
 };
 
 
-/*
- * Locking in uhci.c
- *
- * Almost everything relating to the hardware schedule and processing
- * of URBs is protected by uhci->lock.  urb->status is protected by
- * urb->lock; that's the one exception.
- *
- * To prevent deadlocks, never lock uhci->lock while holding urb->lock.
- * The safe order of locking is:
- *
- * #1 uhci->lock
- * #2 urb->lock
- */
-
-
 /* Some special IDs */
 
 #define PCI_VENDOR_ID_GENESYS          0x17a0
index bab5672665597b29f28ea6c6b3f7b3c948fba1d9..e5d60d5b105a8975979c731a401eee26b0effd62 100644 (file)
@@ -1480,7 +1480,7 @@ done:
  * Finish unlinking an URB and give it back
  */
 static void uhci_giveback_urb(struct uhci_hcd *uhci, struct uhci_qh *qh,
-               struct urb *urb)
+               struct urb *urb, int status)
 __releases(uhci->lock)
 __acquires(uhci->lock)
 {
@@ -1520,7 +1520,7 @@ __acquires(uhci->lock)
        usb_hcd_unlink_urb_from_ep(uhci_to_hcd(uhci), urb);
 
        spin_unlock(&uhci->lock);
-       usb_hcd_giveback_urb(uhci_to_hcd(uhci), urb);
+       usb_hcd_giveback_urb(uhci_to_hcd(uhci), urb, status);
        spin_lock(&uhci->lock);
 
        /* If the queue is now empty, we can unlink the QH and give up its
@@ -1556,10 +1556,6 @@ static void uhci_scan_qh(struct uhci_hcd *uhci, struct uhci_qh *qh)
                if (status == -EINPROGRESS)
                        break;
 
-               spin_lock(&urb->lock);
-               urb->status = status;
-               spin_unlock(&urb->lock);
-
                /* Dequeued but completed URBs can't be given back unless
                 * the QH is stopped or has finished unlinking. */
                if (urb->unlinked) {
@@ -1569,7 +1565,7 @@ static void uhci_scan_qh(struct uhci_hcd *uhci, struct uhci_qh *qh)
                                return;
                }
 
-               uhci_giveback_urb(uhci, qh, urb);
+               uhci_giveback_urb(uhci, qh, urb, status);
                if (status < 0)
                        break;
        }
@@ -1594,7 +1590,7 @@ restart:
                                qh->is_stopped = 0;
                                return;
                        }
-                       uhci_giveback_urb(uhci, qh, urb);
+                       uhci_giveback_urb(uhci, qh, urb, 0);
                        goto restart;
                }
        }