i.MX kernel hangup caused by chipidea USB gadget driver

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

A simple way to reproduce the issue could be:

1. Copy a big file (like 500MB) from host PC to device with scp

2. While the file copy is ongoing, suspend & resume the device like:

    $ echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

3. The device will hang up there

I reproduced on the following kernels:

- Mainline kernel
- NXP kernel lf-6.6.y
- NXP kernel lf-6.12.y

But NXP kernel lf-6.1.y doesn't have this problem. I tracked it down to Peter's commit [1] on lf-6.1.y, and found that the gadget disconnect & connect calls got lost from suspend & resume hooks, when the commit were split and pushed upstream. I confirm that adding the calls back fixes the hangup.

---8<--------------------

drivers/usb/chipidea/udc.c +3 -0

index 8a9b31fd5c89..72329a7eac4d 100644
@@ -2374,6 +2374,9 @@ 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->driver && ci->vbus_active && (ci->gadget.state != USB_STATE_SUSPENDED))
+               usb_gadget_disconnect(&ci->gadget);
 }
 
 static void udc_resume(struct ci_hdrc *ci, bool power_lost)

@@ -2384,6 +2387,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
                                        OTGSC_BSVIS | OTGSC_BSVIE);

But it's unclear to me why the hangup happens and how the change above fix the problem. Do you guys have any insight here?o

Shawn

[1] https://github.com/reMarkable/linux-imx/commit/0791d25578cb0e46fd93ae7a3c36ff7a424f3547

Hi Shawn,

Thanks for your reports!

Quoted 7 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

Quoted 19 lines · Xu Yang

Hi Shawn,

Thanks for your reports!

Quoted 7 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

Quoted 6 lines

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

The situation is more complicated than this.

In general, a USB gadget cannot allow itself to be suspended while the USB bus it is connected to remains active. Not unless it can be set to wake up when a USB packet arrives, and even that probably won't work because the wakeup sequence would take too long and the USB transfer would time out on the host.

The best way to fix this problem is for the gadget to disconnect itself from the USB bus whenever it goes into suspend, and to reconnect when it resumes.

Quoted 19 lines · Xu Yang

Hi Shawn,

Thanks for your reports!

Quoted 7 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

Quoted 6 lines

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

The situation is more complicated than this.

In general, a USB gadget cannot allow itself to be suspended while the USB bus it is connected to remains active. Not unless it can be set to wake up when a USB packet arrives, and even that probably won't work because the wakeup sequence would take too long and the USB transfer would time out on the host.

The best way to fix this problem is for the gadget to disconnect itself from the USB bus whenever it goes into suspend, and to reconnect when it resumes.

Alan Stern

Hi Shawn,

Thanks for your reports!

Quoted 29 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

A simple way to reproduce the issue could be:

1. Copy a big file (like 500MB) from host PC to device with scp

2. While the file copy is ongoing, suspend & resume the device like:

    $ echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

3. The device will hang up there

I reproduced on the following kernels:

- Mainline kernel
- NXP kernel lf-6.6.y
- NXP kernel lf-6.12.y

But NXP kernel lf-6.1.y doesn't have this problem. I tracked it down to Peter's commit [1] on lf-6.1.y, and found that the gadget disconnect & connect calls got lost from suspend & resume hooks, when the commit were split and pushed upstream. I confirm that adding the calls back fixes the hangup.

Quoted 19 lines

---8<--------------------

drivers/usb/chipidea/udc.c +3 -0

index 8a9b31fd5c89..72329a7eac4d 100644
@@ -2374,6 +2374,9 @@ 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->driver && ci->vbus_active && (ci->gadget.state != USB_STATE_SUSPENDED))
+               usb_gadget_disconnect(&ci->gadget);
 }
 
 static void udc_resume(struct ci_hdrc *ci, bool power_lost)

@@ -2384,6 +2387,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
                                        OTGSC_BSVIS | OTGSC_BSVIE);

drivers/usb/chipidea/udc.c static void udc_resume(struct ci_hdrc *ci, bool power_lost)

Quoted 10 lines

drivers/usb/chipidea/udc.c +3 -1

                if (ci->vbus_active)
                        usb_gadget_vbus_disconnect(&ci->gadget);
+       } else {
+               if (ci->driver && ci->vbus_active)
+                       usb_gadget_connect(&ci->gadget);
        }
 
        /* Restore value 0 if it was set for power lost check */
 
---->8------------------

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

---8<--------------------

drivers/usb/chipidea/udc.c +4 -0

index 8a9b31fd5c89..27a7674ed62c 100644
@@ -2367,6 +2367,8 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
 #ifdef CONFIG_PM_SLEEP
 static void udc_suspend(struct ci_hdrc *ci)
 {
+       ci->driver->suspend(&ci->gadget);
+
        /*
         * Set OP_ENDPTLISTADDR to be non-zero for
         * checking if controller resume from power lost

@@ -2389,6 +2391,8 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
        /* Restore value 0 if it was set for power lost check */
        if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0xFFFFFFFF)
                hw_write(ci, OP_ENDPTLISTADDR, ~0, 0);
+
+       ci->driver->resume(&ci->gadget);
 }
 #endif
 

---->8------------------

Thanks,
Xu Yang

Quoted 8 lines

But it's unclear to me why the hangup happens and how the change above fix the problem. Do you guys have any insight here?o

Shawn

[1] https://github.com/reMarkable/linux-imx/commit/0791d25578cb0e46fd93ae7a3c36ff7a424f3547

Hi Shawn, Xu,

Quoted 34 lines · Xu Yang

Hi Shawn,

Thanks for your reports!

Quoted 29 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

A simple way to reproduce the issue could be:

1. Copy a big file (like 500MB) from host PC to device with scp

2. While the file copy is ongoing, suspend & resume the device like:

    $ echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

3. The device will hang up there

I reproduced on the following kernels:

- Mainline kernel
- NXP kernel lf-6.6.y
- NXP kernel lf-6.12.y

But NXP kernel lf-6.1.y doesn't have this problem. I tracked it down to Peter's commit [1] on lf-6.1.y, and found that the gadget disconnect & connect calls got lost from suspend & resume hooks, when the commit were split and pushed upstream. I confirm that adding the calls back fixes the hangup.

We probably ran into the same problem trying to bring onboard 6.12, going from 6.1, on iMX8QXP. I managed to trace the hang to EP priming through a combination of debug tracing and BUG_ON experiments. See if it starts splatin with the below change.

----------------->8------------------

From 092599ab6f9e20412a7ca1eb118dd2be80cd18ff Mon Sep 17 00:00:00 2001
From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:01 +0200
Subject: [PATCH] USB: ci: gadget: Panic if priming when gadget off

---
 drivers/usb/chipidea/udc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/chipidea/udc.c +3 -2

index 2fea263a5e30..544aa4fa2d1d 100644
@@ -203,8 +203,10 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 
    hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
-   while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
+   while (hw_read(ci, OP_ENDPTPRIME, BIT(n))) {
        cpu_relax();
+       BUG_ON(dir == TX && !hw_read(ci, OP_ENDPTCTRL + num, ENDPTCTRL_TXE));
+   }
    if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
        return -EAGAIN;
 
----------------->8------------------

On the iMX8QXP you may additionally run into asychronous aborts and SError due to resource being disabled.

Quoted 67 lines

Quoted 19 lines

---8<--------------------

drivers/usb/chipidea/udc.c +3 -0

index 8a9b31fd5c89..72329a7eac4d 100644
@@ -2374,6 +2374,9 @@ 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->driver && ci->vbus_active && (ci->gadget.state != USB_STATE_SUSPENDED))
+               usb_gadget_disconnect(&ci->gadget);
 }
 
 static void udc_resume(struct ci_hdrc *ci, bool power_lost)

@@ -2384,6 +2387,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
                                        OTGSC_BSVIS | OTGSC_BSVIE);

drivers/usb/chipidea/udc.c static void udc_resume(struct ci_hdrc *ci, bool power_lost)

Quoted 10 lines

drivers/usb/chipidea/udc.c +3 -1

                if (ci->vbus_active)
                        usb_gadget_vbus_disconnect(&ci->gadget);
+       } else {
+               if (ci->driver && ci->vbus_active)
+                       usb_gadget_connect(&ci->gadget);
        }
 
        /* Restore value 0 if it was set for power lost check */
 
---->8------------------

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

---8<--------------------

drivers/usb/chipidea/udc.c +4 -0

index 8a9b31fd5c89..27a7674ed62c 100644
@@ -2367,6 +2367,8 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
 #ifdef CONFIG_PM_SLEEP
 static void udc_suspend(struct ci_hdrc *ci)
 {
+       ci->driver->suspend(&ci->gadget);
+
        /*
         * Set OP_ENDPTLISTADDR to be non-zero for
         * checking if controller resume from power lost

@@ -2389,6 +2391,8 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
        /* Restore value 0 if it was set for power lost check */
        if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0xFFFFFFFF)
                hw_write(ci, OP_ENDPTLISTADDR, ~0, 0);
+
+       ci->driver->resume(&ci->gadget);
 }
 #endif
 

Quoted 1 line

---->8------------------

I tested this during my debugging and it doesn't work because suspend/resume callbacks on the gadgets are designed for USB triggered suspend/resume and not system triggered suspend/resume. Meaning that the link will just be woken up again by the next USB transfer.

Quoted 12 lines

Thanks,
Xu Yang

Quoted 8 lines

But it's unclear to me why the hangup happens and how the change above fix the problem. Do you guys have any insight here?o

Shawn

[1] https://github.com/reMarkable/linux-imx/commit/0791d25578cb0e46fd93ae7a3c36ff7a424f3547

I didn't find the missing lines of code that Shawn found and instead ended up looking at why the UDC core isn't suspending the gadgets when the system is going to suspend. Because to me it feels like a job of UDC core.

I ended up with the monstrosity below that I have been intended to send as an RFC when I'm done thinking about it. It currently applies on 6.12.20.

But since Shawn also ran into the problem I'm including it for the sake of discussion about what the correct path of solving it is.

Best regards // John Ernberg

----------------->8------------------

From 3c1d167f1eff0bd010b797530e3d03f6939db322 Mon Sep 17 00:00:00 2001
From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:50 +0200
Subject: [PATCH] WIP: Suspend getherlink on system suspend

---
 drivers/usb/gadget/composite.c        | 68 +++++++++++++++++++++++++++
 drivers/usb/gadget/configfs.c         | 53 +++++++++++++++++++++
 drivers/usb/gadget/function/f_ecm.c   | 22 +++++++++
 drivers/usb/gadget/function/u_ether.c | 34 ++++++++++++++
 drivers/usb/gadget/function/u_ether.h |  2 +
 drivers/usb/gadget/udc/core.c         | 29 ++++++++++++
 include/linux/usb/composite.h         |  4 ++
 include/linux/usb/gadget.h            |  2 +
 8 files changed, 214 insertions(+)

drivers/usb/gadget/composite.c +68 -0

index 8402a86176f4..f1ed1db1e1d0 100644
@@ -2669,6 +2669,72 @@ void composite_resume(struct usb_gadget *gadget)
 	cdev->suspended = 0;
 }
 
+int composite_system_suspend(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
+	struct usb_function		*f;
+	int				ret;
+
+	DBG(cdev, "system suspend\n");
+	if (cdev->config) {
+		list_for_each_entry(f, &cdev->config->functions, list) {
+			if (f->system_suspend) {
+				ret = f->system_suspend(f);
+				if (ret)
+					return ret;
+			}
+		}
+	}
+
+	if (cdev->config &&
+	    cdev->config->bmAttributes & USB_CONFIG_ATT_SELFPOWER)
+		usb_gadget_set_selfpowered(gadget);
+
+	usb_gadget_vbus_draw(gadget, 2);
+
+	return 0;
+}
+
+int composite_system_resume(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
+	struct usb_function		*f;
+	unsigned			maxpower;
+	int				ret;
+
+	DBG(cdev, "system resume\n");
+	if (cdev->config) {
+		list_for_each_entry(f, &cdev->config->functions, list) {
+			if (f->system_resume) {
+				ret = f->system_resume(f);
+				if (ret)
+					return ret;
+			}
+		}
+
+		maxpower = cdev->config->MaxPower ?
+			cdev->config->MaxPower : CONFIG_USB_GADGET_VBUS_DRAW;
+		if (gadget->speed < USB_SPEED_SUPER)
+			maxpower = min(maxpower, 500U);
+		else
+			maxpower = min(maxpower, 900U);
+
+		if (maxpower > USB_SELF_POWER_VBUS_MAX_DRAW ||
+		    !(cdev->config->bmAttributes & USB_CONFIG_ATT_SELFPOWER))
+			usb_gadget_clear_selfpowered(gadget);
+		else
+			usb_gadget_set_selfpowered(gadget);
+
+		usb_gadget_vbus_draw(gadget, maxpower);
+	} else {
+		maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+		maxpower = min(maxpower, 100U);
+		usb_gadget_vbus_draw(gadget, maxpower);
+	}
+
+	return 0;
+}
+
 /*-------------------------------------------------------------------------*/
 
 static const struct usb_gadget_driver composite_driver_template = {

@@ -2681,6 +2747,8 @@ static const struct usb_gadget_driver composite_driver_template = {
 
 	.suspend	= composite_suspend,
 	.resume		= composite_resume,
+	.system_suspend	= composite_system_suspend,
+	.system_resume	= composite_system_resume,
 
 	.driver	= {
 		.owner		= THIS_MODULE,

drivers/usb/gadget/configfs.c +53 -0

index 29390d573e23..e0d2f0998e86 100644
@@ -1962,6 +1962,57 @@ static void configfs_composite_resume(struct usb_gadget *gadget)
 	spin_unlock_irqrestore(&gi->spinlock, flags);
 }
 
+static int configfs_composite_system_suspend(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev *cdev;
+	struct gadget_info *gi;
+	unsigned long flags;
+	int ret;
+
+	cdev = get_gadget_data(gadget);
+	if (!cdev)
+		return 0;
+
+	gi = container_of(cdev, struct gadget_info, cdev);
+	spin_lock_irqsave(&gi->spinlock, flags);
+	cdev = get_gadget_data(gadget);
+	if (!cdev || gi->unbind) {
+		spin_unlock_irqrestore(&gi->spinlock, flags);
+		return 0;
+	}
+
+	ret = composite_system_suspend(gadget);
+	spin_unlock_irqrestore(&gi->spinlock, flags);
+
+	return ret;
+}
+
+static int configfs_composite_system_resume(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev *cdev;
+	struct gadget_info *gi;
+	unsigned long flags;
+	int ret;
+
+	cdev = get_gadget_data(gadget);
+	if (!cdev)
+		return 0;
+
+	gi = container_of(cdev, struct gadget_info, cdev);
+	spin_lock_irqsave(&gi->spinlock, flags);
+	cdev = get_gadget_data(gadget);
+	if (!cdev || gi->unbind) {
+		spin_unlock_irqrestore(&gi->spinlock, flags);
+		return 0;
+	}
+
+	ret = composite_system_resume(gadget);
+	spin_unlock_irqrestore(&gi->spinlock, flags);
+
+	return ret;
+}
+
+
 static const struct usb_gadget_driver configfs_driver_template = {
 	.bind           = configfs_composite_bind,
 	.unbind         = configfs_composite_unbind,

@@ -1972,6 +2023,8 @@ static const struct usb_gadget_driver configfs_driver_template = {
 
 	.suspend	= configfs_composite_suspend,
 	.resume		= configfs_composite_resume,
+	.system_suspend	= configfs_composite_system_suspend,
+	.system_resume	= configfs_composite_system_resume,
 
 	.max_speed	= USB_SPEED_SUPER_PLUS,
 	.driver = {

drivers/usb/gadget/function/f_ecm.c +22 -0

index 6cb7771e8a69..4df67d5ee0fa 100644
@@ -892,6 +892,26 @@ static void ecm_resume(struct usb_function *f)
 	gether_resume(&ecm->port);
 }
 
+static int ecm_system_suspend(struct usb_function *f)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	DBG(cdev, "ECM System Suspend\n");
+
+	return gether_system_suspend(&ecm->port);
+}
+
+static int ecm_system_resume(struct usb_function *f)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	DBG(cdev, "ECM System Resume\n");
+
+	return gether_system_resume(&ecm->port);
+}
+
 static void ecm_free(struct usb_function *f)
 {
 	struct f_ecm *ecm;

@@ -961,6 +981,8 @@ static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
 	ecm->port.func.free_func = ecm_free;
 	ecm->port.func.suspend = ecm_suspend;
 	ecm->port.func.resume = ecm_resume;
+	ecm->port.func.system_suspend = ecm_system_suspend;
+	ecm->port.func.system_resume = ecm_system_resume;
 
 	return &ecm->port.func;
 }

drivers/usb/gadget/function/u_ether.c +34 -0

index f58590bf5e02..d4f0e28ffd4d 100644
@@ -1078,6 +1078,40 @@ void gether_resume(struct gether *link)
 }
 EXPORT_SYMBOL_GPL(gether_resume);
 
+int gether_system_suspend(struct gether *link)
+{
+	struct eth_dev *dev = link->ioport;
+	struct net_device *ndev = dev->net;
+
+	rtnl_lock();
+	if (netif_running(ndev)) {
+		netif_tx_lock_bh(ndev);
+		netif_device_detach(ndev);
+		netif_tx_unlock_bh(ndev);
+	}
+	rtnl_unlock();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gether_system_suspend);
+
+int gether_system_resume(struct gether *link)
+{
+	struct eth_dev *dev = link->ioport;
+	struct net_device *ndev = dev->net;
+
+	rtnl_lock();
+	if (netif_running(ndev)) {
+		netif_tx_lock_bh(ndev);
+		netif_device_attach(ndev);
+		netif_tx_unlock_bh(ndev);
+	}
+	rtnl_unlock();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gether_system_resume);
+
 /*
  * gether_cleanup - remove Ethernet-over-USB device
  * Context: may sleep

drivers/usb/gadget/function/u_ether.h +2 -0

index 34be220cef77..ffd023b7be7b 100644
@@ -261,6 +261,8 @@ void gether_cleanup(struct eth_dev *dev);
 
 void gether_suspend(struct gether *link);
 void gether_resume(struct gether *link);
+int gether_system_suspend(struct gether *link);
+int gether_system_resume(struct gether *link);
 
 /* connect/disconnect is handled by individual functions */
 struct net_device *gether_connect(struct gether *);

drivers/usb/gadget/udc/core.c +29 -0

index 4b3d5075621a..1e4ee5ffcfbf 100644
@@ -1683,6 +1683,30 @@ static void gadget_unbind_driver(struct device *dev)
 	kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
 }
 
+static int gadget_suspend_driver(struct device *dev)
+{
+	struct usb_gadget *gadget = dev_to_usb_gadget(dev);
+	struct usb_udc *udc = gadget->udc;
+	struct usb_gadget_driver *driver = udc->driver;
+
+	if (driver->system_suspend)
+		return driver->system_suspend(gadget);
+
+	return 0;
+}
+
+static int gadget_resume_driver(struct device *dev)
+{
+	struct usb_gadget *gadget = dev_to_usb_gadget(dev);
+	struct usb_udc *udc = gadget->udc;
+	struct usb_gadget_driver *driver = udc->driver;
+
+	if (driver->system_resume)
+		return driver->system_resume(gadget);
+
+	return 0;
+}
+
 /* ------------------------------------------------------------------------- */
 
 int usb_gadget_register_driver_owner(struct usb_gadget_driver *driver,

@@ -1896,11 +1920,16 @@ static const struct class udc_class = {
 	.dev_uevent	= usb_udc_uevent,
 };
 
+static const struct dev_pm_ops gadget_bus_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(gadget_suspend_driver, gadget_resume_driver)
+};
+
 static const struct bus_type gadget_bus_type = {
 	.name = "gadget",
 	.probe = gadget_bind_driver,
 	.remove = gadget_unbind_driver,
 	.match = gadget_match_driver,
+	.pm = &gadget_bus_pm_ops,
 };
 
 static int __init usb_udc_init(void)

include/linux/usb/composite.h +4 -0

index 6e38fb9d2117..f42ba1cfd181 100644
@@ -226,6 +226,8 @@ struct usb_function {
 					bool config0);
 	void			(*suspend)(struct usb_function *);
 	void			(*resume)(struct usb_function *);
+	int			(*system_suspend)(struct usb_function *);
+	int			(*system_resume)(struct usb_function *);
 
 	/* USB 3.0 additions */
 	int			(*get_status)(struct usb_function *);

@@ -522,6 +524,8 @@ extern int composite_setup(struct usb_gadget *gadget,
 		const struct usb_ctrlrequest *ctrl);
 extern void composite_suspend(struct usb_gadget *gadget);
 extern void composite_resume(struct usb_gadget *gadget);
+extern int composite_system_suspend(struct usb_gadget *gadget);
+extern int composite_system_resume(struct usb_gadget *gadget);
 
 /*
  * Some systems will need runtime overrides for the  product identifiers

include/linux/usb/gadget.h +2 -0

index df33333650a0..8cdfdece1561 100644
@@ -744,6 +744,8 @@ struct usb_gadget_driver {
 	void			(*disconnect)(struct usb_gadget *);
 	void			(*suspend)(struct usb_gadget *);
 	void			(*resume)(struct usb_gadget *);
+	int			(*system_suspend)(struct usb_gadget *);
+	int			(*system_resume)(struct usb_gadget *);
 	void			(*reset)(struct usb_gadget *);
 
 	/* FIXME support safe rmmod */

Like Alan and Xu's suggestion, there are probably two problems here:
- When the system enters the suspend, the USB bus may neither at suspend nor disconnect state if USB controller/phy's power is not off and VBUS is there. So, the host still considers the device is active, it could trigger transfer any time. If the transfer occurs during system resume, the USB controller triggers interrupt to CPU, and USB's interrupt handler is triggered. If the USB's hardware is still at low power mode (or clock is gated off), it may cause system hang (CPU gets error response from USB) after access register.

With Shawn's change, it pulls D+ down during the suspend, and the host is notified of disconnection, so the host will not trigger transfer until D+ is pulled up by calling usb_gadget_connect. The USB leaves low power mode (and turn clock on) before that, the access register will not cause system hang.

- The current chipidea driver doesn't notify gadget driver when it enters system suspend routine. In fact, during system suspend/resume, the controller driver may not respond middle layer's (network) request well due to it enters low power mode, so calling usb_gadget_driver-> disconnect (composite_disconnect) is needed during controller's suspend routine, it calls function->disable for USB function driver and ends/stop middle layer process.

Quoted 38 lines · John Ernberg

Hi Shawn, Xu,

Quoted 34 lines · Xu Yang

Hi Shawn,

Thanks for your reports!

Quoted 29 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

A simple way to reproduce the issue could be:

1. Copy a big file (like 500MB) from host PC to device with scp

2. While the file copy is ongoing, suspend & resume the device like:

    $ echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

3. The device will hang up there

I reproduced on the following kernels:

- Mainline kernel
- NXP kernel lf-6.6.y
- NXP kernel lf-6.12.y

But NXP kernel lf-6.1.y doesn't have this problem. I tracked it down to Peter's commit [1] on lf-6.1.y, and found that the gadget disconnect & connect calls got lost from suspend & resume hooks, when the commit were split and pushed upstream. I confirm that adding the calls back fixes the hangup.

Quoted 4 lines

We probably ran into the same problem trying to bring onboard 6.12, going from 6.1, on iMX8QXP. I managed to trace the hang to EP priming through a combination of debug tracing and BUG_ON experiments. See if it starts splatin with the below change.

Hi John and Shawn,

Like Alan and Xu's suggestion, there are probably two problems here:
- When the system enters the suspend, the USB bus may neither at suspend nor disconnect state if USB controller/phy's power is not off and VBUS is there. So, the host still considers the device is active, it could trigger transfer any time. If the transfer occurs during system resume, the USB controller triggers interrupt to CPU, and USB's interrupt handler is triggered. If the USB's hardware is still at low power mode (or clock is gated off), it may cause system hang (CPU gets error response from USB) after access register.

With Shawn's change, it pulls D+ down during the suspend, and the host is notified of disconnection, so the host will not trigger transfer until D+ is pulled up by calling usb_gadget_connect. The USB leaves low power mode (and turn clock on) before that, the access register will not cause system hang.

- The current chipidea driver doesn't notify gadget driver when it enters system suspend routine. In fact, during system suspend/resume, the controller driver may not respond middle layer's (network) request well due to it enters low power mode, so calling usb_gadget_driver-> disconnect (composite_disconnect) is needed during controller's suspend routine, it calls function->disable for USB function driver and ends/stop middle layer process.

Peter

Quoted 492 lines

----------------->8------------------

From 092599ab6f9e20412a7ca1eb118dd2be80cd18ff Mon Sep 17 00:00:00 2001
From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:01 +0200
Subject: [PATCH] USB: ci: gadget: Panic if priming when gadget off

---
 drivers/usb/chipidea/udc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/chipidea/udc.c +3 -2

index 2fea263a5e30..544aa4fa2d1d 100644
@@ -203,8 +203,10 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 
    hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
-   while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
+   while (hw_read(ci, OP_ENDPTPRIME, BIT(n))) {
        cpu_relax();
+       BUG_ON(dir == TX && !hw_read(ci, OP_ENDPTCTRL + num, ENDPTCTRL_TXE));
+   }
    if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
        return -EAGAIN;
 
----------------->8------------------

On the iMX8QXP you may additionally run into asychronous aborts and SError due to resource being disabled.

Quoted 67 lines

Quoted 19 lines

---8<--------------------

drivers/usb/chipidea/udc.c +3 -0

index 8a9b31fd5c89..72329a7eac4d 100644
@@ -2374,6 +2374,9 @@ 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->driver && ci->vbus_active && (ci->gadget.state != USB_STATE_SUSPENDED))
+               usb_gadget_disconnect(&ci->gadget);
 }
 
 static void udc_resume(struct ci_hdrc *ci, bool power_lost)

@@ -2384,6 +2387,9 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
                                        OTGSC_BSVIS | OTGSC_BSVIE);

drivers/usb/chipidea/udc.c static void udc_resume(struct ci_hdrc *ci, bool power_lost)

Quoted 10 lines

drivers/usb/chipidea/udc.c +3 -1

                if (ci->vbus_active)
                        usb_gadget_vbus_disconnect(&ci->gadget);
+       } else {
+               if (ci->driver && ci->vbus_active)
+                       usb_gadget_connect(&ci->gadget);
        }
 
        /* Restore value 0 if it was set for power lost check */
 
---->8------------------

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

---8<--------------------

drivers/usb/chipidea/udc.c +4 -0

index 8a9b31fd5c89..27a7674ed62c 100644
@@ -2367,6 +2367,8 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
 #ifdef CONFIG_PM_SLEEP
 static void udc_suspend(struct ci_hdrc *ci)
 {
+       ci->driver->suspend(&ci->gadget);
+
        /*
         * Set OP_ENDPTLISTADDR to be non-zero for
         * checking if controller resume from power lost

@@ -2389,6 +2391,8 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
        /* Restore value 0 if it was set for power lost check */
        if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0xFFFFFFFF)
                hw_write(ci, OP_ENDPTLISTADDR, ~0, 0);
+
+       ci->driver->resume(&ci->gadget);
 }
 #endif
 

Quoted 1 line

---->8------------------

I tested this during my debugging and it doesn't work because suspend/resume callbacks on the gadgets are designed for USB triggered suspend/resume and not system triggered suspend/resume. Meaning that the link will just be woken up again by the next USB transfer.

Quoted 12 lines

Thanks,
Xu Yang

Quoted 8 lines

But it's unclear to me why the hangup happens and how the change above fix the problem. Do you guys have any insight here?o

Shawn

[1] https://github.com/reMarkable/linux-imx/commit/0791d25578cb0e46fd93ae7a3c36ff7a424f3547

I didn't find the missing lines of code that Shawn found and instead ended up looking at why the UDC core isn't suspending the gadgets when the system is going to suspend. Because to me it feels like a job of UDC core.

I ended up with the monstrosity below that I have been intended to send as an RFC when I'm done thinking about it. It currently applies on 6.12.20.

But since Shawn also ran into the problem I'm including it for the sake of discussion about what the correct path of solving it is.

Best regards // John Ernberg

----------------->8------------------

From 3c1d167f1eff0bd010b797530e3d03f6939db322 Mon Sep 17 00:00:00 2001
From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:50 +0200
Subject: [PATCH] WIP: Suspend getherlink on system suspend

---
 drivers/usb/gadget/composite.c        | 68 +++++++++++++++++++++++++++
 drivers/usb/gadget/configfs.c         | 53 +++++++++++++++++++++
 drivers/usb/gadget/function/f_ecm.c   | 22 +++++++++
 drivers/usb/gadget/function/u_ether.c | 34 ++++++++++++++
 drivers/usb/gadget/function/u_ether.h |  2 +
 drivers/usb/gadget/udc/core.c         | 29 ++++++++++++
 include/linux/usb/composite.h         |  4 ++
 include/linux/usb/gadget.h            |  2 +
 8 files changed, 214 insertions(+)

drivers/usb/gadget/composite.c +68 -0

index 8402a86176f4..f1ed1db1e1d0 100644
@@ -2669,6 +2669,72 @@ void composite_resume(struct usb_gadget *gadget)
 	cdev->suspended = 0;
 }
 
+int composite_system_suspend(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
+	struct usb_function		*f;
+	int				ret;
+
+	DBG(cdev, "system suspend\n");
+	if (cdev->config) {
+		list_for_each_entry(f, &cdev->config->functions, list) {
+			if (f->system_suspend) {
+				ret = f->system_suspend(f);
+				if (ret)
+					return ret;
+			}
+		}
+	}
+
+	if (cdev->config &&
+	    cdev->config->bmAttributes & USB_CONFIG_ATT_SELFPOWER)
+		usb_gadget_set_selfpowered(gadget);
+
+	usb_gadget_vbus_draw(gadget, 2);
+
+	return 0;
+}
+
+int composite_system_resume(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
+	struct usb_function		*f;
+	unsigned			maxpower;
+	int				ret;
+
+	DBG(cdev, "system resume\n");
+	if (cdev->config) {
+		list_for_each_entry(f, &cdev->config->functions, list) {
+			if (f->system_resume) {
+				ret = f->system_resume(f);
+				if (ret)
+					return ret;
+			}
+		}
+
+		maxpower = cdev->config->MaxPower ?
+			cdev->config->MaxPower : CONFIG_USB_GADGET_VBUS_DRAW;
+		if (gadget->speed < USB_SPEED_SUPER)
+			maxpower = min(maxpower, 500U);
+		else
+			maxpower = min(maxpower, 900U);
+
+		if (maxpower > USB_SELF_POWER_VBUS_MAX_DRAW ||
+		    !(cdev->config->bmAttributes & USB_CONFIG_ATT_SELFPOWER))
+			usb_gadget_clear_selfpowered(gadget);
+		else
+			usb_gadget_set_selfpowered(gadget);
+
+		usb_gadget_vbus_draw(gadget, maxpower);
+	} else {
+		maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+		maxpower = min(maxpower, 100U);
+		usb_gadget_vbus_draw(gadget, maxpower);
+	}
+
+	return 0;
+}
+
 /*-------------------------------------------------------------------------*/
 
 static const struct usb_gadget_driver composite_driver_template = {

@@ -2681,6 +2747,8 @@ static const struct usb_gadget_driver composite_driver_template = {
 
 	.suspend	= composite_suspend,
 	.resume		= composite_resume,
+	.system_suspend	= composite_system_suspend,
+	.system_resume	= composite_system_resume,
 
 	.driver	= {
 		.owner		= THIS_MODULE,

drivers/usb/gadget/configfs.c +53 -0

index 29390d573e23..e0d2f0998e86 100644
@@ -1962,6 +1962,57 @@ static void configfs_composite_resume(struct usb_gadget *gadget)
 	spin_unlock_irqrestore(&gi->spinlock, flags);
 }
 
+static int configfs_composite_system_suspend(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev *cdev;
+	struct gadget_info *gi;
+	unsigned long flags;
+	int ret;
+
+	cdev = get_gadget_data(gadget);
+	if (!cdev)
+		return 0;
+
+	gi = container_of(cdev, struct gadget_info, cdev);
+	spin_lock_irqsave(&gi->spinlock, flags);
+	cdev = get_gadget_data(gadget);
+	if (!cdev || gi->unbind) {
+		spin_unlock_irqrestore(&gi->spinlock, flags);
+		return 0;
+	}
+
+	ret = composite_system_suspend(gadget);
+	spin_unlock_irqrestore(&gi->spinlock, flags);
+
+	return ret;
+}
+
+static int configfs_composite_system_resume(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev *cdev;
+	struct gadget_info *gi;
+	unsigned long flags;
+	int ret;
+
+	cdev = get_gadget_data(gadget);
+	if (!cdev)
+		return 0;
+
+	gi = container_of(cdev, struct gadget_info, cdev);
+	spin_lock_irqsave(&gi->spinlock, flags);
+	cdev = get_gadget_data(gadget);
+	if (!cdev || gi->unbind) {
+		spin_unlock_irqrestore(&gi->spinlock, flags);
+		return 0;
+	}
+
+	ret = composite_system_resume(gadget);
+	spin_unlock_irqrestore(&gi->spinlock, flags);
+
+	return ret;
+}
+
+
 static const struct usb_gadget_driver configfs_driver_template = {
 	.bind           = configfs_composite_bind,
 	.unbind         = configfs_composite_unbind,

@@ -1972,6 +2023,8 @@ static const struct usb_gadget_driver configfs_driver_template = {
 
 	.suspend	= configfs_composite_suspend,
 	.resume		= configfs_composite_resume,
+	.system_suspend	= configfs_composite_system_suspend,
+	.system_resume	= configfs_composite_system_resume,
 
 	.max_speed	= USB_SPEED_SUPER_PLUS,
 	.driver = {

drivers/usb/gadget/function/f_ecm.c +22 -0

index 6cb7771e8a69..4df67d5ee0fa 100644
@@ -892,6 +892,26 @@ static void ecm_resume(struct usb_function *f)
 	gether_resume(&ecm->port);
 }
 
+static int ecm_system_suspend(struct usb_function *f)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	DBG(cdev, "ECM System Suspend\n");
+
+	return gether_system_suspend(&ecm->port);
+}
+
+static int ecm_system_resume(struct usb_function *f)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	DBG(cdev, "ECM System Resume\n");
+
+	return gether_system_resume(&ecm->port);
+}
+
 static void ecm_free(struct usb_function *f)
 {
 	struct f_ecm *ecm;

@@ -961,6 +981,8 @@ static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
 	ecm->port.func.free_func = ecm_free;
 	ecm->port.func.suspend = ecm_suspend;
 	ecm->port.func.resume = ecm_resume;
+	ecm->port.func.system_suspend = ecm_system_suspend;
+	ecm->port.func.system_resume = ecm_system_resume;
 
 	return &ecm->port.func;
 }

drivers/usb/gadget/function/u_ether.c +34 -0

index f58590bf5e02..d4f0e28ffd4d 100644
@@ -1078,6 +1078,40 @@ void gether_resume(struct gether *link)
 }
 EXPORT_SYMBOL_GPL(gether_resume);
 
+int gether_system_suspend(struct gether *link)
+{
+	struct eth_dev *dev = link->ioport;
+	struct net_device *ndev = dev->net;
+
+	rtnl_lock();
+	if (netif_running(ndev)) {
+		netif_tx_lock_bh(ndev);
+		netif_device_detach(ndev);
+		netif_tx_unlock_bh(ndev);
+	}
+	rtnl_unlock();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gether_system_suspend);
+
+int gether_system_resume(struct gether *link)
+{
+	struct eth_dev *dev = link->ioport;
+	struct net_device *ndev = dev->net;
+
+	rtnl_lock();
+	if (netif_running(ndev)) {
+		netif_tx_lock_bh(ndev);
+		netif_device_attach(ndev);
+		netif_tx_unlock_bh(ndev);
+	}
+	rtnl_unlock();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gether_system_resume);
+
 /*
  * gether_cleanup - remove Ethernet-over-USB device
  * Context: may sleep

drivers/usb/gadget/function/u_ether.h +2 -0

index 34be220cef77..ffd023b7be7b 100644
@@ -261,6 +261,8 @@ void gether_cleanup(struct eth_dev *dev);
 
 void gether_suspend(struct gether *link);
 void gether_resume(struct gether *link);
+int gether_system_suspend(struct gether *link);
+int gether_system_resume(struct gether *link);
 
 /* connect/disconnect is handled by individual functions */
 struct net_device *gether_connect(struct gether *);

drivers/usb/gadget/udc/core.c +29 -0

index 4b3d5075621a..1e4ee5ffcfbf 100644
@@ -1683,6 +1683,30 @@ static void gadget_unbind_driver(struct device *dev)
 	kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
 }
 
+static int gadget_suspend_driver(struct device *dev)
+{
+	struct usb_gadget *gadget = dev_to_usb_gadget(dev);
+	struct usb_udc *udc = gadget->udc;
+	struct usb_gadget_driver *driver = udc->driver;
+
+	if (driver->system_suspend)
+		return driver->system_suspend(gadget);
+
+	return 0;
+}
+
+static int gadget_resume_driver(struct device *dev)
+{
+	struct usb_gadget *gadget = dev_to_usb_gadget(dev);
+	struct usb_udc *udc = gadget->udc;
+	struct usb_gadget_driver *driver = udc->driver;
+
+	if (driver->system_resume)
+		return driver->system_resume(gadget);
+
+	return 0;
+}
+
 /* ------------------------------------------------------------------------- */
 
 int usb_gadget_register_driver_owner(struct usb_gadget_driver *driver,

@@ -1896,11 +1920,16 @@ static const struct class udc_class = {
 	.dev_uevent	= usb_udc_uevent,
 };
 
+static const struct dev_pm_ops gadget_bus_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(gadget_suspend_driver, gadget_resume_driver)
+};
+
 static const struct bus_type gadget_bus_type = {
 	.name = "gadget",
 	.probe = gadget_bind_driver,
 	.remove = gadget_unbind_driver,
 	.match = gadget_match_driver,
+	.pm = &gadget_bus_pm_ops,
 };
 
 static int __init usb_udc_init(void)

include/linux/usb/composite.h +4 -0

index 6e38fb9d2117..f42ba1cfd181 100644
@@ -226,6 +226,8 @@ struct usb_function {
 					bool config0);
 	void			(*suspend)(struct usb_function *);
 	void			(*resume)(struct usb_function *);
+	int			(*system_suspend)(struct usb_function *);
+	int			(*system_resume)(struct usb_function *);
 
 	/* USB 3.0 additions */
 	int			(*get_status)(struct usb_function *);

@@ -522,6 +524,8 @@ extern int composite_setup(struct usb_gadget *gadget,
 		const struct usb_ctrlrequest *ctrl);
 extern void composite_suspend(struct usb_gadget *gadget);
 extern void composite_resume(struct usb_gadget *gadget);
+extern int composite_system_suspend(struct usb_gadget *gadget);
+extern int composite_system_resume(struct usb_gadget *gadget);
 
 /*
  * Some systems will need runtime overrides for the  product identifiers

include/linux/usb/gadget.h +2 -0

index df33333650a0..8cdfdece1561 100644
@@ -744,6 +744,8 @@ struct usb_gadget_driver {
 	void			(*disconnect)(struct usb_gadget *);
 	void			(*suspend)(struct usb_gadget *);
 	void			(*resume)(struct usb_gadget *);
+	int			(*system_suspend)(struct usb_gadget *);
+	int			(*system_resume)(struct usb_gadget *);
 	void			(*reset)(struct usb_gadget *);
 
 	/* FIXME support safe rmmod */

Signature · 2 lines

Best regards,
Peter

We probably ran into the same problem trying to bring onboard 6.12, going from 6.1, on iMX8QXP. I managed to trace the hang to EP priming through a combination of debug tracing and BUG_ON experiments. See if it starts splatin with the below change.

----------------->8------------------

From 092599ab6f9e20412a7ca1eb118dd2be80cd18ff Mon Sep 17 00:00:00 2001
From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:01 +0200
Subject: [PATCH] USB: ci: gadget: Panic if priming when gadget off

---
 drivers/usb/chipidea/udc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/chipidea/udc.c +0 -0

index 2fea263a5e30..544aa4fa2d1d 100644
@@ -203,8 +203,10 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 
    hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));

Hi John,

On Mon, Jun 09, 2025 at 02:17:30PM +0000, John Ernberg wrote:

<snip>

Quoted 32 lines

We probably ran into the same problem trying to bring onboard 6.12, going from 6.1, on iMX8QXP. I managed to trace the hang to EP priming through a combination of debug tracing and BUG_ON experiments. See if it starts splatin with the below change.

----------------->8------------------

From 092599ab6f9e20412a7ca1eb118dd2be80cd18ff Mon Sep 17 00:00:00 2001
From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:01 +0200
Subject: [PATCH] USB: ci: gadget: Panic if priming when gadget off

---
  drivers/usb/chipidea/udc.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/chipidea/udc.c +3 -1

index 2fea263a5e30..544aa4fa2d1d 100644
@@ -203,8 +203,10 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 
     hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
-   while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
+   while (hw_read(ci, OP_ENDPTPRIME, BIT(n))) {
         cpu_relax();
+       BUG_ON(dir == TX && !hw_read(ci, OP_ENDPTCTRL + num, ENDPTCTRL_TXE));
+   }
     if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
         return -EAGAIN;
 

Quoted 1 line

----------------->8------------------

Hmm, I just tested the change on i.MX8MM but didn't see the splatting. Maybe we are running into a slightly different problems?

Shawn

Hi Shawn, Xu,

Quoted 34 lines · Xu Yang

Hi Shawn,

Thanks for your reports!

Quoted 29 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

A simple way to reproduce the issue could be:

1. Copy a big file (like 500MB) from host PC to device with scp

2. While the file copy is ongoing, suspend & resume the device like:

    $ echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

3. The device will hang up there

I reproduced on the following kernels:

- Mainline kernel
- NXP kernel lf-6.6.y
- NXP kernel lf-6.12.y

But NXP kernel lf-6.1.y doesn't have this problem. I tracked it down to Peter's commit [1] on lf-6.1.y, and found that the gadget disconnect & connect calls got lost from suspend & resume hooks, when the commit were split and pushed upstream. I confirm that adding the calls back fixes the hangup.

We probably ran into the same problem trying to bring onboard 6.12, going from 6.1, on iMX8QXP. I managed to trace the hang to EP priming through a combination of debug tracing and BUG_ON experiments. See if it starts splatin with the below change.

----------------->8------------------

Quoted 1 line

From 092599ab6f9e20412a7ca1eb118dd2be80cd18ff Mon Sep 17 00:00:00 2001

From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:01 +0200
Subject: [PATCH] USB: ci: gadget: Panic if priming when gadget off

---
 drivers/usb/chipidea/udc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/chipidea/udc.c +3 -2

index 2fea263a5e30..544aa4fa2d1d 100644
@@ -203,8 +203,10 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 
    hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
-   while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
+   while (hw_read(ci, OP_ENDPTPRIME, BIT(n))) {
        cpu_relax();
+       BUG_ON(dir == TX && !hw_read(ci, OP_ENDPTCTRL + num, ENDPTCTRL_TXE));
+   }
    if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
        return -EAGAIN;
 
----------------->8------------------

On the iMX8QXP you may additionally run into asychronous aborts and SError due to resource being disabled.

Quoted 26 lines

Quoted 26 lines

---8<--------------------

drivers/usb/chipidea/udc.c +6 -0

index 8a9b31fd5c89..72329a7eac4d 100644
@@ -2374,6 +2374,9 @@ 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->driver && ci->vbus_active && (ci->gadget.state != USB_STATE_SUSPENDED))
+               usb_gadget_disconnect(&ci->gadget);
 }
 
 static void udc_resume(struct ci_hdrc *ci, bool power_lost)

@@ -2384,6 +2387,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->driver && ci->vbus_active)
+                       usb_gadget_connect(&ci->gadget);
        }
 

Quoted 3 lines

drivers/usb/chipidea/udc.c static void udc_resume(struct ci_hdrc *ci, bool power_lost)

Quoted 3 lines

drivers/usb/chipidea/udc.c +0 -1

        /* Restore value 0 if it was set for power lost check */
 
---->8------------------

Quoted 38 lines

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

---8<--------------------

drivers/usb/chipidea/udc.c +4 -0

index 8a9b31fd5c89..27a7674ed62c 100644
@@ -2367,6 +2367,8 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
 #ifdef CONFIG_PM_SLEEP
 static void udc_suspend(struct ci_hdrc *ci)
 {
+       ci->driver->suspend(&ci->gadget);
+
        /*
         * Set OP_ENDPTLISTADDR to be non-zero for
         * checking if controller resume from power lost

@@ -2389,6 +2391,8 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
        /* Restore value 0 if it was set for power lost check */
        if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0xFFFFFFFF)
                hw_write(ci, OP_ENDPTLISTADDR, ~0, 0);
+
+       ci->driver->resume(&ci->gadget);
 }
 #endif
 

Quoted 1 line

---->8------------------

I tested this during my debugging and it doesn't work because suspend/resume callbacks on the gadgets are designed for USB triggered suspend/resume and not system triggered suspend/resume. Meaning that the link will just be woken up again by the next USB transfer.

Quoted 13 lines

Thanks,
Xu Yang

Quoted 8 lines

But it's unclear to me why the hangup happens and how the change above fix the problem. Do you guys have any insight here?o

Shawn

[1] https://github.com/reMarkable/linux-imx/commit/0791d25578cb0e46fd93ae7a3c36ff7a424f3547

I didn't find the missing lines of code that Shawn found and instead ended up looking at why the UDC core isn't suspending the gadgets when the system is going to suspend. Because to me it feels like a job of UDC core.

I ended up with the monstrosity below that I have been intended to send as an RFC when I'm done thinking about it. It currently applies on 6.12.20.

But since Shawn also ran into the problem I'm including it for the sake of discussion about what the correct path of solving it is.

Best regards // John Ernberg

----------------->8------------------

Quoted 1 line

From 3c1d167f1eff0bd010b797530e3d03f6939db322 Mon Sep 17 00:00:00 2001

From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:50 +0200
Subject: [PATCH] WIP: Suspend getherlink on system suspend

---
 drivers/usb/gadget/composite.c        | 68 +++++++++++++++++++++++++++
 drivers/usb/gadget/configfs.c         | 53 +++++++++++++++++++++
 drivers/usb/gadget/function/f_ecm.c   | 22 +++++++++
 drivers/usb/gadget/function/u_ether.c | 34 ++++++++++++++
 drivers/usb/gadget/function/u_ether.h |  2 +
 drivers/usb/gadget/udc/core.c         | 29 ++++++++++++
 include/linux/usb/composite.h         |  4 ++
 include/linux/usb/gadget.h            |  2 +
 8 files changed, 214 insertions(+)

drivers/usb/gadget/composite.c +68 -0

index 8402a86176f4..f1ed1db1e1d0 100644
@@ -2669,6 +2669,72 @@ void composite_resume(struct usb_gadget *gadget)
 	cdev->suspended = 0;
 }
 
+int composite_system_suspend(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
+	struct usb_function		*f;
+	int				ret;
+
+	DBG(cdev, "system suspend\n");
+	if (cdev->config) {
+		list_for_each_entry(f, &cdev->config->functions, list) {
+			if (f->system_suspend) {
+				ret = f->system_suspend(f);
+				if (ret)
+					return ret;
+			}
+		}
+	}
+
+	if (cdev->config &&
+	    cdev->config->bmAttributes & USB_CONFIG_ATT_SELFPOWER)
+		usb_gadget_set_selfpowered(gadget);
+
+	usb_gadget_vbus_draw(gadget, 2);
+
+	return 0;
+}
+
+int composite_system_resume(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
+	struct usb_function		*f;
+	unsigned			maxpower;
+	int				ret;
+
+	DBG(cdev, "system resume\n");
+	if (cdev->config) {
+		list_for_each_entry(f, &cdev->config->functions, list) {
+			if (f->system_resume) {
+				ret = f->system_resume(f);
+				if (ret)
+					return ret;
+			}
+		}
+
+		maxpower = cdev->config->MaxPower ?
+			cdev->config->MaxPower : CONFIG_USB_GADGET_VBUS_DRAW;
+		if (gadget->speed < USB_SPEED_SUPER)
+			maxpower = min(maxpower, 500U);
+		else
+			maxpower = min(maxpower, 900U);
+
+		if (maxpower > USB_SELF_POWER_VBUS_MAX_DRAW ||
+		    !(cdev->config->bmAttributes & USB_CONFIG_ATT_SELFPOWER))
+			usb_gadget_clear_selfpowered(gadget);
+		else
+			usb_gadget_set_selfpowered(gadget);
+
+		usb_gadget_vbus_draw(gadget, maxpower);
+	} else {
+		maxpower = CONFIG_USB_GADGET_VBUS_DRAW;
+		maxpower = min(maxpower, 100U);
+		usb_gadget_vbus_draw(gadget, maxpower);
+	}
+
+	return 0;
+}
+
 /*-------------------------------------------------------------------------*/
 
 static const struct usb_gadget_driver composite_driver_template = {

@@ -2681,6 +2747,8 @@ static const struct usb_gadget_driver composite_driver_template = {
 
 	.suspend	= composite_suspend,
 	.resume		= composite_resume,
+	.system_suspend	= composite_system_suspend,
+	.system_resume	= composite_system_resume,
 
 	.driver	= {
 		.owner		= THIS_MODULE,

drivers/usb/gadget/configfs.c +53 -0

index 29390d573e23..e0d2f0998e86 100644
@@ -1962,6 +1962,57 @@ static void configfs_composite_resume(struct usb_gadget *gadget)
 	spin_unlock_irqrestore(&gi->spinlock, flags);
 }
 
+static int configfs_composite_system_suspend(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev *cdev;
+	struct gadget_info *gi;
+	unsigned long flags;
+	int ret;
+
+	cdev = get_gadget_data(gadget);
+	if (!cdev)
+		return 0;
+
+	gi = container_of(cdev, struct gadget_info, cdev);
+	spin_lock_irqsave(&gi->spinlock, flags);
+	cdev = get_gadget_data(gadget);
+	if (!cdev || gi->unbind) {
+		spin_unlock_irqrestore(&gi->spinlock, flags);
+		return 0;
+	}
+
+	ret = composite_system_suspend(gadget);
+	spin_unlock_irqrestore(&gi->spinlock, flags);
+
+	return ret;
+}
+
+static int configfs_composite_system_resume(struct usb_gadget *gadget)
+{
+	struct usb_composite_dev *cdev;
+	struct gadget_info *gi;
+	unsigned long flags;
+	int ret;
+
+	cdev = get_gadget_data(gadget);
+	if (!cdev)
+		return 0;
+
+	gi = container_of(cdev, struct gadget_info, cdev);
+	spin_lock_irqsave(&gi->spinlock, flags);
+	cdev = get_gadget_data(gadget);
+	if (!cdev || gi->unbind) {
+		spin_unlock_irqrestore(&gi->spinlock, flags);
+		return 0;
+	}
+
+	ret = composite_system_resume(gadget);
+	spin_unlock_irqrestore(&gi->spinlock, flags);
+
+	return ret;
+}
+
+
 static const struct usb_gadget_driver configfs_driver_template = {
 	.bind           = configfs_composite_bind,
 	.unbind         = configfs_composite_unbind,

@@ -1972,6 +2023,8 @@ static const struct usb_gadget_driver configfs_driver_template = {
 
 	.suspend	= configfs_composite_suspend,
 	.resume		= configfs_composite_resume,
+	.system_suspend	= configfs_composite_system_suspend,
+	.system_resume	= configfs_composite_system_resume,
 
 	.max_speed	= USB_SPEED_SUPER_PLUS,
 	.driver = {

drivers/usb/gadget/function/f_ecm.c +22 -0

index 6cb7771e8a69..4df67d5ee0fa 100644
@@ -892,6 +892,26 @@ static void ecm_resume(struct usb_function *f)
 	gether_resume(&ecm->port);
 }
 
+static int ecm_system_suspend(struct usb_function *f)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	DBG(cdev, "ECM System Suspend\n");
+
+	return gether_system_suspend(&ecm->port);
+}
+
+static int ecm_system_resume(struct usb_function *f)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	DBG(cdev, "ECM System Resume\n");
+
+	return gether_system_resume(&ecm->port);
+}
+
 static void ecm_free(struct usb_function *f)
 {
 	struct f_ecm *ecm;

@@ -961,6 +981,8 @@ static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
 	ecm->port.func.free_func = ecm_free;
 	ecm->port.func.suspend = ecm_suspend;
 	ecm->port.func.resume = ecm_resume;
+	ecm->port.func.system_suspend = ecm_system_suspend;
+	ecm->port.func.system_resume = ecm_system_resume;
 
 	return &ecm->port.func;
 }

drivers/usb/gadget/function/u_ether.c +34 -0

index f58590bf5e02..d4f0e28ffd4d 100644
@@ -1078,6 +1078,40 @@ void gether_resume(struct gether *link)
 }
 EXPORT_SYMBOL_GPL(gether_resume);
 
+int gether_system_suspend(struct gether *link)
+{
+	struct eth_dev *dev = link->ioport;
+	struct net_device *ndev = dev->net;
+
+	rtnl_lock();
+	if (netif_running(ndev)) {
+		netif_tx_lock_bh(ndev);
+		netif_device_detach(ndev);
+		netif_tx_unlock_bh(ndev);
+	}
+	rtnl_unlock();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gether_system_suspend);
+
+int gether_system_resume(struct gether *link)
+{
+	struct eth_dev *dev = link->ioport;
+	struct net_device *ndev = dev->net;
+
+	rtnl_lock();
+	if (netif_running(ndev)) {
+		netif_tx_lock_bh(ndev);
+		netif_device_attach(ndev);
+		netif_tx_unlock_bh(ndev);
+	}
+	rtnl_unlock();
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(gether_system_resume);
+
 /*
  * gether_cleanup - remove Ethernet-over-USB device
  * Context: may sleep

drivers/usb/gadget/function/u_ether.h +2 -0

index 34be220cef77..ffd023b7be7b 100644
@@ -261,6 +261,8 @@ void gether_cleanup(struct eth_dev *dev);
 
 void gether_suspend(struct gether *link);
 void gether_resume(struct gether *link);
+int gether_system_suspend(struct gether *link);
+int gether_system_resume(struct gether *link);
 
 /* connect/disconnect is handled by individual functions */
 struct net_device *gether_connect(struct gether *);

drivers/usb/gadget/udc/core.c +29 -0

index 4b3d5075621a..1e4ee5ffcfbf 100644
@@ -1683,6 +1683,30 @@ static void gadget_unbind_driver(struct device *dev)
 	kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
 }
 
+static int gadget_suspend_driver(struct device *dev)
+{
+	struct usb_gadget *gadget = dev_to_usb_gadget(dev);
+	struct usb_udc *udc = gadget->udc;
+	struct usb_gadget_driver *driver = udc->driver;
+
+	if (driver->system_suspend)
+		return driver->system_suspend(gadget);
+
+	return 0;
+}
+
+static int gadget_resume_driver(struct device *dev)
+{
+	struct usb_gadget *gadget = dev_to_usb_gadget(dev);
+	struct usb_udc *udc = gadget->udc;
+	struct usb_gadget_driver *driver = udc->driver;
+
+	if (driver->system_resume)
+		return driver->system_resume(gadget);
+
+	return 0;
+}
+
 /* ------------------------------------------------------------------------- */
 
 int usb_gadget_register_driver_owner(struct usb_gadget_driver *driver,

@@ -1896,11 +1920,16 @@ static const struct class udc_class = {
 	.dev_uevent	= usb_udc_uevent,
 };
 
+static const struct dev_pm_ops gadget_bus_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(gadget_suspend_driver, gadget_resume_driver)
+};
+
 static const struct bus_type gadget_bus_type = {
 	.name = "gadget",
 	.probe = gadget_bind_driver,
 	.remove = gadget_unbind_driver,
 	.match = gadget_match_driver,
+	.pm = &gadget_bus_pm_ops,
 };
 
 static int __init usb_udc_init(void)

include/linux/usb/composite.h +4 -0

index 6e38fb9d2117..f42ba1cfd181 100644
@@ -226,6 +226,8 @@ struct usb_function {
 					bool config0);
 	void			(*suspend)(struct usb_function *);
 	void			(*resume)(struct usb_function *);
+	int			(*system_suspend)(struct usb_function *);
+	int			(*system_resume)(struct usb_function *);
 
 	/* USB 3.0 additions */
 	int			(*get_status)(struct usb_function *);

@@ -522,6 +524,8 @@ extern int composite_setup(struct usb_gadget *gadget,
 		const struct usb_ctrlrequest *ctrl);
 extern void composite_suspend(struct usb_gadget *gadget);
 extern void composite_resume(struct usb_gadget *gadget);
+extern int composite_system_suspend(struct usb_gadget *gadget);
+extern int composite_system_resume(struct usb_gadget *gadget);
 
 /*
  * Some systems will need runtime overrides for the  product identifiers

include/linux/usb/gadget.h +2 -0

index df33333650a0..8cdfdece1561 100644
@@ -744,6 +744,8 @@ struct usb_gadget_driver {
 	void			(*disconnect)(struct usb_gadget *);
 	void			(*suspend)(struct usb_gadget *);
 	void			(*resume)(struct usb_gadget *);
+	int			(*system_suspend)(struct usb_gadget *);
+	int			(*system_resume)(struct usb_gadget *);
 	void			(*reset)(struct usb_gadget *);
 
 	/* FIXME support safe rmmod */

Hi John,

Quoted 101 lines · John Ernberg

Hi Shawn, Xu,

Quoted 34 lines · Xu Yang

Hi Shawn,

Thanks for your reports!

Quoted 29 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

A simple way to reproduce the issue could be:

1. Copy a big file (like 500MB) from host PC to device with scp

2. While the file copy is ongoing, suspend & resume the device like:

     $ echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

3. The device will hang up there

I reproduced on the following kernels:

- Mainline kernel
- NXP kernel lf-6.6.y
- NXP kernel lf-6.12.y

But NXP kernel lf-6.1.y doesn't have this problem. I tracked it down to Peter's commit [1] on lf-6.1.y, and found that the gadget disconnect & connect calls got lost from suspend & resume hooks, when the commit were split and pushed upstream. I confirm that adding the calls back fixes the hangup.

We probably ran into the same problem trying to bring onboard 6.12, going from 6.1, on iMX8QXP. I managed to trace the hang to EP priming through a combination of debug tracing and BUG_ON experiments. See if it starts splatin with the below change.

----------------->8------------------

Quoted 1 line

From 092599ab6f9e20412a7ca1eb118dd2be80cd18ff Mon Sep 17 00:00:00 2001

From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:01 +0200
Subject: [PATCH] USB: ci: gadget: Panic if priming when gadget off

---
  drivers/usb/chipidea/udc.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/chipidea/udc.c +3 -2

index 2fea263a5e30..544aa4fa2d1d 100644
@@ -203,8 +203,10 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 
     hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
-   while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
+   while (hw_read(ci, OP_ENDPTPRIME, BIT(n))) {
         cpu_relax();
+       BUG_ON(dir == TX && !hw_read(ci, OP_ENDPTCTRL + num, ENDPTCTRL_TXE));
+   }
     if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
         return -EAGAIN;
 
----------------->8------------------

On the iMX8QXP you may additionally run into asychronous aborts and SError due to resource being disabled.

Quoted 26 lines

Quoted 26 lines

---8<--------------------

drivers/usb/chipidea/udc.c +6 -0

index 8a9b31fd5c89..72329a7eac4d 100644
@@ -2374,6 +2374,9 @@ 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->driver && ci->vbus_active && (ci->gadget.state != USB_STATE_SUSPENDED))
+               usb_gadget_disconnect(&ci->gadget);
  }
 
  static void udc_resume(struct ci_hdrc *ci, bool power_lost)

@@ -2384,6 +2387,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->driver && ci->vbus_active)
+                       usb_gadget_connect(&ci->gadget);
         }
 

Quoted 3 lines

Quoted 3 lines

drivers/usb/chipidea/udc.c static void udc_resume(struct ci_hdrc *ci, bool power_lost)

Quoted 3 lines

drivers/usb/chipidea/udc.c +0 -1

         /* Restore value 0 if it was set for power lost check */
 
---->8------------------

Does above change work for you?

Hi John,

Quoted 101 lines · John Ernberg

Hi Shawn, Xu,

Quoted 34 lines · Xu Yang

Hi Shawn,

Thanks for your reports!

Quoted 29 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

A simple way to reproduce the issue could be:

1. Copy a big file (like 500MB) from host PC to device with scp

2. While the file copy is ongoing, suspend & resume the device like:

     $ echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

3. The device will hang up there

I reproduced on the following kernels:

- Mainline kernel
- NXP kernel lf-6.6.y
- NXP kernel lf-6.12.y

But NXP kernel lf-6.1.y doesn't have this problem. I tracked it down to Peter's commit [1] on lf-6.1.y, and found that the gadget disconnect & connect calls got lost from suspend & resume hooks, when the commit were split and pushed upstream. I confirm that adding the calls back fixes the hangup.

We probably ran into the same problem trying to bring onboard 6.12, going from 6.1, on iMX8QXP. I managed to trace the hang to EP priming through a combination of debug tracing and BUG_ON experiments. See if it starts splatin with the below change.

----------------->8------------------

Quoted 1 line

From 092599ab6f9e20412a7ca1eb118dd2be80cd18ff Mon Sep 17 00:00:00 2001

From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:01 +0200
Subject: [PATCH] USB: ci: gadget: Panic if priming when gadget off

---
  drivers/usb/chipidea/udc.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/chipidea/udc.c +3 -2

index 2fea263a5e30..544aa4fa2d1d 100644
@@ -203,8 +203,10 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 
     hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
-   while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
+   while (hw_read(ci, OP_ENDPTPRIME, BIT(n))) {
         cpu_relax();
+       BUG_ON(dir == TX && !hw_read(ci, OP_ENDPTCTRL + num, ENDPTCTRL_TXE));
+   }
     if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
         return -EAGAIN;
 
----------------->8------------------

On the iMX8QXP you may additionally run into asychronous aborts and SError due to resource being disabled.

Quoted 26 lines

Quoted 26 lines

---8<--------------------

drivers/usb/chipidea/udc.c +6 -0

index 8a9b31fd5c89..72329a7eac4d 100644
@@ -2374,6 +2374,9 @@ 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->driver && ci->vbus_active && (ci->gadget.state != USB_STATE_SUSPENDED))
+               usb_gadget_disconnect(&ci->gadget);
  }
 
  static void udc_resume(struct ci_hdrc *ci, bool power_lost)

@@ -2384,6 +2387,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->driver && ci->vbus_active)
+                       usb_gadget_connect(&ci->gadget);
         }
 

Quoted 3 lines

Quoted 3 lines

drivers/usb/chipidea/udc.c static void udc_resume(struct ci_hdrc *ci, bool power_lost)

Quoted 3 lines

drivers/usb/chipidea/udc.c +0 -1

         /* Restore value 0 if it was set for power lost check */
 
---->8------------------

Does above change work for you?

Hi Xu, Shawn,

Quoted 104 lines · Xu Yang

Hi John,

Quoted 101 lines · John Ernberg

Hi Shawn, Xu,

Quoted 34 lines · Xu Yang

Hi Shawn,

Thanks for your reports!

Quoted 29 lines · Shawn Guo

Hi Xu, Peter,

I'm seeing a kernel hangup on imx8mm-evk board. It happens when:

- USB gadget is enabled as Ethernet
- There is data transfer over USB Ethernet
- Device is going in/out suspend

A simple way to reproduce the issue could be:

1. Copy a big file (like 500MB) from host PC to device with scp

2. While the file copy is ongoing, suspend & resume the device like:

     $ echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

3. The device will hang up there

I reproduced on the following kernels:

- Mainline kernel
- NXP kernel lf-6.6.y
- NXP kernel lf-6.12.y

But NXP kernel lf-6.1.y doesn't have this problem. I tracked it down to Peter's commit [1] on lf-6.1.y, and found that the gadget disconnect & connect calls got lost from suspend & resume hooks, when the commit were split and pushed upstream. I confirm that adding the calls back fixes the hangup.

We probably ran into the same problem trying to bring onboard 6.12, going from 6.1, on iMX8QXP. I managed to trace the hang to EP priming through a combination of debug tracing and BUG_ON experiments. See if it starts splatin with the below change.

----------------->8------------------

Quoted 1 line

From 092599ab6f9e20412a7ca1eb118dd2be80cd18ff Mon Sep 17 00:00:00 2001

From: John Ernberg <[email protected]>
Date: Mon, 5 May 2025 09:09:01 +0200
Subject: [PATCH] USB: ci: gadget: Panic if priming when gadget off

---
  drivers/usb/chipidea/udc.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

drivers/usb/chipidea/udc.c +3 -2

index 2fea263a5e30..544aa4fa2d1d 100644
@@ -203,8 +203,10 @@ static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 
     hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 
-   while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
+   while (hw_read(ci, OP_ENDPTPRIME, BIT(n))) {
         cpu_relax();
+       BUG_ON(dir == TX && !hw_read(ci, OP_ENDPTCTRL + num, ENDPTCTRL_TXE));
+   }
     if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
         return -EAGAIN;
 
----------------->8------------------

On the iMX8QXP you may additionally run into asychronous aborts and SError due to resource being disabled.

Quoted 26 lines

Quoted 26 lines

---8<--------------------

drivers/usb/chipidea/udc.c +6 -0

index 8a9b31fd5c89..72329a7eac4d 100644
@@ -2374,6 +2374,9 @@ 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->driver && ci->vbus_active && (ci->gadget.state != USB_STATE_SUSPENDED))
+               usb_gadget_disconnect(&ci->gadget);
  }
 
  static void udc_resume(struct ci_hdrc *ci, bool power_lost)

@@ -2384,6 +2387,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->driver && ci->vbus_active)
+                       usb_gadget_connect(&ci->gadget);
         }
 

Quoted 6 lines

Quoted 3 lines

Quoted 3 lines

drivers/usb/chipidea/udc.c static void udc_resume(struct ci_hdrc *ci, bool power_lost)

Quoted 3 lines

drivers/usb/chipidea/udc.c +0 -1

         /* Restore value 0 if it was set for power lost check */
 
---->8------------------

Does above change work for you?

I have ran suspend/resume tests for about 12 hours now with this change and not had any trouble on iMX8QXP, where it was not possible to run such tests for so long before.

Please pick up if you submit this formally:

Tested-by John Ernberg <[email protected]> # iMX8QXP

Thanks! // John Ernberg

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

---8<--------------------

drivers/usb/chipidea/udc.c +4 -0

index 8a9b31fd5c89..27a7674ed62c 100644
@@ -2367,6 +2367,8 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
 #ifdef CONFIG_PM_SLEEP
 static void udc_suspend(struct ci_hdrc *ci)
 {
+       ci->driver->suspend(&ci->gadget);
+
        /*
         * Set OP_ENDPTLISTADDR to be non-zero for
         * checking if controller resume from power lost

@@ -2389,6 +2391,8 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
        /* Restore value 0 if it was set for power lost check */
        if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0xFFFFFFFF)
                hw_write(ci, OP_ENDPTLISTADDR, ~0, 0);
+
+       ci->driver->resume(&ci->gadget);
 }
 #endif
 

---->8------------------

On Mon, Jun 09, 2025 at 07:53:22PM +0800, Xu Yang wrote:

<snip>

Quoted 6 lines

During the scp process, the usb host won't put usb device to suspend state. In current design, then the ether driver doesn't know the system has suspended after echo mem. The root cause is that ether driver is still tring to queue usb request after usb controller has suspended where usb clock is off, then the system hang.

Quoted 6 lines

With the above changes, I think the ether driver will fail to eth_start_xmit() at an ealier stage, so the issue can't be triggered.

I think the ether driver needs call gether_suspend() accordingly, to do this, the controller driver need explicitly call suspend() function when it's going to be suspended. Could you check whether below patch fix the issue?

Thanks for the patch, Xu! It does fix the hangup but seems to be less reliable than my/Peter's change (disconnecting gadget), per my testing on a custom i.MX8MM board. With your change, host/PC doesn't disconnect gadget when the board suspends. After a few suspend cycles, Ethernet gadget stops working and the following workqueue lockup is seen. There seems to some be other bugs?

[  223.047990] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[  223.054097] rcu:     1-...0: (7 ticks this GP) idle=bb7c/1/0x4000000000000000 softirq=5368/5370 fqs=2431
[  223.063318] rcu:     (detected by 0, t=5252 jiffies, g=4705, q=2400 ncpus=4)
[  223.070105] Task dump for CPU 1:
[  223.073330] task:systemd-network state:R  running task     stack:0     pid:406   ppid:1      flags:0x00000202
[  223.083248] Call trace:
[  223.085692]  __switch_to+0xc0/0x124
[  246.747996] BUG: workqueue lockup - pool cpus=1 node=0 flags=0x0 nice=0 stuck for 43s!

However, your change seems working fine on i.MX8MM EVK. It's probably due to the fact that host disconnects gadget for some reason when EVK suspends. This is a different behavior from the custom board above. We do not really expect this disconnecting, do we?

Shawn

Quoted 25 lines

---8<--------------------

drivers/usb/chipidea/udc.c +4 -0

index 8a9b31fd5c89..27a7674ed62c 100644
@@ -2367,6 +2367,8 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
 #ifdef CONFIG_PM_SLEEP
 static void udc_suspend(struct ci_hdrc *ci)
 {
+       ci->driver->suspend(&ci->gadget);
+
        /*
         * Set OP_ENDPTLISTADDR to be non-zero for
         * checking if controller resume from power lost

@@ -2389,6 +2391,8 @@ static void udc_resume(struct ci_hdrc *ci, bool power_lost)
        /* Restore value 0 if it was set for power lost check */
        if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0xFFFFFFFF)
                hw_write(ci, OP_ENDPTLISTADDR, ~0, 0);
+
+       ci->driver->resume(&ci->gadget);
 }
 #endif
 

---->8------------------

Quoted 4 lines

However, your change seems working fine on i.MX8MM EVK. It's probably due to the fact that host disconnects gadget for some reason when EVK suspends. This is a different behavior from the custom board above. We do not really expect this disconnecting, do we?

It's an expected behavior on i.MX8MM EVK. The host will see device disconnected after imx8mm do system suspend. It's because the usb phy power will be off after system suspend. If you enable wakeup capabiklity for usb phy, the usb phy power will keep on and you will not see disconnect event anymore.

# echo enabled > /sys/devices/platform/usbphynop1/power/wakeup