USB: musb: host endpoint_disable() oops fixes
Sergei Shtylyov [Sat, 21 Feb 2009 23:31:01 +0000 (15:31 -0800)]
The musb_h_disable() routine can oops in some cases:

 - It's not safe to read hep->hcpriv outside musb->lock,
   since it gets changed on completion IRQ paths.

 - The list iterators aren't safe to use in that way;
   just remove the first element while !list_empty(),
   so deletions on other code paths can't make trouble.

We need two "scrub the list" loops because only one branch
should touch hardware and advance the schedule.

[ dbrownell@users.sourceforge.net: massively simplify
  patch description; add key points as code comments ]

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

drivers/usb/musb/musb_host.c

index e323239..c74ebad 100644 (file)
@@ -2103,15 +2103,16 @@ musb_h_disable(struct usb_hcd *hcd, struct usb_host_endpoint *hep)
        unsigned long           flags;
        struct musb             *musb = hcd_to_musb(hcd);
        u8                      is_in = epnum & USB_DIR_IN;
-       struct musb_qh          *qh = hep->hcpriv;
-       struct urb              *urb, *tmp;
+       struct musb_qh          *qh;
+       struct urb              *urb;
        struct list_head        *sched;
 
-       if (!qh)
-               return;
-
        spin_lock_irqsave(&musb->lock, flags);
 
+       qh = hep->hcpriv;
+       if (qh == NULL)
+               goto exit;
+
        switch (qh->type) {
        case USB_ENDPOINT_XFER_CONTROL:
                sched = &musb->control;
@@ -2145,13 +2146,28 @@ musb_h_disable(struct usb_hcd *hcd, struct usb_host_endpoint *hep)
 
                /* cleanup */
                musb_cleanup_urb(urb, qh, urb->pipe & USB_DIR_IN);
-       } else
-               urb = NULL;
 
-       /* then just nuke all the others */
-       list_for_each_entry_safe_from(urb, tmp, &hep->urb_list, urb_list)
-               musb_giveback(qh, urb, -ESHUTDOWN);
+               /* Then nuke all the others ... and advance the
+                * queue on hw_ep (e.g. bulk ring) when we're done.
+                */
+               while (!list_empty(&hep->urb_list)) {
+                       urb = next_urb(qh);
+                       urb->status = -ESHUTDOWN;
+                       musb_advance_schedule(musb, urb, qh->hw_ep, is_in);
+               }
+       } else {
+               /* Just empty the queue; the hardware is busy with
+                * other transfers, and since !qh->is_ready nothing
+                * will activate any of these as it advances.
+                */
+               while (!list_empty(&hep->urb_list))
+                       __musb_giveback(musb, next_urb(qh), -ESHUTDOWN);
 
+               hep->hcpriv = NULL;
+               list_del(&qh->ring);
+               kfree(qh);
+       }
+exit:
        spin_unlock_irqrestore(&musb->lock, flags);
 }