Hi Zixun, Thank you for the patch.
On Tue, May 12, 2026 at 15:11, Zixun LI <[email protected]> wrote: > Endpoints should not be disabled on bus reset event inside UDC driver, > otherwise a racing condition will happen between gadget driver. Gadget nitpick: a racing condition -> a race condition > driver will free the requests and disable endpoints in disconnect ops > later. > Add: Fixes: 59310d1ecb9f ("usb: gadget: introduce 'enabled' flag in struct usb_ep") > Signed-off-by: Zixun LI <[email protected]> Just above your Signed-off-by: to mark this as a fix. > --- > drivers/usb/gadget/atmel_usba_udc.c | 14 -------------- > 1 file changed, 14 deletions(-) Earlier in the driver, in the usba_ep_disable() function, there is the following section: """ if (!ep->desc) { spin_unlock_irqrestore(&udc->lock, flags); /* REVISIT because this driver disables endpoints in * reset_all_endpoints() before calling disconnect(), * most gadget drivers would trigger this non-error ... */ if (udc->gadget.speed != USB_SPEED_UNKNOWN) DBG(DBG_ERR, "ep_disable: %s not enabled\n", ep->ep.name); return -EINVAL; } ep->desc = NULL; """ With this patch, the REVISIT comment is no longer valid. Can we remove it? Also, I believe (but I may be wrong, please try it out) that we can maybe remove the whole if (!ep->desc) block from usba_ep_disable() since that should be handled by the enabled flag wrapper introduced by commit 59310d1ecb9f ("usb: gadget: introduce 'enabled' flag in struct usb_ep") > > diff --git a/drivers/usb/gadget/atmel_usba_udc.c > b/drivers/usb/gadget/atmel_usba_udc.c > index f7a92ded6da..b0de22961de 100644 > --- a/drivers/usb/gadget/atmel_usba_udc.c > +++ b/drivers/usb/gadget/atmel_usba_udc.c > @@ -571,20 +571,6 @@ static void reset_all_endpoints(struct usba_udc *udc) > list_del_init(&req->queue); > request_complete(ep, req, -ECONNRESET); > } > - > - /* NOTE: normally, the next call to the gadget driver is in > - * charge of disabling endpoints... usually disconnect(). > - * The exception would be entering a high speed test mode. > - * > - * FIXME remove this code ... and retest thoroughly. > - */ > - list_for_each_entry(ep, &udc->gadget.ep_list, ep.ep_list) { > - if (ep->desc) { > - spin_unlock(&udc->lock); > - usba_ep_disable(&ep->ep); > - spin_lock(&udc->lock); > - } > - } > } > > static struct usba_ep *get_ep_by_addr(struct usba_udc *udc, u16 wIndex) > > --- > base-commit: 5732bd0f457b4c671e46574d64d4acb099c0f0a5 > change-id: 20260512-udc_ep-51a3f7a6befb > > Best regards, > -- > Zixun LI <[email protected]>
