Commit dec3c23c authored by Alan Stern's avatar Alan Stern Committed by Felipe Balbi

USB: net2280: Fix erroneous synchronization change

Commit f16443a0 ("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 7dbd8f4c ("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
f16443a0.  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: default avatarAlan Stern <stern@rowland.harvard.edu>
Fixes: f16443a0 ("USB: gadgetfs, dummy-hcd, net2280: fix locking for callbacks")
Reported-by: default avatarD. Ziesche <dziesche@zes.com>
Tested-by: default avatarD. Ziesche <dziesche@zes.com>
CC: <stable@vger.kernel.org>
Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
parent 66174b69
...@@ -1545,11 +1545,14 @@ static int net2280_pullup(struct usb_gadget *_gadget, int is_on) ...@@ -1545,11 +1545,14 @@ static int net2280_pullup(struct usb_gadget *_gadget, int is_on)
writel(tmp | BIT(USB_DETECT_ENABLE), &dev->usb->usbctl); writel(tmp | BIT(USB_DETECT_ENABLE), &dev->usb->usbctl);
} else { } else {
writel(tmp & ~BIT(USB_DETECT_ENABLE), &dev->usb->usbctl); writel(tmp & ~BIT(USB_DETECT_ENABLE), &dev->usb->usbctl);
stop_activity(dev, dev->driver); stop_activity(dev, NULL);
} }
spin_unlock_irqrestore(&dev->lock, flags); spin_unlock_irqrestore(&dev->lock, flags);
if (!is_on && dev->driver)
dev->driver->disconnect(&dev->gadget);
return 0; return 0;
} }
...@@ -2466,8 +2469,11 @@ static void stop_activity(struct net2280 *dev, struct usb_gadget_driver *driver) ...@@ -2466,8 +2469,11 @@ static void stop_activity(struct net2280 *dev, struct usb_gadget_driver *driver)
nuke(&dev->ep[i]); nuke(&dev->ep[i]);
/* report disconnect; the driver is already quiesced */ /* report disconnect; the driver is already quiesced */
if (driver) if (driver) {
spin_unlock(&dev->lock);
driver->disconnect(&dev->gadget); driver->disconnect(&dev->gadget);
spin_lock(&dev->lock);
}
usb_reinit(dev); usb_reinit(dev);
} }
...@@ -3341,6 +3347,8 @@ static void handle_stat0_irqs(struct net2280 *dev, u32 stat) ...@@ -3341,6 +3347,8 @@ static void handle_stat0_irqs(struct net2280 *dev, u32 stat)
BIT(PCI_RETRY_ABORT_INTERRUPT)) BIT(PCI_RETRY_ABORT_INTERRUPT))
static void handle_stat1_irqs(struct net2280 *dev, u32 stat) static void handle_stat1_irqs(struct net2280 *dev, u32 stat)
__releases(dev->lock)
__acquires(dev->lock)
{ {
struct net2280_ep *ep; struct net2280_ep *ep;
u32 tmp, num, mask, scratch; u32 tmp, num, mask, scratch;
...@@ -3381,12 +3389,14 @@ static void handle_stat1_irqs(struct net2280 *dev, u32 stat) ...@@ -3381,12 +3389,14 @@ static void handle_stat1_irqs(struct net2280 *dev, u32 stat)
if (disconnect || reset) { if (disconnect || reset) {
stop_activity(dev, dev->driver); stop_activity(dev, dev->driver);
ep0_start(dev); ep0_start(dev);
spin_unlock(&dev->lock);
if (reset) if (reset)
usb_gadget_udc_reset usb_gadget_udc_reset
(&dev->gadget, dev->driver); (&dev->gadget, dev->driver);
else else
(dev->driver->disconnect) (dev->driver->disconnect)
(&dev->gadget); (&dev->gadget);
spin_lock(&dev->lock);
return; return;
} }
} }
...@@ -3405,6 +3415,7 @@ static void handle_stat1_irqs(struct net2280 *dev, u32 stat) ...@@ -3405,6 +3415,7 @@ static void handle_stat1_irqs(struct net2280 *dev, u32 stat)
tmp = BIT(SUSPEND_REQUEST_CHANGE_INTERRUPT); tmp = BIT(SUSPEND_REQUEST_CHANGE_INTERRUPT);
if (stat & tmp) { if (stat & tmp) {
writel(tmp, &dev->regs->irqstat1); writel(tmp, &dev->regs->irqstat1);
spin_unlock(&dev->lock);
if (stat & BIT(SUSPEND_REQUEST_INTERRUPT)) { if (stat & BIT(SUSPEND_REQUEST_INTERRUPT)) {
if (dev->driver->suspend) if (dev->driver->suspend)
dev->driver->suspend(&dev->gadget); dev->driver->suspend(&dev->gadget);
...@@ -3415,6 +3426,7 @@ static void handle_stat1_irqs(struct net2280 *dev, u32 stat) ...@@ -3415,6 +3426,7 @@ static void handle_stat1_irqs(struct net2280 *dev, u32 stat)
dev->driver->resume(&dev->gadget); dev->driver->resume(&dev->gadget);
/* at high speed, note erratum 0133 */ /* at high speed, note erratum 0133 */
} }
spin_lock(&dev->lock);
stat &= ~tmp; stat &= ~tmp;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment