[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87eg5m5y6l.fsf@linux.intel.com>
Date: Thu, 18 Aug 2016 10:33:06 +0300
From: Felipe Balbi <balbi@...nel.org>
To: Baolin Wang <baolin.wang@...aro.org>, gregkh@...uxfoundation.org
Cc: mathias.nyman@...el.com, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, broonie@...nel.org,
baolin.wang@...aro.org
Subject: Re: [PATCH 4/4] usb: dwc3: core: Support the dwc3 host suspend/resume
Hi,
Baolin Wang <baolin.wang@...aro.org> writes:
> 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, and
> we also should enable the 'XHCI_SLOW_SUSPEND' quirk for extraordinary delay when
> suspending the xhci.
>
> Signed-off-by: Baolin Wang <baolin.wang@...aro.org>
> ---
> drivers/usb/dwc3/Kconfig | 7 +++++++
> drivers/usb/dwc3/core.c | 25 ++++++++++++++++++++++++-
> drivers/usb/dwc3/core.h | 15 +++++++++++++++
> drivers/usb/dwc3/host.c | 32 ++++++++++++++++++++++++++++++++
> 4 files changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index a64ce1c..725d2bd 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -45,6 +45,13 @@ config USB_DWC3_DUAL_ROLE
> This is the default mode of working of DWC3 controller where
> both host and gadget features are enabled.
>
> +config USB_DWC3_HOST_SUSPEND
> + bool "Choose if the 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.
> +
> endchoice
>
> comment "Platform Glue Driver Support"
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 1485480..5140b4d 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1103,15 +1103,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;
> @@ -1133,12 +1145,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 45d6de5..0ba203e 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1177,4 +1177,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
> { }
> #endif
>
> +#if IS_ENABLED(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 2e960ed..2ec3eff 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -17,8 +17,11 @@
>
> #include <linux/platform_device.h>
> #include <linux/usb/xhci_pdriver.h>
> +#include <linux/usb.h>
> +#include <linux/usb/hcd.h>
>
> #include "core.h"
> +#include "../host/xhci.h"
>
> int dwc3_host_init(struct dwc3 *dwc)
> {
> @@ -91,6 +94,8 @@ int dwc3_host_init(struct dwc3 *dwc)
> memset(&pdata, 0, sizeof(pdata));
>
> pdata.usb3_lpm_capable = dwc->usb3_lpm_capable;
> + /* dwc3 controller need an extraordinary delay when suspending xhci. */
> + pdata.usb3_slow_suspend = 1;
>
> ret = platform_device_add_data(xhci, &pdata, sizeof(pdata));
> if (ret) {
> @@ -128,3 +133,30 @@ void dwc3_host_exit(struct dwc3 *dwc)
> dev_name(&dwc->xhci->dev));
> platform_device_unregister(dwc->xhci);
> }
> +
> +int dwc3_host_suspend(struct dwc3 *dwc)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(&dwc->xhci->dev);
> + struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + int ret, cnt = 20;
> +
> +try_again:
> + /* We should wait for xhci bus has been into suspend mode firstly. */
> + ret = xhci_suspend(xhci, device_may_wakeup(&dwc->xhci->dev));
> + if (ret && --cnt > 0) {
> + dev_warn(dwc->dev, "xhci suspend failed %d, try again...\n",
> + ret);
> + msleep(200);
> + goto try_again;
> + }
> +
> + return ret;
> +}
> +
> +int dwc3_host_resume(struct dwc3 *dwc)
> +{
> + struct usb_hcd *hcd = dev_get_drvdata(&dwc->xhci->dev);
> + struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> +
> + return xhci_resume(xhci, 0);
> +}
This is wrong. XHCI is a child of dwc3, when dwc3 suspends,
xhci_supend() has already been called. Why isn't it called for you?
--
balbi
Download attachment "signature.asc" of type "application/pgp-signature" (801 bytes)
Powered by blists - more mailing lists