| Message ID | 92118292e053f3a1a9238facfec91630468ba752.1609865348.git.Thinh.Nguyen@synopsys.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] usb: dwc3: gadget: Check if the gadget had started | expand |
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.
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 --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)
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(-)