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

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

On 10/7/2026 7:27 PM, Selvarasu Ganesan wrote:
> On 10/7/2026 7:36 AM, Thinh Nguyen wrote:
>> On Tue, Oct 06, 2026, Selvarasu Ganesan wrote:
>>>>     	/* Clear out the ep descriptors for non-ep0 */
>>>> @@ -1792,9 +1815,9 @@ static int __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool int
>>>>     
>>>>     	dep->resource_index = 0;
>>>>     
>>>> -	if (!interrupt)
>>>> +	if (!interrupt || ret)
>>> Hi Thinh,
>>>
>>> Thanks for your code changes. The given code changes look good to me and
>>> are working as expected.
>>>
>>> There is one more concern about an uncovered endpoint resource failure
>>> in some corner cases where END TRANSFER timeout is observed (ret = -110).
>>>
>>> The sequence is explained below,
>>>
>>> Step 1:
>>>     __dwc3_gadget_ep_set_halt(dep, value=0)
>>>      ->dwc3_stop_active_transfer(dep, true, true)
>> Looks like you still do ForceRM=true. Can you apply this patch:
>>
>>     b58e6200450d ("usb: dwc3: clear forceRM when issuing EndTransfer")
> We have this fix in our dwc3 driver code. but still observing EP end
> transfer timeout.
>
>
>>>        ->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)
>>>
>>>
>>> Could you please give your suggestions on this issue case?
>>>
>> Thanks for testing. Can you check whether the End Transfer command ever
>> completes with the endpoint completion event after the -ETIMEDOUT error?
>
> No, the endpoint completion event is not seen after the -ETIMEDOUT error.
>
> The proposed fix works well for the __dwc3_gadget_ep_set_halt sequence,
> where DWC3_EP_END_TRANSFER_PENDING must be set to prevent dwc3_ep_queue
> from starting a new transfer during a EP transfer timeout.
>
> But, this is unnecessary for __dwc3_gadget_ep_disable. Since there's no
> way to clear the pending flag if the interrupt is missed and no
> dwc3_ep_queue calls occur until the EP is re-enabled, preserving
> DWC3_EP_END_TRANSFER_PENDING here provides no benefit.
> So, the below changes is not necessary in ep disable,
>
> @@ -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); +
>
> Instead, keep our suggestion changes that prevent the manipulation of
> dep->flags due to the race condition between dwc3_gadget_ep_disable()
> and dwc3_gadget_ep_queue(), since this race observing in long run rndis
> test.
>
> + + /* + * When dwc3_gadget_ep_disable() calls dwc3_gadget_giveback(), +
> * the dwc->lock is temporarily released. If dwc3_gadget_ep_queue() + *
> runs in that window it may set the DWC3_EP_TRANSFER_STARTED flag as + *
> part of dwc3_send_gadget_ep_cmd. The original code cleared the flag + *
> unconditionally in the mask operation, which could overwrite the + *
> concurrent modification. + * + * As a workaround for the interrupt
> context constraint where we cannot + * wait for endpoint flushing,
> preserve the DWC3_EP_TRANSFER_STARTED + * flag if it is set, avoiding
> resource conflicts until the framework + * is fixed to properly
> synchronize endpoint lifecycle management. + */ + if (dep->flags &
> DWC3_EP_TRANSFER_STARTED) + mask |= DWC3_EP_TRANSFER_STARTED; +
>
>
> Thanks,
> Selva

Sorry for the format issue. The updated answers as below,

No, the endpoint completion event is not seen after the -ETIMEDOUT error.

The proposed fix works well for the __dwc3_gadget_ep_set_halt sequence, 
where DWC3_EP_END_TRANSFER_PENDING must be set to prevent dwc3_ep_queue 
from starting a new transfer during a EP transfer timeout.

But, this is unnecessary for __dwc3_gadget_ep_disable. Since there's no 
way to clear the pending flag if the interrupt is missed and no 
dwc3_ep_queue calls occur until the EP is re-enabled, preserving 
DWC3_EP_END_TRANSFER_PENDING here provides no benefit.

So, the below changes is not necessary in ep disable,


@@ -1096,6 +1144,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);
+

Instead, keep our suggestion changes that prevent the manipulation of 
dep->flags due to the race condition between dwc3_gadget_ep_disable() 
and dwc3_gadget_ep_queue(), since this race observing in long run rndis 
test.

+
+       /*
+        * When dwc3_gadget_ep_disable() calls dwc3_gadget_giveback(),
+        * the dwc->lock is temporarily released. If dwc3_gadget_ep_queue()
+        * runs in that window it may set the DWC3_EP_TRANSFER_STARTED 
flag as
+        * part of dwc3_send_gadget_ep_cmd. The original code cleared 
the flag
+        * unconditionally in the mask operation, which could overwrite the
+        * concurrent modification.
+        *
+        * As a workaround for the interrupt context constraint where we 
cannot
+        * wait for endpoint flushing, preserve the DWC3_EP_TRANSFER_STARTED
+        * flag if it is set, avoiding resource conflicts until the 
framework
+        * is fixed to properly synchronize endpoint lifecycle management.
+        */
+       if (dep->flags & DWC3_EP_TRANSFER_STARTED)
+               mask |= DWC3_EP_TRANSFER_STARTED;
+


Thanks,
Selva
>> Try with the below.
>>
>> Thanks,
>> Thinh
>>
>> 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);