1. Patchew
  2. linux
  3. View series

[PATCH] usb: core: adds support for PM control of specific USB dev skip suspend.

huanglei814 posted 1 patch 1 month ago
There is a newer version of this series
drivers/usb/core/Kconfig     | 12 ++++++++++++
drivers/usb/core/driver.c    | 14 ++++++++++++++
drivers/usb/host/xhci-plat.c |  7 +++++++
include/linux/usb.h          |  9 +++++++++
4 files changed, 42 insertions(+)
[PATCH] usb: core: adds support for PM control of specific USB dev skip suspend.
Posted by huanglei814 1 month ago 
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>
---
 drivers/usb/core/Kconfig     | 12 ++++++++++++
 drivers/usb/core/driver.c    | 14 ++++++++++++++
 drivers/usb/host/xhci-plat.c |  7 +++++++
 include/linux/usb.h          |  9 +++++++++
 4 files changed, 42 insertions(+)

diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index 58e3ca7e4793..fe178c90d167 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -143,3 +143,15 @@ config USB_DEFAULT_AUTHORIZATION_MODE
 	  ACPI selecting value 2 is analogous to selecting value 0.
 
 	  If unsure, keep the default value.
+
+config USB_SKIP_SUSPEND
+	bool "Vendor USB support skip suspend"
+	depends on USB
+	default n
+	help
+	  Select this the associate USB devices will skip suspend when pm control.
+
+	  This option adds support skip suspend for PM control of USB devices
+	  in specific manufacturers platforms.
+
+	  If unsure, keep the default value.
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 0c3f12daac79..05fe921f8297 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1583,6 +1583,15 @@ int usb_suspend(struct device *dev, pm_message_t msg)
 	struct usb_device	*udev = to_usb_device(dev);
 	int r;
 
+#ifdef CONFIG_USB_SKIP_SUSPEND
+	if (udev->bus->skip_suspend && (msg.event == PM_EVENT_SUSPEND)) {
+		if (udev->state != USB_STATE_SUSPENDED)
+			dev_err(dev, "abort suspend\n");
+
+		return 0;
+	}
+#endif
+
 	unbind_no_pm_drivers_interfaces(udev);
 
 	/* From now on we are sure all drivers support suspend/resume
@@ -1619,6 +1628,11 @@ int usb_resume(struct device *dev, pm_message_t msg)
 	struct usb_device	*udev = to_usb_device(dev);
 	int			status;
 
+#ifdef CONFIG_USB_SKIP_SUSPEND
+	if (udev->bus->skip_suspend && (msg.event == PM_EVENT_RESUME))
+		return 0;
+#endif
+
 	/* 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..8cbc666ab5c6 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -265,6 +265,13 @@ 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;
 
+#ifdef CONFIG_USB_SKIP_SUSPEND
+		if (device_property_read_bool(tmpdev, "usb-skip-suspend")) {
+			hcd_to_bus(hcd)->skip_suspend = true;
+			hcd_to_bus(xhci->shared_hcd)->skip_suspend = true;
+		}
+#endif
+
 		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..5f88850fc42d 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -487,6 +487,15 @@ struct usb_bus {
 	struct mon_bus *mon_bus;	/* non-null when associated */
 	int monitored;			/* non-zero when monitored */
 #endif
+
+#ifdef CONFIG_USB_SKIP_SUSPEND
+	unsigned int 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.
+					 */
+#endif
 };
 
 struct usb_dev_state;
-- 
2.17.1
Re: [PATCH] usb: core: adds support for PM control of specific USB dev skip suspend.
Posted by Greg KH 1 month ago 
On Mon, Oct 21, 2024 at 08:47:41PM +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.

Why is it desireable to do that?  Why can't you just do so from
userspace today?

What hardware requires this?

> Signed-off-by: huanglei <huanglei@kylinos.cn>
> ---
>  drivers/usb/core/Kconfig     | 12 ++++++++++++
>  drivers/usb/core/driver.c    | 14 ++++++++++++++
>  drivers/usb/host/xhci-plat.c |  7 +++++++
>  include/linux/usb.h          |  9 +++++++++
>  4 files changed, 42 insertions(+)
> 
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index 58e3ca7e4793..fe178c90d167 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -143,3 +143,15 @@ config USB_DEFAULT_AUTHORIZATION_MODE
>  	  ACPI selecting value 2 is analogous to selecting value 0.
>  
>  	  If unsure, keep the default value.
> +
> +config USB_SKIP_SUSPEND
> +	bool "Vendor USB support skip suspend"
> +	depends on USB
> +	default n

No need for this line, 'n' is the default.

> +	help
> +	  Select this the associate USB devices will skip suspend when pm control.
> +
> +	  This option adds support skip suspend for PM control of USB devices
> +	  in specific manufacturers platforms.
> +
> +	  If unsure, keep the default value.
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index 0c3f12daac79..05fe921f8297 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -1583,6 +1583,15 @@ int usb_suspend(struct device *dev, pm_message_t msg)
>  	struct usb_device	*udev = to_usb_device(dev);
>  	int r;
>  
> +#ifdef CONFIG_USB_SKIP_SUSPEND
> +	if (udev->bus->skip_suspend && (msg.event == PM_EVENT_SUSPEND)) {
> +		if (udev->state != USB_STATE_SUSPENDED)
> +			dev_err(dev, "abort suspend\n");
> +
> +		return 0;
> +	}
> +#endif

Please do not put #ifdef lines in .c files if at all possible.

> +
>  	unbind_no_pm_drivers_interfaces(udev);
>  
>  	/* From now on we are sure all drivers support suspend/resume
> @@ -1619,6 +1628,11 @@ int usb_resume(struct device *dev, pm_message_t msg)
>  	struct usb_device	*udev = to_usb_device(dev);
>  	int			status;
>  
> +#ifdef CONFIG_USB_SKIP_SUSPEND
> +	if (udev->bus->skip_suspend && (msg.event == PM_EVENT_RESUME))
> +		return 0;
> +#endif
> +
>  	/* 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..8cbc666ab5c6 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -265,6 +265,13 @@ 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;
>  
> +#ifdef CONFIG_USB_SKIP_SUSPEND
> +		if (device_property_read_bool(tmpdev, "usb-skip-suspend")) {
> +			hcd_to_bus(hcd)->skip_suspend = true;
> +			hcd_to_bus(xhci->shared_hcd)->skip_suspend = true;
> +		}
> +#endif

Why are you only doing this for xhci platform drivers?


> +
>  		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..5f88850fc42d 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -487,6 +487,15 @@ struct usb_bus {
>  	struct mon_bus *mon_bus;	/* non-null when associated */
>  	int monitored;			/* non-zero when monitored */
>  #endif
> +
> +#ifdef CONFIG_USB_SKIP_SUSPEND
> +	unsigned int 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.
> +					 */

Shouldn't this be a boolean?

thanks,

greg k-h
Re:Re: [PATCH] usb: core: adds support for PM control of specific USB dev skip suspend.
Posted by huanglei 1 month ago 
Thank you for your reply！

In some industrial control fields, it is not desirable for USB devices to power off after the system automatically enter suspend mode;

There are also some scenes， the SOC integrated platform USB controllers developed by custom manufacturers to avoid external USB devices enter suspend mode, which can prevent stability issues with the controller; And It can also be used for debugging USB controllers;

The userspace cannot do it because it needs to determine whether to enable this feature based on the usb_skip_suspend field describing the controller dts；

In addition, UserSpace provides a runtime PM mechanism that can stop automatic suspend, but its function is different from this patch. This patch completely avoids all devices under the USB controller bus to enter suspend.


thanks，
best regards！


在 2024/10/21 21:01, Greg KH 写道:
> On Mon, Oct 21, 2024 at 08:47:41PM +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.
>
> Why is it desireable to do that?  Why can't you just do so from
> userspace today?
>
> What hardware requires this?
>
>> Signed-off-by: huanglei <huanglei@kylinos.cn>
>> ---
>>  drivers/usb/core/Kconfig     | 12 ++++++++++++
>>  drivers/usb/core/driver.c    | 14 ++++++++++++++
>>  drivers/usb/host/xhci-plat.c |  7 +++++++
>>  include/linux/usb.h          |  9 +++++++++
>>  4 files changed, 42 insertions(+)
>>
>> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
>> index 58e3ca7e4793..fe178c90d167 100644
>> --- a/drivers/usb/core/Kconfig
>> +++ b/drivers/usb/core/Kconfig
>> @@ -143,3 +143,15 @@ config USB_DEFAULT_AUTHORIZATION_MODE
>>        ACPI selecting value 2 is analogous to selecting value 0.
>>  
>>        If unsure, keep the default value.
>> +
>> +config USB_SKIP_SUSPEND
>> +    bool "Vendor USB support skip suspend"
>> +    depends on USB
>> +    default n
>
> No need for this line, 'n' is the default.  PATCH V2 has modify
>
>> +    help
>> +      Select this the associate USB devices will skip suspend when pm control.
>> +
>> +      This option adds support skip suspend for PM control of USB devices
>> +      in specific manufacturers platforms.
>> +
>> +      If unsure, keep the default value.
>> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
>> index 0c3f12daac79..05fe921f8297 100644
>> --- a/drivers/usb/core/driver.c
>> +++ b/drivers/usb/core/driver.c
>> @@ -1583,6 +1583,15 @@ int usb_suspend(struct device *dev, pm_message_t msg)
>>      struct usb_device    *udev = to_usb_device(dev);
>>      int r;
>>  
>> +#ifdef CONFIG_USB_SKIP_SUSPEND
>> +    if (udev->bus->skip_suspend && (msg.event == PM_EVENT_SUSPEND)) {
>> +        if (udev->state != USB_STATE_SUSPENDED)
>> +            dev_err(dev, "abort suspend\n");
>> +
>> +        return 0;
>> +    }
>> +#endif
>
> Please do not put #ifdef lines in .c files if at all possible.   The use of #ifdef is to ensure that it does not have any impact on the kernel when this feature is not supported
>
>> +
>>      unbind_no_pm_drivers_interfaces(udev);
>>  
>>      /* From now on we are sure all drivers support suspend/resume
>> @@ -1619,6 +1628,11 @@ int usb_resume(struct device *dev, pm_message_t msg)
>>      struct usb_device    *udev = to_usb_device(dev);
>>      int            status;
>>  
>> +#ifdef CONFIG_USB_SKIP_SUSPEND
>> +    if (udev->bus->skip_suspend && (msg.event == PM_EVENT_RESUME))
>> +        return 0;
>> +#endif
>> +
>>      /* 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..8cbc666ab5c6 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -265,6 +265,13 @@ 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;
>>  
>> +#ifdef CONFIG_USB_SKIP_SUSPEND
>> +        if (device_property_read_bool(tmpdev, "usb-skip-suspend")) {
>> +            hcd_to_bus(hcd)->skip_suspend = true;
>> +            hcd_to_bus(xhci->shared_hcd)->skip_suspend = true;
>> +        }
>> +#endif
>
> Why are you only doing this for xhci platform drivers?     this issue find only on  SOC integrated xhci platform  controller.
>
>
>> +
>>          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..5f88850fc42d 100644
>> --- a/include/linux/usb.h
>> +++ b/include/linux/usb.h
>> @@ -487,6 +487,15 @@ struct usb_bus {
>>      struct mon_bus *mon_bus;    /* non-null when associated */
>>      int monitored;            /* non-zero when monitored */
>>  #endif
>> +
>> +#ifdef CONFIG_USB_SKIP_SUSPEND
>> +    unsigned int 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.
>> +                     */
>
> Shouldn't this be a boolean?    PATCH V2 has modify
>
> thanks,
>
> greg k-h

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.