All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()
@ 2026-05-15 10:05 Zixun LI
  2026-05-18  7:36 ` Mattijs Korpershoek
  2026-05-20 10:10 ` Mattijs Korpershoek
  0 siblings, 2 replies; 3+ messages in thread
From: Zixun LI @ 2026-05-15 10:05 UTC (permalink / raw)
  To: u-boot
  Cc: Lukasz Majewski, Mattijs Korpershoek, Marek Vasut, Tom Rini,
	Zixun LI, Andrew Goodbody, Zixun LI, Stephan Gerhold

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

Also remove outdated comment about it in usba_ep_disable().

Fixes: 59310d1ecb9f ("usb: gadget: introduce 'enabled' flag in struct usb_ep")

Signed-off-by: Zixun LI <admin@hifiphile.com>
---
Changes in v2:
- Remove outdated comment in usba_ep_disable()
- Reword commit message
- Link to v1: https://patch.msgid.link/20260512-udc_ep-v1-1-8a783e44cf7f@hifiphile.com

To: Mattijs Korpershoek <mkorpershoek@kernel.org>
To: Stephan Gerhold <stephan.gerhold@linaro.org>
To: u-boot@lists.denx.de
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@konsulko.com>
Cc: Zixun LI <admin@hifiphile.com>
Cc: Andrew Goodbody <andrew.goodbody@linaro.org>
---
 drivers/usb/gadget/atmel_usba_udc.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
index f7a92ded6da..a2eee2bca2c 100644
--- a/drivers/usb/gadget/atmel_usba_udc.c
+++ b/drivers/usb/gadget/atmel_usba_udc.c
@@ -289,10 +289,6 @@ static int usba_ep_disable(struct usb_ep *_ep)
 
 	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);
@@ -571,20 +567,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>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()
  2026-05-15 10:05 [PATCH v2] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints() Zixun LI
@ 2026-05-18  7:36 ` Mattijs Korpershoek
  2026-05-20 10:10 ` Mattijs Korpershoek
  1 sibling, 0 replies; 3+ messages in thread
From: Mattijs Korpershoek @ 2026-05-18  7:36 UTC (permalink / raw)
  To: Zixun LI, u-boot
  Cc: Lukasz Majewski, Mattijs Korpershoek, Marek Vasut, Tom Rini,
	Zixun LI, Andrew Goodbody, Zixun LI, Stephan Gerhold

Hi Zixun,

Thank you for the patch.

On Fri, May 15, 2026 at 12:05, Zixun LI <admin@hifiphile.com> wrote:

> Endpoints should not be disabled on bus reset inside UDC driver,
> otherwise a race condition will happen between gadget driver. Gadget
> driver will free the requests and disable endpoints in disconnect ops.
>
> Also remove outdated comment about it in usba_ep_disable().
>
> Fixes: 59310d1ecb9f ("usb: gadget: introduce 'enabled' flag in struct usb_ep")
>
> Signed-off-by: Zixun LI <admin@hifiphile.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

Note: there should not be an empty newline between the fixes and the signed-off.
It should be:

Fixes: 59310d1ecb9f ("usb: gadget: introduce 'enabled' flag in struct usb_ep")
Signed-off-by: Zixun LI <admin@hifiphile.com>

Not:

Fixes: 59310d1ecb9f ("usb: gadget: introduce 'enabled' flag in struct usb_ep")
<empty newline>
Signed-off-by: Zixun LI <admin@hifiphile.com>

No need to resend a v3 for it, I can fix it up while applying.

> ---
> Changes in v2:
> - Remove outdated comment in usba_ep_disable()
> - Reword commit message
> - Link to v1: https://patch.msgid.link/20260512-udc_ep-v1-1-8a783e44cf7f@hifiphile.com
>
> To: Mattijs Korpershoek <mkorpershoek@kernel.org>
> To: Stephan Gerhold <stephan.gerhold@linaro.org>
> To: u-boot@lists.denx.de
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Zixun LI <admin@hifiphile.com>
> Cc: Andrew Goodbody <andrew.goodbody@linaro.org>
> ---
>  drivers/usb/gadget/atmel_usba_udc.c | 18 ------------------
>  1 file changed, 18 deletions(-)
>
> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
> index f7a92ded6da..a2eee2bca2c 100644
> --- a/drivers/usb/gadget/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/atmel_usba_udc.c
> @@ -289,10 +289,6 @@ static int usba_ep_disable(struct usb_ep *_ep)
>  
>  	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);
> @@ -571,20 +567,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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()
  2026-05-15 10:05 [PATCH v2] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints() Zixun LI
  2026-05-18  7:36 ` Mattijs Korpershoek
@ 2026-05-20 10:10 ` Mattijs Korpershoek
  1 sibling, 0 replies; 3+ messages in thread
From: Mattijs Korpershoek @ 2026-05-20 10:10 UTC (permalink / raw)
  To: u-boot, Zixun LI
  Cc: Lukasz Majewski, Marek Vasut, Tom Rini, Zixun LI, Andrew Goodbody,
	Stephan Gerhold

Hi,

On Fri, 15 May 2026 12:05:41 +0200, Zixun LI wrote:
> Endpoints should not be disabled on bus reset inside UDC driver,
> otherwise a race condition will happen between gadget driver. Gadget
> driver will free the requests and disable endpoints in disconnect ops.
> 
> Also remove outdated comment about it in usba_ep_disable().
> 
> Fixes: 59310d1ecb9f ("usb: gadget: introduce 'enabled' flag in struct usb_ep")
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu)

[1/1] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/7f34bb50a59804f6ebe02c1f62b7b29093eb103c

--
Mattijs

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-20 10:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-15 10:05 [PATCH v2] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints() Zixun LI
2026-05-18  7:36 ` Mattijs Korpershoek
2026-05-20 10:10 ` Mattijs Korpershoek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.