All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mattijs Korpershoek <mkorpershoek@kernel.org>
To: Zixun LI <admin@hifiphile.com>, u-boot@lists.denx.de
Cc: Lukasz Majewski <lukma@denx.de>, Marek Vasut <marex@denx.de>,
	Tom Rini <trini@konsulko.com>, Zixun LI <zli@ogga.fr>,
	Andrew Goodbody <andrew.goodbody@linaro.org>,
	Zixun LI <admin@hifiphile.com>
Subject: Re: [PATCH] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints()
Date: Wed, 13 May 2026 13:24:07 +0200	[thread overview]
Message-ID: <87h5obv9go.fsf@kernel.org> (raw)
In-Reply-To: <20260512-udc_ep-v1-1-8a783e44cf7f@hifiphile.com>

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>

  reply	other threads:[~2026-05-13 11:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 13:11 [PATCH] usb: gadget: atmel: do not disable endpoints in reset_all_endpoints() Zixun LI
2026-05-13 11:24 ` Mattijs Korpershoek [this message]
2026-05-13 14:05   ` Zixun LI
2026-05-18  7:37     ` Mattijs Korpershoek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h5obv9go.fsf@kernel.org \
    --to=mkorpershoek@kernel.org \
    --cc=admin@hifiphile.com \
    --cc=andrew.goodbody@linaro.org \
    --cc=lukma@denx.de \
    --cc=marex@denx.de \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=zli@ogga.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.