On Mon, 4 Feb 2019, Guido Kiener wrote: > From: Guido Kiener <guido.kiener@xxxxxxxxxxxxxxxxx> > > A reset e.g. calling ep_reset_338x() can happen while endpoints > are enabled. How can this happen? That routine is called from only two places. One is in net2280_disable(), so after the endpoint has already been disabled. The other is in usb_reinit_338x() via usb_reinit() via stop_activity(), which disconnects the gadget driver and thereby disables all the endpoints. > The ep_reset_338x() sets ep->desc = NULL to mark > endpoint being invalid. A subsequent call of net2280_disable will > fail and return -EINVAL to parent function usb_ep_disable(), > which will fail, too, and do not set the member ep->enabled = false. So maybe a better fix (assuming there really is a problem) is to make usb_ep_disable() clear ep->enabled always. After all, the only reasonable way for usb_ep_disable() to fail is if the endpoint isn't enabled in the first place. Alan Stern > See: > https://elixir.bootlin.com/linux/v5.0-rc5/source/drivers/usb/gadget/udc/core.c#L139 > > This fix ignores dp->desc and allows net2280_disable() to succeed. > Subsequent calls to usb_ep_enable()/usb_ep_disable() succeeds. > > Signed-off-by: Guido Kiener <guido.kiener@xxxxxxxxxxxxxxxxx> > --- > drivers/usb/gadget/udc/net2280.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c > index e7dae5379e04..7154f00dea40 100644 > --- a/drivers/usb/gadget/udc/net2280.c > +++ b/drivers/usb/gadget/udc/net2280.c > @@ -516,8 +516,8 @@ static int net2280_disable(struct usb_ep *_ep) > unsigned long flags; > > ep = container_of(_ep, struct net2280_ep, ep); > - if (!_ep || !ep->desc || _ep->name == ep0name) { > - pr_err("%s: Invalid ep=%p or ep->desc\n", __func__, _ep); > + if (!_ep || _ep->name == ep0name) { > + pr_err("%s: Invalid ep=%p\n", __func__, _ep); > return -EINVAL; > } > spin_lock_irqsave(&ep->dev->lock, flags); >
