[PATCH] usb: chipidea: udc: disconnect/reconnect from host when do suspend/resume

[PATCH] usb: chipidea: udc: disconnect/reconnect from host when do suspend/resume

[Xu Yang]

Shawn and John reported a hang issue during system suspend as below:

 - USB gadget is enabled as Ethernet
 - There is data transfer over USB Ethernet (scp a big file between host
                                             and device)
 - Device is going in/out suspend (echo mem > /sys/power/state)

The root cause is the USB device controller is suspended but the USB bus
is still active which caused the USB host continues to transfer data with
device and the device continues to queue USB requests (in this case, a
delayed TCP ACK packet trigger the issue) after controller is suspended,
however the USB controller clock is already gated off. Then if udc driver
access registers after that point, the system will hang.

The correct way to avoid such issue is to disconnect device from host when
the USB bus is not at suspend state. Then the host will receive disconnect
event and stop data transfer in time. To continue make USB gadget device
work after system resume, this will reconnect device automatically.

To make usb wakeup work if USB bus is already at suspend state, this will
keep connection for it only when USB device controller has enabled wakeup
capability.

Reported-by: Shawn Guo <shawnguo@kernel.org>
Reported-by: John Ernberg <john.ernberg@actia.se>
Closes: https://lore.kernel.org/linux-usb/aEZxmlHmjeWcXiF3@dragon/
Tested-by: John Ernberg <john.ernberg@actia.se> # iMX8QXP
Fixes: 235ffc17d014 ("usb: chipidea: udc: add suspend/resume support for device controller")
Cc: stable@vger.kernel.org
Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/chipidea/udc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 8a9b31fd5c89..1a48e6440e6c 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -2374,6 +2374,10 @@ static void udc_suspend(struct ci_hdrc *ci)
 	 */
 	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0)
 		hw_write(ci, OP_ENDPTLISTADDR, ~0, ~0);
+
+	if (ci->gadget.connected &&
+	    (!ci->suspended || !device_may_wakeup(ci->dev)))
+		usb_gadget_disconnect(&ci->gadget);
 }
 
 static void udc_resume(struct ci_hdrc *ci, bool power_lost)
@@ -2384,6 +2388,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
 					OTGSC_BSVIS | OTGSC_BSVIE);
 		if (ci->vbus_active)
 			usb_gadget_vbus_disconnect(&ci->gadget);
+	} else if (ci->vbus_active && ci->driver &&
+		   !ci->gadget.connected) {
+		usb_gadget_connect(&ci->gadget);
 	}
 
 	/* Restore value 0 if it was set for power lost check */
-- 
2.34.1

Re: [PATCH] usb: chipidea: udc: disconnect/reconnect from host when do suspend/resume

[Peter Chen (CIX)]

On 25-06-14 20:49:14, Xu Yang wrote:
> Shawn and John reported a hang issue during system suspend as below:
> 
>  - USB gadget is enabled as Ethernet
>  - There is data transfer over USB Ethernet (scp a big file between host
>                                              and device)
>  - Device is going in/out suspend (echo mem > /sys/power/state)
> 
> The root cause is the USB device controller is suspended but the USB bus
> is still active which caused the USB host continues to transfer data with
> device and the device continues to queue USB requests (in this case, a
> delayed TCP ACK packet trigger the issue) after controller is suspended,
> however the USB controller clock is already gated off. Then if udc driver
> access registers after that point, the system will hang.
> 
> The correct way to avoid such issue is to disconnect device from host when
> the USB bus is not at suspend state. Then the host will receive disconnect
> event and stop data transfer in time. To continue make USB gadget device
> work after system resume, this will reconnect device automatically.
> 
> To make usb wakeup work if USB bus is already at suspend state, this will
> keep connection for it only when USB device controller has enabled wakeup
> capability.
> 
> Reported-by: Shawn Guo <shawnguo@kernel.org>
> Reported-by: John Ernberg <john.ernberg@actia.se>
> Closes: https://lore.kernel.org/linux-usb/aEZxmlHmjeWcXiF3@dragon/
> Tested-by: John Ernberg <john.ernberg@actia.se> # iMX8QXP
> Fixes: 235ffc17d014 ("usb: chipidea: udc: add suspend/resume support for device controller")
> Cc: stable@vger.kernel.org
> Reviewed-by: Jun Li <jun.li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---
>  drivers/usb/chipidea/udc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 8a9b31fd5c89..1a48e6440e6c 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -2374,6 +2374,10 @@ static void udc_suspend(struct ci_hdrc *ci)
>  	 */
>  	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0)
>  		hw_write(ci, OP_ENDPTLISTADDR, ~0, ~0);
> +
> +	if (ci->gadget.connected &&
> +	    (!ci->suspended || !device_may_wakeup(ci->dev)))
> +		usb_gadget_disconnect(&ci->gadget);
>  }

Don't we need to notify gadget(class) side the disconnect event?

Peter
>  
>  static void udc_resume(struct ci_hdrc *ci, bool power_lost)
> @@ -2384,6 +2388,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
>  					OTGSC_BSVIS | OTGSC_BSVIE);
>  		if (ci->vbus_active)
>  			usb_gadget_vbus_disconnect(&ci->gadget);
> +	} else if (ci->vbus_active && ci->driver &&
> +		   !ci->gadget.connected) {
> +		usb_gadget_connect(&ci->gadget);
>  	}
>  
>  	/* Restore value 0 if it was set for power lost check */
> -- 
> 2.34.1
> 

-- 

Best regards,
Peter

Re: [PATCH] usb: chipidea: udc: disconnect/reconnect from host when do suspend/resume

[Xu Yang]

On Tue, Jun 17, 2025 at 04:22:20PM +0800, Peter Chen (CIX) wrote:
> On 25-06-14 20:49:14, Xu Yang wrote:
> > Shawn and John reported a hang issue during system suspend as below:
> > 
> >  - USB gadget is enabled as Ethernet
> >  - There is data transfer over USB Ethernet (scp a big file between host
> >                                              and device)
> >  - Device is going in/out suspend (echo mem > /sys/power/state)
> > 
> > The root cause is the USB device controller is suspended but the USB bus
> > is still active which caused the USB host continues to transfer data with
> > device and the device continues to queue USB requests (in this case, a
> > delayed TCP ACK packet trigger the issue) after controller is suspended,
> > however the USB controller clock is already gated off. Then if udc driver
> > access registers after that point, the system will hang.
> > 
> > The correct way to avoid such issue is to disconnect device from host when
> > the USB bus is not at suspend state. Then the host will receive disconnect
> > event and stop data transfer in time. To continue make USB gadget device
> > work after system resume, this will reconnect device automatically.
> > 
> > To make usb wakeup work if USB bus is already at suspend state, this will
> > keep connection for it only when USB device controller has enabled wakeup
> > capability.
> > 
> > Reported-by: Shawn Guo <shawnguo@kernel.org>
> > Reported-by: John Ernberg <john.ernberg@actia.se>
> > Closes: https://lore.kernel.org/linux-usb/aEZxmlHmjeWcXiF3@dragon/
> > Tested-by: John Ernberg <john.ernberg@actia.se> # iMX8QXP
> > Fixes: 235ffc17d014 ("usb: chipidea: udc: add suspend/resume support for device controller")
> > Cc: stable@vger.kernel.org
> > Reviewed-by: Jun Li <jun.li@nxp.com>
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
> >  drivers/usb/chipidea/udc.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index 8a9b31fd5c89..1a48e6440e6c 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -2374,6 +2374,10 @@ static void udc_suspend(struct ci_hdrc *ci)
> >  	 */
> >  	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0)
> >  		hw_write(ci, OP_ENDPTLISTADDR, ~0, ~0);
> > +
> > +	if (ci->gadget.connected &&
> > +	    (!ci->suspended || !device_may_wakeup(ci->dev)))
> > +		usb_gadget_disconnect(&ci->gadget);
> >  }
> 
> Don't we need to notify gadget(class) side the disconnect event?

Func usb_gadget_disconnect() will disconnect gadget at the end:

if (gadget->udc->driver)
	gadget->udc->driver->disconnect(gadget);

So gadget driver could be notified.

I thought you want me to call usb_gadget_set_state() after it is
disconnected, the user space processes is freezed before usb controller
system suspend, so it's unnecessary to change state because no user can
be notified during this period too.

Thanks,
Xu Yang

> 
> Peter
> >  
> >  static void udc_resume(struct ci_hdrc *ci, bool power_lost)
> > @@ -2384,6 +2388,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
> >  					OTGSC_BSVIS | OTGSC_BSVIE);
> >  		if (ci->vbus_active)
> >  			usb_gadget_vbus_disconnect(&ci->gadget);
> > +	} else if (ci->vbus_active && ci->driver &&
> > +		   !ci->gadget.connected) {
> > +		usb_gadget_connect(&ci->gadget);
> >  	}
> >  
> >  	/* Restore value 0 if it was set for power lost check */
> > -- 
> > 2.34.1
> > 
> 
> -- 
> 
> Best regards,
> Peter

Re: [PATCH] usb: chipidea: udc: disconnect/reconnect from host when do suspend/resume

[Peter Chen (CIX)]

On 25-06-14 20:49:14, Xu Yang wrote:
> Shawn and John reported a hang issue during system suspend as below:
> 
>  - USB gadget is enabled as Ethernet
>  - There is data transfer over USB Ethernet (scp a big file between host
>                                              and device)
>  - Device is going in/out suspend (echo mem > /sys/power/state)
> 
> The root cause is the USB device controller is suspended but the USB bus
> is still active which caused the USB host continues to transfer data with
> device and the device continues to queue USB requests (in this case, a
> delayed TCP ACK packet trigger the issue) after controller is suspended,
> however the USB controller clock is already gated off. Then if udc driver
> access registers after that point, the system will hang.
> 
> The correct way to avoid such issue is to disconnect device from host when
> the USB bus is not at suspend state. Then the host will receive disconnect
> event and stop data transfer in time. To continue make USB gadget device
> work after system resume, this will reconnect device automatically.
> 
> To make usb wakeup work if USB bus is already at suspend state, this will
> keep connection for it only when USB device controller has enabled wakeup
> capability.
> 
> Reported-by: Shawn Guo <shawnguo@kernel.org>
> Reported-by: John Ernberg <john.ernberg@actia.se>
> Closes: https://lore.kernel.org/linux-usb/aEZxmlHmjeWcXiF3@dragon/
> Tested-by: John Ernberg <john.ernberg@actia.se> # iMX8QXP
> Fixes: 235ffc17d014 ("usb: chipidea: udc: add suspend/resume support for device controller")
> Cc: stable@vger.kernel.org
> Reviewed-by: Jun Li <jun.li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
>  drivers/usb/chipidea/udc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 8a9b31fd5c89..1a48e6440e6c 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -2374,6 +2374,10 @@ static void udc_suspend(struct ci_hdrc *ci)
>  	 */
>  	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0)
>  		hw_write(ci, OP_ENDPTLISTADDR, ~0, ~0);
> +
> +	if (ci->gadget.connected &&
> +	    (!ci->suspended || !device_may_wakeup(ci->dev)))
> +		usb_gadget_disconnect(&ci->gadget);
>  }
>  
>  static void udc_resume(struct ci_hdrc *ci, bool power_lost)
> @@ -2384,6 +2388,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
>  					OTGSC_BSVIS | OTGSC_BSVIE);
>  		if (ci->vbus_active)
>  			usb_gadget_vbus_disconnect(&ci->gadget);
> +	} else if (ci->vbus_active && ci->driver &&
> +		   !ci->gadget.connected) {
> +		usb_gadget_connect(&ci->gadget);
>  	}
>  
>  	/* Restore value 0 if it was set for power lost check */
> -- 
> 2.34.1
> 

-- 

Best regards,
Peter