From: huanglei <huanglei@kylinos.cn>

All USB devices are brought into suspend power state after system suspend.
It is desirable for some specific manufacturers buses to keep their devices
in normal state even after system suspend.

Signed-off-by: huanglei <huanglei@kylinos.cn>
---
v3->v4:
- Cancel SUSPENDED state judgment when enter suspend，because udev will not enter.
- Change "usb-skip-suspend" to "usb-never-suspend"
- Change dev_err to dev_info.
- Remove Kconfig option, it's redundant indeed.
- Update commit message style.
v2->v3:
- Rebase and update commit message.
v1->v2:
- Change to bool type for skip_suspend.
- Kconfig remove "default n", 'n' is the default.
---
 drivers/usb/core/driver.c    | 8 ++++++++
 drivers/usb/host/xhci-plat.c | 5 +++++
 include/linux/usb.h          | 7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 0c3f12daac79..93137c7c34df 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1583,6 +1583,11 @@ int usb_suspend(struct device *dev, pm_message_t msg)
 	struct usb_device	*udev = to_usb_device(dev);
 	int r;
 
+	if (udev->bus->skip_suspend && (msg.event == PM_EVENT_SUSPEND)) {
+		dev_info(dev, "abort suspend\n");
+		return 0;
+	}
+
 	unbind_no_pm_drivers_interfaces(udev);
 
 	/* From now on we are sure all drivers support suspend/resume
@@ -1619,6 +1624,9 @@ int usb_resume(struct device *dev, pm_message_t msg)
 	struct usb_device	*udev = to_usb_device(dev);
 	int			status;
 
+	if (udev->bus->skip_suspend && (msg.event == PM_EVENT_RESUME))
+		return 0;
+
 	/* For all calls, take the device back to full power and
 	 * tell the PM core in case it was autosuspended previously.
 	 * Unbind the interfaces that will need rebinding later,
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ecaa75718e59..35062aa19a32 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -265,6 +265,11 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
 		if (device_property_read_bool(tmpdev, "xhci-skip-phy-init-quirk"))
 			xhci->quirks |= XHCI_SKIP_PHY_INIT;
 
+		if (device_property_read_bool(tmpdev, "usb-never-suspend")) {
+			hcd_to_bus(hcd)->skip_suspend = true;
+			hcd_to_bus(xhci->shared_hcd)->skip_suspend = true;
+		}
+
 		device_property_read_u32(tmpdev, "imod-interval-ns",
 					 &xhci->imod_interval);
 	}
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 672d8fc2abdb..c854d1f622ec 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -487,6 +487,13 @@ struct usb_bus {
 	struct mon_bus *mon_bus;	/* non-null when associated */
 	int monitored;			/* non-zero when monitored */
 #endif
+
+	bool skip_suspend;		/* All USB devices are brought into suspend
+					 * power state after system suspend. It is
+					 * desirable for some specific manufacturers
+					 * buses to keep their devices in normal
+					 * state even after system suspend.
+					 */
 };
 
 struct usb_dev_state;
-- 
2.17.1

On Wed, Oct 23, 2024 at 11:44:57AM +0800, huanglei814 wrote:
> From: huanglei <huanglei@kylinos.cn>
> 
> All USB devices are brought into suspend power state after system suspend.
> It is desirable for some specific manufacturers buses to keep their devices
> in normal state even after system suspend.

Here you should explain _why_ this is desirable.  People will want to 
know, so that they can judge whether this is a good thing to do.

> Signed-off-by: huanglei <huanglei@kylinos.cn>
> ---
> v3->v4:
> - Cancel SUSPENDED state judgment when enter suspend，because udev will not enter.
> - Change "usb-skip-suspend" to "usb-never-suspend"
> - Change dev_err to dev_info.
> - Remove Kconfig option, it's redundant indeed.
> - Update commit message style.
> v2->v3:
> - Rebase and update commit message.
> v1->v2:
> - Change to bool type for skip_suspend.
> - Kconfig remove "default n", 'n' is the default.
> ---
>  drivers/usb/core/driver.c    | 8 ++++++++
>  drivers/usb/host/xhci-plat.c | 5 +++++
>  include/linux/usb.h          | 7 +++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index 0c3f12daac79..93137c7c34df 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -1583,6 +1583,11 @@ int usb_suspend(struct device *dev, pm_message_t msg)
>  	struct usb_device	*udev = to_usb_device(dev);
>  	int r;
>  
> +	if (udev->bus->skip_suspend && (msg.event == PM_EVENT_SUSPEND)) {
> +		dev_info(dev, "abort suspend\n");
> +		return 0;

Do you really need this message at all?  Maybe it should be dev_dbg, 
because it's useful only to developers, not to normal users.

Also, you're not really aborting the suspend; you're skipping it.

> +	}
> +
>  	unbind_no_pm_drivers_interfaces(udev);
>  
>  	/* From now on we are sure all drivers support suspend/resume
> @@ -1619,6 +1624,9 @@ int usb_resume(struct device *dev, pm_message_t msg)
>  	struct usb_device	*udev = to_usb_device(dev);
>  	int			status;
>  
> +	if (udev->bus->skip_suspend && (msg.event == PM_EVENT_RESUME))
> +		return 0;
> +
>  	/* For all calls, take the device back to full power and
>  	 * tell the PM core in case it was autosuspended previously.
>  	 * Unbind the interfaces that will need rebinding later,
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ecaa75718e59..35062aa19a32 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -265,6 +265,11 @@ int xhci_plat_probe(struct platform_device *pdev, struct device *sysdev, const s
>  		if (device_property_read_bool(tmpdev, "xhci-skip-phy-init-quirk"))
>  			xhci->quirks |= XHCI_SKIP_PHY_INIT;
>  
> +		if (device_property_read_bool(tmpdev, "usb-never-suspend")) {

Hmmm.  "usb-never-suspend" implies that the bus won't even be put into 
runtime suspend, but that's not what you mean.  What you really mean is 
more like "stay-powered-during-system-suspend", but that name is a bit 
long.  Maybe you can think of something better.

Also, this sounds more like the manufacturer's policy decision rather 
than a hardware limitation.  Is that correct?  If it is, I doubt that 
the setting really belongs in DT.

> +			hcd_to_bus(hcd)->skip_suspend = true;
> +			hcd_to_bus(xhci->shared_hcd)->skip_suspend = true;
> +		}
> +
>  		device_property_read_u32(tmpdev, "imod-interval-ns",
>  					 &xhci->imod_interval);
>  	}
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 672d8fc2abdb..c854d1f622ec 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -487,6 +487,13 @@ struct usb_bus {
>  	struct mon_bus *mon_bus;	/* non-null when associated */
>  	int monitored;			/* non-zero when monitored */
>  #endif
> +
> +	bool skip_suspend;		/* All USB devices are brought into suspend
> +					 * power state after system suspend. It is
> +					 * desirable for some specific manufacturers
> +					 * buses to keep their devices in normal
> +					 * state even after system suspend.
> +					 */

This is not a good comment.  It doesn't say what the skip_suspend flag 
means.  You don't need to explain the reason for the flag here; just 
explain what it does.

Alan Stern

>  };
>  
>  struct usb_dev_state;
> -- 
> 2.17.1
> 
> 

On 23/10/2024 05:44, huanglei814 wrote:
> From: huanglei <huanglei@kylinos.cn>
> 
> All USB devices are brought into suspend power state after system suspend.
> It is desirable for some specific manufacturers buses to keep their devices
> in normal state even after system suspend.
> 
> Signed-off-by: huanglei <huanglei@kylinos.cn>
> ---
> v3->v4:
> - Cancel SUSPENDED state judgment when enter suspend，because udev will not enter.
> - Change "usb-skip-suspend" to "usb-never-suspend"

Nothing improved in respect of bindings.

Still NAK.

Best regards,
Krzysztof