3.16.62-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
commit dec3c23c9aa1815f07d98ae0375b4cbc10971e13 upstream.
Commit f16443a034c7 ("USB: gadgetfs, dummy-hcd, net2280: fix locking
for callbacks") was based on a serious misunderstanding. It
introduced regressions into both the dummy-hcd and net2280 drivers.
The problem in dummy-hcd was fixed by commit 7dbd8f4cabd9 ("USB:
dummy-hcd: Fix erroneous synchronization change"), but the problem in
net2280 remains. Namely: the ->disconnect(), ->suspend(), ->resume(),
and ->reset() callbacks must be invoked without the private lock held;
otherwise a deadlock will occur when the callback routine tries to
interact with the UDC driver.
This patch largely is a reversion of the relevant parts of
f16443a034c7. It also drops the private lock around the calls to
->suspend() and ->resume() (something the earlier patch forgot to do).
This is safe from races with device interrupts because it occurs
within the interrupt handler.
Finally, the patch changes where the ->disconnect() callback is
invoked when net2280_pullup() turns the pullup off. Rather than
making the callback from within stop_activity() at a time when dropping
the private lock could be unsafe, the callback is moved to a point
after the lock has already been dropped.
Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Fixes: f16443a034c7 ("USB: gadgetfs, dummy-hcd, net2280: fix locking for callbacks")
Reported-by: D. Ziesche <dziesche@xxxxxxx>
Tested-by: D. Ziesche <dziesche@xxxxxxx>
Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
[bwh: Backported to 3.16:
- Drop inapplicable change to disconnection handling in handle_stat1_irqs()
- Adjust filename, context]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -1399,11 +1399,14 @@ static int net2280_pullup(struct usb_gad
writel(tmp | BIT(USB_DETECT_ENABLE), &dev->usb->usbctl);
} else {
writel(tmp & ~BIT(USB_DETECT_ENABLE), &dev->usb->usbctl);
- stop_activity(dev, dev->driver);
+ stop_activity(dev, NULL);
}
spin_unlock_irqrestore (&dev->lock, flags);
+ if (!is_on && dev->driver)
+ dev->driver->disconnect(&dev->gadget);
+
return 0;
}
@@ -1948,8 +1951,11 @@ stop_activity (struct net2280 *dev, stru
nuke (&dev->ep [i]);
/* report disconnect; the driver is already quiesced */
- if (driver)
+ if (driver) {
+ spin_unlock(&dev->lock);
driver->disconnect(&dev->gadget);
+ spin_lock(&dev->lock);
+ }
usb_reinit (dev);
}
@@ -2452,6 +2458,8 @@ next_endpoints:
| (1 << PCI_RETRY_ABORT_INTERRUPT))
static void handle_stat1_irqs (struct net2280 *dev, u32 stat)
+__releases(dev->lock)
+__acquires(dev->lock)
{
struct net2280_ep *ep;
u32 tmp, num, mask, scratch;
@@ -2494,6 +2502,7 @@ static void handle_stat1_irqs (struct ne
tmp = (1 << SUSPEND_REQUEST_CHANGE_INTERRUPT);
if (stat & tmp) {
writel (tmp, &dev->regs->irqstat1);
+ spin_unlock(&dev->lock);
if (stat & (1 << SUSPEND_REQUEST_INTERRUPT)) {
if (dev->driver->suspend)
dev->driver->suspend (&dev->gadget);
@@ -2504,6 +2513,7 @@ static void handle_stat1_irqs (struct ne
dev->driver->resume (&dev->gadget);
/* at high speed, note erratum 0133 */
}
+ spin_lock(&dev->lock);
stat &= ~tmp;
}
