[10/14] usb: dwc3: gadget: remove wait_end_transfer

[Felipe Balbi <felipe.balbi@linux.intel.com>]

Hi,

Thinh Nguyen <thinh.nguyen@synopsys.com> writes:
> @@ -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?

> 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.
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index bed2ff42780b..d61cf9180332 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -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;
> @@ -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.