From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754010AbcHRM0U (ORCPT ); Thu, 18 Aug 2016 08:26:20 -0400 Received: from mga04.intel.com ([192.55.52.120]:36163 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753900AbcHRM0S (ORCPT ); Thu, 18 Aug 2016 08:26:18 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,539,1464678000"; d="asc'?scan'208";a="1027657246" From: Felipe Balbi To: Baolin Wang Cc: Greg KH , mathias.nyman@intel.com, USB , LKML , Mark Brown Subject: Re: [PATCH 4/4] usb: dwc3: core: Support the dwc3 host suspend/resume In-Reply-To: References: <9cca1b5a0487676a4fa912e957a03642a330c20a.1468571634.git.baolin.wang@linaro.org> <87eg5m5y6l.fsf@linux.intel.com> User-Agent: Notmuch/0.22.1+63~g994277e (https://notmuchmail.org) Emacs/25.1.1 (x86_64-pc-linux-gnu) Date: Thu, 18 Aug 2016 15:25:59 +0300 Message-ID: <87fuq24620.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Baolin Wang writes: > Hi Felipe, > > On 18 August 2016 at 15:33, Felipe Balbi wrote: >> >> Hi, >> >> Baolin Wang writes: >>> For some mobile devices with strict power management, we also want to s= uspend >>> the host when the slave is detached for power saving. >>> >>> Thus we add the host suspend/resume functions to support this requireme= nt, and >>> we also should enable the 'XHCI_SLOW_SUSPEND' quirk for extraordinary d= elay when >>> suspending the xhci. >>> >>> Signed-off-by: Baolin Wang >>> --- >>> 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=3Dy || USB_DWC3_DUAL_ROLE=3Dy >>> + help >>> + We can suspend the host when the slave is detached for power sa= ving, >>> + 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 =3D 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 =3D 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 =3D 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 =3D 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 *d= wc) >>> { } >>> #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 >>> #include >>> +#include >>> +#include >>> >>> #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 =3D dwc->usb3_lpm_capable; >>> + /* dwc3 controller need an extraordinary delay when suspending xh= ci. */ >>> + pdata.usb3_slow_suspend =3D 1; >>> >>> ret =3D 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 =3D dev_get_drvdata(&dwc->xhci->dev); >>> + struct xhci_hcd *xhci =3D hcd_to_xhci(hcd); >>> + int ret, cnt =3D 20; >>> + >>> +try_again: >>> + /* We should wait for xhci bus has been into suspend mode firstl= y. */ >>> + ret =3D 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 =3D dev_get_drvdata(&dwc->xhci->dev); >>> + struct xhci_hcd *xhci =3D 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? > > xhci-plat.c did not set the runtime PM callbacks, how can we issued > xhci_supend() by runtime PM from dwc3? wouldn't it be nicer to just initialize PM runtime callbacks from xhci-plat? It just needs to be added and verified, right? =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJXtalXAAoJEIaOsuA1yqREMYMP/RdMmtxs1fR7zPYn4HgtMxUC XBftBGCWMhmyRR99XhZ0wsbp1LECBRRiLsb4yJunYmiO0Y6YR3Bc5XkY2dQcC7SF RbcEMYNHQ+7ouOU3pUEqF2yvSfLaRsTdokI7lgHoiOXvHupG+ij2OBEk5a/SvtJY QoXh2G0xvH8Mc7gJyducxVvfC9j8aNVm+gDH8LXuUyZloqtWb8/eNR6c+8ZwLRZL yVJymztgJPcKmTmXZpVbY0cuptE/6NpEZ4Ln6FdIZqp7TKoBu7L8nm9fEo5gRTMB ncO901TjfX5wozx4pdyXKLhuwE/BGFAV6d1uP2rnfQCIBgBXlV6DmvJ5gScV0Qvu 1fWdiZRIJn8RblCib4SlesDK2w21fpj0b3yhHcLFvCNFd73f31gbvEh5UBvnijTy zeA+wdmbza0/wEhq5rIxTFniMW6rCvJILJeyDfgI+RrQevc7Ly4CgnWQIeTvOhTI 0RyBSzXjWvBxnCLaKm/tZNJsn5bHUYqP/Nwuo3HWDAZe1lbjoQbV/tAwBrhXZ18i WrhheI2lBeCOrMVcpvQLAVkhoX+p6QJrLSEzvOz7Odd2EXPMxxb99PuA+Cm4M57q yZhKNQ5hcUFLlWdvni3qHWuq4qNI6N3BLZXM6WDjr4R8cwYFkpooW/86i0yOJ2OI yLlEt0MldBmPX1UyXr+k =6C7U -----END PGP SIGNATURE----- --=-=-=--