Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752898AbcLHRyG (ORCPT ); Thu, 8 Dec 2016 12:54:06 -0500 Received: from mga03.intel.com ([134.134.136.65]:38701 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751610AbcLHRyE (ORCPT ); Thu, 8 Dec 2016 12:54:04 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,320,1477983600"; d="scan'208";a="40298482" From: Felipe Balbi To: Baolin Wang Cc: mathias.nyman@intel.com, Greg KH , Mark Brown , USB , LKML Subject: Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume In-Reply-To: References: <5e67c4a6680bbc7c02e92305d81eda5449b8a4af.1479991143.git.baolin.wang@linaro.org> <87oa0mkbd6.fsf@linux.intel.com> <87d1h2k7kj.fsf@linux.intel.com> Date: Thu, 08 Dec 2016 19:52:56 +0200 Message-ID: <877f7apatj.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3477 Lines: 86 Hi, Baolin Wang writes: >> Baolin Wang writes: >>>>> On 28 November 2016 at 14:43, Baolin Wang 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 >>>>>> --- >>>>>> 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