| Message ID | 20230502162133.148821-3-rogerq@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: dwc3: gadget: Improve dwc3_gadget_suspend/resume | expand |
On Tue, May 02, 2023, Roger Quadros wrote: > Prevent -ETIMEDOUT error on .suspend(). > e.g. If gadget driver is loaded and we are connected to a USB host, > all transfers must be stopped before stopping the controller else > we will not get a clean stop i.e. dwc3_gadget_run_stop() will take > several seconds to complete and will return -ETIMEDOUT. > > Handle error cases properly in dwc3_gadget_suspend(). > Simplify dwc3_gadget_resume() by using the introduced helper function. > > Fixes: 9f8a67b65a49 ("usb: dwc3: gadget: fix gadget suspend/resume") > Cc: stable@vger.kernel.org > Suggested-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > Signed-off-by: Roger Quadros <rogerq@kernel.org> > --- > drivers/usb/dwc3/gadget.c | 66 +++++++++++++++++++-------------------- > 1 file changed, 33 insertions(+), 33 deletions(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index b5170374cd18..869f1695565d 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2699,6 +2699,21 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc) > return ret; > } > > +static int dwc3_gadget_soft_connect(struct dwc3 *dwc) > +{ > + /* > + * In the Synopsys DWC_usb31 1.90a programming guide section > + * 4.1.9, it specifies that for a reconnect after a > + * device-initiated disconnect requires a core soft reset > + * (DCTL.CSftRst) before enabling the run/stop bit. > + */ > + dwc3_core_soft_reset(dwc); > + > + dwc3_event_buffers_setup(dwc); > + __dwc3_gadget_start(dwc); > + return dwc3_gadget_run_stop(dwc, true); > +} > + > static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > { > struct dwc3 *dwc = gadget_to_dwc(g); > @@ -2737,21 +2752,10 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > > synchronize_irq(dwc->irq_gadget); > > - if (!is_on) { > + if (!is_on) > ret = dwc3_gadget_soft_disconnect(dwc); > - } else { > - /* > - * In the Synopsys DWC_usb31 1.90a programming guide section > - * 4.1.9, it specifies that for a reconnect after a > - * device-initiated disconnect requires a core soft reset > - * (DCTL.CSftRst) before enabling the run/stop bit. > - */ > - dwc3_core_soft_reset(dwc); > - > - dwc3_event_buffers_setup(dwc); > - __dwc3_gadget_start(dwc); > - ret = dwc3_gadget_run_stop(dwc, true); > - } > + else > + ret = dwc3_gadget_soft_connect(dwc); > > pm_runtime_put(dwc->dev); > > @@ -4655,42 +4659,38 @@ void dwc3_gadget_exit(struct dwc3 *dwc) > int dwc3_gadget_suspend(struct dwc3 *dwc) > { > unsigned long flags; > + int ret; > > if (!dwc->gadget_driver || !dwc->softconnect) > return 0; > > - dwc3_gadget_run_stop(dwc, false); > + ret = dwc3_gadget_soft_disconnect(dwc); > + if (ret) > + goto err; > > spin_lock_irqsave(&dwc->lock, flags); > dwc3_disconnect_gadget(dwc); > - __dwc3_gadget_stop(dwc); > spin_unlock_irqrestore(&dwc->lock, flags); > > return 0; > + > +err: > + /* > + * Attempt to reset the controller's state. Likely no > + * communication can be established until the host > + * performs a port reset. > + */ > + dwc3_gadget_soft_connect(dwc); > + > + return ret; > } > > int dwc3_gadget_resume(struct dwc3 *dwc) > { > - int ret; > - > if (!dwc->gadget_driver || !dwc->softconnect) > return 0; > > - ret = __dwc3_gadget_start(dwc); > - if (ret < 0) > - goto err0; > - > - ret = dwc3_gadget_run_stop(dwc, true); > - if (ret < 0) > - goto err1; > - > - return 0; > - > -err1: > - __dwc3_gadget_stop(dwc); > - > -err0: > - return ret; > + return dwc3_gadget_soft_connect(dwc); > } > > void dwc3_gadget_process_pending_events(struct dwc3 *dwc) > -- > 2.34.1 > Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> Thanks, Thinh
On 03/05/2023 00:18, Thinh Nguyen wrote: > On Tue, May 02, 2023, Roger Quadros wrote: >> Prevent -ETIMEDOUT error on .suspend(). >> e.g. If gadget driver is loaded and we are connected to a USB host, >> all transfers must be stopped before stopping the controller else >> we will not get a clean stop i.e. dwc3_gadget_run_stop() will take >> several seconds to complete and will return -ETIMEDOUT. >> >> Handle error cases properly in dwc3_gadget_suspend(). >> Simplify dwc3_gadget_resume() by using the introduced helper function. >> >> Fixes: 9f8a67b65a49 ("usb: dwc3: gadget: fix gadget suspend/resume") >> Cc: stable@vger.kernel.org >> Suggested-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> drivers/usb/dwc3/gadget.c | 66 +++++++++++++++++++-------------------- >> 1 file changed, 33 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >> index b5170374cd18..869f1695565d 100644 >> --- a/drivers/usb/dwc3/gadget.c >> +++ b/drivers/usb/dwc3/gadget.c >> @@ -2699,6 +2699,21 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc) >> return ret; >> } >> >> +static int dwc3_gadget_soft_connect(struct dwc3 *dwc) >> +{ >> + /* >> + * In the Synopsys DWC_usb31 1.90a programming guide section >> + * 4.1.9, it specifies that for a reconnect after a >> + * device-initiated disconnect requires a core soft reset >> + * (DCTL.CSftRst) before enabling the run/stop bit. >> + */ >> + dwc3_core_soft_reset(dwc); >> + >> + dwc3_event_buffers_setup(dwc); >> + __dwc3_gadget_start(dwc); >> + return dwc3_gadget_run_stop(dwc, true); >> +} >> + >> static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) >> { >> struct dwc3 *dwc = gadget_to_dwc(g); >> @@ -2737,21 +2752,10 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) >> >> synchronize_irq(dwc->irq_gadget); >> >> - if (!is_on) { >> + if (!is_on) >> ret = dwc3_gadget_soft_disconnect(dwc); >> - } else { >> - /* >> - * In the Synopsys DWC_usb31 1.90a programming guide section >> - * 4.1.9, it specifies that for a reconnect after a >> - * device-initiated disconnect requires a core soft reset >> - * (DCTL.CSftRst) before enabling the run/stop bit. >> - */ >> - dwc3_core_soft_reset(dwc); >> - >> - dwc3_event_buffers_setup(dwc); >> - __dwc3_gadget_start(dwc); >> - ret = dwc3_gadget_run_stop(dwc, true); >> - } >> + else >> + ret = dwc3_gadget_soft_connect(dwc); >> >> pm_runtime_put(dwc->dev); >> >> @@ -4655,42 +4659,38 @@ void dwc3_gadget_exit(struct dwc3 *dwc) >> int dwc3_gadget_suspend(struct dwc3 *dwc) >> { >> unsigned long flags; >> + int ret; >> >> if (!dwc->gadget_driver || !dwc->softconnect) >> return 0; >> >> - dwc3_gadget_run_stop(dwc, false); >> + ret = dwc3_gadget_soft_disconnect(dwc); >> + if (ret) >> + goto err; >> >> spin_lock_irqsave(&dwc->lock, flags); >> dwc3_disconnect_gadget(dwc); >> - __dwc3_gadget_stop(dwc); >> spin_unlock_irqrestore(&dwc->lock, flags); >> >> return 0; >> + >> +err: As we don't pick the first patch, we will need to check for dwc->softconnect here. I'll send a follow up patch. >> + /* >> + * Attempt to reset the controller's state. Likely no >> + * communication can be established until the host >> + * performs a port reset. >> + */ >> + dwc3_gadget_soft_connect(dwc); >> + >> + return ret; >> } >> >> int dwc3_gadget_resume(struct dwc3 *dwc) >> { >> - int ret; >> - >> if (!dwc->gadget_driver || !dwc->softconnect) >> return 0; >> >> - ret = __dwc3_gadget_start(dwc); >> - if (ret < 0) >> - goto err0; >> - >> - ret = dwc3_gadget_run_stop(dwc, true); >> - if (ret < 0) >> - goto err1; >> - >> - return 0; >> - >> -err1: >> - __dwc3_gadget_stop(dwc); >> - >> -err0: >> - return ret; >> + return dwc3_gadget_soft_connect(dwc); >> } >> >> void dwc3_gadget_process_pending_events(struct dwc3 *dwc) >> -- >> 2.34.1 >> > > Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > > Thanks, > Thinh cheers, -roger
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index b5170374cd18..869f1695565d 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2699,6 +2699,21 @@ static int dwc3_gadget_soft_disconnect(struct dwc3 *dwc) return ret; } +static int dwc3_gadget_soft_connect(struct dwc3 *dwc) +{ + /* + * In the Synopsys DWC_usb31 1.90a programming guide section + * 4.1.9, it specifies that for a reconnect after a + * device-initiated disconnect requires a core soft reset + * (DCTL.CSftRst) before enabling the run/stop bit. + */ + dwc3_core_soft_reset(dwc); + + dwc3_event_buffers_setup(dwc); + __dwc3_gadget_start(dwc); + return dwc3_gadget_run_stop(dwc, true); +} + static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) { struct dwc3 *dwc = gadget_to_dwc(g); @@ -2737,21 +2752,10 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) synchronize_irq(dwc->irq_gadget); - if (!is_on) { + if (!is_on) ret = dwc3_gadget_soft_disconnect(dwc); - } else { - /* - * In the Synopsys DWC_usb31 1.90a programming guide section - * 4.1.9, it specifies that for a reconnect after a - * device-initiated disconnect requires a core soft reset - * (DCTL.CSftRst) before enabling the run/stop bit. - */ - dwc3_core_soft_reset(dwc); - - dwc3_event_buffers_setup(dwc); - __dwc3_gadget_start(dwc); - ret = dwc3_gadget_run_stop(dwc, true); - } + else + ret = dwc3_gadget_soft_connect(dwc); pm_runtime_put(dwc->dev); @@ -4655,42 +4659,38 @@ void dwc3_gadget_exit(struct dwc3 *dwc) int dwc3_gadget_suspend(struct dwc3 *dwc) { unsigned long flags; + int ret; if (!dwc->gadget_driver || !dwc->softconnect) return 0; - dwc3_gadget_run_stop(dwc, false); + ret = dwc3_gadget_soft_disconnect(dwc); + if (ret) + goto err; spin_lock_irqsave(&dwc->lock, flags); dwc3_disconnect_gadget(dwc); - __dwc3_gadget_stop(dwc); spin_unlock_irqrestore(&dwc->lock, flags); return 0; + +err: + /* + * Attempt to reset the controller's state. Likely no + * communication can be established until the host + * performs a port reset. + */ + dwc3_gadget_soft_connect(dwc); + + return ret; } int dwc3_gadget_resume(struct dwc3 *dwc) { - int ret; - if (!dwc->gadget_driver || !dwc->softconnect) return 0; - ret = __dwc3_gadget_start(dwc); - if (ret < 0) - goto err0; - - ret = dwc3_gadget_run_stop(dwc, true); - if (ret < 0) - goto err1; - - return 0; - -err1: - __dwc3_gadget_stop(dwc); - -err0: - return ret; + return dwc3_gadget_soft_connect(dwc); } void dwc3_gadget_process_pending_events(struct dwc3 *dwc)
Prevent -ETIMEDOUT error on .suspend(). e.g. If gadget driver is loaded and we are connected to a USB host, all transfers must be stopped before stopping the controller else we will not get a clean stop i.e. dwc3_gadget_run_stop() will take several seconds to complete and will return -ETIMEDOUT. Handle error cases properly in dwc3_gadget_suspend(). Simplify dwc3_gadget_resume() by using the introduced helper function. Fixes: 9f8a67b65a49 ("usb: dwc3: gadget: fix gadget suspend/resume") Cc: stable@vger.kernel.org Suggested-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> Signed-off-by: Roger Quadros <rogerq@kernel.org> --- drivers/usb/dwc3/gadget.c | 66 +++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 33 deletions(-)