[PATCH v15 2/4] usb: offload: add apis for offload usage tracking

Guan-Yu Lin posted 4 patches 2 months ago
There is a newer version of this series
[PATCH v15 2/4] usb: offload: add apis for offload usage tracking
Posted by Guan-Yu Lin 2 months ago
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/Kconfig   |  10 +++
 drivers/usb/core/Makefile  |   1 +
 drivers/usb/core/offload.c | 136 +++++++++++++++++++++++++++++++++++++
 drivers/usb/core/usb.c     |   1 +
 include/linux/usb.h        |  18 +++++
 5 files changed, 166 insertions(+)
 create mode 100644 drivers/usb/core/offload.c

diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index 58e3ca7e4793..d5d38657f929 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -143,3 +143,13 @@ config USB_DEFAULT_AUTHORIZATION_MODE
 	  ACPI selecting value 2 is analogous to selecting value 0.
 
 	  If unsure, keep the default value.
+
+config USB_OFFLOAD
+	bool "Enable USB offload feature"
+	depends on USB
+	depends on USB_XHCI_SIDEBAND_SUSPEND
+	help
+	  Offload denotes that there is another co-processor accessing the
+	  USB device via the same USB host controller, creating the
+	  "offloaded USB transfers". Say Y to allow offloaded USB
+	  transfers during system sleep (Suspend-to-RAM).
diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index ac006abd13b3..df26c2885dd8 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -9,6 +9,7 @@ usbcore-y += devio.o notify.o generic.o quirks.o devices.o
 usbcore-y += phy.o port.o
 
 usbcore-$(CONFIG_OF)		+= of.o
+usbcore-$(CONFIG_USB_OFFLOAD)		+= offload.o
 usbcore-$(CONFIG_USB_PCI)		+= hcd-pci.o
 usbcore-$(CONFIG_ACPI)		+= usb-acpi.o
 
diff --git a/drivers/usb/core/offload.c b/drivers/usb/core/offload.c
new file mode 100644
index 000000000000..7c699f1b8d2b
--- /dev/null
+++ b/drivers/usb/core/offload.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * offload.c - USB offload related functions
+ *
+ * Copyright (c) 2025, Google LLC.
+ *
+ * Author: Guan-Yu Lin
+ */
+
+#include <linux/usb.h>
+
+#include "usb.h"
+
+/**
+ * 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.
+ *
+ * Return: 0 on success. A negative error code otherwise.
+ */
+int usb_offload_get(struct usb_device *udev)
+{
+	int ret;
+
+	usb_lock_device(udev);
+	if (udev->state == USB_STATE_NOTATTACHED) {
+		usb_unlock_device(udev);
+		return -ENODEV;
+	}
+
+	if (udev->state == USB_STATE_SUSPENDED ||
+		   udev->offload_at_suspend) {
+		usb_unlock_device(udev);
+		return -EBUSY;
+	}
+
+	/*
+	 * 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) {
+		usb_unlock_device(udev);
+		return ret;
+	}
+
+	udev->offload_usage++;
+	usb_autosuspend_device(udev);
+	usb_unlock_device(udev);
+
+	return ret;
+}
+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.
+ *
+ * Return: 0 on success. A negative error code otherwise.
+ */
+int usb_offload_put(struct usb_device *udev)
+{
+	int ret;
+
+	usb_lock_device(udev);
+	if (udev->state == USB_STATE_NOTATTACHED) {
+		usb_unlock_device(udev);
+		return -ENODEV;
+	}
+
+	if (udev->state == USB_STATE_SUSPENDED ||
+		   udev->offload_at_suspend) {
+		usb_unlock_device(udev);
+		return -EBUSY;
+	}
+
+	/*
+	 * 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) {
+		usb_unlock_device(udev);
+		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);
+	usb_unlock_device(udev);
+
+	return ret;
+}
+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. In addition, the caller should
+ * ensure downstream usb devices are all either suspended or marked as
+ * "offload_at_suspend" to ensure the correctness of the return value.
+ *
+ * Returns true on any offload activity, false otherwise.
+ */
+bool usb_offload_check(struct usb_device *udev) __must_hold(&udev->dev->mutex)
+{
+	struct usb_device *child;
+	bool active;
+	int port1;
+
+	usb_hub_for_each_child(udev, port1, child) {
+		usb_lock_device(child);
+		active = usb_offload_check(child);
+		usb_unlock_device(child);
+		if (active)
+			return true;
+	}
+
+	return !!udev->offload_usage;
+}
+EXPORT_SYMBOL_GPL(usb_offload_check);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index fca7735fc660..e6b9ab8b4a34 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/include/linux/usb.h b/include/linux/usb.h
index 9d662c6abb4d..81fc0d93783f 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -636,6 +636,8 @@ struct usb3_lpm_parameters {
  * @do_remote_wakeup:  remote wakeup should be enabled
  * @reset_resume: needs reset instead of resume
  * @port_is_suspended: the upstream port is suspended (L2 or U3)
+ * @offload_at_suspend: offload activities during suspend is enabled.
+ * @offload_usage: number of offload activities happening on this usb device.
  * @slot_id: Slot ID assigned by xHCI
  * @l1_params: best effor service latency for USB2 L1 LPM state, and L1 timeout.
  * @u1_params: exit latencies for USB3 U1 LPM state, and hub-initiated timeout.
@@ -724,6 +726,8 @@ struct usb_device {
 	unsigned do_remote_wakeup:1;
 	unsigned reset_resume:1;
 	unsigned port_is_suspended:1;
+	unsigned offload_at_suspend:1;
+	int offload_usage;
 	enum usb_link_tunnel_mode tunnel_mode;
 	struct device_link *usb4_link;
 
@@ -841,6 +845,20 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
 { }
 #endif
 
+#if IS_ENABLED(CONFIG_USB_OFFLOAD)
+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.50.1.565.gc32cd1483b-goog
Re: [PATCH v15 2/4] usb: offload: add apis for offload usage tracking
Posted by Greg KH 1 month, 3 weeks ago
On Fri, Aug 01, 2025 at 03:39:31AM +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/Kconfig   |  10 +++
>  drivers/usb/core/Makefile  |   1 +
>  drivers/usb/core/offload.c | 136 +++++++++++++++++++++++++++++++++++++
>  drivers/usb/core/usb.c     |   1 +
>  include/linux/usb.h        |  18 +++++
>  5 files changed, 166 insertions(+)
>  create mode 100644 drivers/usb/core/offload.c
> 
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index 58e3ca7e4793..d5d38657f929 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -143,3 +143,13 @@ config USB_DEFAULT_AUTHORIZATION_MODE
>  	  ACPI selecting value 2 is analogous to selecting value 0.
>  
>  	  If unsure, keep the default value.
> +
> +config USB_OFFLOAD
> +	bool "Enable USB offload feature"

I'm confused, we already have a "USB offload feature" that went into the
last kernel release, why do we need a separate config option for this as
well?  Shouldn't this code only get built if the drivers that need it
select it automatically?  Forcing distros to configure this isn't
generally a good idea if at all possible.


> +	depends on USB
> +	depends on USB_XHCI_SIDEBAND_SUSPEND
> +	help
> +	  Offload denotes that there is another co-processor accessing the
> +	  USB device via the same USB host controller, creating the
> +	  "offloaded USB transfers". Say Y to allow offloaded USB
> +	  transfers during system sleep (Suspend-to-RAM).

Especially because all "desktops" do not want this code selected, so
having it in all distros feels like a waste to me.

thanks,

greg k-h
Re: [PATCH v15 2/4] usb: offload: add apis for offload usage tracking
Posted by Guan-Yu Lin 1 month, 1 week ago
On Wed, Aug 13, 2025 at 10:50 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Aug 01, 2025 at 03:39:31AM +0000, Guan-Yu Lin wrote:
> > +
> > +config USB_OFFLOAD
> > +     bool "Enable USB offload feature"
>
> I'm confused, we already have a "USB offload feature" that went into the
> last kernel release, why do we need a separate config option for this as
> well?  Shouldn't this code only get built if the drivers that need it
> select it automatically?  Forcing distros to configure this isn't
> generally a good idea if at all possible.
>

Based on the discussion in v13, a new, separate USB configuration
option is required to avoid core USB functions being enabled or
disabled via an xhci-specific option. The USB offload feature involves
a sideband entity that can access xhci resources, which, from the USB
driver's viewpoint, means USB transfers are offloaded to this other
entity. Therefore, the name "USB_OFFLOAD" was chosen to reflect this
functionality.

>
> > +     depends on USB
> > +     depends on USB_XHCI_SIDEBAND_SUSPEND
>
> Especially because all "desktops" do not want this code selected, so
> having it in all distros feels like a waste to me.
>
> thanks,
>
> greg k-h

For the config keywords, we could automatically select USB_OFFLOAD
once USB_XHCI_SIDEBAND_SUSPEND is selected to reduce configuration
efforts.

Regards,
Guan-Yu
Re: [PATCH v15 2/4] usb: offload: add apis for offload usage tracking
Posted by Greg KH 4 weeks, 1 day ago
On Tue, Aug 26, 2025 at 11:59:00AM +0800, Guan-Yu Lin wrote:
> On Wed, Aug 13, 2025 at 10:50 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Aug 01, 2025 at 03:39:31AM +0000, Guan-Yu Lin wrote:
> > > +
> > > +config USB_OFFLOAD
> > > +     bool "Enable USB offload feature"
> >
> > I'm confused, we already have a "USB offload feature" that went into the
> > last kernel release, why do we need a separate config option for this as
> > well?  Shouldn't this code only get built if the drivers that need it
> > select it automatically?  Forcing distros to configure this isn't
> > generally a good idea if at all possible.
> >
> 
> Based on the discussion in v13, a new, separate USB configuration
> option is required to avoid core USB functions being enabled or
> disabled via an xhci-specific option. The USB offload feature involves
> a sideband entity that can access xhci resources, which, from the USB
> driver's viewpoint, means USB transfers are offloaded to this other
> entity. Therefore, the name "USB_OFFLOAD" was chosen to reflect this
> functionality.

Again, you are increasing the number of config options here, which does
not make sense.  Why would anyone only want a subset, and not just the
whole thing?

Yes, USB_OFFLOAD only works today on xhci, and that's fine, so let's
just keep it that way.

> > > +     depends on USB
> > > +     depends on USB_XHCI_SIDEBAND_SUSPEND
> >
> > Especially because all "desktops" do not want this code selected, so
> > having it in all distros feels like a waste to me.
> >
> > thanks,
> >
> > greg k-h
> 
> For the config keywords, we could automatically select USB_OFFLOAD
> once USB_XHCI_SIDEBAND_SUSPEND is selected to reduce configuration
> efforts.

select is a nightmare to maintain and understand.  Please just reduce
configuration efforts by not adding new options at all :)

thanks,

greg k-h
Re: [PATCH v15 2/4] usb: offload: add apis for offload usage tracking
Posted by Guan-Yu Lin 3 weeks, 3 days ago
On Sat, Sep 6, 2025 at 9:13 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Aug 26, 2025 at 11:59:00AM +0800, Guan-Yu Lin wrote:
> > On Wed, Aug 13, 2025 at 10:50 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Aug 01, 2025 at 03:39:31AM +0000, Guan-Yu Lin wrote:
> > > > +
> > > > +config USB_OFFLOAD
> > > > +     bool "Enable USB offload feature"
> > >
> > > I'm confused, we already have a "USB offload feature" that went into the
> > > last kernel release, why do we need a separate config option for this as
> > > well?  Shouldn't this code only get built if the drivers that need it
> > > select it automatically?  Forcing distros to configure this isn't
> > > generally a good idea if at all possible.
> > >
> >
> > Based on the discussion in v13, a new, separate USB configuration
> > option is required to avoid core USB functions being enabled or
> > disabled via an xhci-specific option. The USB offload feature involves
> > a sideband entity that can access xhci resources, which, from the USB
> > driver's viewpoint, means USB transfers are offloaded to this other
> > entity. Therefore, the name "USB_OFFLOAD" was chosen to reflect this
> > functionality.
>
> Again, you are increasing the number of config options here, which does
> not make sense.  Why would anyone only want a subset, and not just the
> whole thing?
>
> Yes, USB_OFFLOAD only works today on xhci, and that's fine, so let's
> just keep it that way.
>
> > > > +     depends on USB
> > > > +     depends on USB_XHCI_SIDEBAND_SUSPEND
> > >
> > > Especially because all "desktops" do not want this code selected, so
> > > having it in all distros feels like a waste to me.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > For the config keywords, we could automatically select USB_OFFLOAD
> > once USB_XHCI_SIDEBAND_SUSPEND is selected to reduce configuration
> > efforts.
>
> select is a nightmare to maintain and understand.  Please just reduce
> configuration efforts by not adding new options at all :)
>
> thanks,
>
> greg k-h

Thanks for the suggestions. I'll use only USB_XHCI_SIDEBAND_SUSPEND to
control all the features, including the core USB functions.

Regards,
Guan-Yu