From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
Selvarasu Ganesan <selvarasu.g@samsung.com>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"jh0801.jung@samsung.com" <jh0801.jung@samsung.com>,
"dh10.jung@samsung.com" <dh10.jung@samsung.com>,
"naushad@samsung.com" <naushad@samsung.com>,
"akash.m5@samsung.com" <akash.m5@samsung.com>,
"h10.kim@samsung.com" <h10.kim@samsung.com>,
"eomji.oh@samsung.com" <eomji.oh@samsung.com>,
"alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
"thiagu.r@samsung.com" <thiagu.r@samsung.com>,
"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH v2] usb: dwc3: gadget: Prevent EPs resource conflict during StartTransfer
Date: Fri, 21 Nov 2025 02:22:02 +0000 [thread overview]
Message-ID: <20251121022156.vbnheb6r2ytov7bt@synopsys.com> (raw)
In-Reply-To: <2b944e45-c39a-4c34-b159-ba91dd627fe4@rowland.harvard.edu>
On Wed, Nov 19, 2025, Alan Stern wrote:
> On Thu, Nov 20, 2025 at 02:07:33AM +0000, Thinh Nguyen wrote:
> > > Function drivers would have to go to great lengths to guarantee that
> > > requests had completed before the endpoint is re-enabled. Right now
> > > their ->set_alt() callback routines are designed to run in interrupt
> > > context; they can't afford to wait for requests to complete.
> >
> > Why is ->set_alt() designed for interrupt context? We can't expect
> > requests to be completed before usb_ep_disable() completes _and_ also
> > expect usb_ep_disable() be able to be called in interrupt context.
>
> ->set_alt() is called by the composite core when a Set-Interface or
> Set-Config control request arrives from the host. It happens within the
> composite_setup() handler, which is called by the UDC driver when a
> control request arrives, which means it happens in the context of the
> UDC driver's interrupt handler. Therefore ->set_alt() callbacks must
> not sleep.
This should be changed. I don't think we can expect set_alt() to
be in interrupt context only.
>
> > > The easiest way out is for usb_ep_disable() to do what the kerneldoc
> > > says: ensure that pending requests do complete before it returns. Can
> > > dwc3 do this? (And what if at some time in the future we want to start
> >
> > The dwc3 can do that, but we need to note that usb_ep_disable() must be
> > executed in process context and might sleep. I suspect we may run into
> > some issues from some function drivers that expected usb_ep_disable() to
> > be executable in interrupt context.
>
> Well, that's part of what I meant to ask. Is it possible to wait for
> all pending requests to be given back while in interrupt context?
The dwc3 controller will need some time (usually less than 2ms) to flush
the endpoints and give back requests. It's probably too long to have
busy poll in interrupt context.
>
> > > using an asynchronous bottom half for request completions, like usbcore
> > > does for URBs?)
> >
> > Which one are you referring to? From what I see, even the host side
> > expected ->endpoint_disable to be executed in process context.
>
> I was referring to the way usb_hcd_giveback_urb() uses system_bh_wq or
> system_bh_highpri_wq to do its work. This makes it impossible for an
> interrupt handler to wait for a giveback to complete.
>
> If the gadget core also switches over to using a work queue for request
> completions, it will then likewise become impossible for an interrupt
> handler to wait for a request to complete.
>
> > Perhaps we can introduce endpoint_flush() on gadget side for
> > synchronization if we want to keep usb_ep_disable() to be asynchronous?
> >
> > >
> > > Let's face it; the situation is a mess.
> > >
> >
> > Glad you're here to help with the mess :)
>
> To do this right, I can't think of any approach other than to make the
> composite core use a work queue or other kernel thread for handling
> Set-Interface and Set-Config calls.
Sounds like it should've been like this initially.
>
> It would be nice if there was a way to invoke the ->set_alt() callback
> that would just disable the interface's endpoints without re-enabling
> anything. Then the composite core could disable the existing
> altsetting, flush the old endpoints, and call ->set_alt() a second time
> to install the new altsetting and enable the new endpoints. But
> implementing this would require us to update every function driver's
> ->set_alt() callback routine.
Yeah..
>
> Without that ability, we will have to audit every function driver to
> make sure the ->set_alt() callbacks do ensure that endpoints are flushed
> before they are re-enabled.
>
> There does not seem to be any way to fix the problem just by changing
> the gadget core.
>
We can have a workaround in dwc3 that can temporarily "work" with what
we have. However, eventually, we will need to properly rework this and
audit the gadget drivers.
Thanks,
Thinh
next prev parent reply other threads:[~2025-11-21 2:22 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20251117160057epcas5p324eddf1866146216495186a50bcd3c01@epcas5p3.samsung.com>
2025-11-17 15:59 ` [PATCH v2] usb: dwc3: gadget: Prevent EPs resource conflict during StartTransfer Selvarasu Ganesan
2025-11-18 2:21 ` Thinh Nguyen
2025-11-18 3:58 ` Selvarasu Ganesan
2025-11-19 1:56 ` Thinh Nguyen
2025-11-18 4:20 ` Alan Stern
2025-11-19 1:49 ` Thinh Nguyen
2025-11-19 4:09 ` Alan Stern
2025-11-20 2:07 ` Thinh Nguyen
2025-11-20 3:33 ` Alan Stern
2025-11-21 2:22 ` Thinh Nguyen [this message]
2025-11-21 3:08 ` Alan Stern
2025-12-03 5:25 ` Selvarasu Ganesan
2025-12-04 1:51 ` Thinh Nguyen
2025-12-04 13:15 ` Selvarasu Ganesan
2025-12-05 0:37 ` Thinh Nguyen
2025-12-05 1:18 ` Thinh Nguyen
2025-12-05 1:19 ` Thinh Nguyen
2025-12-11 10:38 ` Selvarasu Ganesan
2025-12-12 1:21 ` Thinh Nguyen
2026-02-26 10:59 ` Selvarasu Ganesan
2026-02-26 18:41 ` Thinh Nguyen
2026-03-02 5:56 ` Selvarasu Ganesan
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=20251121022156.vbnheb6r2ytov7bt@synopsys.com \
--to=thinh.nguyen@synopsys.com \
--cc=akash.m5@samsung.com \
--cc=alim.akhtar@samsung.com \
--cc=dh10.jung@samsung.com \
--cc=eomji.oh@samsung.com \
--cc=gregkh@linuxfoundation.org \
--cc=h10.kim@samsung.com \
--cc=jh0801.jung@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=naushad@samsung.com \
--cc=selvarasu.g@samsung.com \
--cc=stable@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
--cc=thiagu.r@samsung.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox