* [PATCH v4 1/2] usb: host: plat: Enable xhci plat runtime PM @ 2016-11-28 6:43 Baolin Wang 2016-11-28 6:43 ` [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume Baolin Wang 2016-12-08 7:04 ` [PATCH v4 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang 0 siblings, 2 replies; 10+ messages in thread From: Baolin Wang @ 2016-11-28 6:43 UTC (permalink / raw) To: mathias.nyman, balbi Cc: gregkh, broonie, linux-usb, linux-kernel, baolin.wang Enable the xhci plat runtime PM for parent device to suspend/resume xhci. Also call pm_runtime_get_noresume() in probe() function in case the parent device doesn't call suspend/resume callback by runtime PM now. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- Changes since v3: - Fix kbuild error. Changes since v2: - Add pm_runtime_get_noresume() in probe() function. - Add pm_runtime_set_suspended()/pm_runtime_put_noidle() in remove() function. Changes since v1: - No updates. --- drivers/usb/host/xhci-plat.c | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index ed56bf9..5805c6a 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -246,6 +246,10 @@ static int xhci_plat_probe(struct platform_device *pdev) if (ret) goto dealloc_usb2_hcd; + pm_runtime_get_noresume(&pdev->dev); + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->dev); + return 0; @@ -274,6 +278,10 @@ static int xhci_plat_remove(struct platform_device *dev) struct xhci_hcd *xhci = hcd_to_xhci(hcd); struct clk *clk = xhci->clk; + pm_runtime_set_suspended(&dev->dev); + pm_runtime_put_noidle(&dev->dev); + pm_runtime_disable(&dev->dev); + usb_remove_hcd(xhci->shared_hcd); usb_phy_shutdown(hcd->usb_phy); @@ -311,14 +319,37 @@ static int xhci_plat_resume(struct device *dev) return xhci_resume(xhci, 0); } +#endif /* CONFIG_PM_SLEEP */ + +#ifdef CONFIG_PM +static int xhci_plat_runtime_suspend(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct xhci_hcd *xhci = hcd_to_xhci(hcd); + + return xhci_suspend(xhci, device_may_wakeup(dev)); +} + +static int xhci_plat_runtime_resume(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct xhci_hcd *xhci = hcd_to_xhci(hcd); + + return xhci_resume(xhci, 0); +} + +static int xhci_plat_runtime_idle(struct device *dev) +{ + return 0; +} +#endif /* CONFIG_PM */ static const struct dev_pm_ops xhci_plat_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume) + + SET_RUNTIME_PM_OPS(xhci_plat_runtime_suspend, xhci_plat_runtime_resume, + xhci_plat_runtime_idle) }; -#define DEV_PM_OPS (&xhci_plat_pm_ops) -#else -#define DEV_PM_OPS NULL -#endif /* CONFIG_PM */ static const struct acpi_device_id usb_xhci_acpi_match[] = { /* XHCI-compliant USB Controller */ @@ -332,7 +363,7 @@ static int xhci_plat_resume(struct device *dev) .remove = xhci_plat_remove, .driver = { .name = "xhci-hcd", - .pm = DEV_PM_OPS, + .pm = &xhci_plat_pm_ops, .of_match_table = of_match_ptr(usb_xhci_of_match), .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match), }, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume 2016-11-28 6:43 [PATCH v4 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang @ 2016-11-28 6:43 ` Baolin Wang 2016-12-08 7:05 ` Baolin Wang 2016-12-08 7:04 ` [PATCH v4 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang 1 sibling, 1 reply; 10+ messages in thread From: Baolin Wang @ 2016-11-28 6:43 UTC (permalink / raw) To: mathias.nyman, balbi Cc: gregkh, broonie, linux-usb, linux-kernel, baolin.wang For some mobile devices with strict power management, we also want to suspend the host when the slave is detached for power saving. Thus we add the host suspend/resume functions to support this requirement. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- Changes since v3: - No updates. Changes since v2: - Remove pm_children_suspended() and other unused macros. Changes since v1: - Add pm_runtime.h head file to avoid kbuild error. --- drivers/usb/dwc3/Kconfig | 7 +++++++ drivers/usb/dwc3/core.c | 26 +++++++++++++++++++++++++- drivers/usb/dwc3/core.h | 15 +++++++++++++++ drivers/usb/dwc3/host.c | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index a45b4f1..47bb2f3 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE endchoice +config USB_DWC3_HOST_SUSPEND + bool "Choose if the DWC3 host (xhci) can be suspend/resume" + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y + help + We can suspend the host when the slave is detached for power saving, + and resume the host when one slave is attached. + comment "Platform Glue Driver Support" config USB_DWC3_OMAP diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 9a4a5e4..7ad4bc3 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) pm_runtime_use_autosuspend(dev); pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); pm_runtime_enable(dev); + pm_suspend_ignore_children(dev, true); ret = pm_runtime_get_sync(dev); if (ret < 0) goto err1; @@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev) static int dwc3_suspend_common(struct dwc3 *dwc) { unsigned long flags; + int ret; switch (dwc->dr_mode) { case USB_DR_MODE_PERIPHERAL: + spin_lock_irqsave(&dwc->lock, flags); + dwc3_gadget_suspend(dwc); + spin_unlock_irqrestore(&dwc->lock, flags); + break; case USB_DR_MODE_OTG: + ret = dwc3_host_suspend(dwc); + if (ret) + return ret; + spin_lock_irqsave(&dwc->lock, flags); dwc3_gadget_suspend(dwc); spin_unlock_irqrestore(&dwc->lock, flags); break; case USB_DR_MODE_HOST: + ret = dwc3_host_suspend(dwc); + if (ret) + return ret; default: /* do nothing */ break; @@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc) switch (dwc->dr_mode) { case USB_DR_MODE_PERIPHERAL: + spin_lock_irqsave(&dwc->lock, flags); + dwc3_gadget_resume(dwc); + spin_unlock_irqrestore(&dwc->lock, flags); + break; case USB_DR_MODE_OTG: + ret = dwc3_host_resume(dwc); + if (ret) + return ret; + spin_lock_irqsave(&dwc->lock, flags); dwc3_gadget_resume(dwc); spin_unlock_irqrestore(&dwc->lock, flags); - /* FALLTHROUGH */ + break; case USB_DR_MODE_HOST: + ret = dwc3_host_resume(dwc); + if (ret) + return ret; default: /* do nothing */ break; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index b585a30..db41908 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc) { } #endif +#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND) +int dwc3_host_suspend(struct dwc3 *dwc); +int dwc3_host_resume(struct dwc3 *dwc); +#else +static inline int dwc3_host_suspend(struct dwc3 *dwc) +{ + return 0; +} + +static inline int dwc3_host_resume(struct dwc3 *dwc) +{ + return 0; +} +#endif + #endif /* __DRIVERS_USB_DWC3_CORE_H */ diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index ed82464..8e5309d6 100644 --- a/drivers/usb/dwc3/host.c +++ b/drivers/usb/dwc3/host.c @@ -16,6 +16,7 @@ */ #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include "core.h" @@ -130,3 +131,39 @@ void dwc3_host_exit(struct dwc3 *dwc) dev_name(&dwc->xhci->dev)); platform_device_unregister(dwc->xhci); } + +#ifdef CONFIG_USB_DWC3_HOST_SUSPEND +int dwc3_host_suspend(struct dwc3 *dwc) +{ + struct device *xhci = &dwc->xhci->dev; + int ret; + + /* + * Note: if we get the -EBUSY, which means the xHCI children devices are + * not in suspend state yet, the glue layer need to wait for a while and + * try to suspend xHCI device again. + */ + ret = pm_runtime_put_sync(xhci); + if (ret) { + dev_err(xhci, "failed to suspend xHCI device\n"); + return ret; + } + + return 0; +} + +int dwc3_host_resume(struct dwc3 *dwc) +{ + struct device *xhci = &dwc->xhci->dev; + int ret; + + /* Resume the xHCI device synchronously. */ + ret = pm_runtime_get_sync(xhci); + if (ret) { + dev_err(xhci, "failed to resume xHCI device\n"); + return ret; + } + + return 0; +} +#endif -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume 2016-11-28 6:43 ` [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume Baolin Wang @ 2016-12-08 7:05 ` Baolin Wang 2016-12-08 9:40 ` Felipe Balbi 0 siblings, 1 reply; 10+ messages in thread From: Baolin Wang @ 2016-12-08 7:05 UTC (permalink / raw) To: mathias.nyman, Felipe Balbi; +Cc: Greg KH, Mark Brown, USB, LKML, Baolin Wang Hi Felipe, On 28 November 2016 at 14:43, Baolin Wang <baolin.wang@linaro.org> wrote: > For some mobile devices with strict power management, we also want to suspend > the host when the slave is detached for power saving. Thus we add the host > suspend/resume functions to support this requirement. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > --- > Changes since v3: > - No updates. > > Changes since v2: > - Remove pm_children_suspended() and other unused macros. > > Changes since v1: > - Add pm_runtime.h head file to avoid kbuild error. > --- > drivers/usb/dwc3/Kconfig | 7 +++++++ > drivers/usb/dwc3/core.c | 26 +++++++++++++++++++++++++- > drivers/usb/dwc3/core.h | 15 +++++++++++++++ > drivers/usb/dwc3/host.c | 37 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 84 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig > index a45b4f1..47bb2f3 100644 > --- a/drivers/usb/dwc3/Kconfig > +++ b/drivers/usb/dwc3/Kconfig > @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE > > endchoice > > +config USB_DWC3_HOST_SUSPEND > + bool "Choose if the DWC3 host (xhci) can be suspend/resume" > + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y > + help > + We can suspend the host when the slave is detached for power saving, > + and resume the host when one slave is attached. > + > comment "Platform Glue Driver Support" > > config USB_DWC3_OMAP > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 9a4a5e4..7ad4bc3 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) > pm_runtime_use_autosuspend(dev); > pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); > pm_runtime_enable(dev); > + pm_suspend_ignore_children(dev, true); > ret = pm_runtime_get_sync(dev); > if (ret < 0) > goto err1; > @@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev) > static int dwc3_suspend_common(struct dwc3 *dwc) > { > unsigned long flags; > + int ret; > > switch (dwc->dr_mode) { > case USB_DR_MODE_PERIPHERAL: > + spin_lock_irqsave(&dwc->lock, flags); > + dwc3_gadget_suspend(dwc); > + spin_unlock_irqrestore(&dwc->lock, flags); > + break; > case USB_DR_MODE_OTG: > + ret = dwc3_host_suspend(dwc); > + if (ret) > + return ret; > + > spin_lock_irqsave(&dwc->lock, flags); > dwc3_gadget_suspend(dwc); > spin_unlock_irqrestore(&dwc->lock, flags); > break; > case USB_DR_MODE_HOST: > + ret = dwc3_host_suspend(dwc); > + if (ret) > + return ret; > default: > /* do nothing */ > break; > @@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc) > > switch (dwc->dr_mode) { > case USB_DR_MODE_PERIPHERAL: > + spin_lock_irqsave(&dwc->lock, flags); > + dwc3_gadget_resume(dwc); > + spin_unlock_irqrestore(&dwc->lock, flags); > + break; > case USB_DR_MODE_OTG: > + ret = dwc3_host_resume(dwc); > + if (ret) > + return ret; > + > spin_lock_irqsave(&dwc->lock, flags); > dwc3_gadget_resume(dwc); > spin_unlock_irqrestore(&dwc->lock, flags); > - /* FALLTHROUGH */ > + break; > case USB_DR_MODE_HOST: > + ret = dwc3_host_resume(dwc); > + if (ret) > + return ret; > default: > /* do nothing */ > break; > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index b585a30..db41908 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc) > { } > #endif > > +#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND) > +int dwc3_host_suspend(struct dwc3 *dwc); > +int dwc3_host_resume(struct dwc3 *dwc); > +#else > +static inline int dwc3_host_suspend(struct dwc3 *dwc) > +{ > + return 0; > +} > + > +static inline int dwc3_host_resume(struct dwc3 *dwc) > +{ > + return 0; > +} > +#endif > + > #endif /* __DRIVERS_USB_DWC3_CORE_H */ > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c > index ed82464..8e5309d6 100644 > --- a/drivers/usb/dwc3/host.c > +++ b/drivers/usb/dwc3/host.c > @@ -16,6 +16,7 @@ > */ > > #include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > > #include "core.h" > > @@ -130,3 +131,39 @@ void dwc3_host_exit(struct dwc3 *dwc) > dev_name(&dwc->xhci->dev)); > platform_device_unregister(dwc->xhci); > } > + > +#ifdef CONFIG_USB_DWC3_HOST_SUSPEND > +int dwc3_host_suspend(struct dwc3 *dwc) > +{ > + struct device *xhci = &dwc->xhci->dev; > + int ret; > + > + /* > + * Note: if we get the -EBUSY, which means the xHCI children devices are > + * not in suspend state yet, the glue layer need to wait for a while and > + * try to suspend xHCI device again. > + */ > + ret = pm_runtime_put_sync(xhci); > + if (ret) { > + dev_err(xhci, "failed to suspend xHCI device\n"); > + return ret; > + } > + > + return 0; > +} > + > +int dwc3_host_resume(struct dwc3 *dwc) > +{ > + struct device *xhci = &dwc->xhci->dev; > + int ret; > + > + /* Resume the xHCI device synchronously. */ > + ret = pm_runtime_get_sync(xhci); > + if (ret) { > + dev_err(xhci, "failed to resume xHCI device\n"); > + return ret; > + } > + > + return 0; > +} > +#endif > -- > 1.7.9.5 > How do you think this patch and do you have any suggestion? Thanks. -- Baolin.wang Best Regards ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume 2016-12-08 7:05 ` Baolin Wang @ 2016-12-08 9:40 ` Felipe Balbi 2016-12-08 10:20 ` Baolin Wang 0 siblings, 1 reply; 10+ messages in thread From: Felipe Balbi @ 2016-12-08 9:40 UTC (permalink / raw) To: Baolin Wang, mathias.nyman; +Cc: Greg KH, Mark Brown, USB, LKML, Baolin Wang [-- Attachment #1: Type: text/plain, Size: 6656 bytes --] Hi, Baolin Wang <baolin.wang@linaro.org> writes: > Hi Felipe, > > On 28 November 2016 at 14:43, Baolin Wang <baolin.wang@linaro.org> wrote: >> For some mobile devices with strict power management, we also want to suspend >> the host when the slave is detached for power saving. Thus we add the host >> suspend/resume functions to support this requirement. >> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >> --- >> Changes since v3: >> - No updates. >> >> Changes since v2: >> - Remove pm_children_suspended() and other unused macros. >> >> Changes since v1: >> - Add pm_runtime.h head file to avoid kbuild error. >> --- >> drivers/usb/dwc3/Kconfig | 7 +++++++ >> drivers/usb/dwc3/core.c | 26 +++++++++++++++++++++++++- >> drivers/usb/dwc3/core.h | 15 +++++++++++++++ >> drivers/usb/dwc3/host.c | 37 +++++++++++++++++++++++++++++++++++++ >> 4 files changed, 84 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >> index a45b4f1..47bb2f3 100644 >> --- a/drivers/usb/dwc3/Kconfig >> +++ b/drivers/usb/dwc3/Kconfig >> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE >> >> endchoice >> >> +config USB_DWC3_HOST_SUSPEND >> + bool "Choose if the DWC3 host (xhci) can be suspend/resume" >> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y why do you need another Kconfig for this? Just enable it already :-p >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 9a4a5e4..7ad4bc3 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) >> pm_runtime_use_autosuspend(dev); >> pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); >> pm_runtime_enable(dev); >> + pm_suspend_ignore_children(dev, true); why do you need this? >> @@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev) >> static int dwc3_suspend_common(struct dwc3 *dwc) >> { >> unsigned long flags; >> + int ret; >> >> switch (dwc->dr_mode) { >> case USB_DR_MODE_PERIPHERAL: >> + spin_lock_irqsave(&dwc->lock, flags); >> + dwc3_gadget_suspend(dwc); >> + spin_unlock_irqrestore(&dwc->lock, flags); >> + break; >> case USB_DR_MODE_OTG: >> + ret = dwc3_host_suspend(dwc); >> + if (ret) >> + return ret; >> + >> spin_lock_irqsave(&dwc->lock, flags); >> dwc3_gadget_suspend(dwc); >> spin_unlock_irqrestore(&dwc->lock, flags); >> break; >> case USB_DR_MODE_HOST: >> + ret = dwc3_host_suspend(dwc); >> + if (ret) >> + return ret; >> default: >> /* do nothing */ >> break; >> @@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc) >> >> switch (dwc->dr_mode) { >> case USB_DR_MODE_PERIPHERAL: >> + spin_lock_irqsave(&dwc->lock, flags); >> + dwc3_gadget_resume(dwc); >> + spin_unlock_irqrestore(&dwc->lock, flags); >> + break; >> case USB_DR_MODE_OTG: >> + ret = dwc3_host_resume(dwc); >> + if (ret) >> + return ret; >> + >> spin_lock_irqsave(&dwc->lock, flags); >> dwc3_gadget_resume(dwc); >> spin_unlock_irqrestore(&dwc->lock, flags); >> - /* FALLTHROUGH */ >> + break; >> case USB_DR_MODE_HOST: >> + ret = dwc3_host_resume(dwc); >> + if (ret) >> + return ret; >> default: >> /* do nothing */ >> break; >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >> index b585a30..db41908 100644 >> --- a/drivers/usb/dwc3/core.h >> +++ b/drivers/usb/dwc3/core.h >> @@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc) >> { } >> #endif >> >> +#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND) >> +int dwc3_host_suspend(struct dwc3 *dwc); >> +int dwc3_host_resume(struct dwc3 *dwc); >> +#else >> +static inline int dwc3_host_suspend(struct dwc3 *dwc) >> +{ >> + return 0; >> +} >> + >> +static inline int dwc3_host_resume(struct dwc3 *dwc) >> +{ >> + return 0; >> +} >> +#endif >> + >> #endif /* __DRIVERS_USB_DWC3_CORE_H */ >> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c >> index ed82464..8e5309d6 100644 >> --- a/drivers/usb/dwc3/host.c >> +++ b/drivers/usb/dwc3/host.c >> @@ -16,6 +16,7 @@ >> */ >> >> #include <linux/platform_device.h> >> +#include <linux/pm_runtime.h> >> >> #include "core.h" >> >> @@ -130,3 +131,39 @@ void dwc3_host_exit(struct dwc3 *dwc) >> dev_name(&dwc->xhci->dev)); >> platform_device_unregister(dwc->xhci); >> } >> + >> +#ifdef CONFIG_USB_DWC3_HOST_SUSPEND >> +int dwc3_host_suspend(struct dwc3 *dwc) >> +{ >> + struct device *xhci = &dwc->xhci->dev; >> + int ret; >> + >> + /* >> + * Note: if we get the -EBUSY, which means the xHCI children devices are >> + * not in suspend state yet, the glue layer need to wait for a while and >> + * try to suspend xHCI device again. >> + */ >> + ret = pm_runtime_put_sync(xhci); >> + if (ret) { >> + dev_err(xhci, "failed to suspend xHCI device\n"); >> + return ret; >> + } return pm_runtime_put_sync() is probably enough here. >> + >> + return 0; >> +} >> + >> +int dwc3_host_resume(struct dwc3 *dwc) >> +{ >> + struct device *xhci = &dwc->xhci->dev; >> + int ret; >> + >> + /* Resume the xHCI device synchronously. */ >> + ret = pm_runtime_get_sync(xhci); likewise. >> + if (ret) { >> + dev_err(xhci, "failed to resume xHCI device\n"); >> + return ret; >> + } >> + >> + return 0; >> +} >> +#endif >> -- >> 1.7.9.5 >> > > How do you think this patch and do you have any suggestion? Thanks. > > -- > Baolin.wang > Best Regards > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume 2016-12-08 9:40 ` Felipe Balbi @ 2016-12-08 10:20 ` Baolin Wang 2016-12-08 11:02 ` Felipe Balbi 0 siblings, 1 reply; 10+ messages in thread From: Baolin Wang @ 2016-12-08 10:20 UTC (permalink / raw) To: Felipe Balbi; +Cc: mathias.nyman, Greg KH, Mark Brown, USB, LKML Hi, On 8 December 2016 at 17:40, Felipe Balbi <balbi@kernel.org> wrote: > > Hi, > > Baolin Wang <baolin.wang@linaro.org> writes: >> Hi Felipe, >> >> On 28 November 2016 at 14:43, Baolin Wang <baolin.wang@linaro.org> wrote: >>> For some mobile devices with strict power management, we also want to suspend >>> the host when the slave is detached for power saving. Thus we add the host >>> suspend/resume functions to support this requirement. >>> >>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >>> --- >>> Changes since v3: >>> - No updates. >>> >>> Changes since v2: >>> - Remove pm_children_suspended() and other unused macros. >>> >>> Changes since v1: >>> - Add pm_runtime.h head file to avoid kbuild error. >>> --- >>> drivers/usb/dwc3/Kconfig | 7 +++++++ >>> drivers/usb/dwc3/core.c | 26 +++++++++++++++++++++++++- >>> drivers/usb/dwc3/core.h | 15 +++++++++++++++ >>> drivers/usb/dwc3/host.c | 37 +++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 84 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >>> index a45b4f1..47bb2f3 100644 >>> --- a/drivers/usb/dwc3/Kconfig >>> +++ b/drivers/usb/dwc3/Kconfig >>> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE >>> >>> endchoice >>> >>> +config USB_DWC3_HOST_SUSPEND >>> + bool "Choose if the DWC3 host (xhci) can be suspend/resume" >>> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y > > why do you need another Kconfig for this? Just enable it already :-p I assume some platforms may do not need this feature. But okay, I can remove this kconfig and enable it. > >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 9a4a5e4..7ad4bc3 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) >>> pm_runtime_use_autosuspend(dev); >>> pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); >>> pm_runtime_enable(dev); >>> + pm_suspend_ignore_children(dev, true); > > why do you need this? Since the dwc3 device is the parent deive of xhci device, if we want to suspend dwc3 device, we need to suspend child device (xhci device) manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In pm_runtime_put_sync(), it will check if the children (xhci device) of dwc3 device have been in suspend state, if not we will suspend dwc3 device failed. We get/put the child device manually in parent device's runtime callback, we need to ignore the child device's runtime state, or it will failed due to the dependency. > >>> @@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev) >>> static int dwc3_suspend_common(struct dwc3 *dwc) >>> { >>> unsigned long flags; >>> + int ret; >>> >>> switch (dwc->dr_mode) { >>> case USB_DR_MODE_PERIPHERAL: >>> + spin_lock_irqsave(&dwc->lock, flags); >>> + dwc3_gadget_suspend(dwc); >>> + spin_unlock_irqrestore(&dwc->lock, flags); >>> + break; >>> case USB_DR_MODE_OTG: >>> + ret = dwc3_host_suspend(dwc); >>> + if (ret) >>> + return ret; >>> + >>> spin_lock_irqsave(&dwc->lock, flags); >>> dwc3_gadget_suspend(dwc); >>> spin_unlock_irqrestore(&dwc->lock, flags); >>> break; >>> case USB_DR_MODE_HOST: >>> + ret = dwc3_host_suspend(dwc); >>> + if (ret) >>> + return ret; >>> default: >>> /* do nothing */ >>> break; >>> @@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc) >>> >>> switch (dwc->dr_mode) { >>> case USB_DR_MODE_PERIPHERAL: >>> + spin_lock_irqsave(&dwc->lock, flags); >>> + dwc3_gadget_resume(dwc); >>> + spin_unlock_irqrestore(&dwc->lock, flags); >>> + break; >>> case USB_DR_MODE_OTG: >>> + ret = dwc3_host_resume(dwc); >>> + if (ret) >>> + return ret; >>> + >>> spin_lock_irqsave(&dwc->lock, flags); >>> dwc3_gadget_resume(dwc); >>> spin_unlock_irqrestore(&dwc->lock, flags); >>> - /* FALLTHROUGH */ >>> + break; >>> case USB_DR_MODE_HOST: >>> + ret = dwc3_host_resume(dwc); >>> + if (ret) >>> + return ret; >>> default: >>> /* do nothing */ >>> break; >>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >>> index b585a30..db41908 100644 >>> --- a/drivers/usb/dwc3/core.h >>> +++ b/drivers/usb/dwc3/core.h >>> @@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc) >>> { } >>> #endif >>> >>> +#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND) >>> +int dwc3_host_suspend(struct dwc3 *dwc); >>> +int dwc3_host_resume(struct dwc3 *dwc); >>> +#else >>> +static inline int dwc3_host_suspend(struct dwc3 *dwc) >>> +{ >>> + return 0; >>> +} >>> + >>> +static inline int dwc3_host_resume(struct dwc3 *dwc) >>> +{ >>> + return 0; >>> +} >>> +#endif >>> + >>> #endif /* __DRIVERS_USB_DWC3_CORE_H */ >>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c >>> index ed82464..8e5309d6 100644 >>> --- a/drivers/usb/dwc3/host.c >>> +++ b/drivers/usb/dwc3/host.c >>> @@ -16,6 +16,7 @@ >>> */ >>> >>> #include <linux/platform_device.h> >>> +#include <linux/pm_runtime.h> >>> >>> #include "core.h" >>> >>> @@ -130,3 +131,39 @@ void dwc3_host_exit(struct dwc3 *dwc) >>> dev_name(&dwc->xhci->dev)); >>> platform_device_unregister(dwc->xhci); >>> } >>> + >>> +#ifdef CONFIG_USB_DWC3_HOST_SUSPEND >>> +int dwc3_host_suspend(struct dwc3 *dwc) >>> +{ >>> + struct device *xhci = &dwc->xhci->dev; >>> + int ret; >>> + >>> + /* >>> + * Note: if we get the -EBUSY, which means the xHCI children devices are >>> + * not in suspend state yet, the glue layer need to wait for a while and >>> + * try to suspend xHCI device again. >>> + */ >>> + ret = pm_runtime_put_sync(xhci); >>> + if (ret) { >>> + dev_err(xhci, "failed to suspend xHCI device\n"); >>> + return ret; >>> + } > > return pm_runtime_put_sync() is probably enough here. OK. > >>> + >>> + return 0; >>> +} >>> + >>> +int dwc3_host_resume(struct dwc3 *dwc) >>> +{ >>> + struct device *xhci = &dwc->xhci->dev; >>> + int ret; >>> + >>> + /* Resume the xHCI device synchronously. */ >>> + ret = pm_runtime_get_sync(xhci); > > likewise. OK. I will fix these in next version. Thanks. > >>> + if (ret) { >>> + dev_err(xhci, "failed to resume xHCI device\n"); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> +#endif >>> -- >>> 1.7.9.5 >>> >> >> How do you think this patch and do you have any suggestion? Thanks. >> >> -- >> Baolin.wang >> Best Regards >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > balbi -- Baolin.wang Best Regards ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume 2016-12-08 10:20 ` Baolin Wang @ 2016-12-08 11:02 ` Felipe Balbi 2016-12-08 11:14 ` Baolin Wang 0 siblings, 1 reply; 10+ messages in thread From: Felipe Balbi @ 2016-12-08 11:02 UTC (permalink / raw) To: Baolin Wang; +Cc: mathias.nyman, Greg KH, Mark Brown, USB, LKML [-- Attachment #1: Type: text/plain, Size: 2980 bytes --] Hi, Baolin Wang <baolin.wang@linaro.org> writes: >>> On 28 November 2016 at 14:43, Baolin Wang <baolin.wang@linaro.org> wrote: >>>> For some mobile devices with strict power management, we also want to suspend >>>> the host when the slave is detached for power saving. Thus we add the host >>>> suspend/resume functions to support this requirement. >>>> >>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >>>> --- >>>> Changes since v3: >>>> - No updates. >>>> >>>> Changes since v2: >>>> - Remove pm_children_suspended() and other unused macros. >>>> >>>> Changes since v1: >>>> - Add pm_runtime.h head file to avoid kbuild error. >>>> --- >>>> drivers/usb/dwc3/Kconfig | 7 +++++++ >>>> drivers/usb/dwc3/core.c | 26 +++++++++++++++++++++++++- >>>> drivers/usb/dwc3/core.h | 15 +++++++++++++++ >>>> drivers/usb/dwc3/host.c | 37 +++++++++++++++++++++++++++++++++++++ >>>> 4 files changed, 84 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >>>> index a45b4f1..47bb2f3 100644 >>>> --- a/drivers/usb/dwc3/Kconfig >>>> +++ b/drivers/usb/dwc3/Kconfig >>>> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE >>>> >>>> endchoice >>>> >>>> +config USB_DWC3_HOST_SUSPEND >>>> + bool "Choose if the DWC3 host (xhci) can be suspend/resume" >>>> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y >> >> why do you need another Kconfig for this? Just enable it already :-p > > I assume some platforms may do not need this feature. But okay, I can > remove this kconfig and enable it. thanks :-) >>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>> index 9a4a5e4..7ad4bc3 100644 >>>> --- a/drivers/usb/dwc3/core.c >>>> +++ b/drivers/usb/dwc3/core.c >>>> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) >>>> pm_runtime_use_autosuspend(dev); >>>> pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); >>>> pm_runtime_enable(dev); >>>> + pm_suspend_ignore_children(dev, true); >> >> why do you need this? > > Since the dwc3 device is the parent deive of xhci device, if we want > to suspend dwc3 device, we need to suspend child device (xhci device) > manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In > pm_runtime_put_sync(), it will check if the children (xhci device) of > dwc3 device have been in suspend state, if not we will suspend dwc3 > device failed. > > We get/put the child device manually in parent device's runtime > callback, we need to ignore the child device's runtime state, or it > will failed due to the dependency. I see. Good explanation :-) There's one thing though, if you want to runtime suspend the gadget and dwc3 is working on peripheral mode, host side (XHCI) should already be runtime suspended because there's nothing attached to it. Why isn't it runtime suspended? -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume 2016-12-08 11:02 ` Felipe Balbi @ 2016-12-08 11:14 ` Baolin Wang 2016-12-08 17:52 ` Felipe Balbi 0 siblings, 1 reply; 10+ messages in thread From: Baolin Wang @ 2016-12-08 11:14 UTC (permalink / raw) To: Felipe Balbi; +Cc: mathias.nyman, Greg KH, Mark Brown, USB, LKML Hi, On 8 December 2016 at 19:02, Felipe Balbi <balbi@kernel.org> wrote: > > Hi, > > Baolin Wang <baolin.wang@linaro.org> writes: >>>> On 28 November 2016 at 14:43, Baolin Wang <baolin.wang@linaro.org> wrote: >>>>> For some mobile devices with strict power management, we also want to suspend >>>>> the host when the slave is detached for power saving. Thus we add the host >>>>> suspend/resume functions to support this requirement. >>>>> >>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >>>>> --- >>>>> Changes since v3: >>>>> - No updates. >>>>> >>>>> Changes since v2: >>>>> - Remove pm_children_suspended() and other unused macros. >>>>> >>>>> Changes since v1: >>>>> - Add pm_runtime.h head file to avoid kbuild error. >>>>> --- >>>>> drivers/usb/dwc3/Kconfig | 7 +++++++ >>>>> drivers/usb/dwc3/core.c | 26 +++++++++++++++++++++++++- >>>>> drivers/usb/dwc3/core.h | 15 +++++++++++++++ >>>>> drivers/usb/dwc3/host.c | 37 +++++++++++++++++++++++++++++++++++++ >>>>> 4 files changed, 84 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >>>>> index a45b4f1..47bb2f3 100644 >>>>> --- a/drivers/usb/dwc3/Kconfig >>>>> +++ b/drivers/usb/dwc3/Kconfig >>>>> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE >>>>> >>>>> endchoice >>>>> >>>>> +config USB_DWC3_HOST_SUSPEND >>>>> + bool "Choose if the DWC3 host (xhci) can be suspend/resume" >>>>> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y >>> >>> why do you need another Kconfig for this? Just enable it already :-p >> >> I assume some platforms may do not need this feature. But okay, I can >> remove this kconfig and enable it. > > thanks :-) > >>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>> index 9a4a5e4..7ad4bc3 100644 >>>>> --- a/drivers/usb/dwc3/core.c >>>>> +++ b/drivers/usb/dwc3/core.c >>>>> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) >>>>> pm_runtime_use_autosuspend(dev); >>>>> pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); >>>>> pm_runtime_enable(dev); >>>>> + pm_suspend_ignore_children(dev, true); >>> >>> why do you need this? >> >> Since the dwc3 device is the parent deive of xhci device, if we want >> to suspend dwc3 device, we need to suspend child device (xhci device) >> manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In >> pm_runtime_put_sync(), it will check if the children (xhci device) of >> dwc3 device have been in suspend state, if not we will suspend dwc3 >> device failed. >> >> We get/put the child device manually in parent device's runtime >> callback, we need to ignore the child device's runtime state, or it >> will failed due to the dependency. > > I see. Good explanation :-) > > There's one thing though, if you want to runtime suspend the gadget and > dwc3 is working on peripheral mode, host side (XHCI) should already be > runtime suspended because there's nothing attached to it. Why isn't it > runtime suspended? Since we have get the runtime count for xHCI device in xhci_plat_probe(), in case it will suspend automatically if some controllers do not want xHCI enters runtime suspend automatically. So the parent device (dwc3 device) need to get/put the xHCI device's runtime count to resume/suspend xHCI. -- Baolin.wang Best Regards ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume 2016-12-08 11:14 ` Baolin Wang @ 2016-12-08 17:52 ` Felipe Balbi 2016-12-09 3:23 ` Baolin Wang 0 siblings, 1 reply; 10+ messages in thread From: Felipe Balbi @ 2016-12-08 17:52 UTC (permalink / raw) To: Baolin Wang; +Cc: mathias.nyman, Greg KH, Mark Brown, USB, LKML Hi, Baolin Wang <baolin.wang@linaro.org> writes: >> Baolin Wang <baolin.wang@linaro.org> writes: >>>>> On 28 November 2016 at 14:43, Baolin Wang <baolin.wang@linaro.org> wrote: >>>>>> For some mobile devices with strict power management, we also want to suspend >>>>>> the host when the slave is detached for power saving. Thus we add the host >>>>>> suspend/resume functions to support this requirement. >>>>>> >>>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >>>>>> --- >>>>>> Changes since v3: >>>>>> - No updates. >>>>>> >>>>>> Changes since v2: >>>>>> - Remove pm_children_suspended() and other unused macros. >>>>>> >>>>>> Changes since v1: >>>>>> - Add pm_runtime.h head file to avoid kbuild error. >>>>>> --- >>>>>> drivers/usb/dwc3/Kconfig | 7 +++++++ >>>>>> drivers/usb/dwc3/core.c | 26 +++++++++++++++++++++++++- >>>>>> drivers/usb/dwc3/core.h | 15 +++++++++++++++ >>>>>> drivers/usb/dwc3/host.c | 37 +++++++++++++++++++++++++++++++++++++ >>>>>> 4 files changed, 84 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >>>>>> index a45b4f1..47bb2f3 100644 >>>>>> --- a/drivers/usb/dwc3/Kconfig >>>>>> +++ b/drivers/usb/dwc3/Kconfig >>>>>> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE >>>>>> >>>>>> endchoice >>>>>> >>>>>> +config USB_DWC3_HOST_SUSPEND >>>>>> + bool "Choose if the DWC3 host (xhci) can be suspend/resume" >>>>>> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y >>>> >>>> why do you need another Kconfig for this? Just enable it already :-p >>> >>> I assume some platforms may do not need this feature. But okay, I can >>> remove this kconfig and enable it. >> >> thanks :-) >> >>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>>> index 9a4a5e4..7ad4bc3 100644 >>>>>> --- a/drivers/usb/dwc3/core.c >>>>>> +++ b/drivers/usb/dwc3/core.c >>>>>> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) >>>>>> pm_runtime_use_autosuspend(dev); >>>>>> pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); >>>>>> pm_runtime_enable(dev); >>>>>> + pm_suspend_ignore_children(dev, true); >>>> >>>> why do you need this? >>> >>> Since the dwc3 device is the parent deive of xhci device, if we want >>> to suspend dwc3 device, we need to suspend child device (xhci device) >>> manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In >>> pm_runtime_put_sync(), it will check if the children (xhci device) of >>> dwc3 device have been in suspend state, if not we will suspend dwc3 >>> device failed. >>> >>> We get/put the child device manually in parent device's runtime >>> callback, we need to ignore the child device's runtime state, or it >>> will failed due to the dependency. >> >> I see. Good explanation :-) >> >> There's one thing though, if you want to runtime suspend the gadget and >> dwc3 is working on peripheral mode, host side (XHCI) should already be >> runtime suspended because there's nothing attached to it. Why isn't it >> runtime suspended? > > Since we have get the runtime count for xHCI device in > xhci_plat_probe(), in case it will suspend automatically if some > controllers do not want xHCI enters runtime suspend automatically. So > the parent device (dwc3 device) need to get/put the xHCI device's > runtime count to resume/suspend xHCI. IMHO, that's a bug in xhci-plat, not something dwc3 should work around. -- balbi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume 2016-12-08 17:52 ` Felipe Balbi @ 2016-12-09 3:23 ` Baolin Wang 0 siblings, 0 replies; 10+ messages in thread From: Baolin Wang @ 2016-12-09 3:23 UTC (permalink / raw) To: Felipe Balbi; +Cc: mathias.nyman, Greg KH, Mark Brown, USB, LKML Hi, On 9 December 2016 at 01:52, Felipe Balbi <balbi@kernel.org> wrote: > > Hi, > > Baolin Wang <baolin.wang@linaro.org> writes: >>> Baolin Wang <baolin.wang@linaro.org> writes: >>>>>> On 28 November 2016 at 14:43, Baolin Wang <baolin.wang@linaro.org> wrote: >>>>>>> For some mobile devices with strict power management, we also want to suspend >>>>>>> the host when the slave is detached for power saving. Thus we add the host >>>>>>> suspend/resume functions to support this requirement. >>>>>>> >>>>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >>>>>>> --- >>>>>>> Changes since v3: >>>>>>> - No updates. >>>>>>> >>>>>>> Changes since v2: >>>>>>> - Remove pm_children_suspended() and other unused macros. >>>>>>> >>>>>>> Changes since v1: >>>>>>> - Add pm_runtime.h head file to avoid kbuild error. >>>>>>> --- >>>>>>> drivers/usb/dwc3/Kconfig | 7 +++++++ >>>>>>> drivers/usb/dwc3/core.c | 26 +++++++++++++++++++++++++- >>>>>>> drivers/usb/dwc3/core.h | 15 +++++++++++++++ >>>>>>> drivers/usb/dwc3/host.c | 37 +++++++++++++++++++++++++++++++++++++ >>>>>>> 4 files changed, 84 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >>>>>>> index a45b4f1..47bb2f3 100644 >>>>>>> --- a/drivers/usb/dwc3/Kconfig >>>>>>> +++ b/drivers/usb/dwc3/Kconfig >>>>>>> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE >>>>>>> >>>>>>> endchoice >>>>>>> >>>>>>> +config USB_DWC3_HOST_SUSPEND >>>>>>> + bool "Choose if the DWC3 host (xhci) can be suspend/resume" >>>>>>> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y >>>>> >>>>> why do you need another Kconfig for this? Just enable it already :-p >>>> >>>> I assume some platforms may do not need this feature. But okay, I can >>>> remove this kconfig and enable it. >>> >>> thanks :-) >>> >>>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>>>> index 9a4a5e4..7ad4bc3 100644 >>>>>>> --- a/drivers/usb/dwc3/core.c >>>>>>> +++ b/drivers/usb/dwc3/core.c >>>>>>> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) >>>>>>> pm_runtime_use_autosuspend(dev); >>>>>>> pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); >>>>>>> pm_runtime_enable(dev); >>>>>>> + pm_suspend_ignore_children(dev, true); >>>>> >>>>> why do you need this? >>>> >>>> Since the dwc3 device is the parent deive of xhci device, if we want >>>> to suspend dwc3 device, we need to suspend child device (xhci device) >>>> manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In >>>> pm_runtime_put_sync(), it will check if the children (xhci device) of >>>> dwc3 device have been in suspend state, if not we will suspend dwc3 >>>> device failed. >>>> >>>> We get/put the child device manually in parent device's runtime >>>> callback, we need to ignore the child device's runtime state, or it >>>> will failed due to the dependency. >>> >>> I see. Good explanation :-) >>> >>> There's one thing though, if you want to runtime suspend the gadget and >>> dwc3 is working on peripheral mode, host side (XHCI) should already be >>> runtime suspended because there's nothing attached to it. Why isn't it >>> runtime suspended? >> >> Since we have get the runtime count for xHCI device in >> xhci_plat_probe(), in case it will suspend automatically if some >> controllers do not want xHCI enters runtime suspend automatically. So >> the parent device (dwc3 device) need to get/put the xHCI device's >> runtime count to resume/suspend xHCI. > > IMHO, that's a bug in xhci-plat, not something dwc3 should work around. Maybe you misunderstood my meaning and I should make it clear. If dwc3 is just working on peripheral mode, then no need to initialize the host (xhci) things, which means it is invalid to runtime suspend/resume xHCI device and we can just runtime suspend/resume the gadget. If dwc3 is working on OTG mode, which will create and add the USB hcd for host side. Then if we want to suspend dwc3, we need to suspend xHCI device manually though now dwc3 acts as peripheral. The USB device (bus) of host side will runtime suspend automatically if there are no slave attached (by usb_runtime_suspend()--->usb_suspend_both()--->usb_suspend_interface()--->usb_suspend_device()--->generic_suspend()--->hcd_bus_suspend()--->xhci_bus_suspend()), but we should not suspend xHCI device (issuing xhci_suspend()) now, since some controllers did not implement the runtime PM to control xHCI device's runtime state, which will cause controllers can not resume xHCI device (issuing xhci_resume()) if the xHCI device suspend automatically by its child device. Thus we should get the runtime count for xHCI device in xhci_plat_probe() in case xHCI device will also suspend automatically by its child device. According to that, for xHCI device's parent: dwc3 device, we should put the xHCI device's runtime count to suspend xHCI device manually. Hope I make it clear. -- Baolin.wang Best Regards ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] usb: host: plat: Enable xhci plat runtime PM 2016-11-28 6:43 [PATCH v4 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang 2016-11-28 6:43 ` [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume Baolin Wang @ 2016-12-08 7:04 ` Baolin Wang 1 sibling, 0 replies; 10+ messages in thread From: Baolin Wang @ 2016-12-08 7:04 UTC (permalink / raw) To: mathias.nyman, Felipe Balbi; +Cc: Greg KH, Mark Brown, USB, LKML, Baolin Wang Hi Mathias and Felipe, On 28 November 2016 at 14:43, Baolin Wang <baolin.wang@linaro.org> wrote: > Enable the xhci plat runtime PM for parent device to suspend/resume xhci. > Also call pm_runtime_get_noresume() in probe() function in case the parent > device doesn't call suspend/resume callback by runtime PM now. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > --- > Changes since v3: > - Fix kbuild error. > > Changes since v2: > - Add pm_runtime_get_noresume() in probe() function. > - Add pm_runtime_set_suspended()/pm_runtime_put_noidle() in remove() function. > > Changes since v1: > - No updates. > --- > drivers/usb/host/xhci-plat.c | 41 ++++++++++++++++++++++++++++++++++++----- > 1 file changed, 36 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > index ed56bf9..5805c6a 100644 > --- a/drivers/usb/host/xhci-plat.c > +++ b/drivers/usb/host/xhci-plat.c > @@ -246,6 +246,10 @@ static int xhci_plat_probe(struct platform_device *pdev) > if (ret) > goto dealloc_usb2_hcd; > > + pm_runtime_get_noresume(&pdev->dev); > + pm_runtime_set_active(&pdev->dev); > + pm_runtime_enable(&pdev->dev); > + > return 0; > > > @@ -274,6 +278,10 @@ static int xhci_plat_remove(struct platform_device *dev) > struct xhci_hcd *xhci = hcd_to_xhci(hcd); > struct clk *clk = xhci->clk; > > + pm_runtime_set_suspended(&dev->dev); > + pm_runtime_put_noidle(&dev->dev); > + pm_runtime_disable(&dev->dev); > + > usb_remove_hcd(xhci->shared_hcd); > usb_phy_shutdown(hcd->usb_phy); > > @@ -311,14 +319,37 @@ static int xhci_plat_resume(struct device *dev) > > return xhci_resume(xhci, 0); > } > +#endif /* CONFIG_PM_SLEEP */ > + > +#ifdef CONFIG_PM > +static int xhci_plat_runtime_suspend(struct device *dev) > +{ > + struct usb_hcd *hcd = dev_get_drvdata(dev); > + struct xhci_hcd *xhci = hcd_to_xhci(hcd); > + > + return xhci_suspend(xhci, device_may_wakeup(dev)); > +} > + > +static int xhci_plat_runtime_resume(struct device *dev) > +{ > + struct usb_hcd *hcd = dev_get_drvdata(dev); > + struct xhci_hcd *xhci = hcd_to_xhci(hcd); > + > + return xhci_resume(xhci, 0); > +} > + > +static int xhci_plat_runtime_idle(struct device *dev) > +{ > + return 0; > +} > +#endif /* CONFIG_PM */ > > static const struct dev_pm_ops xhci_plat_pm_ops = { > SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume) > + > + SET_RUNTIME_PM_OPS(xhci_plat_runtime_suspend, xhci_plat_runtime_resume, > + xhci_plat_runtime_idle) > }; > -#define DEV_PM_OPS (&xhci_plat_pm_ops) > -#else > -#define DEV_PM_OPS NULL > -#endif /* CONFIG_PM */ > > static const struct acpi_device_id usb_xhci_acpi_match[] = { > /* XHCI-compliant USB Controller */ > @@ -332,7 +363,7 @@ static int xhci_plat_resume(struct device *dev) > .remove = xhci_plat_remove, > .driver = { > .name = "xhci-hcd", > - .pm = DEV_PM_OPS, > + .pm = &xhci_plat_pm_ops, > .of_match_table = of_match_ptr(usb_xhci_of_match), > .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match), > }, > -- > 1.7.9.5 > Do you have any comments about this patch? Thanks. -- Baolin.wang Best Regards ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-12-09 3:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-28 6:43 [PATCH v4 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang 2016-11-28 6:43 ` [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume Baolin Wang 2016-12-08 7:05 ` Baolin Wang 2016-12-08 9:40 ` Felipe Balbi 2016-12-08 10:20 ` Baolin Wang 2016-12-08 11:02 ` Felipe Balbi 2016-12-08 11:14 ` Baolin Wang 2016-12-08 17:52 ` Felipe Balbi 2016-12-09 3:23 ` Baolin Wang 2016-12-08 7:04 ` [PATCH v4 1/2] usb: host: plat: Enable xhci plat runtime PM Baolin Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).