Re: [PATCH v2] usb: dwc3: gadget: Handle EP0 request dequeuing properly

[Thinh Nguyen]

On Wed, Dec 06, 2023, Wesley Cheng wrote:
> Current EP0 dequeue path will share the same as other EPs.  However, there
> are some special considerations that need to be made for EP0 transfers:
> 
>   - EP0 transfers never transition into the started_list
>   - EP0 only has one active request at a time
> 
> In case there is a vendor specific control message for a function over USB
> FFS, then there is no guarantee on the timeline which the DATA/STATUS stage
> is responded to.  While this occurs, any attempt to end transfers on
> non-control EPs will end up having the DWC3_EP_DELAY_STOP flag set, and
> defer issuing of the end transfer command.  If the USB FFS application
> decides to timeout the control transfer, or if USB FFS AIO path exits, the
> USB FFS driver will issue a call to usb_ep_dequeue() for the ep0 request.
> 
> In case of the AIO exit path, the AIO FS blocks until all pending USB
> requests utilizing the AIO path is completed.  However, since the dequeue
> of ep0 req does not happen properly, all non-control EPs with the
> DWC3_EP_DELAY_STOP flag set will not be handled, and the AIO exit path will
> be stuck waiting for the USB FFS data endpoints to receive a completion
> callback.
> 
> Fix is to utilize dwc3_ep0_reset_state() in the dequeue API to ensure EP0
> is brought back to the SETUP state, and ensures that any deferred end
> transfer commands are handled.  This also will end any active transfers
> on EP0, compared to the previous implementation which directly called
> giveback only.
> 
> Fixes: fcd2def66392 ("usb: dwc3: gadget: Refactor dwc3_gadget_ep_dequeue")
> Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
> ---
> Changes from v1:
>  - Added fixes tag
> 
>  drivers/usb/dwc3/gadget.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 858fe4c299b7..88d8d589f014 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2103,7 +2103,17 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>  
>  	list_for_each_entry(r, &dep->pending_list, list) {
>  		if (r == req) {
> -			dwc3_gadget_giveback(dep, req, -ECONNRESET);
> +			/*
> +			 * Explicitly check for EP0/1 as dequeue for those
> +			 * EPs need to be handled differently.  Control EP
> +			 * only deals with one USB req, and giveback will
> +			 * occur during dwc3_ep0_stall_and_restart().  EP0
> +			 * requests are never added to started_list.
> +			 */
> +			if (dep->number > 1)
> +				dwc3_gadget_giveback(dep, req, -ECONNRESET);
> +			else
> +				dwc3_ep0_reset_state(dwc);
>  			goto out;
>  		}
>  	}

Acked-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>

Thanks,
Thinh