Re: [PATCH v2] usb: dwc3: gadget: Clear wait flag on dequeue

Hi,

Thinh Nguyen [off-list ref] writes:

quoted

Thinh Nguyen [off-list ref] writes:

quoted

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@vger.kernel.org
Fixes: e0d19563eb6c ("usb: dwc3: gadget: Wait for transfer completion")
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
---
 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@kernel.org>

-- 
balbi