Received: by 10.223.185.111 with SMTP id b44csp184161wrg; Fri, 9 Mar 2018 03:20:30 -0800 (PST) X-Google-Smtp-Source: AG47ELvH3LvIyLnAj+mOeTxidZ6BaRXpQ66gF5tiMQ7JZSzpqu4qrNQo1BKbZ5UWtf31YIWsZkTo X-Received: by 10.99.96.210 with SMTP id u201mr1136989pgb.124.1520594430258; Fri, 09 Mar 2018 03:20:30 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1520594430; cv=none; d=google.com; s=arc-20160816; b=yBkAKhmviv8d144nurUFKsiTATn2yLS/mKKJtfSZ8ZjlkSj7Ozgy1k/6mzuiy3Y0wA 5i1/K99Ag7+e1rdJBMuXffJ/pSENWj4ubrDa8m433uOysh6934SmNxhsByO1xztRradH yNF/+rrqbGHDQ7XH4u9/IYBvkYIdn7C6hK+aFXh404f/R/YKtcwZquuleKg7if1fttUp zUEXY4tk57VSPw5MsvwXRgB9log348MVaodvqVjLKVsiMjFFFSp+0gEAMI/QCbQHHZ18 QqlV5iSXmD2WkY3bguAAGTdsMlP2BXGobcGOPbpcbAAcLJS3TZ39UYtI/HhMw48LnxHG YfkQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:arc-authentication-results; bh=wduXQgVmgSrvSda086+v2qJIONOvQLOHpuVJ54u0Fu4=; b=OpDPjrcESTOoLLv2/rbuCIqs4VhFPRrQjezaRDaCPPv+X6JDtzRYcrMR4VxOKOsVlF pPMGrObypbPwL9FSCbZcBcOr3MW2ALMuyg15c/jO9HiNBo0L6w1anroFV2f4+15XoQBt 2VHgmAYyDDWOQ8b3GTwhz3v5YghsokWnbXIGK/fV4sQ8lz4AJZxGzAr57S7y9JzvFH6Q QhJkIK6vT2s5S7tH6RTjahFLPL5/WPekpuYrTSNW9Fl2UMfMGUwQETH9RN7o9iKlv5Jx GM4N9LBvadO0vlYc/WsOKX4lROi8vp8P0ul+aNXpLEVEUMmB/yUhTXdjUtqLdK8rtUuo Jecg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id u18si588982pgv.374.2018.03.09.03.20.14; Fri, 09 Mar 2018 03:20:30 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=nvidia.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751048AbeCILTT convert rfc822-to-8bit (ORCPT + 99 others); Fri, 9 Mar 2018 06:19:19 -0500 Received: from hqemgate16.nvidia.com ([216.228.121.65]:17406 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750883AbeCILTR (ORCPT ); Fri, 9 Mar 2018 06:19:17 -0500 Received: from hqpgpgate102.nvidia.com (Not Verified[216.228.121.13]) by hqemgate16.nvidia.com id ; Fri, 09 Mar 2018 03:19:15 -0800 Received: from HQMAIL107.nvidia.com ([172.20.161.6]) by hqpgpgate102.nvidia.com (PGP Universal service); Fri, 09 Mar 2018 03:19:15 -0800 X-PGP-Universal: processed; by hqpgpgate102.nvidia.com on Fri, 09 Mar 2018 03:19:15 -0800 Received: from UKMAIL101.nvidia.com (10.26.138.13) by HQMAIL107.nvidia.com (172.20.187.13) with Microsoft SMTP Server (TLS) id 15.0.1347.2; Fri, 9 Mar 2018 11:19:16 +0000 Received: from [10.21.132.129] (10.21.132.129) by UKMAIL101.nvidia.com (10.26.138.13) with Microsoft SMTP Server (TLS) id 15.0.1347.2; Fri, 9 Mar 2018 11:19:12 +0000 Subject: Re: [PATCH 2/3] usb: xhci: tegra: Add runtime PM support To: Mathias Nyman , Thierry Reding CC: Mathias Nyman , Greg Kroah-Hartman , , , References: <1518626085-29102-1-git-send-email-jonathanh@nvidia.com> <1518626085-29102-2-git-send-email-jonathanh@nvidia.com> <54bd00b7-2835-a253-0399-370e8c8203b8@linux.intel.com> <20180309083629.GA13877@ulmo> <1fff9fc1-2ad2-dba1-cb1d-d531254984ce@intel.com> From: Jon Hunter Message-ID: <81534ac4-c446-62da-2656-e4d5d0fd1da2@nvidia.com> Date: Fri, 9 Mar 2018 11:19:11 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <1fff9fc1-2ad2-dba1-cb1d-d531254984ce@intel.com> X-Originating-IP: [10.21.132.129] X-ClientProxiedBy: UKMAIL101.nvidia.com (10.26.138.13) To UKMAIL101.nvidia.com (10.26.138.13) Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/03/18 09:13, Mathias Nyman wrote: > On 09.03.2018 10:36, Thierry Reding wrote: >> On Thu, Mar 08, 2018 at 09:31:07PM +0000, Jon Hunter wrote: >>> >>> On 01/03/18 14:18, Mathias Nyman wrote: >>>> On 14.02.2018 18:34, Jon Hunter wrote: >>>>> Add runtime PM support to the Tegra XHCI driver and move the function >>>>> calls to enable/disable the clocks, regulators and PHY into the >>>>> runtime >>>>> PM callbacks. >>>>> >>>>> Signed-off-by: Jon Hunter >>>>> --- >>>>> drivers/usb/host/xhci-tegra.c | 80 >>>>> ++++++++++++++++++++++++++++++------------- >>>>> 1 file changed, 56 insertions(+), 24 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/host/xhci-tegra.c >>>>> b/drivers/usb/host/xhci-tegra.c >>>>> index 02b0b24faa58..42aa67858b53 100644 >>>>> --- a/drivers/usb/host/xhci-tegra.c >>>>> +++ b/drivers/usb/host/xhci-tegra.c >>>>> @@ -18,6 +18,7 @@ >>>>> #include >>>>> #include >>>>> #include >>>>> +#include >>>>> #include >>>>> #include >>>>> #include >>>>> @@ -1067,22 +1068,12 @@ static int tegra_xusb_probe(struct >>>>> platform_device *pdev) >>>>> */ >>>>> platform_set_drvdata(pdev, tegra); >>>>> - err = tegra_xusb_clk_enable(tegra); >>>>> - if (err) { >>>>> - dev_err(&pdev->dev, "failed to enable clocks: %d\n", err); >>>>> - goto put_usb2; >>>>> - } >>>>> - >>>>> - err = regulator_bulk_enable(tegra->soc->num_supplies, >>>>> tegra->supplies); >>>>> - if (err) { >>>>> - dev_err(&pdev->dev, "failed to enable regulators: %d\n", >>>>> err); >>>>> - goto disable_clk; >>>>> - } >>>>> + pm_runtime_enable(&pdev->dev); >>>>> - err = tegra_xusb_phy_enable(tegra); >>>>> + err = pm_runtime_get_sync(&pdev->dev); >>>>> if (err < 0) { >>>> >>>> Does this mean that if runtime PM is disabled then clocks and regulator >>>> will never be enabled >>>> for Tegra xhci? >>>> >>>> How about keeping the clock and regualtor enabling in probe, and >>>> instead >>>> add something like: >>>> >>>> pm_runtime_set_active(&pdev->dev); >>>> pm_runtime_enable(&pdev->dev); >>>> pm_runtime_get_noresume(&pdev->dev); >>> >>> For 64-bit Tegra there is a dependency on CONFIG_PM, but for 32-bit >>> AFAIK there is not and so yes we should handle the case when PM_RUNTIME >>> is disabled. >>> >>> Typically we do something like ... >>> >>> pm_runtime_enable(&pdev->dev); >>> if (!pm_runtime_enabled(&pdev->dev)) >>> ret = tegra_xusb_runtime_resume(&pdev->dev); >>> else >>> ret = pm_runtime_get_sync(&pdev->dev); >>> >>> That way we can keep the regulator and clock stuff in the handler. I >>> will update this series. >> >> Is there any good reason why we don't depend on PM for 32-bit as well? >> I'm not aware of any differences in drivers that are 32-bit specific for >> Tegra, and I'm not even sure the !PM case gets any testing at all. And >> even if, do we really still want to support that? >> >> I don't see any advantage these days for having it disabled. > > I don't know much about Tegra, but I'd still like to turn this question > around: > > Is there any reason why clks and regulators can't initially be turned on > in probe, > and then let runtime PM handle them later if PM is supported? I personally prefer having the regulator, clock, etc handling for enabling and disabling for a device in their own handler. Duplicating the calls to the regulator and clock frameworks in probe and the rpm handlers seems more prone to mistakes. Hence, that's why I would prefer the option I suggested above, which IMO is the best of both worlds. > Shouldn't this work in all cases, and it avoids creating new dependencies? Yes but when you want to use frameworks such as genpd it becomes more complex to support !PM and that is why for 64-bit Tegra we have a dependency today. Cheers Jon -- nvpublic