[PATCH] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()

[PATCH] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()

[Zixun LI]

Endpoints should not be disabled on bus reset event inside UDC driver,
otherwise a racing condition will happen between gadget driver. Gadget
driver will free the requests and disable endpoints in disconnect ops
later.

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
 drivers/usb/gadget/atmel_usba_udc.c | 14 --------------
 1 file changed, 14 deletions(-)

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 <admin@hifiphile.com>

Re: [PATCH] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()

[Mattijs Korpershoek]

Hi Zixun,

Thank you for the patch.

On Tue, May 12, 2026 at 15:11, Zixun LI <admin@hifiphile.com> 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 <admin@hifiphile.com>

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 <admin@hifiphile.com>

Re: [PATCH] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()

[Zixun LI]

Hi Mattijs,

On Wed, May 13, 2026 at 1:24 PM Mattijs Korpershoek <mkorpershoek@kernel.org>
wrote:

>
> 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")
>
>
I agree we can remove the whole if block but it will desync with kernel
code.
Do you want me to send a patch ?

Best regards,
Zixun LI

Re: [PATCH] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()

[Mattijs Korpershoek]

On Wed, May 13, 2026 at 16:05, Zixun LI <admin@hifiphile.com> wrote:

> Hi Mattijs,
>
> On Wed, May 13, 2026 at 1:24 PM Mattijs Korpershoek <mkorpershoek@kernel.org>
> wrote:
>
>>
>> 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")
>>
>>
> I agree we can remove the whole if block but it will desync with kernel
> code.
> Do you want me to send a patch ?

I see a follow-up has been send here:
https://lore.kernel.org/all/20260515-udc_ep-v2-1-cd335b4e62e4@hifiphile.com/

That one looks good to me. I indeed prefer to stay synced with the
kernel code!

>
> Best regards,
> Zixun LI