Hi, Danilo Krummrich <danilokrummrich@xxxxxxxxxxxxx> writes: > udc_stop needs to be called before gadget driver unbind. Otherwise it > might happen that udc drivers still call into the gadget driver (e.g. > to reset gadget after OTG event). If this happens it is likely to get > panics from gadget driver dereferencing NULL ptr, as gadget's drvdata > is set to NULL on unbind. seems like the problem here is with the OTG layer, not UDC core. > Signed-off-by: Danilo Krummrich <danilokrummrich@xxxxxxxxxxxxx> > --- > Actually there could still be a race: > (CPU1 code taken from dwc3 drivers dwc3_disconnect_gadget() as exsample) > > CPU0 CPU1 > ---- ---- > usb_gadget_disconnect(udc->gadget); > udc->driver->disconnect(udc->gadget); > if (dwc->gadget_driver && dwc->gadget_driver->disconnect) > usb_gadget_udc_stop(udc); > udc->driver->unbind(udc->gadget); > dwc->gadget_driver->disconnect(&dwc->gadget); > > UDC drivers typically set their gadget driver pointer to NULL in udc_stop > and check for it before calling into the gadget driver. To fix the issue > above every udc driver could apply a lock around this. > > If you see the need for having this or another solutions I can provide > further patches. This patch could also just serve as a base for discussion > if someone knows a smarter solution. > > I saw this problem causing a panic on hikey960 board and provided a quick > workaround for the same problem here: > https://android-review.googlesource.com/#/c/kernel/common/+/457476/ > (panic log in the commit message of the linked patch) > --- > drivers/usb/gadget/udc/core.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c > index efce68e9a8e0..8155468afc0d 100644 > --- a/drivers/usb/gadget/udc/core.c > +++ b/drivers/usb/gadget/udc/core.c > @@ -1234,8 +1234,12 @@ static void usb_gadget_remove_driver(struct usb_udc *udc) > > usb_gadget_disconnect(udc->gadget); > udc->driver->disconnect(udc->gadget); > - udc->driver->unbind(udc->gadget); > + /* udc_stop needs to be called before gadget driver unbind to prevent > + * udc driver calls into gadget driver after unbind which could cause > + * a nullptr exception. > + */ > usb_gadget_udc_stop(udc); > + udc->driver->unbind(udc->gadget); This patch is incorrect, it will prevent us from giving back requests to gadget driver properly. ->unbind() has to happen before ->udc_stop(). -- balbi
Attachment:
signature.asc
Description: PGP signature
