[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eb19ca1d-1919-4ead-9795-5dd24ca1950c@samsung.com>
Date: Tue, 18 Nov 2025 09:28:55 +0530
From: Selvarasu Ganesan <selvarasu.g@...sung.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>, Alan Stern
<stern@...land.harvard.edu>
Cc: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jh0801.jung@...sung.com" <jh0801.jung@...sung.com>, "dh10.jung@...sung.com"
<dh10.jung@...sung.com>, "naushad@...sung.com" <naushad@...sung.com>,
"akash.m5@...sung.com" <akash.m5@...sung.com>, "h10.kim@...sung.com"
<h10.kim@...sung.com>, "eomji.oh@...sung.com" <eomji.oh@...sung.com>,
"alim.akhtar@...sung.com" <alim.akhtar@...sung.com>, "thiagu.r@...sung.com"
<thiagu.r@...sung.com>, "stable@...r.kernel.org" <stable@...r.kernel.org>,
"hongpooh.kim@...sung.com" <hongpooh.kim@...sung.com>,
"muhammed.ali@...sung.com" <muhammed.ali@...sung.com>
Subject: Re: [PATCH v2] usb: dwc3: gadget: Prevent EPs resource conflict
during StartTransfer
On 11/18/2025 7:51 AM, Thinh Nguyen wrote:
> On Mon, Nov 17, 2025, Selvarasu Ganesan wrote:
>> The below “No resource for ep” warning appears when a StartTransfer
>> command is issued for bulk or interrupt endpoints in
>> `dwc3_gadget_ep_enable` while a previous StartTransfer on the same
>> endpoint is still in progress. The gadget functions drivers can invoke
>> `usb_ep_enable` (which triggers a new StartTransfer command) before the
>> earlier transfer has completed. Because the previous StartTransfer is
>> still active, `dwc3_gadget_ep_disable` can skip the required
>> `EndTransfer` due to `DWC3_EP_DELAY_STOP`, leading to the endpoint
>> resources are busy for previous StartTransfer and warning ("No resource
>> for ep") from dwc3 driver.
>>
>> To resolve this, a check is added to `dwc3_gadget_ep_enable` that
>> checks the `DWC3_EP_TRANSFER_STARTED` flag before issuing a new
>> StartTransfer. By preventing a second StartTransfer on an already busy
>> endpoint, the resource conflict is eliminated, the warning disappears,
>> and potential kernel panics caused by `panic_on_warn` are avoided.
>>
>> ------------[ cut here ]------------
>> dwc3 13200000.dwc3: No resource for ep1out
>> WARNING: CPU: 0 PID: 700 at drivers/usb/dwc3/gadget.c:398 dwc3_send_gadget_ep_cmd+0x2f8/0x76c
>> Call trace:
>> dwc3_send_gadget_ep_cmd+0x2f8/0x76c
>> __dwc3_gadget_ep_enable+0x490/0x7c0
>> dwc3_gadget_ep_enable+0x6c/0xe4
>> usb_ep_enable+0x5c/0x15c
>> mp_eth_stop+0xd4/0x11c
>> __dev_close_many+0x160/0x1c8
>> __dev_change_flags+0xfc/0x220
>> dev_change_flags+0x24/0x70
>> devinet_ioctl+0x434/0x524
>> inet_ioctl+0xa8/0x224
>> sock_do_ioctl+0x74/0x128
>> sock_ioctl+0x3bc/0x468
>> __arm64_sys_ioctl+0xa8/0xe4
>> invoke_syscall+0x58/0x10c
>> el0_svc_common+0xa8/0xdc
>> do_el0_svc+0x1c/0x28
>> el0_svc+0x38/0x88
>> el0t_64_sync_handler+0x70/0xbc
>> el0t_64_sync+0x1a8/0x1ac
>>
>> Fixes: a97ea994605e ("usb: dwc3: gadget: offset Start Transfer latency for bulk EPs")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Selvarasu Ganesan <selvarasu.g@...sung.com>
>> ---
>>
>> Changes in v2:
>> - Removed change-id.
>> - Updated commit message.
>> Link to v1: https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20251117152812.622-1-selvarasu.g@samsung.com/__;!!A4F2R9G_pg!dQNEaMvjzrOiP0c9U6lyj31ysXGkjBAs8c_KjgDCp6ONSZcTrF15DXaJeFTK02v0RLS3w0NQ2K5_pvXak_7c8tIxKL8$
>> ---
>> drivers/usb/dwc3/gadget.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 1f67fb6aead5..8d3caa71ea12 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -963,8 +963,9 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, unsigned int action)
>> * Issue StartTransfer here with no-op TRB so we can always rely on No
>> * Response Update Transfer command.
>> */
>> - if (usb_endpoint_xfer_bulk(desc) ||
>> - usb_endpoint_xfer_int(desc)) {
>> + if ((usb_endpoint_xfer_bulk(desc) ||
>> + usb_endpoint_xfer_int(desc)) &&
>> + !(dep->flags & DWC3_EP_TRANSFER_STARTED)) {
>> struct dwc3_gadget_ep_cmd_params params;
>> struct dwc3_trb *trb;
>> dma_addr_t trb_dma;
>> --
>> 2.34.1
>>
> Thanks for the catch. The problem is that the "ep_disable" process
> should be completed after usb_ep_disable is completed. But currently it
> may be an async call.
>
> This brings up some conflicting wording of the gadget API regarding
> usb_ep_disable. Here's the doc regarding usb_ep_disable:
>
> /**
> * usb_ep_disable - endpoint is no longer usable
> * @ep:the endpoint being unconfigured. may not be the endpoint named "ep0".
> *
> * no other task may be using this endpoint when this is called.
> * any pending and uncompleted requests will complete with status
> * indicating disconnect (-ESHUTDOWN) before this call returns.
> * gadget drivers must call usb_ep_enable() again before queueing
> * requests to the endpoint.
> *
> * This routine may be called in an atomic (interrupt) context.
> *
> * returns zero, or a negative error code.
> */
>
> It expects all requests to be completed and given back on completion. It
> also notes that this can also be called in interrupt context. Currently,
> there's a scenario where dwc3 may not want to give back the requests
> right away (ie. DWC3_EP_DELAY_STOP). To fix that in dwc3, it would need
Hi Thinh,
Thanks for your comments.
I agree with you on dwc3 may not want to give back the requests right
away (ie. DWC3_EP_DELAY_STOP) and might also ignore the End Transfer
command.
Consequently, there’s no point in scheduling a new Start Transfer before
the previous one has completed. The traces below illustrate this: for
EP1OUT the End Transfer never occurs, so a new Start Transfer is issued,
which eventually ends with a “No Resource” error.
1. EP disable for ep1in: (working EP)
----------------------------------
dwc3_gadget_ep_disable: ep1in: mps 512/1024 streams 16 burst 0 ring 8/0
flags E:swBp:< dwc3_gadget_ep_cmd: ep1in: cmd 'End Transfer' [30c08]
params 00000000 00000000 00000000 --> status: Successful
dwc3_gadget_giveback: ep1in: req 00000000cfc03ba0 length 0/194 Zsi ==>
-108 dwc3_gadget_giveback: ep1in: req 0000000075196149 length 0/194 Zsi
==> -108 dwc3_gadget_giveback: ep1in: req 000000002b2ffaa2 length 0/86
Zsi ==> -108
2. EP enable for ep1out:(Not working EP) -->(No End Transfer)
----------------------------------------------------------
dwc3_gadget_ep_disable: ep1out: mps 512/682 streams 16 burst 0 ring
114/74 flags E:swBp:>
3. EP disable for ep1in:
-----------------------
dwc3_gadget_ep_cmd: ep1in: cmd 'Set Endpoint Configuration' [401] params
00021004 06000200 00000000 --> status: Successful dwc3_gadget_ep_cmd:
ep1in: cmd 'Start Transfer' [406] params 00000008 ad6b9000 00000000 -->
status: Successful dwc3_gadget_ep_enable: ep1in: mps 512/1024 streams 16
burst 0 ring 0/0 flags E:swBp:<
4. EP enable for ep1out:(Triggered start Transfer without End Transfer)
----------------------------------------------------------------------
dwc3_gadget_ep_cmd: ep1out: cmd 'Set Endpoint Configuration' [401]
params 00001004 04000200 00000000 --> status: Successful
dwc3_gadget_ep_cmd: ep1out: cmd 'Start Transfer' [406] params 00000008
ad6b7000 00000000 --> status: No Resource dwc3_gadget_ep_enable: ep1out:
mps 512/682 streams 16 burst 0 ring 114/74 flags E:swBp:>
The given fix will prevent in this failure case ,
dwc3_gadget_ep_enable:
---------------------
+ if ((usb_endpoint_xfer_bulk(desc) ||
+ usb_endpoint_xfer_int(desc)) &&
+ !(dep->flags & DWC3_EP_TRANSFER_STARTED)) {
Thanks,
Selva
> to "wait" for the right condition. But waiting does not make sense in
> interrupt context. (We could busy-poll to satisfy the interrupt context,
> but that doesn't sound right either)
>
> This was updated from process context only to may be called in interrupt
> context:
>
> b0d5d2a71641 ("usb: gadget: udc: core: Revise comments for USB ep enable/disable")
>
>
> Hi Alan,
>
> Can you help give your opinion on this?
>
> Thanks,
> Thinh
Powered by blists - more mailing lists