Re: [PATCH] USB: DWC3: Fix giveback of queued request

[Felipe Balbi <balbi@xxxxxx>]

Hi,

On Tue, Jun 12, 2012 at 05:13:56PM +0530, Pratyush Anand wrote:
> In case of ep_disable and reset interrupt callback for all the queued request
> must be called, after stopping the endpoint.
> In case of ep_dequeue , callback of dequeued request is called.
> 
> Signed-off-by: Pratyush Anand <pratyush.anand@xxxxxx>

I think you should split this patch (see below)

> ---
>  drivers/usb/dwc3/gadget.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index cc5423f..3f555f0 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -579,8 +579,11 @@ static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep)
>  		 */
>  		udelay(100);
>  
> -		req = next_request(&dep->req_queued);
> -		dwc3_gadget_giveback(dep, req, -ESHUTDOWN);
> +		while (!list_empty(&dep->req_queued)) {
> +			req = next_request(&dep->req_queued);
> +
> +			dwc3_gadget_giveback(dep, req, -ESHUTDOWN);
> +		}
>  	}
>  
>  	while (!list_empty(&dep->request_list)) {

this is fine, we really need to do this in the driver, but it can wait
v3.6 merge window, I guess...

Another thing, I'm not sure we should call dwc3_remove_requests() from
dwc3_ep_disable(), but I'm still considering that. What do you think ? I
suppose we could/should assume gadget driver will dequeue all requests
before disabling endpoint...

> @@ -1204,7 +1207,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>  		if (r == req) {
>  			/* wait until it is processed */
>  			dwc3_stop_active_transfer(dwc, dep->number);
> -			goto out0;
> +			goto out1;
>  		}
>  		dev_err(dwc->dev, "request %p was not queued to %s\n",
>  				request, ep->name);
> @@ -1212,6 +1215,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>  		goto out0;
>  	}
>  
> +out1:
>  	/* giveback the request */
>  	dwc3_gadget_giveback(dep, req, -ECONNRESET);

these two hunks though, should go in ASAP and should be Cced to
stable@xxxxxxxxxxxxxxx and be backported to v3.2+. Looks like this bug
has been there forever.

-- 
balbi

Attachment: signature.asc
Description: Digital signature