diff mbox series

usb: dwc3: gadget: Do not clear ep delayed stop flag during ep disable

Message ID 20220919231213.21364-1-quic_wcheng@quicinc.com (mailing list archive)
State Accepted
Commit 76bff31c7fba6cc21bf8f9785572484d54d31878
Headers show
Series usb: dwc3: gadget: Do not clear ep delayed stop flag during ep disable | expand

Commit Message

Wesley Cheng Sept. 19, 2022, 11:12 p.m. UTC
DWC3_EP_DELAYED_STOP is utilized to defer issuing the end transfer command
until the subsequent SETUP stage, in order to avoid end transfer timeouts.
During cable disconnect scenarios, __dwc3_gadget_ep_disable() is
responsible for ensuring endpoints have no active transfers pending.  Since
dwc3_remove_request() can now exit early if the EP delayed stop is set,
avoid clearing all DEP flags, otherwise the transition back into the SETUP
stage won't issue an endxfer command.

Fixes: 2b2da6574e77 ("usb: dwc3: Avoid unmapping USB requests if endxfer is not complete")
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
 drivers/usb/dwc3/gadget.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Thinh Nguyen Sept. 21, 2022, 2:18 a.m. UTC | #1
On Mon, Sep 19, 2022, Wesley Cheng wrote:
> DWC3_EP_DELAYED_STOP is utilized to defer issuing the end transfer command
> until the subsequent SETUP stage, in order to avoid end transfer timeouts.
> During cable disconnect scenarios, __dwc3_gadget_ep_disable() is
> responsible for ensuring endpoints have no active transfers pending.  Since
> dwc3_remove_request() can now exit early if the EP delayed stop is set,
> avoid clearing all DEP flags, otherwise the transition back into the SETUP
> stage won't issue an endxfer command.
> 
> Fixes: 2b2da6574e77 ("usb: dwc3: Avoid unmapping USB requests if endxfer is not complete")
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> ---
>  drivers/usb/dwc3/gadget.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index b75e1b8b3f05..3e2baf22824b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1011,6 +1011,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
>  {
>  	struct dwc3		*dwc = dep->dwc;
>  	u32			reg;
> +	u32			mask;
>  
>  	trace_dwc3_gadget_ep_disable(dep);
>  
> @@ -1032,7 +1033,15 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
>  
>  	dep->stream_capable = false;
>  	dep->type = 0;
> -	dep->flags &= DWC3_EP_TXFIFO_RESIZED;
> +	mask = DWC3_EP_TXFIFO_RESIZED;
> +	/*
> +	 * dwc3_remove_requests() can exit early if DWC3 EP delayed stop is
> +	 * set.  Do not clear DEP flags, so that the end transfer command will
> +	 * be reattempted during the next SETUP stage.
> +	 */
> +	if (dep->flags & DWC3_EP_DELAY_STOP)
> +		mask |= (DWC3_EP_DELAY_STOP | DWC3_EP_TRANSFER_STARTED);
> +	dep->flags &= mask;
>  
>  	return 0;
>  }

The conditions are starting to get complicated... It would be nice if we
can clear all the flags after the transfer had ended. If the gadget
driver misbehaves and decides to queue a new request after it had
disabled the endpoint but before the end transfer command completes,
then DWC3_EP_DELAY_START may get set. Then things can go wrong?

Regardless, I think this should be fine.

Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index b75e1b8b3f05..3e2baf22824b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1011,6 +1011,7 @@  static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
 {
 	struct dwc3		*dwc = dep->dwc;
 	u32			reg;
+	u32			mask;
 
 	trace_dwc3_gadget_ep_disable(dep);
 
@@ -1032,7 +1033,15 @@  static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
 
 	dep->stream_capable = false;
 	dep->type = 0;
-	dep->flags &= DWC3_EP_TXFIFO_RESIZED;
+	mask = DWC3_EP_TXFIFO_RESIZED;
+	/*
+	 * dwc3_remove_requests() can exit early if DWC3 EP delayed stop is
+	 * set.  Do not clear DEP flags, so that the end transfer command will
+	 * be reattempted during the next SETUP stage.
+	 */
+	if (dep->flags & DWC3_EP_DELAY_STOP)
+		mask |= (DWC3_EP_DELAY_STOP | DWC3_EP_TRANSFER_STARTED);
+	dep->flags &= mask;
 
 	return 0;
 }