[v1] usb: dwc3: Ignore late xferNotReady event to prevent halt timeout

[PATCH] usb: dwc3: Ignore late xferNotReady event to prevent halt timeout

Posted by Kuen-Han Tsai 2 months ago

During a device-initiated disconnect, the End Transfer command resets
the event filter, allowing a new xferNotReady event to be generated
before the controller is fully halted. Processing this late event
incorrectly triggers a Start Transfer, which prevents the controller
from halting and results in a DSTS.DEVCTLHLT bit polling timeout.

Ignore the late xferNotReady event if the controller is already in a
disconnected state.

Fixes: 8f608e8ab628 ("usb: dwc3: gadget: remove unnecessary 'dwc' parameter")
Cc: stable <stable@kernel.org>
Signed-off-by: Kuen-Han Tsai <khtsai@google.com>
---
Tracing:

# Stop active transfers by sending End Transfer commands
dwc3_gadget_ep_cmd: ep1out: cmd 'End Transfer' [20d08] params 00000000 00000000 00000000 --> status: Successful
dwc3_gadget_ep_cmd: ep1in: cmd 'End Transfer' [40d08] params 00000000 00000000 00000000 --> status: Successful
 ...
# Recieve an xferNotReady event on an ISOC IN endpoint
dwc3_event: event (35d010c6): ep1in: Transfer Not Ready [000035d0] (Not Active)
dwc3_gadget_ep_cmd: ep1in: cmd 'Start Transfer' [35d60406] params 00000000 ffffb620 00000000 --> status: Successful
dwc3_gadget_ep_cmd: ep2in: cmd 'End Transfer' [30d08] params 00000000 00000000 00000000 --> status: Timed Out
 ...
# Start polling DSTS.DEVCTRLHLT
dwc3_gadget_run_stop: start polling DWC3_DSTS_DEVCTRLHLT
 ...
# HALT timeout and print out the endpoint status for debugging
dwc3_gadget_run_stop: finish polling DWC3_DSTS_DEVCTRLHLT, is_on=0, reg=0
dwc3_gadget_ep_status: ep1out: mps 1024/2765 streams 16 burst 5 ring 64/56 flags E:swbp:>
dwc3_gadget_ep_status: ep1in: mps 1024/1024 streams 16 burst 2 ring 21/64 flags E:swBp:<
dwc3_gadget_ep_status: ep2out: mps 1024/2765 streams 16 burst 5 ring 56/48 flags e:swbp:>
---
 drivers/usb/dwc3/gadget.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 25db36c63951..ad89cbeea761 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3777,6 +3777,15 @@ static void dwc3_gadget_endpoint_transfer_complete(struct dwc3_ep *dep,
 static void dwc3_gadget_endpoint_transfer_not_ready(struct dwc3_ep *dep,
 		const struct dwc3_event_depevt *event)
 {
+	/*
+	 * During a device-initiated disconnect, a late xferNotReady event can
+	 * be generated after the End Transfer command resets the event filter,
+	 * but before the controller is halted. Ignore it to prevent a new
+	 * transfer from starting.
+	 */
+	if (dep->dwc->connected)
+		return;
+
 	dwc3_gadget_endpoint_frame_from_event(dep, event);

 	/*
--
2.50.1.565.gc32cd1483b-goog

Re: [PATCH] usb: dwc3: Ignore late xferNotReady event to prevent halt timeout

Posted by Thinh Nguyen 2 months ago

On Tue, Aug 05, 2025, Kuen-Han Tsai wrote:
> During a device-initiated disconnect, the End Transfer command resets
> the event filter, allowing a new xferNotReady event to be generated
> before the controller is fully halted. Processing this late event
> incorrectly triggers a Start Transfer, which prevents the controller
> from halting and results in a DSTS.DEVCTLHLT bit polling timeout.
> 
> Ignore the late xferNotReady event if the controller is already in a
> disconnected state.
> 
> Fixes: 8f608e8ab628 ("usb: dwc3: gadget: remove unnecessary 'dwc' parameter")

The Fixes should target since the beginning of the driver:
72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")

> Cc: stable <stable@kernel.org>
> Signed-off-by: Kuen-Han Tsai <khtsai@google.com>
> ---
> Tracing:
> 
> # Stop active transfers by sending End Transfer commands
> dwc3_gadget_ep_cmd: ep1out: cmd 'End Transfer' [20d08] params 00000000 00000000 00000000 --> status: Successful
> dwc3_gadget_ep_cmd: ep1in: cmd 'End Transfer' [40d08] params 00000000 00000000 00000000 --> status: Successful
>  ...
> # Recieve an xferNotReady event on an ISOC IN endpoint
> dwc3_event: event (35d010c6): ep1in: Transfer Not Ready [000035d0] (Not Active)
> dwc3_gadget_ep_cmd: ep1in: cmd 'Start Transfer' [35d60406] params 00000000 ffffb620 00000000 --> status: Successful
> dwc3_gadget_ep_cmd: ep2in: cmd 'End Transfer' [30d08] params 00000000 00000000 00000000 --> status: Timed Out
>  ...
> # Start polling DSTS.DEVCTRLHLT
> dwc3_gadget_run_stop: start polling DWC3_DSTS_DEVCTRLHLT
>  ...
> # HALT timeout and print out the endpoint status for debugging
> dwc3_gadget_run_stop: finish polling DWC3_DSTS_DEVCTRLHLT, is_on=0, reg=0
> dwc3_gadget_ep_status: ep1out: mps 1024/2765 streams 16 burst 5 ring 64/56 flags E:swbp:>
> dwc3_gadget_ep_status: ep1in: mps 1024/1024 streams 16 burst 2 ring 21/64 flags E:swBp:<
> dwc3_gadget_ep_status: ep2out: mps 1024/2765 streams 16 burst 5 ring 56/48 flags e:swbp:>
> ---
>  drivers/usb/dwc3/gadget.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 25db36c63951..ad89cbeea761 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3777,6 +3777,15 @@ static void dwc3_gadget_endpoint_transfer_complete(struct dwc3_ep *dep,
>  static void dwc3_gadget_endpoint_transfer_not_ready(struct dwc3_ep *dep,
>  		const struct dwc3_event_depevt *event)
>  {
> +	/*
> +	 * During a device-initiated disconnect, a late xferNotReady event can
> +	 * be generated after the End Transfer command resets the event filter,
> +	 * but before the controller is halted. Ignore it to prevent a new
> +	 * transfer from starting.
> +	 */
> +	if (dep->dwc->connected)

Did you test this? This is supposed to be if (!dep->dwc->connected)
right?

BR,
Thinh

> +		return;
> +
>  	dwc3_gadget_endpoint_frame_from_event(dep, event);
> 
>  	/*
> --
> 2.50.1.565.gc32cd1483b-goog
>

Re: [PATCH] usb: dwc3: Ignore late xferNotReady event to prevent halt timeout

Posted by Kuen-Han Tsai 1 month, 4 weeks ago

Hi Thinh,

Thank you for your review.

On Wed, Aug 6, 2025 at 7:28 AM Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>
> On Tue, Aug 05, 2025, Kuen-Han Tsai wrote:
> > During a device-initiated disconnect, the End Transfer command resets
> > the event filter, allowing a new xferNotReady event to be generated
> > before the controller is fully halted. Processing this late event
> > incorrectly triggers a Start Transfer, which prevents the controller
> > from halting and results in a DSTS.DEVCTLHLT bit polling timeout.
> >
> > Ignore the late xferNotReady event if the controller is already in a
> > disconnected state.
> >
> > Fixes: 8f608e8ab628 ("usb: dwc3: gadget: remove unnecessary 'dwc' parameter")
>
> The Fixes should target since the beginning of the driver:
> 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")

Commit 72246da40f37 doesn't have the dwc->connected member. Should we
change the Fixes tag to f09ddcfcb8c5 ("usb: dwc3: gadget: Prevent EP
queuing while stopping transfers")?
That's the commit where the "dwc->connected = false" logic was moved before
stopping active transfers within the pullup function.

>
> > Cc: stable <stable@kernel.org>
> > Signed-off-by: Kuen-Han Tsai <khtsai@google.com>
> > ---
> > Tracing:
> >
> > # Stop active transfers by sending End Transfer commands
> > dwc3_gadget_ep_cmd: ep1out: cmd 'End Transfer' [20d08] params 00000000 00000000 00000000 --> status: Successful
> > dwc3_gadget_ep_cmd: ep1in: cmd 'End Transfer' [40d08] params 00000000 00000000 00000000 --> status: Successful
> >  ...
> > # Recieve an xferNotReady event on an ISOC IN endpoint
> > dwc3_event: event (35d010c6): ep1in: Transfer Not Ready [000035d0] (Not Active)
> > dwc3_gadget_ep_cmd: ep1in: cmd 'Start Transfer' [35d60406] params 00000000 ffffb620 00000000 --> status: Successful
> > dwc3_gadget_ep_cmd: ep2in: cmd 'End Transfer' [30d08] params 00000000 00000000 00000000 --> status: Timed Out
> >  ...
> > # Start polling DSTS.DEVCTRLHLT
> > dwc3_gadget_run_stop: start polling DWC3_DSTS_DEVCTRLHLT
> >  ...
> > # HALT timeout and print out the endpoint status for debugging
> > dwc3_gadget_run_stop: finish polling DWC3_DSTS_DEVCTRLHLT, is_on=0, reg=0
> > dwc3_gadget_ep_status: ep1out: mps 1024/2765 streams 16 burst 5 ring 64/56 flags E:swbp:>
> > dwc3_gadget_ep_status: ep1in: mps 1024/1024 streams 16 burst 2 ring 21/64 flags E:swBp:<
> > dwc3_gadget_ep_status: ep2out: mps 1024/2765 streams 16 burst 5 ring 56/48 flags e:swbp:>
> > ---
> >  drivers/usb/dwc3/gadget.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 25db36c63951..ad89cbeea761 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -3777,6 +3777,15 @@ static void dwc3_gadget_endpoint_transfer_complete(struct dwc3_ep *dep,
> >  static void dwc3_gadget_endpoint_transfer_not_ready(struct dwc3_ep *dep,
> >               const struct dwc3_event_depevt *event)
> >  {
> > +     /*
> > +      * During a device-initiated disconnect, a late xferNotReady event can
> > +      * be generated after the End Transfer command resets the event filter,
> > +      * but before the controller is halted. Ignore it to prevent a new
> > +      * transfer from starting.
> > +      */
> > +     if (dep->dwc->connected)
>
> Did you test this? This is supposed to be if (!dep->dwc->connected)
> right?
>

You're absolutely right, I'll update your suggested change in the next
patch. Sorry for the mistake.

The code in my Android testing environment was correct and we've
verified that it resolves the halt problem.

Regards,
Kuen-Han




> > +             return;
> > +
> >       dwc3_gadget_endpoint_frame_from_event(dep, event);
> >
> >       /*
> > --
> > 2.50.1.565.gc32cd1483b-goog
> >