[PATCH] usb: gadget: bcm63xx_udc: don't sleep under spinlock in udc_stop

[PATCH] usb: gadget: bcm63xx_udc: don't sleep under spinlock in udc_stop

[Hyunjung Ko]

bcm63xx_udc_stop() clears udc->driver and then calls msleep(100) plus the
PHY/clock teardown while still holding udc->lock, which is taken with
spin_lock_irqsave(). Sleeping in that atomic context triggers "BUG:
scheduling while atomic" (and a CONFIG_DEBUG_ATOMIC_SLEEP splat) when the
gadget driver is unbound.

The lock is only needed to clear udc->driver, which serializes against the
interrupt handler. The settle delay and the bcm63xx_select_phy_mode() /
set_clocks() teardown only touch hardware and are already called without
the lock during init, so drop the lock right after clearing udc->driver
and run the sleep and teardown unlocked.

Fixes: 613065e53cb1 ("usb: gadget: bcm63xx UDC driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 drivers/usb/gadget/udc/bcm63xx_udc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index ba91b1ebbf57..269063d19f10 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -1847,8 +1847,8 @@ static int bcm63xx_udc_stop(struct usb_gadget *gadget)
 	unsigned long flags;
 
 	spin_lock_irqsave(&udc->lock, flags);
-
 	udc->driver = NULL;
+	spin_unlock_irqrestore(&udc->lock, flags);
 
 	/*
 	 * If we switch the PHY too abruptly after dropping D+, the host
@@ -1861,8 +1861,6 @@ static int bcm63xx_udc_stop(struct usb_gadget *gadget)
 	bcm63xx_select_phy_mode(udc, false);
 	set_clocks(udc, false);
 
-	spin_unlock_irqrestore(&udc->lock, flags);
-
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH] usb: gadget: bcm63xx_udc: don't sleep under spinlock in udc_stop

[sashiko-bot]

> bcm63xx_udc_stop() clears udc->driver and then calls msleep(100) plus the
> PHY/clock teardown while still holding udc->lock, which is taken with
> spin_lock_irqsave(). Sleeping in that atomic context triggers "BUG:
> scheduling while atomic" (and a CONFIG_DEBUG_ATOMIC_SLEEP splat) when the
> gadget driver is unbound.
> 
> The lock is only needed to clear udc->driver, which serializes against the
> interrupt handler. The settle delay and the bcm63xx_select_phy_mode() /
> set_clocks() teardown only touch hardware and are already called without
> the lock during init, so drop the lock right after clearing udc->driver
> and run the sleep and teardown unlocked.
> 
> Fixes: 613065e53cb1 ("usb: gadget: bcm63xx UDC driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hyunjung Ko <hj351016@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111072310.3005580-1-hj351016@gmail.com?part=1