Hi,
Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>> If an active transfer is dequeued, then the endpoint is freed to start a
>>> new transfer. Make sure to clear the endpoint's transfer wait flag for
>>> this case.
>>>
>>> Cc: stable@xxxxxxxxxxxxxxx
>>> Fixes: e0d19563eb6c ("usb: dwc3: gadget: Wait for transfer completion")
>>> Signed-off-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
>>> ---
>>> Changes in v2:
>>> - Only clear the wait flag if the selected request is of an active transfer.
>>> Otherwise, any dequeue will change the endpoint's state even if it's for
>>> some random request.
>>>
>>> drivers/usb/dwc3/gadget.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 78cb4db8a6e4..9a00dcaca010 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -1763,6 +1763,8 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>>> list_for_each_entry_safe(r, t, &dep->started_list, list)
>>> dwc3_gadget_move_cancelled_request(r);
>>>
>>> + dep->flags &= ~DWC3_EP_WAIT_TRANSFER_COMPLETE;
>> I'm not sure this is correct. This could create a race condition between
>> clearing this bit and getting the transfer complete interrupt. It also
>> seems to break the assumptions made by
>> dwc3_gadget_endpoint_trbs_complete() (actually its users), specially
>> regarding ISOC endpoints.
>>
>> Have you verified all transfer types with this commit?
>>
>
> It shouldn't race. It's protected by the spinlock irq and it doesn't
> matter whether dwc3_gadget_endpoint_trbs_complete() or this dequeue
> function clears it first. The flag DWC3_EP_WAIT_TRANSFER_COMPLETE is
> only applicable to stream transfer as the driver needs to wait for 1
> stream to finish before starting another.
>
> This is verified with our test environment (which includes UASP CV and
> others).
Fair enough, I'll take your word for it :-)
Acked-by: Felipe Balbi <balbi@xxxxxxxxxx>
--
balbi
