Skip to contentSkip to conversation

usb: dwc3: gadget: remove wait_end_transfer

PATCH New Felipe Balbi 1 participant 1 message Jan 21, 2019

Subsystems (2)
USB DesignWare USB3 DRD IP driver
lore
Share & tools
lore

Felipe Balbi

author
#0 lore raw

PATCH 10/14 usb: dwc3: gadget: remove wait_end_transfer

Hi,

Quoted 44 lines · Felipe Balbi

Hi again,

Felipe Balbi <[email protected]> writes:

<snip>

Quoted 2 lines

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;
Quoted 5 lines

[...]
}

So this will wait forever. Here's a patch that takes into consideration this possibility:

a version that compiles now (#facepalm):

drivers/usb/dwc3/gadget.c +10 -15
index 17eb6619376a..f88184b6cb5f 100644
@@ -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;