[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2024102137-senate-yarn-34ed@gregkh>
Date: Mon, 21 Oct 2024 15:01:33 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: huanglei814 <huanglei814@....com>
Cc: mathias.nyman@...el.com, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, huanglei <huanglei@...inos.cn>
Subject: Re: [PATCH] usb: core: adds support for PM control of specific USB
dev skip suspend.
On Mon, Oct 21, 2024 at 08:47:41PM +0800, huanglei814 wrote:
> From: huanglei <huanglei@...inos.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@...inos.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
Powered by blists - more mailing lists