| Message ID | 20241106083501.408074-6-guanyulin@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support system sleep with offloaded usb transfers | expand |
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.
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 */
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(+)