From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754348AbcHZWMU (ORCPT ); Fri, 26 Aug 2016 18:12:20 -0400 Received: from mail-pa0-f46.google.com ([209.85.220.46]:33557 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752806AbcHZWMT (ORCPT ); Fri, 26 Aug 2016 18:12:19 -0400 Date: Fri, 26 Aug 2016 15:11:36 -0700 From: Brian Norris To: Robert Foss Cc: mathias.nyman@intel.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Julius Werner , Andrew Bresticker , Felipe Balbi , Baolin Wang , zyx@rock-chips.com, wulf@rock-chips.com Subject: Re: [PACTH,v6,1/2] usb: xhci: plat: Enable runtime PM Message-ID: <20160826221134.GA10849@localhost> References: <1470861136-23017-2-git-send-email-robert.foss@collabora.com> <20160823032304.GA1781@google.com> <3bf26670-6af5-e580-76cc-304c759befaf@collabora.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3bf26670-6af5-e580-76cc-304c759befaf@collabora.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Wed, Aug 24, 2016 at 04:48:01PM -0400, Robert Foss wrote: > On 2016-08-22 11:23 PM, Brian Norris wrote: > >+ others > > > >Hi Robert and Felipe, > > > >I have a few questions for one or both of you. I'm not really an expert > >on runtime PM, so please take my questions with a grain of salt. > > > >On Wed, Aug 10, 2016 at 04:32:15PM -0400, robert.foss@collabora.com wrote: > >>From: Robert Foss > >> > >>Enable runtime PM for the xhci-plat device so that the parent device > >>may implement runtime PM. > >> > >>Signed-off-by: Robert Foss > >> > >>Tested-by: Robert Foss > >>--- > >> drivers/usb/host/xhci-plat.c | 29 +++++++++++++++++++++++++++-- > >> 1 file changed, 27 insertions(+), 2 deletions(-) > >> > >>diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c > >>index ed56bf9..ba4efe7 100644 > >>--- a/drivers/usb/host/xhci-plat.c > >>+++ b/drivers/usb/host/xhci-plat.c > >>@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev) > >> if (ret) > >> goto dealloc_usb2_hcd; > >> > >>+ pm_runtime_set_active(&pdev->dev); > >>+ pm_runtime_enable(&pdev->dev); > >>+ > > > >How does it help to enable PM runtime like this, if you don't have any > >kind of runtime_{suspend,resume}() callbacks? > > Andrew, I think you understand the inner workings of this code > better than me, maybe you could give a short summary? I believe Andrew is fairly busy, and I already talked with him a bit about this. This is needed (as per your (or his?) commit message) only if you have a parent device that wants to implement runtime PM. Now depending on what you want to do with "runtime PM", this might mean the parent device has to suspend xhci_{suspend,resume}() on behalf of xhci-plat.c. Not very nice layering IMO, but it has been done before... So I guess this comes down to "what does a parent device/driver want to do"? If that's (e.g.) just putting some PHY into a slightly lower power mode, then maybe it's fine for xhci-plat not to do anything else. But if you actually want to completely power down the parent bus, reset an accompanying dual-role/OTG controller, etc., then this really isn't sufficient, AFAICT. But maybe that's just an indictment of the poor structure of dwc3's host-mode support, more than it is an indictment of this patch. Brian