diff mbox series

[1/2] usb: dwc3: gadget: Check if the gadget had started

Message ID 92118292e053f3a1a9238facfec91630468ba752.1609865348.git.Thinh.Nguyen@synopsys.com (mailing list archive)
State New, archived
Headers show
Series usb: dwc3: gadget: Check for multiple start/stop | expand

Commit Message

Thinh Nguyen Jan. 5, 2021, 4:56 p.m. UTC
If the gadget had already started, don't try to start again. Otherwise,
we may request the same threaded irq with the same dev_id, it will mess
up the interrupt freeing logic. This can happen if a user tries to
trigger a soft-connect from soft_connect sysfs multiple times. Check to
make sure that the gadget had started before proceeding to request
threaded irq. Fix this by checking if there's bounded gadget driver.

Cc: stable@vger.kernel.org
Fixes: b0d7ffd44ba9 ("usb: dwc3: gadget: don't request IRQs in atomic")
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
 drivers/usb/dwc3/gadget.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

Comments

Felipe Balbi Jan. 6, 2021, 7:53 a.m. UTC | #1
Hi,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
> If the gadget had already started, don't try to start again. Otherwise,
> we may request the same threaded irq with the same dev_id, it will mess
> up the interrupt freeing logic. This can happen if a user tries to
> trigger a soft-connect from soft_connect sysfs multiple times. Check to
> make sure that the gadget had started before proceeding to request
> threaded irq. Fix this by checking if there's bounded gadget driver.

Looks like this should be fixed at the framework level, otherwise we
will have to patch every single UDC. After that is done, we can remove
the dwc->gadget_driver check from here.
Thinh Nguyen Jan. 6, 2021, 9:35 a.m. UTC | #2
Hi,

Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes:
>> If the gadget had already started, don't try to start again. Otherwise,
>> we may request the same threaded irq with the same dev_id, it will mess
>> up the interrupt freeing logic. This can happen if a user tries to
>> trigger a soft-connect from soft_connect sysfs multiple times. Check to
>> make sure that the gadget had started before proceeding to request
>> threaded irq. Fix this by checking if there's bounded gadget driver.
> Looks like this should be fixed at the framework level, otherwise we
> will have to patch every single UDC. After that is done, we can remove
> the dwc->gadget_driver check from here.
>

Sure. We can do that.

Thanks,
Thinh
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 25f654b79e48..51d81a32ce78 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2303,35 +2303,27 @@  static int dwc3_gadget_start(struct usb_gadget *g,
 	int			ret = 0;
 	int			irq;
 
+	if (dwc->gadget_driver) {
+		dev_err(dwc->dev, "%s is already bound to %s\n",
+				dwc->gadget->name,
+				dwc->gadget_driver->driver.name);
+		return -EBUSY;
+	}
+
 	irq = dwc->irq_gadget;
 	ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt,
 			IRQF_SHARED, "dwc3", dwc->ev_buf);
 	if (ret) {
 		dev_err(dwc->dev, "failed to request irq #%d --> %d\n",
 				irq, ret);
-		goto err0;
+		return ret;
 	}
 
 	spin_lock_irqsave(&dwc->lock, flags);
-	if (dwc->gadget_driver) {
-		dev_err(dwc->dev, "%s is already bound to %s\n",
-				dwc->gadget->name,
-				dwc->gadget_driver->driver.name);
-		ret = -EBUSY;
-		goto err1;
-	}
-
 	dwc->gadget_driver	= driver;
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	return 0;
-
-err1:
-	spin_unlock_irqrestore(&dwc->lock, flags);
-	free_irq(irq, dwc);
-
-err0:
-	return ret;
 }
 
 static void __dwc3_gadget_stop(struct dwc3 *dwc)