Skip to contentSkip to conversation

usb: dwc3: gadget: remove wait_end_transfer

PATCH New Felipe Balbi 1 participant 1 message Jan 18, 2019 USB

lore
Share & tools
lore
#0 lore raw

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

Hi,

Quoted 8 lines · Thinh Nguyen
(quoted diff) +0 -0
@@ -1409,15 +1407,11 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
                if (r == req) {
                        /* wait until it is processed */
                        dwc3_stop_active_transfer(dep, true);
 

I ran into a regression with this patch. DWC3 will cleanup cancelled requests on END_TRANSFER command completion. However, if for some reason, the driver is unable to send the command, it will never

by why wouldn't the driver send the command? That seems to be the error we should be looking at. Got some tracepoints available?

Quoted 34 lines

cleanup cancelled requests and give them back to the upper layer. The failure doesn't happen often, and I'm investigating the actual cause. If you have any idea, please let me know.

I had a workaround as below, let me know what you think.

drivers/usb/dwc3/gadget.c +4 -3
index bed2ff42780b..d61cf9180332 100644
@@ -698,7 +698,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, unsigned int action)
        return 0;
 }
 
-static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force);
+static int dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force);
 static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep)
 {
        struct dwc3_request             *req;
@@ -1547,10 +1547,11 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
                }
                if (r == req) {
                        /* wait until it is processed */
-                       dwc3_stop_active_transfer(dep, true);
+                       if (dwc3_stop_active_transfer(dep, true))
+                               goto out1;
 
                        if (!r->trb)
-                               goto out0;
+                               goto out1;
 
                        dwc3_gadget_move_cancelled_request(req);
                        goto out0;

drivers/usb/dwc3/gadget.c static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, · line 1566

Quoted 5 lines
drivers/usb/dwc3/gadget.c +1 -0
@@ -1561,6 +1562,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
                goto out0;
        }
 
+out1:
        dwc3_gadget_giveback(dep, req, -ECONNRESET);

This will create other regressions. For example, this could be recycled and requeued before END_TRANSFER completes, which means the core is still processing the TRB. Then we go ahead and change the TRB's HWO bit while the core is still using it.

What we need is a way to ensure that either END_TRANSFER happens, or prove that for cases where END_TRANSFER isn't issued, is because of suspend/resume or driver removal. In either case, we can safely giveback the TRBs since core will be reinitialized later on.

Still, if you have some tracepoints, I'd like to see why is it that core doesn't issue END_TRANSFER.