Re: [PATCH v3] usb: dwc3: gadget: Prevent EP resource conflicts during StartTransfer

[Selvarasu Ganesan <selvarasu.g@samsung.com>]

On 10/13/2026 7:22 AM, Thinh Nguyen wrote:
> On Mon, Oct 12, 2026, Selvarasu Ganesan wrote:
>> On 10/9/2026 5:57 AM, Thinh Nguyen wrote:
>>> On Thu, Oct 08, 2026, Selvarasu Ganesan wrote:
>>>>> The End Transfer command not completing is also a separate issue. I
>>>>> asked you to check whether the command would eventually complete with
>>>>> the additional code, and it appears it doesn't.
>>>>     We have confirmed that the completion event is never seen, once the
>>>> timeout occurs for end transfer. It appears the command is indeed
>>>> hanging in the hardware, as you suspected.
>>>>
>>>>> Can you provide some more info on your setup:
>>>>>     * IP and version
>>>>>     * connected speed
>>>>>     * endpoint direction (is it always OUT?)
>>>>>     * Is there any active transfer
>>>>>     * tracepoints
>>>> * IP and version : 0xc120:   0x33313130
>>> and version (0xc1a0)?
>>
>> VER_NUMBER = 0x3231302a
>> VER_TYPE = 0x67612a2a
>>>> * connected speed : HS mode
>>>> * endpoint direction (is it always OUT?) : Yes its always OUT
>>>> * Is there any active transfer : No, There is evidences from the log to
>>>> say there is a active transfer.
>>> The host may have sent data that would not appear in the log until
>>> completion. Are there any active transfer on other endpoints?
>>
>> Yes, we can see active transfer on other endpoints.
> Was there another SETUP packet aborting the ClearFeature(ep_halt) while
> this is going on?
We dont see any  evidence from the log to say SETUP packet aborting, 
anyway we will add a debug logs to confirm this one.
>
>>>> * tracepoints: There is no dwc3 traces for this issue as of now due to
>>>> its low reproduction rate in the customer's setup.
>>>>
>>> I think you may need to file a case with our support team and provide
>>> additional debug info such as scan dump to investigate this further.
>>
>> Sure We will file a case,
>> I agree that the ENDTRANSFER timeout may be a controller level issue
>> that we can debug separately.  But, there is also a software bug here.
>> Let see the in the below error case, "dep->resource_index" is reset to
>> "0" unconditionally, even when the first ENDTRANSFER times out. A
>> subsequent ENDTRANSFER with resource index "0" may succeed and
>> incorrectly clear the pending and transfer started flags, making the
>> driver assume the previous transfer has completed. Since the original
>> transfer was never properly terminated, the endpoint may not recover,
>> resulting in a no-resources error on the next STARTTRANSFER.
>>
>> Therefore, we need to fix this software state tracking issue
>> independently of the controller timeout.
>>
>> Step 1:
>> __dwc3_gadget_ep_set_halt(dep, value=0)
>>    ->dwc3_stop_active_transfer(dep, true, true)
>>     ->END(e.g, ep2out, resource_index=7) issued, but timeout occurs
>>      -> resource_index cleared to 0
>> ->dwc3_send_clear_stall_ep_cmd(dep)
>>    -> failed to clear STALL on ep2out
>>
>>
>> Step 2:
>> __dwc3_gadget_ep_set_halt(dep, value=0) ->Re triggered clear stall for
>> same EP
>>    -> dwc3_stop_active_transfer(dep, true, true)
>>      -> END(ep2out, resource_index=0) issued (wrong resource!)
>>       -> Command succeeds, DWC3_EP_TRANSFER_STARTED cleared in completion
>> handler
>>
>> Step 3:
>> usb_ep_queue()
>> -> dwc3_gadget_ep_queue()
>>    -> __dwc3_gadget_kick_transfer()
>>     -> starting = !(dep->flags & DWC3_EP_TRANSFER_STARTED) -> starting=0
>>      -> Issues STARTTRANSFER (because DWC3_EP_TRANSFER_STARTED is not set)
>>        -> Hardware rejects with NO_RESOURCE (resource 7 still held)
>>
>>
>> We are fine with the following suggestions fix to prevent resource issue
>> in timeout case,
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 1f973e546219..bac0328423cd 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1819,7 +1819,11 @@ static int __dwc3_stop_active_transfer(struct
>> dwc3_ep *dep, bool force, bool int
>>
>>        dep->resource_index = 0;
>>
>> -    if (!interrupt || ret)
>> +    if (!interrupt || (ret && ret != -ETIMEDOUT))
>>            dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
>>        else
>>            dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
>> @@ -3895,6 +3899,8 @@ static void
>> dwc3_gadget_endpoint_command_complete(struct dwc3_ep *dep,
>>        if (dep->stream_capable)
>>            dep->flags |= DWC3_EP_IGNORE_NEXT_NOSTREAM;
>>
>> +    dep->flags &= ~DWC3_EP_DELAY_STOP;
>>        dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
>>        dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
>>        dwc3_gadget_ep_cleanup_cancelled_requests(dep);
>>
>>
>> But regarding below changes in ep_disable that you suggested, is this
>> really required? It seems this could cause a bug during an endtransfer
>> timeout, if command completion never happens, the PENDING flag won't be
>> cleared, making recovery impossible.
> Yes, it's required. The suggested patch now enables IOC for ep_disable.
> So we need to track its completion with DWC3_EP_END_TRANSFER_PENDING.
> This handles the case where the End Transfer completes after ep_disable
> returns. If the function driver re-enables and queues to the endpoint
> before then, the driver would issue Start Transfer while the resource is
> still held. That's the original issue.
>
> If the End Transfer never completes, the endpoint is stuck. Clearing the
> flag doesn't recover it. The dwc3_schedule_err_recovery() is now in

I mean 'not recover here' is the failure to clear the 
DWC3_EP_END_TRANSFER_PENDING flag upon cable reconnection, caused by a 
missing command completion from the prior connection.

Sure we can use dwc3_schedule_err_recovery(), and thanks for the suggestion.

> mainline. We can use that as recovery for -ETIMEDOUT.
>
> Looking at the recovery function now, we also need to reset the
> dep->flags on recovery which I missed when reviewing the change. I'll
> send a patch for this.

Great, Thank you.
>
>> @@ -1096,6 +1110,15 @@ static int __dwc3_gadget_ep_disable(struct
>> dwc3_ep *dep)
>>         */
>>        if (dep->flags & DWC3_EP_DELAY_STOP)
>>            mask |= (DWC3_EP_DELAY_STOP | DWC3_EP_TRANSFER_STARTED);
>> +
>> +    /*
>> +     * The End Transfer command is still in progress.  Do not clear the
>> +     * flags, so that the ep is only rearmed once the command completes.
>> +     */
>> +    if (dep->flags & DWC3_EP_END_TRANSFER_PENDING)
>> +        mask |= (DWC3_EP_END_TRANSFER_PENDING |
>> +             DWC3_EP_TRANSFER_STARTED);
>>
>>
>> As you mentioned Clearing the flag (DWC3_EP_END_TRANSFER_PENDING) in
>> ep_disble only lead to a NO_RESOURCE error later. Yes i agree, but this
>> can prevent by below new suggestion change, so above changes (do not
>> clear DWC3_EP_END_TRANSFER_PENDING in ep_disable) is not required.
> The ep_queue change only covers the giveback window inside ep_disable,
> not this case.
agree.
>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index
>> ee837235630a..eb6666b7bb98 100644 --- a/drivers/usb/dwc3/gadget.c +++
>> b/drivers/usb/dwc3/gadget.c @@ -1084,6 +1084,8 @@ static int
>> __dwc3_gadget_ep_disable(struct dwc3_ep *dep) reg &=
>> ~DWC3_DALEPENA_EP(dep->number); dwc3_writel(dwc, DWC3_DALEPENA, reg); +
>> dep->flags &= ~DWC3_EP_ENABLED; + dwc3_remove_requests(dwc, dep,
>> -ESHUTDOWN); dep->stream_capable = false; @@ -1990,7 +1992,8 @@ static
>> int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request
>> *req) { struct dwc3 *dwc = dep->dwc; - if (!dep->endpoint.desc ||
>> !dwc->pullups_connected || !dwc->connected) { + if (!(dep->flags &
>> DWC3_EP_ENABLED) || !dep->endpoint.desc || + !dwc->pullups_connected ||
>> !dwc->connected) { dev_dbg(dwc->dev, "%s: can't queue to disabled
>> endpoint\n", dep->name); return -ESHUTDOWN;
>>
> (Can you help fix your mail client? I had to unmangle the diff above to
> know what it was)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index ee837235630a..eb6666b7bb98 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1084,6 +1084,8 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep 
*dep)
      reg &= ~DWC3_DALEPENA_EP(dep->number);
      dwc3_writel(dwc, DWC3_DALEPENA, reg);

+    dep->flags &= ~DWC3_EP_ENABLED;
+
      dwc3_remove_requests(dwc, dep, -ESHUTDOWN);

      dep->stream_capable = false;
@@ -1990,7 +1992,8 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep 
*dep, struct dwc3_request *req)
  {
      struct dwc3        *dwc = dep->dwc;

-    if (!dep->endpoint.desc || !dwc->pullups_connected || 
!dwc->connected) {
+    if (!(dep->flags & DWC3_EP_ENABLED) || !dep->endpoint.desc ||
+        !dwc->pullups_connected || !dwc->connected) {
          dev_dbg(dwc->dev, "%s: can't queue to disabled endpoint\n",
                  dep->name);
          return -ESHUTDOWN;
>
> Thanks,
> Thinh