public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/5] Support system sleep with offloaded usb transfers
@ 2024-11-06  8:32 Guan-Yu Lin
  2024-11-06  8:32 ` [PATCH v6 1/5] usb: dwc3: separate dev_pm_ops for each pm_event Guan-Yu Lin
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Guan-Yu Lin @ 2024-11-06  8:32 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, mathias.nyman, stern, sumit.garg, dianders,
	kekrby, oneukum, yajun.deng, niko.mauno, christophe.jaillet, tj,
	stanley_chang, andreyknvl, quic_jjohnson, ricardo
  Cc: linux-usb, linux-kernel, Guan-Yu Lin

Wesley Cheng and Mathias Nyman's USB offload design enables a co-processor
to handle some USB transfers, potentially allowing the main system to
sleep and save power. However, Linux's power management system halts the
USB host controller when the main system isn't managing any USB transfers.
To address this, the proposal modifies the system to recognize offloaded
USB transfers and manage power accordingly.

This involves two key steps:
1. Transfer Status Tracking: Propose xhci_sideband_get and
xhci_sideband_put to track USB transfers on the co-processor, ensuring the
system is aware of any ongoing activity.
2. Power Management Adjustment:  Modifications to the USB driver stack
(dwc3 controller driver, xhci host controller driver, and USB device
drivers) allow the system to sleep without disrupting co-processor managed
USB transfers. This involves adding conditional checks to bypass some
power management operations.

patches depends on series "Introduce QC USB SND audio offloading support" 
https://lore.kernel.org/lkml/20240925010000.2231406-11-quic_wcheng@quicinc.com/T/

changelog
----------
Changes in v6:
- Fix build errors when CONFIG_USB_XHCI_SIDEBAND is disabled.
- Explicitly specify the data structure of the drvdata refereced in
  dwc3_suspend(), dwc3_resume().
- Move the initialization of counters to the patches introducing them.

Changes in v5:
- Walk through the USB children in usb_sideband_check() to determine the
  sideband activity under the specific USB device. 
- Replace atomic_t by refcount_t.
- Reduce logs by using dev_dbg & remove __func__.

Changes in v4:
- Isolate the feature into USB driver stack.
- Integrate with series "Introduce QC USB SND audio offloading support"

Changes in v3:
- Integrate the feature with the pm core framework.

Changes in v2:
- Cosmetics changes on coding style.

[v3] PM / core: conditionally skip system pm in device/driver model
[v2] usb: host: enable suspend-to-RAM control in userspace
[v1] [RFC] usb: host: Allow userspace to control usb suspend flows
---

Guan-Yu Lin (5):
  usb: dwc3: separate dev_pm_ops for each pm_event
  usb: xhci-plat: separate dev_pm_ops for each pm_event
  usb: add apis for sideband usage tracking
  xhci: sideband: add api to trace sideband usage
  usb: host: enable sideband transfer during system sleep

 drivers/usb/core/driver.c         | 87 +++++++++++++++++++++++++++
 drivers/usb/core/hcd.c            |  4 ++
 drivers/usb/core/usb.c            |  4 ++
 drivers/usb/dwc3/core.c           | 97 ++++++++++++++++++++++++++++++-
 drivers/usb/dwc3/core.h           |  1 +
 drivers/usb/host/xhci-plat.c      | 38 ++++++++++--
 drivers/usb/host/xhci-sideband.c  | 92 +++++++++++++++++++++++++++++
 include/linux/usb.h               | 20 +++++++
 include/linux/usb/hcd.h           | 13 +++++
 include/linux/usb/xhci-sideband.h |  5 ++
 10 files changed, 356 insertions(+), 5 deletions(-)

-- 
2.47.0.199.ga7371fff76-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v6 1/5] usb: dwc3: separate dev_pm_ops for each pm_event
  2024-11-06  8:32 [PATCH v6 0/5] Support system sleep with offloaded usb transfers Guan-Yu Lin
@ 2024-11-06  8:32 ` Guan-Yu Lin
  2024-11-06  8:32 ` [PATCH v6 2/5] usb: xhci-plat: " Guan-Yu Lin
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Guan-Yu Lin @ 2024-11-06  8:32 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, mathias.nyman, stern, sumit.garg, dianders,
	kekrby, oneukum, yajun.deng, niko.mauno, christophe.jaillet, tj,
	stanley_chang, andreyknvl, quic_jjohnson, ricardo
  Cc: linux-usb, linux-kernel, Guan-Yu Lin

Separate dev_pm_ops for different power events such as suspend, thaw,
and hibernation. This is crucial when dwc3 driver needs to adapt its
behavior based on different power state changes.

Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
---
 drivers/usb/dwc3/core.c | 77 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b25d80f318a9..2fdafbcbe44c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2582,6 +2582,76 @@ static int dwc3_resume(struct device *dev)
 	return 0;
 }
 
+static int dwc3_freeze(struct device *dev)
+{
+	struct dwc3	*dwc = dev_get_drvdata(dev);
+	int		ret;
+
+	ret = dwc3_suspend_common(dwc, PMSG_FREEZE);
+	if (ret)
+		return ret;
+
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int dwc3_thaw(struct device *dev)
+{
+	struct dwc3	*dwc = dev_get_drvdata(dev);
+	int		ret;
+
+	pinctrl_pm_select_default_state(dev);
+
+	pm_runtime_disable(dev);
+	pm_runtime_set_active(dev);
+
+	ret = dwc3_resume_common(dwc, PMSG_THAW);
+	if (ret) {
+		pm_runtime_set_suspended(dev);
+		return ret;
+	}
+
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
+static int dwc3_poweroff(struct device *dev)
+{
+	struct dwc3	*dwc = dev_get_drvdata(dev);
+	int		ret;
+
+	ret = dwc3_suspend_common(dwc, PMSG_HIBERNATE);
+	if (ret)
+		return ret;
+
+	pinctrl_pm_select_sleep_state(dev);
+
+	return 0;
+}
+
+static int dwc3_restore(struct device *dev)
+{
+	struct dwc3	*dwc = dev_get_drvdata(dev);
+	int		ret;
+
+	pinctrl_pm_select_default_state(dev);
+
+	pm_runtime_disable(dev);
+	pm_runtime_set_active(dev);
+
+	ret = dwc3_resume_common(dwc, PMSG_RESTORE);
+	if (ret) {
+		pm_runtime_set_suspended(dev);
+		return ret;
+	}
+
+	pm_runtime_enable(dev);
+
+	return 0;
+}
+
 static void dwc3_complete(struct device *dev)
 {
 	struct dwc3	*dwc = dev_get_drvdata(dev);
@@ -2599,7 +2669,12 @@ static void dwc3_complete(struct device *dev)
 #endif /* CONFIG_PM_SLEEP */
 
 static const struct dev_pm_ops dwc3_dev_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
+	.suspend = pm_sleep_ptr(dwc3_suspend),
+	.resume = pm_sleep_ptr(dwc3_resume),
+	.freeze = pm_sleep_ptr(dwc3_freeze),
+	.thaw = pm_sleep_ptr(dwc3_thaw),
+	.poweroff = pm_sleep_ptr(dwc3_poweroff),
+	.restore = pm_sleep_ptr(dwc3_restore),
 	.complete = dwc3_complete,
 	SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
 			dwc3_runtime_idle)
-- 
2.47.0.199.ga7371fff76-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 2/5] usb: xhci-plat: separate dev_pm_ops for each pm_event
  2024-11-06  8:32 [PATCH v6 0/5] Support system sleep with offloaded usb transfers Guan-Yu Lin
  2024-11-06  8:32 ` [PATCH v6 1/5] usb: dwc3: separate dev_pm_ops for each pm_event Guan-Yu Lin
@ 2024-11-06  8:32 ` Guan-Yu Lin
  2024-11-06  8:32 ` [PATCH v6 3/5] usb: add apis for sideband usage tracking Guan-Yu Lin
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Guan-Yu Lin @ 2024-11-06  8:32 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, mathias.nyman, stern, sumit.garg, dianders,
	kekrby, oneukum, yajun.deng, niko.mauno, christophe.jaillet, tj,
	stanley_chang, andreyknvl, quic_jjohnson, ricardo
  Cc: linux-usb, linux-kernel, Guan-Yu Lin

Separate dev_pm_ops for different power events such as suspend, thaw,
and hibernation. This is crucial when xhci-plat driver needs to adapt
its behavior based on different power state changes.

Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
---
 drivers/usb/host/xhci-plat.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 8dc23812b204..6e49ef1908eb 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -450,7 +450,7 @@ void xhci_plat_remove(struct platform_device *dev)
 }
 EXPORT_SYMBOL_GPL(xhci_plat_remove);
 
-static int xhci_plat_suspend(struct device *dev)
+static int xhci_plat_suspend_common(struct device *dev, struct pm_message pmsg)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
@@ -478,6 +478,21 @@ static int xhci_plat_suspend(struct device *dev)
 	return 0;
 }
 
+static int xhci_plat_suspend(struct device *dev)
+{
+	return xhci_plat_suspend_common(dev, PMSG_SUSPEND);
+}
+
+static int xhci_plat_freeze(struct device *dev)
+{
+	return xhci_plat_suspend_common(dev, PMSG_FREEZE);
+}
+
+static int xhci_plat_poweroff(struct device *dev)
+{
+	return xhci_plat_suspend_common(dev, PMSG_HIBERNATE);
+}
+
 static int xhci_plat_resume_common(struct device *dev, struct pm_message pmsg)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
@@ -524,6 +539,11 @@ static int xhci_plat_resume(struct device *dev)
 	return xhci_plat_resume_common(dev, PMSG_RESUME);
 }
 
+static int xhci_plat_thaw(struct device *dev)
+{
+	return xhci_plat_resume_common(dev, PMSG_THAW);
+}
+
 static int xhci_plat_restore(struct device *dev)
 {
 	return xhci_plat_resume_common(dev, PMSG_RESTORE);
@@ -553,9 +573,9 @@ static int __maybe_unused xhci_plat_runtime_resume(struct device *dev)
 const struct dev_pm_ops xhci_plat_pm_ops = {
 	.suspend = pm_sleep_ptr(xhci_plat_suspend),
 	.resume = pm_sleep_ptr(xhci_plat_resume),
-	.freeze = pm_sleep_ptr(xhci_plat_suspend),
-	.thaw = pm_sleep_ptr(xhci_plat_resume),
-	.poweroff = pm_sleep_ptr(xhci_plat_suspend),
+	.freeze = pm_sleep_ptr(xhci_plat_freeze),
+	.thaw = pm_sleep_ptr(xhci_plat_thaw),
+	.poweroff = pm_sleep_ptr(xhci_plat_poweroff),
 	.restore = pm_sleep_ptr(xhci_plat_restore),
 
 	SET_RUNTIME_PM_OPS(xhci_plat_runtime_suspend,
-- 
2.47.0.199.ga7371fff76-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 3/5] usb: add apis for sideband usage tracking
  2024-11-06  8:32 [PATCH v6 0/5] Support system sleep with offloaded usb transfers Guan-Yu Lin
  2024-11-06  8:32 ` [PATCH v6 1/5] usb: dwc3: separate dev_pm_ops for each pm_event Guan-Yu Lin
  2024-11-06  8:32 ` [PATCH v6 2/5] usb: xhci-plat: " Guan-Yu Lin
@ 2024-11-06  8:32 ` Guan-Yu Lin
  2024-11-06  8:32 ` [PATCH v6 4/5] xhci: sideband: add api to trace sideband usage Guan-Yu Lin
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Guan-Yu Lin @ 2024-11-06  8:32 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, mathias.nyman, stern, sumit.garg, dianders,
	kekrby, oneukum, yajun.deng, niko.mauno, christophe.jaillet, tj,
	stanley_chang, andreyknvl, quic_jjohnson, ricardo
  Cc: linux-usb, linux-kernel, Guan-Yu Lin

Introduce sb_usage_count and corresponding apis to track sideband usage
on each USB device. A sideband refers to the co-processor that accesses
the usb_device via shared control on the same USB host controller. To
optimize power usage, it's essential to monitor whether ther sideband is
actively using the USB device. 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 | 77 +++++++++++++++++++++++++++++++++++++++
 drivers/usb/core/usb.c    |  4 ++
 include/linux/usb.h       | 20 ++++++++++
 3 files changed, 101 insertions(+)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 0c3f12daac79..e53cb4c267b3 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -2041,6 +2041,83 @@ int usb_disable_usb2_hardware_lpm(struct usb_device *udev)
 
 #endif /* CONFIG_PM */
 
+#ifdef CONFIG_USB_XHCI_SIDEBAND
+
+/**
+ * usb_sideband_get - increment the sb_usage_count of a USB device
+ * @udev: the USB device to increment its sb_usage_count
+ *
+ * Incrementing the sb_usage_count of a usb_device indicates that a sideband is
+ * currently using the device; that is, another entity is actively handling USB
+ * transfers. This information allows the USB driver to adjust its power
+ * management policy based on sideband activity.
+ *
+ * The caller must hold @udev's device lock.
+ *
+ * Return: 0 on success. A negative error code otherwise.
+ */
+int usb_sideband_get(struct usb_device *udev)
+{
+	if (udev->state == USB_STATE_NOTATTACHED ||
+			udev->state == USB_STATE_SUSPENDED)
+		return -EAGAIN;
+
+	refcount_inc(&udev->sb_usage_count);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_sideband_get);
+
+/**
+ * usb_sideband_put - drop the sb_usage_count of a USB device
+ * @udev: the USB device to drop its sb_usage_count
+ *
+ * The inverse operation of usb_sideband_get, which drops the sb_usage_count of
+ * a USB device. This information allows the USB driver to adjust its power
+ * management policy based on sideband activity.
+ *
+ * The caller must hold @udev's device lock.
+ *
+ * Return: 0 on success. A negative error code otherwise.
+ */
+int usb_sideband_put(struct usb_device *udev)
+{
+	if (udev->state == USB_STATE_NOTATTACHED ||
+			udev->state == USB_STATE_SUSPENDED)
+		return -EAGAIN;
+
+	refcount_dec(&udev->sb_usage_count);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_sideband_put);
+
+/**
+ * usb_sideband_check - check sideband activities on a USB device
+ * @udev: the USB device to check its sideband activity.
+ *
+ * Check if there are any sideband activity on the USB device right now. This
+ * information could be used for power management or other forms or resource
+ * management.
+ *
+ * Returns true on any active sideband existence, false otherwise
+ */
+bool usb_sideband_check(struct usb_device *udev)
+{
+	struct usb_device *child;
+	int port1;
+
+	usb_hub_for_each_child(udev, port1, child) {
+		if (usb_sideband_check(child))
+			return true;
+	}
+
+	return !!refcount_read(&udev->sb_usage_count);
+}
+EXPORT_SYMBOL_GPL(usb_sideband_check);
+
+#endif /* CONFIG_USB_XHCI_SIDEBAND */
+
 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 0b4685aad2d5..00bb00d15875 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -672,6 +672,10 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent,
 	dev->lpm_disable_count = 1;
 	atomic_set(&dev->urbnum, 0);
 
+#ifdef CONFIG_USB_XHCI_SIDEBAND
+	refcount_set(&dev->sb_usage_count, 0);
+#endif
+
 	INIT_LIST_HEAD(&dev->ep0.urb_list);
 	dev->ep0.desc.bLength = USB_DT_ENDPOINT_SIZE;
 	dev->ep0.desc.bDescriptorType = USB_DT_ENDPOINT;
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 672d8fc2abdb..c5586532cd12 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -645,6 +645,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.
+ * @sb_usage_count: number of active sideband accessing this usb device.
  *
  * Notes:
  * Usbcore drivers should not set usbdev->state directly.  Instead use
@@ -731,6 +732,10 @@ struct usb_device {
 
 	u16 hub_delay;
 	unsigned use_generic_driver:1;
+
+#ifdef CONFIG_USB_XHCI_SIDEBAND
+	refcount_t sb_usage_count;
+#endif
 };
 
 #define to_usb_device(__dev)	container_of_const(__dev, struct usb_device, dev)
@@ -837,6 +842,21 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
 { }
 #endif
 
+#ifdef CONFIG_USB_XHCI_SIDEBAND
+extern int usb_sideband_get(struct usb_device *udev);
+extern int usb_sideband_put(struct usb_device *udev);
+extern bool usb_sideband_check(struct usb_device *udev);
+
+#else
+
+static inline int usb_sideband_get(struct usb_device *udev)
+{ return 0; }
+static inline int usb_sideband_put(struct usb_device *udev)
+{ return 0; }
+static inline bool usb_sideband_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.47.0.199.ga7371fff76-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 4/5] xhci: sideband: add api to trace sideband usage
  2024-11-06  8:32 [PATCH v6 0/5] Support system sleep with offloaded usb transfers Guan-Yu Lin
                   ` (2 preceding siblings ...)
  2024-11-06  8:32 ` [PATCH v6 3/5] usb: add apis for sideband usage tracking Guan-Yu Lin
@ 2024-11-06  8:32 ` Guan-Yu Lin
  2024-11-06  8:32 ` [PATCH v6 5/5] usb: host: enable sideband transfer during system sleep Guan-Yu Lin
  2024-11-07 23:51 ` [PATCH v6 0/5] Support system sleep with offloaded usb transfers Thinh Nguyen
  5 siblings, 0 replies; 8+ messages in thread
From: Guan-Yu Lin @ 2024-11-06  8:32 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, mathias.nyman, stern, sumit.garg, dianders,
	kekrby, oneukum, yajun.deng, niko.mauno, christophe.jaillet, tj,
	stanley_chang, andreyknvl, quic_jjohnson, ricardo
  Cc: linux-usb, linux-kernel, Guan-Yu Lin

The existing sideband driver only registers sidebands without tracking
their active usage. To address this, new apis are introduced to:
- mark sideband usage: record the sideband usage information in the USB
  host controller driver and USB device driver.
- query sideband status: provide a means for other drivers to fetch
  sideband activity information on a USB host controller.

Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
---
 drivers/usb/core/hcd.c            |  4 ++
 drivers/usb/host/xhci-sideband.c  | 92 +++++++++++++++++++++++++++++++
 include/linux/usb/hcd.h           |  6 ++
 include/linux/usb/xhci-sideband.h |  5 ++
 4 files changed, 107 insertions(+)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 1ff7d901fede..6aca6c69bf74 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2595,6 +2595,10 @@ struct usb_hcd *__usb_create_hcd(const struct hc_driver *driver,
 	INIT_WORK(&hcd->wakeup_work, hcd_resume_work);
 #endif
 
+#ifdef CONFIG_USB_XHCI_SIDEBAND
+	refcount_set(&hcd->sb_usage_count, 0);
+#endif
+
 	INIT_WORK(&hcd->died_work, hcd_died_work);
 
 	hcd->driver = driver;
diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
index d04cf0af57ae..bd0fc1564052 100644
--- a/drivers/usb/host/xhci-sideband.c
+++ b/drivers/usb/host/xhci-sideband.c
@@ -334,6 +334,98 @@ xhci_sideband_interrupter_id(struct xhci_sideband *sb)
 }
 EXPORT_SYMBOL_GPL(xhci_sideband_interrupter_id);
 
+/**
+ * xhci_sideband_get - inform related drivers there's a new active sideband
+ * @sb: sideband instance for this usb device
+ *
+ * An active sideband indicates that another entity is currently using the host
+ * controller. Inform the host controller and related usb devices by increasing
+ * their sb_usage_count. This allows the corresponding drivers to dynamically
+ * adjust power management actions based on current sideband activity.
+ *
+ * Returns 0 on success, negative error otherwise
+ */
+int xhci_sideband_get(struct xhci_sideband *sb)
+{
+	struct usb_device *udev;
+	struct usb_hcd *hcd;
+	int ret;
+
+	if (!sb || !sb->xhci)
+		return -ENODEV;
+
+	hcd = xhci_to_hcd(sb->xhci);
+	refcount_inc(&hcd->sb_usage_count);
+
+	udev = sb->vdev->udev;
+
+	device_lock(&udev->dev);
+	ret = usb_sideband_get(udev);
+	device_unlock(&udev->dev);
+
+	if (ret) {
+		refcount_dec(&hcd->sb_usage_count);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_sideband_get);
+
+/**
+ * xhci_sideband_put - inform related drivers there's a sideband deactivated
+ * @sb: sideband instance for this usb device
+ *
+ * The inverse operation of xhci_sideband_get, which informs the host
+ * controller and related usb devices by decreasing their sb_usage_count. This
+ * allows the corresponding drivers to dynamically adjust power management
+ * actions based on current sideband activity.
+ *
+ * Returns 0 on success, negative error otherwise
+ */
+int xhci_sideband_put(struct xhci_sideband *sb)
+{
+	struct usb_device *udev;
+	struct usb_hcd *hcd;
+	int ret;
+
+	if (!sb || !sb->xhci)
+		return -ENODEV;
+
+	hcd = xhci_to_hcd(sb->xhci);
+	refcount_dec(&hcd->sb_usage_count);
+
+	udev = sb->vdev->udev;
+
+	device_lock(&udev->dev);
+	ret = usb_sideband_put(udev);
+	device_unlock(&udev->dev);
+
+	if (ret) {
+		refcount_inc(&hcd->sb_usage_count);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xhci_sideband_put);
+
+/**
+ * xhci_sideband_check - check sideband activities on the host controller
+ * @hcd: the host controller driver associated with the target host controller
+ *
+ * Allow other drivers, such as usb controller driver, to check if there are
+ * any sideband activity on the host controller right now. This information
+ * could be used for power management or other forms or resource management.
+ *
+ * Returns true on any active sideband existence, false otherwise
+ */
+bool xhci_sideband_check(struct usb_hcd *hcd)
+{
+	return !!refcount_read(&hcd->sb_usage_count);
+}
+EXPORT_SYMBOL_GPL(xhci_sideband_check);
+
 /**
  * xhci_sideband_register - register a sideband for a usb device
  * @udev: usb device to be accessed via sideband
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index ac95e7c89df5..9867c178d188 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -85,6 +85,12 @@ struct usb_hcd {
 #ifdef CONFIG_PM
 	struct work_struct	wakeup_work;	/* for remote wakeup */
 #endif
+
+#ifdef CONFIG_USB_XHCI_SIDEBAND
+	/* Number of active sideband accessing the host controller. */
+	refcount_t		sb_usage_count;
+#endif
+
 	struct work_struct	died_work;	/* for when the device dies */
 
 	/*
diff --git a/include/linux/usb/xhci-sideband.h b/include/linux/usb/xhci-sideband.h
index f0223c5535e0..4850fc826e00 100644
--- a/include/linux/usb/xhci-sideband.h
+++ b/include/linux/usb/xhci-sideband.h
@@ -12,6 +12,7 @@
 
 #include <linux/scatterlist.h>
 #include <linux/usb.h>
+#include <linux/usb/hcd.h>
 
 #define	EP_CTX_PER_DEV		31	/* FIXME defined twice, from xhci.h */
 
@@ -57,6 +58,10 @@ xhci_sideband_get_endpoint_buffer(struct xhci_sideband *sb,
 struct sg_table *
 xhci_sideband_get_event_buffer(struct xhci_sideband *sb);
 
+int xhci_sideband_get(struct xhci_sideband *sb);
+int xhci_sideband_put(struct xhci_sideband *sb);
+bool xhci_sideband_check(struct usb_hcd *hcd);
+
 int
 xhci_sideband_create_interrupter(struct xhci_sideband *sb, int num_seg,
 				 bool ip_autoclear, u32 imod_interval, int intr_num);
-- 
2.47.0.199.ga7371fff76-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v6 5/5] usb: host: enable sideband transfer during system sleep
  2024-11-06  8:32 [PATCH v6 0/5] Support system sleep with offloaded usb transfers Guan-Yu Lin
                   ` (3 preceding siblings ...)
  2024-11-06  8:32 ` [PATCH v6 4/5] xhci: sideband: add api to trace sideband usage Guan-Yu Lin
@ 2024-11-06  8:32 ` Guan-Yu Lin
  2024-11-06  8:38   ` Guan-Yu Lin
  2024-11-07 23:51 ` [PATCH v6 0/5] Support system sleep with offloaded usb transfers Thinh Nguyen
  5 siblings, 1 reply; 8+ messages in thread
From: Guan-Yu Lin @ 2024-11-06  8:32 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, mathias.nyman, stern, sumit.garg, dianders,
	kekrby, oneukum, yajun.deng, niko.mauno, christophe.jaillet, tj,
	stanley_chang, andreyknvl, quic_jjohnson, ricardo
  Cc: linux-usb, linux-kernel, Guan-Yu Lin

Sharing a USB controller with another entity via xhci-sideband driver
creates power management complexities. To prevent the USB controller
from being inadvertently deactivated while in use by the other entity, a
usage-count based mechanism is implemented. This allows the system to
manage power effectively, ensuring the controller remains available
whenever needed.

Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
---
 drivers/usb/core/driver.c    | 10 ++++++++++
 drivers/usb/dwc3/core.c      | 20 ++++++++++++++++++++
 drivers/usb/dwc3/core.h      |  1 +
 drivers/usb/host/xhci-plat.c | 10 ++++++++++
 include/linux/usb/hcd.h      |  7 +++++++
 5 files changed, 48 insertions(+)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index e53cb4c267b3..e5bb26e6c71a 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 (msg.event == PM_EVENT_SUSPEND && usb_sideband_check(udev)) {
+		dev_dbg(dev, "device accessed via sideband\n");
+		return 0;
+	}
+
 	unbind_no_pm_drivers_interfaces(udev);
 
 	/* From now on we are sure all drivers support suspend/resume
@@ -1619,6 +1624,11 @@ int usb_resume(struct device *dev, pm_message_t msg)
 	struct usb_device	*udev = to_usb_device(dev);
 	int			status;
 
+	if (msg.event == PM_EVENT_RESUME && usb_sideband_check(udev)) {
+		dev_dbg(dev, "device accessed via sideband\n");
+		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/dwc3/core.c b/drivers/usb/dwc3/core.c
index 2fdafbcbe44c..d85c68d5eba4 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2550,8 +2550,18 @@ static int dwc3_runtime_idle(struct device *dev)
 static int dwc3_suspend(struct device *dev)
 {
 	struct dwc3	*dwc = dev_get_drvdata(dev);
+	struct platform_device *xhci = dwc->xhci;
+	struct usb_hcd  *hcd;
 	int		ret;
 
+	if (xhci) {
+		hcd = dev_get_drvdata(&xhci->dev);
+		if (xhci_sideband_check(hcd)) {
+			dev_dbg(dev, "device accessed via sideband\n");
+			return 0;
+		}
+	}
+
 	ret = dwc3_suspend_common(dwc, PMSG_SUSPEND);
 	if (ret)
 		return ret;
@@ -2564,8 +2574,18 @@ static int dwc3_suspend(struct device *dev)
 static int dwc3_resume(struct device *dev)
 {
 	struct dwc3	*dwc = dev_get_drvdata(dev);
+	struct platform_device *xhci = dwc->xhci;
+	struct usb_hcd  *hcd;
 	int		ret;
 
+	if (xhci) {
+		hcd = dev_get_drvdata(&xhci->dev);
+		if (xhci_sideband_check(hcd)) {
+			dev_dbg(dev, "device accessed via sideband\n");
+			return 0;
+		}
+	}
+
 	pinctrl_pm_select_default_state(dev);
 
 	pm_runtime_disable(dev);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 80047d0df179..a585e9d80e59 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -26,6 +26,7 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/otg.h>
+#include <linux/usb/hcd.h>
 #include <linux/usb/role.h>
 #include <linux/ulpi/interface.h>
 
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 6e49ef1908eb..5fdbdf0c7f1a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -456,6 +456,11 @@ static int xhci_plat_suspend_common(struct device *dev, struct pm_message pmsg)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	int ret;
 
+	if (pmsg.event == PM_EVENT_SUSPEND && xhci_sideband_check(hcd)) {
+		dev_dbg(dev, "device accessed via sideband\n");
+		return 0;
+	}
+
 	if (pm_runtime_suspended(dev))
 		pm_runtime_resume(dev);
 
@@ -499,6 +504,11 @@ static int xhci_plat_resume_common(struct device *dev, struct pm_message pmsg)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	int ret;
 
+	if (pmsg.event == PM_EVENT_RESUME && xhci_sideband_check(hcd)) {
+		dev_dbg(dev, "device accessed via sideband\n");
+		return 0;
+	}
+
 	if (!device_may_wakeup(dev) && (xhci->quirks & XHCI_SUSPEND_RESUME_CLKS)) {
 		ret = clk_prepare_enable(xhci->clk);
 		if (ret)
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 9867c178d188..b22d25ccdf7c 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -772,6 +772,13 @@ extern struct rw_semaphore ehci_cf_port_reset_rwsem;
 #define USB_EHCI_LOADED		2
 extern unsigned long usb_hcds_loaded;
 
+#if IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND)
+extern bool xhci_sideband_check(struct usb_hcd *hcd);
+#else
+static inline bool xhci_sideband_check(struct usb_hcd *hcd)
+{ return false; }
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif /* __USB_CORE_HCD_H */
-- 
2.47.0.199.ga7371fff76-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 5/5] usb: host: enable sideband transfer during system sleep
  2024-11-06  8:32 ` [PATCH v6 5/5] usb: host: enable sideband transfer during system sleep Guan-Yu Lin
@ 2024-11-06  8:38   ` Guan-Yu Lin
  0 siblings, 0 replies; 8+ messages in thread
From: Guan-Yu Lin @ 2024-11-06  8:38 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, mathias.nyman, stern, sumit.garg, dianders,
	kekrby, oneukum, yajun.deng, niko.mauno, christophe.jaillet, tj,
	stanley_chang, andreyknvl, quic_jjohnson, ricardo
  Cc: linux-usb, linux-kernel

On Wed, Nov 6, 2024 at 4:35 PM Guan-Yu Lin <guanyulin@google.com> wrote:
>
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index e53cb4c267b3..e5bb26e6c71a 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 (msg.event == PM_EVENT_SUSPEND && usb_sideband_check(udev)) {
> +               dev_dbg(dev, "device accessed via sideband\n");
> +               return 0;
> +       }
> +
>         unbind_no_pm_drivers_interfaces(udev);
>
>         /* From now on we are sure all drivers support suspend/resume
> @@ -1619,6 +1624,11 @@ int usb_resume(struct device *dev, pm_message_t msg)
>         struct usb_device       *udev = to_usb_device(dev);
>         int                     status;
>
> +       if (msg.event == PM_EVENT_RESUME && usb_sideband_check(udev)) {
> +               dev_dbg(dev, "device accessed via sideband\n");
> +               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,

In v5, Greg points out the race window between checking sideband
activity and handling power management of usb devices. We should
consider a lock mechanism to address the race window. Given that the
design hasn't locked down and the race window might change from time
to time. I'll address this after the discussion of suspending USB
devices/interfaces has converged.

In addition, Alan suggests to only keep USB devices active but suspend
the USB interfaces. However, hub events and key events require active
USB interfaces to function. In the sideband model, the sideband driver
only handles USB transfers on specific endpoints, leaving other
functionalities like connection changes and key events to the Linux
USB kernel drivers. Therefore, a potential design modification is to
shift the sideband model's focus from voting for USB devices to voting
for USB interfaces. This way, the driver could selectively hold
necessary interfaces active during system suspend. This adjustment
accommodates use cases where specific interfaces must remain active to
support overall USB functionality when partial interfaces are
offloaded to a sideband.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v6 0/5] Support system sleep with offloaded usb transfers
  2024-11-06  8:32 [PATCH v6 0/5] Support system sleep with offloaded usb transfers Guan-Yu Lin
                   ` (4 preceding siblings ...)
  2024-11-06  8:32 ` [PATCH v6 5/5] usb: host: enable sideband transfer during system sleep Guan-Yu Lin
@ 2024-11-07 23:51 ` Thinh Nguyen
  5 siblings, 0 replies; 8+ messages in thread
From: Thinh Nguyen @ 2024-11-07 23:51 UTC (permalink / raw)
  To: Guan-Yu Lin
  Cc: gregkh@linuxfoundation.org, Thinh Nguyen, mathias.nyman@intel.com,
	stern@rowland.harvard.edu, sumit.garg@linaro.org,
	dianders@chromium.org, kekrby@gmail.com, oneukum@suse.com,
	yajun.deng@linux.dev, niko.mauno@vaisala.com,
	christophe.jaillet@wanadoo.fr, tj@kernel.org,
	stanley_chang@realtek.com, andreyknvl@gmail.com,
	quic_jjohnson@quicinc.com, ricardo@marliere.net,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org

Hi Guan-Yu,

On Wed, Nov 06, 2024, Guan-Yu Lin wrote:
> Wesley Cheng and Mathias Nyman's USB offload design enables a co-processor
> to handle some USB transfers, potentially allowing the main system to
> sleep and save power. However, Linux's power management system halts the
> USB host controller when the main system isn't managing any USB transfers.
> To address this, the proposal modifies the system to recognize offloaded
> USB transfers and manage power accordingly.
> 
> This involves two key steps:
> 1. Transfer Status Tracking: Propose xhci_sideband_get and
> xhci_sideband_put to track USB transfers on the co-processor, ensuring the
> system is aware of any ongoing activity.
> 2. Power Management Adjustment:  Modifications to the USB driver stack
> (dwc3 controller driver, xhci host controller driver, and USB device
> drivers) allow the system to sleep without disrupting co-processor managed
> USB transfers. This involves adding conditional checks to bypass some
> power management operations.
> 
> patches depends on series "Introduce QC USB SND audio offloading support" 
> https://urldefense.com/v3/__https://lore.kernel.org/lkml/20240925010000.2231406-11-quic_wcheng@quicinc.com/T/__;!!A4F2R9G_pg!YpthUIdOkNF85SqaTIkixxzfpTvUMjWaP_-kFI_ie1biOtI2DXkIOZluZeer0zv5nRWloMT4qG6Vrw7fJHLOkZU$ 
> 
> changelog
> ----------
> Changes in v6:
> - Fix build errors when CONFIG_USB_XHCI_SIDEBAND is disabled.
> - Explicitly specify the data structure of the drvdata refereced in
>   dwc3_suspend(), dwc3_resume().
> - Move the initialization of counters to the patches introducing them.
> 
> Changes in v5:
> - Walk through the USB children in usb_sideband_check() to determine the
>   sideband activity under the specific USB device. 
> - Replace atomic_t by refcount_t.
> - Reduce logs by using dev_dbg & remove __func__.
> 
> Changes in v4:
> - Isolate the feature into USB driver stack.
> - Integrate with series "Introduce QC USB SND audio offloading support"
> 
> Changes in v3:
> - Integrate the feature with the pm core framework.
> 
> Changes in v2:
> - Cosmetics changes on coding style.
> 
> [v3] PM / core: conditionally skip system pm in device/driver model
> [v2] usb: host: enable suspend-to-RAM control in userspace
> [v1] [RFC] usb: host: Allow userspace to control usb suspend flows
> ---
> 
> Guan-Yu Lin (5):
>   usb: dwc3: separate dev_pm_ops for each pm_event
>   usb: xhci-plat: separate dev_pm_ops for each pm_event
>   usb: add apis for sideband usage tracking
>   xhci: sideband: add api to trace sideband usage
>   usb: host: enable sideband transfer during system sleep
> 
>  drivers/usb/core/driver.c         | 87 +++++++++++++++++++++++++++
>  drivers/usb/core/hcd.c            |  4 ++
>  drivers/usb/core/usb.c            |  4 ++
>  drivers/usb/dwc3/core.c           | 97 ++++++++++++++++++++++++++++++-
>  drivers/usb/dwc3/core.h           |  1 +
>  drivers/usb/host/xhci-plat.c      | 38 ++++++++++--
>  drivers/usb/host/xhci-sideband.c  | 92 +++++++++++++++++++++++++++++
>  include/linux/usb.h               | 20 +++++++
>  include/linux/usb/hcd.h           | 13 +++++
>  include/linux/usb/xhci-sideband.h |  5 ++
>  10 files changed, 356 insertions(+), 5 deletions(-)
> 
> -- 
> 2.47.0.199.ga7371fff76-goog
> 

This series is highly dependent on the series "Introduce QC USB SND
audio offloading support". Since that series is still in review and
under active discussion, I'll wait until that series is approved and
merged before reviewing dwc3 related changes.

Thanks,
Thinh

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-11-07 23:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-06  8:32 [PATCH v6 0/5] Support system sleep with offloaded usb transfers Guan-Yu Lin
2024-11-06  8:32 ` [PATCH v6 1/5] usb: dwc3: separate dev_pm_ops for each pm_event Guan-Yu Lin
2024-11-06  8:32 ` [PATCH v6 2/5] usb: xhci-plat: " Guan-Yu Lin
2024-11-06  8:32 ` [PATCH v6 3/5] usb: add apis for sideband usage tracking Guan-Yu Lin
2024-11-06  8:32 ` [PATCH v6 4/5] xhci: sideband: add api to trace sideband usage Guan-Yu Lin
2024-11-06  8:32 ` [PATCH v6 5/5] usb: host: enable sideband transfer during system sleep Guan-Yu Lin
2024-11-06  8:38   ` Guan-Yu Lin
2024-11-07 23:51 ` [PATCH v6 0/5] Support system sleep with offloaded usb transfers Thinh Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox