| Message ID | 20230614134922.23516-1-quic_kriskura@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3: gadget: Propagate core init errors to UDC during pullup | expand |
On Thu, Jun 15, 2023, Thinh Nguyen wrote: > On Wed, Jun 14, 2023, Krishna Kurapati wrote: > > In scenarios where pullup relies on resume (get sync) to initialize > > the controller and set the run stop bit, then core_init is followed by > > gadget_resume which will eventually set run stop bit. > > > > But in cases where the core_init fails, the return value is not sent > > back to udc appropriately. So according to UDC the controller has > > started but in reality we never set the run stop bit. > > > > On systems like Android, there are uevents sent to HAL depending on > > whether the configfs_bind / configfs_disconnect were invoked. In the > > above mentioned scnenario, if the core init fails, the run stop won't > > be set and the cable plug-out won't result in generation of any > > disconnect event and userspace would never get any uevent regarding > > cable plug out and we never call pullup(0) again. Furthermore none of > > the next Plug-In/Plug-Out's would be known to configfs. > > > > Return back the appropriate result to UDC to let the userspace/ > > configfs know that the pullup failed so they can take appropriate > > action. > > > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > > --- > > drivers/usb/dwc3/gadget.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > index 578804dc29ca..27cb671e18e3 100644 > > --- a/drivers/usb/dwc3/gadget.c > > +++ b/drivers/usb/dwc3/gadget.c > > @@ -2747,7 +2747,9 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > > ret = pm_runtime_get_sync(dwc->dev); > > if (!ret || ret < 0) { > > pm_runtime_put(dwc->dev); > > - return 0; > > + if (ret < 0) > > + pm_runtime_set_suspended(dwc->dev); > > + return ret; > > } > > > > if (dwc->pullups_connected == is_on) { > > -- > > 2.40.0 > > > > Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> > I think this patch qualifies as a fix. Can you also add a fixes tag? Thanks! Thinh
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 578804dc29ca..27cb671e18e3 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2747,7 +2747,9 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) ret = pm_runtime_get_sync(dwc->dev); if (!ret || ret < 0) { pm_runtime_put(dwc->dev); - return 0; + if (ret < 0) + pm_runtime_set_suspended(dwc->dev); + return ret; } if (dwc->pullups_connected == is_on) {
In scenarios where pullup relies on resume (get sync) to initialize the controller and set the run stop bit, then core_init is followed by gadget_resume which will eventually set run stop bit. But in cases where the core_init fails, the return value is not sent back to udc appropriately. So according to UDC the controller has started but in reality we never set the run stop bit. On systems like Android, there are uevents sent to HAL depending on whether the configfs_bind / configfs_disconnect were invoked. In the above mentioned scnenario, if the core init fails, the run stop won't be set and the cable plug-out won't result in generation of any disconnect event and userspace would never get any uevent regarding cable plug out and we never call pullup(0) again. Furthermore none of the next Plug-In/Plug-Out's would be known to configfs. Return back the appropriate result to UDC to let the userspace/ configfs know that the pullup failed so they can take appropriate action. Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> --- drivers/usb/dwc3/gadget.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)