diff options
author | Sergei Shtylyov <sshtylyov@ru.mvista.com> | 2009-02-21 18:31:01 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2009-02-27 17:40:51 -0500 |
commit | dc61d238b8c850c34632ae1fbbdea529f8c41d16 (patch) | |
tree | e800cb6312d12f8347ca60fbab2c8b829d733c17 /drivers/usb | |
parent | a2fd814e6a9e172f7077b68a2a9391bbde777a92 (diff) |
USB: musb: host endpoint_disable() oops fixes
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>
Diffstat (limited to 'drivers/usb')
-rw-r--r-- | drivers/usb/musb/musb_host.c | 36 |
1 files changed, 26 insertions, 10 deletions
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index e32323938e98..c74ebadd4b9d 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c | |||
@@ -2103,15 +2103,16 @@ musb_h_disable(struct usb_hcd *hcd, struct usb_host_endpoint *hep) | |||
2103 | unsigned long flags; | 2103 | unsigned long flags; |
2104 | struct musb *musb = hcd_to_musb(hcd); | 2104 | struct musb *musb = hcd_to_musb(hcd); |
2105 | u8 is_in = epnum & USB_DIR_IN; | 2105 | u8 is_in = epnum & USB_DIR_IN; |
2106 | struct musb_qh *qh = hep->hcpriv; | 2106 | struct musb_qh *qh; |
2107 | struct urb *urb, *tmp; | 2107 | struct urb *urb; |
2108 | struct list_head *sched; | 2108 | struct list_head *sched; |
2109 | 2109 | ||
2110 | if (!qh) | ||
2111 | return; | ||
2112 | |||
2113 | spin_lock_irqsave(&musb->lock, flags); | 2110 | spin_lock_irqsave(&musb->lock, flags); |
2114 | 2111 | ||
2112 | qh = hep->hcpriv; | ||
2113 | if (qh == NULL) | ||
2114 | goto exit; | ||
2115 | |||
2115 | switch (qh->type) { | 2116 | switch (qh->type) { |
2116 | case USB_ENDPOINT_XFER_CONTROL: | 2117 | case USB_ENDPOINT_XFER_CONTROL: |
2117 | sched = &musb->control; | 2118 | sched = &musb->control; |
@@ -2145,13 +2146,28 @@ musb_h_disable(struct usb_hcd *hcd, struct usb_host_endpoint *hep) | |||
2145 | 2146 | ||
2146 | /* cleanup */ | 2147 | /* cleanup */ |
2147 | musb_cleanup_urb(urb, qh, urb->pipe & USB_DIR_IN); | 2148 | musb_cleanup_urb(urb, qh, urb->pipe & USB_DIR_IN); |
2148 | } else | ||
2149 | urb = NULL; | ||
2150 | 2149 | ||
2151 | /* then just nuke all the others */ | 2150 | /* Then nuke all the others ... and advance the |
2152 | list_for_each_entry_safe_from(urb, tmp, &hep->urb_list, urb_list) | 2151 | * queue on hw_ep (e.g. bulk ring) when we're done. |
2153 | musb_giveback(qh, urb, -ESHUTDOWN); | 2152 | */ |
2153 | while (!list_empty(&hep->urb_list)) { | ||
2154 | urb = next_urb(qh); | ||
2155 | urb->status = -ESHUTDOWN; | ||
2156 | musb_advance_schedule(musb, urb, qh->hw_ep, is_in); | ||
2157 | } | ||
2158 | } else { | ||
2159 | /* Just empty the queue; the hardware is busy with | ||
2160 | * other transfers, and since !qh->is_ready nothing | ||
2161 | * will activate any of these as it advances. | ||
2162 | */ | ||
2163 | while (!list_empty(&hep->urb_list)) | ||
2164 | __musb_giveback(musb, next_urb(qh), -ESHUTDOWN); | ||
2154 | 2165 | ||
2166 | hep->hcpriv = NULL; | ||
2167 | list_del(&qh->ring); | ||
2168 | kfree(qh); | ||
2169 | } | ||
2170 | exit: | ||
2155 | spin_unlock_irqrestore(&musb->lock, flags); | 2171 | spin_unlock_irqrestore(&musb->lock, flags); |
2156 | } | 2172 | } |
2157 | 2173 | ||