| Message ID | b81cd5b5281cfbfdadb002c4bcf5c9be7c017cfd.1609828485.git.Thinh.Nguyen@synopsys.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] usb: dwc3: gadget: Clear wait flag on dequeue | expand |
Hi Felipe, Felipe Balbi wrote: > Hi, > > Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@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). BR, Thinh
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; + goto out; } }
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(+) base-commit: 2edc7af892d0913bf06f5b35e49ec463f03d5ed8