Hi, Felipe Balbi <[email protected]> writes: > Hi again, > > Felipe Balbi <[email protected]> writes: > > <snip> > >> Try to dequeue a request that was already completed. Odd. Why are we >> missing a call to giveback?? > > Got a little more information: > > file-storage-3982 [006] d... 131.010663: dwc3_ep_queue: ep1in: req > 00000000eccaa10f length 0/16384 zsI ==> -115 > file-storage-3982 [006] d... 131.010667: dwc3_prepare_trb: ep1in: trb > 000000002ab8a1f9 buf 00000000bde24000 size 16384 ctrl 00000811 > (Hlcs:sC:normal) > file-storage-3982 [006] d... 131.010674: dwc3_gadget_ep_cmd: ep1in: > cmd 'Update Transfer' [30007] params 00000000 00000000 00000000 --> status: > Successful > irq/16-dwc3-3983 [004] d... 131.010942: dwc3_event: event (00004086): > ep1in: Transfer In Progress [0] (sIm) > irq/16-dwc3-3983 [004] d... 131.010942: dwc3_complete_trb: ep1in: trb > 00000000426cd8cf buf 00000000bde20000 size 0 ctrl 00000810 (hlcs:sC:normal) > irq/16-dwc3-3983 [004] d... 131.010944: dwc3_gadget_giveback: ep1in: > req 00000000f7765e56 length 16384/16384 zsI ==> 0 > file-storage-3982 [006] d... 131.010994: dwc3_ep_queue: ep1in: req > 00000000f7765e56 length 0/16384 zsI ==> -115 > file-storage-3982 [006] d... 131.010998: dwc3_prepare_trb: ep1in: trb > 0000000065d9143d buf 00000000bde28000 size 16384 ctrl 00000811 > (Hlcs:sC:normal) > file-storage-3982 [006] d... 131.011005: dwc3_gadget_ep_cmd: ep1in: > cmd 'Update Transfer' [30007] params 00000000 00000000 00000000 --> status: > Successful > irq/16-dwc3-3983 [004] d... 131.065517: dwc3_event: event (00000001): > Disconnect: [U0] > file-storage-3982 [006] .... 131.065558: dwc3_ep_dequeue: ep1in: req > 00000000f7765e56 length 0/16384 zsI ==> -115 > file-storage-3982 [006] d... 131.065687: dwc3_gadget_ep_cmd: ep1in: > cmd 'End Transfer' [30d08] params 00000000 00000000 00000000 --> status: > Successful > irq/16-dwc3-3983 [004] d... 131.065729: dwc3_event: event (080301c6): > ep1in: Endpoint Command Complete > irq/16-dwc3-3983 [004] d... 131.065731: dwc3_gadget_giveback: ep1in: > req 00000000f7765e56 length 0/16384 zsI ==> -104 > file-storage-3982 [006] .... 131.065766: dwc3_ep_dequeue: ep1in: req > 00000000eccaa10f length 0/16384 zsI ==> -115 > irq/16-dwc3-3983 [004] d... 135.071714: dwc3_event: event (00000101): > Reset [U0] > irq/16-dwc3-3983 [004] d... 135.126440: dwc3_event: event (00000201): > Connection Done [U0] > > From this snippet above it seems like we got End Transfer completed > *before* we tried to dequeue the request. This is a race in the driver, > since it will wait for End Transfer complete forever :-p We can see that > ep_dequeue won't send a new End Transfer unless it's necessary: > > static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force) > { > struct dwc3 *dwc = dep->dwc; > struct dwc3_gadget_ep_cmd_params params; > u32 cmd; > int ret; > > if ((dep->flags & DWC3_EP_END_TRANSFER_PENDING) || > !dep->resource_index) > return; > [...] > } > > So this will wait forever. Here's a patch that takes into consideration > this possibility:
a version that compiles now (#facepalm):
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 17eb6619376a..f88184b6cb5f 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -384,19 +384,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned
cmd,
trace_dwc3_gadget_ep_cmd(dep, cmd, params, cmd_status);
- if (ret == 0) {
- switch (DWC3_DEPCMD_CMD(cmd)) {
- case DWC3_DEPCMD_STARTTRANSFER:
- dep->flags |= DWC3_EP_TRANSFER_STARTED;
- dwc3_gadget_ep_get_transfer_index(dep);
- break;
- case DWC3_DEPCMD_ENDTRANSFER:
- dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
- break;
- default:
- /* nothing */
- break;
- }
+ if (ret == 0 && DWC3_DEPCMD_CMD(cmd) == DWC3_DEPCMD_STARTTRANSFER) {
+ dep->flags |= DWC3_EP_TRANSFER_STARTED;
+ dwc3_gadget_ep_get_transfer_index(dep);
}
if (saved_config) {
@@ -1560,7 +1550,10 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
goto out0;
dwc3_gadget_move_cancelled_request(req);
- goto out0;
+ if (dep->flags & DWC3_EP_TRANSFER_STARTED)
+ goto out0;
+ else
+ goto out1;
}
dev_err(dwc->dev, "request %pK was not queued to %s\n",
request, ep->name);
@@ -1568,6 +1561,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
goto out0;
}
+out1:
dwc3_gadget_giveback(dep, req, -ECONNRESET);
out0:
@@ -2578,7 +2572,8 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
cmd = DEPEVT_PARAMETER_CMD(event->parameters);
if (cmd == DWC3_DEPCMD_ENDTRANSFER) {
- dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
+ dep->flags &= ~(DWC3_EP_END_TRANSFER_PENDING |
+ DWC3_EP_TRANSFER_STARTED);
dwc3_gadget_ep_cleanup_cancelled_requests(dep);
}
break;
--
balbi
signature.asc
Description: PGP signature
