Introduce offload_usage and corresponding apis to track offload usage
on each USB device. Offload denotes that there is another co-processor
accessing the USB device via the same USB host controller. To optimize
power usage, it's essential to monitor whether the USB device is
actively used by other co-processor. This information is vital when
determining if a USB device can be safely suspended during system power
state transitions.
Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
---
drivers/usb/core/driver.c | 109 ++++++++++++++++++++++++++++++++++++++
drivers/usb/core/usb.c | 1 +
drivers/usb/host/Kconfig | 11 ++++
include/linux/usb.h | 17 ++++++
4 files changed, 138 insertions(+)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 460d4dde5994..76372690add0 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -2036,6 +2036,115 @@ int usb_disable_usb2_hardware_lpm(struct usb_device *udev)
#endif /* CONFIG_PM */
+#if IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND_SUSPEND)
+
+/**
+ * usb_offload_get - increment the offload_usage of a USB device
+ * @udev: the USB device to increment its offload_usage
+ *
+ * Incrementing the offload_usage of a usb_device indicates that offload is
+ * enabled on this usb_device; that is, another entity is actively handling USB
+ * transfers. This information allows the USB driver to adjust its power
+ * management policy based on offload activity.
+ *
+ * The caller must hold @udev's device lock.
+ *
+ * Return: 0 on success. A negative error code otherwise.
+ */
+int usb_offload_get(struct usb_device *udev)
+{
+ int ret;
+
+ if (udev->state == USB_STATE_NOTATTACHED ||
+ udev->state == USB_STATE_SUSPENDED)
+ return -EAGAIN;
+
+ /*
+ * offload_usage could only be modified when the device is active, since
+ * it will alter the suspend flow of the device.
+ */
+ ret = usb_autoresume_device(udev);
+
+ if (ret < 0)
+ return ret;
+
+ udev->offload_usage++;
+ usb_autosuspend_device(udev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(usb_offload_get);
+
+/**
+ * usb_offload_put - drop the offload_usage of a USB device
+ * @udev: the USB device to drop its offload_usage
+ *
+ * The inverse operation of usb_offload_get, which drops the offload_usage of
+ * a USB device. This information allows the USB driver to adjust its power
+ * management policy based on offload activity.
+ *
+ * The caller must hold @udev's device lock.
+ *
+ * Return: 0 on success. A negative error code otherwise.
+ */
+int usb_offload_put(struct usb_device *udev)
+{
+ int ret;
+
+ if (udev->state == USB_STATE_NOTATTACHED ||
+ udev->state == USB_STATE_SUSPENDED)
+ return -EAGAIN;
+
+ /*
+ * offload_usage could only be modified when the device is active, since
+ * it will alter the suspend flow of the device.
+ */
+ ret = usb_autoresume_device(udev);
+
+ if (ret < 0)
+ return ret;
+
+ /* Drop the count when it wasn't 0, ignore the operation otherwise. */
+ if (udev->offload_usage)
+ udev->offload_usage--;
+ usb_autosuspend_device(udev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(usb_offload_put);
+
+/**
+ * usb_offload_check - check offload activities on a USB device
+ * @udev: the USB device to check its offload activity.
+ *
+ * Check if there are any offload activity on the USB device right now. This
+ * information could be used for power management or other forms of resource
+ * management.
+ *
+ * The caller must hold @udev's device lock.
+ *
+ * Returns true on any offload activity, false otherwise.
+ */
+bool usb_offload_check(struct usb_device *udev)
+{
+ struct usb_device *child;
+ bool active;
+ int port1;
+
+ usb_hub_for_each_child(udev, port1, child) {
+ device_lock(&child->dev);
+ active = usb_offload_check(child);
+ device_unlock(&child->dev);
+ if (active)
+ return true;
+ }
+
+ return !!udev->offload_usage;
+}
+EXPORT_SYMBOL_GPL(usb_offload_check);
+
+#endif /* IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND_SUSPEND) */
+
const struct bus_type usb_bus_type = {
.name = "usb",
.match = usb_device_match,
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 118fa4c93a79..43efd9d939c0 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -670,6 +670,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
set_dev_node(&dev->dev, dev_to_node(bus->sysdev));
dev->state = USB_STATE_ATTACHED;
dev->lpm_disable_count = 1;
+ dev->offload_usage = 0;
atomic_set(&dev->urbnum, 0);
INIT_LIST_HEAD(&dev->ep0.urb_list);
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 033a9a4b51fe..b564f46f1c87 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -113,6 +113,17 @@ config USB_XHCI_SIDEBAND
xHCI USB endpoints directly, allowing CPU to sleep while playing
audio.
+config USB_XHCI_SIDEBAND_SUSPEND
+ bool "xHCI support for sideband during system suspend"
+ depends on USB_XHCI_SIDEBAND
+ depends on USB_XHCI_PLATFORM
+ depends on SUSPEND
+ help
+ Say 'Y' to enable the support for the xHCI sideband capability
+ after system suspended. In addition to USB_XHCI_SIDEBAND, this
+ config allows endpoints and interrupters associated with the
+ sideband function when system is suspended.
+
config USB_XHCI_TEGRA
tristate "xHCI support for NVIDIA Tegra SoCs"
depends on PHY_TEGRA_XUSB
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 1b2545b4363b..ec8d839e1e2b 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -647,6 +647,7 @@ struct usb3_lpm_parameters {
* parent->hub_delay + wHubDelay + tTPTransmissionDelay (40ns)
* Will be used as wValue for SetIsochDelay requests.
* @use_generic_driver: ask driver core to reprobe using the generic driver.
+ * @offload_usage: number of offload activities happening on this usb device.
*
* Notes:
* Usbcore drivers should not set usbdev->state directly. Instead use
@@ -733,6 +734,8 @@ struct usb_device {
u16 hub_delay;
unsigned use_generic_driver:1;
+
+ int offload_usage;
};
#define to_usb_device(__dev) container_of_const(__dev, struct usb_device, dev)
@@ -839,6 +842,20 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
{ }
#endif
+#if IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND_SUSPEND)
+int usb_offload_get(struct usb_device *udev);
+int usb_offload_put(struct usb_device *udev);
+bool usb_offload_check(struct usb_device *udev);
+#else
+
+static inline int usb_offload_get(struct usb_device *udev)
+{ return 0; }
+static inline int usb_offload_put(struct usb_device *udev)
+{ return 0; }
+static inline bool usb_offload_check(struct usb_device *udev)
+{ return false; }
+#endif
+
extern int usb_disable_lpm(struct usb_device *udev);
extern void usb_enable_lpm(struct usb_device *udev);
/* Same as above, but these functions lock/unlock the bandwidth_mutex. */
--
2.49.0.604.gff1f9ca942-googOn Wed, Apr 16, 2025 at 02:43:02PM +0000, Guan-Yu Lin wrote:
> Introduce offload_usage and corresponding apis to track offload usage
> on each USB device. Offload denotes that there is another co-processor
> accessing the USB device via the same USB host controller. To optimize
> power usage, it's essential to monitor whether the USB device is
> actively used by other co-processor. This information is vital when
> determining if a USB device can be safely suspended during system power
> state transitions.
>
> Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
> ---
> drivers/usb/core/driver.c | 109 ++++++++++++++++++++++++++++++++++++++
> drivers/usb/core/usb.c | 1 +
> drivers/usb/host/Kconfig | 11 ++++
> include/linux/usb.h | 17 ++++++
> 4 files changed, 138 insertions(+)
>
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index 460d4dde5994..76372690add0 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -2036,6 +2036,115 @@ int usb_disable_usb2_hardware_lpm(struct usb_device *udev)
>
> #endif /* CONFIG_PM */
>
> +#if IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND_SUSPEND)
> +
> +/**
> + * usb_offload_get - increment the offload_usage of a USB device
> + * @udev: the USB device to increment its offload_usage
> + *
> + * Incrementing the offload_usage of a usb_device indicates that offload is
> + * enabled on this usb_device; that is, another entity is actively handling USB
> + * transfers. This information allows the USB driver to adjust its power
> + * management policy based on offload activity.
> + *
> + * The caller must hold @udev's device lock.
> + *
> + * Return: 0 on success. A negative error code otherwise.
> + */
> +int usb_offload_get(struct usb_device *udev)
> +{
> + int ret;
> +
> + if (udev->state == USB_STATE_NOTATTACHED ||
> + udev->state == USB_STATE_SUSPENDED)
> + return -EAGAIN;
> +
> + /*
> + * offload_usage could only be modified when the device is active, since
> + * it will alter the suspend flow of the device.
> + */
> + ret = usb_autoresume_device(udev);
> +
> + if (ret < 0)
> + return ret;
> +
> + udev->offload_usage++;
> + usb_autosuspend_device(udev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_offload_get);
> +
> +/**
> + * usb_offload_put - drop the offload_usage of a USB device
> + * @udev: the USB device to drop its offload_usage
> + *
> + * The inverse operation of usb_offload_get, which drops the offload_usage of
> + * a USB device. This information allows the USB driver to adjust its power
> + * management policy based on offload activity.
> + *
> + * The caller must hold @udev's device lock.
> + *
> + * Return: 0 on success. A negative error code otherwise.
> + */
> +int usb_offload_put(struct usb_device *udev)
> +{
> + int ret;
> +
> + if (udev->state == USB_STATE_NOTATTACHED ||
> + udev->state == USB_STATE_SUSPENDED)
> + return -EAGAIN;
What's to prevent the state of the device from changing right after you
check for this?
And why -EAGAIN, you don't mention that in the comment for the function.
Also, to pile on, sorry, the coding style needs to be fixed up here :)
> +
> + /*
> + * offload_usage could only be modified when the device is active, since
> + * it will alter the suspend flow of the device.
> + */
> + ret = usb_autoresume_device(udev);
> +
> + if (ret < 0)
> + return ret;
> +
> + /* Drop the count when it wasn't 0, ignore the operation otherwise. */
> + if (udev->offload_usage)
> + udev->offload_usage--;
> + usb_autosuspend_device(udev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_offload_put);
> +
> +/**
> + * usb_offload_check - check offload activities on a USB device
> + * @udev: the USB device to check its offload activity.
> + *
> + * Check if there are any offload activity on the USB device right now. This
> + * information could be used for power management or other forms of resource
> + * management.
> + *
> + * The caller must hold @udev's device lock.
> + *
> + * Returns true on any offload activity, false otherwise.
> + */
> +bool usb_offload_check(struct usb_device *udev)
> +{
> + struct usb_device *child;
> + bool active;
> + int port1;
> +
> + usb_hub_for_each_child(udev, port1, child) {
No locking is needed for this loop at all? What happens if a device is
added or removed while it is looping?
> + device_lock(&child->dev);
> + active = usb_offload_check(child);
> + device_unlock(&child->dev);
> + if (active)
> + return true;
> + }
> +
> + return !!udev->offload_usage;
But the state can change right afterwards, so no one can do anything
with this value, right? What is is used for?
thanks,
greg k-h
On Fri, Apr 25, 2025 at 7:12 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Apr 16, 2025 at 02:43:02PM +0000, Guan-Yu Lin wrote:
> > +int usb_offload_put(struct usb_device *udev)
> > +{
> > + int ret;
> > +
> > + if (udev->state == USB_STATE_NOTATTACHED ||
> > + udev->state == USB_STATE_SUSPENDED)
> > + return -EAGAIN;
>
> What's to prevent the state of the device from changing right after you
> check for this?
>
The caller of usb_offload_put() should hold the device lock, so I
think the state of the usb device will remain the same within
usb_offload_put().
> And why -EAGAIN, you don't mention that in the comment for the function.
>
> Also, to pile on, sorry, the coding style needs to be fixed up here :)
>
I'll separate these 2 states into two error handling checks and
provide appropriate error code respectively. Thanks for your advice.
> > +bool usb_offload_check(struct usb_device *udev)
> > +{
> > + struct usb_device *child;
> > + bool active;
> > + int port1;
> > +
> > + usb_hub_for_each_child(udev, port1, child) {
>
> No locking is needed for this loop at all? What happens if a device is
> added or removed while it is looping?
>
Currently the expectation is that all the downstream usb devices
should either go to suspend or be marked as "offload_at_suspend".
Based on this, is there still a chance that usb devices are being
added or removed? My understanding is device addition/removal requires
locks for the upstream usb device, which we've already acquired before
entering usb_offload_check().
> > + device_lock(&child->dev);
> > + active = usb_offload_check(child);
> > + device_unlock(&child->dev);
> > + if (active)
> > + return true;
> > + }
> > +
> > + return !!udev->offload_usage;
>
> But the state can change right afterwards, so no one can do anything
> with this value, right? What is is used for?
>
> thanks,
>
> greg k-h
If we could ensure that all the downstream usb devices satisfy the
following conditions, could the state still change?
1. usb devices either are suspended or marked as "offload_at_suspend".
2. "offload_usage" could only be modified when the usb device is
neither suspended nor marked as "offload_at_suspend".
Regarding point 1, I'll update the function description to state the
function should only be called after we ensure the downstream usb
devices are either suspended or marked as "offload_at_suspend".
Regarding point 2, I'll update the usb_offload_get()/usb_offload_put()
so that "offload_usage" changes only when the device is active and not
marked as "offload_at_suspend".
Regards,
Guan-Yu
© 2016 - 2025 Red Hat, Inc.