Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754045AbaJVPog (ORCPT ); Wed, 22 Oct 2014 11:44:36 -0400 Received: from mail-lb0-f176.google.com ([209.85.217.176]:33634 "EHLO mail-lb0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753356AbaJVPod (ORCPT ); Wed, 22 Oct 2014 11:44:33 -0400 MIME-Version: 1.0 In-Reply-To: References: <1413809764-21995-1-git-send-email-grygorii.strashko@ti.com> <1413809764-21995-3-git-send-email-grygorii.strashko@ti.com> <5446A065.9050308@gmail.com> <544793B5.6080601@ti.com> Date: Wed, 22 Oct 2014 17:44:30 +0200 X-Google-Sender-Auth: iDMGYxK1QulGIc6q6Zv9tEclFCc Message-ID: Subject: Re: [PATCH v2 2/3] ARM: keystone: pm: switch to use generic pm domains From: Geert Uytterhoeven To: Ulf Hansson Cc: Grygorii Strashko , Santosh Shilimkar , ssantosh@kernel.org, "Rafael J. Wysocki" , Kevin Hilman , Geert Uytterhoeven , "linux-pm@vger.kernel.org" , Rob Herring , Grant Likely , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 22, 2014 at 5:28 PM, Ulf Hansson wrote: > On 22 October 2014 17:09, Geert Uytterhoeven wrote: >> On Wed, Oct 22, 2014 at 5:01 PM, Ulf Hansson wrote: >>>>>> +void keystone_pm_domain_attach_dev(struct device *dev) >>>>>> { >>>>>> + struct clk *clk; >>>>>> int ret; >>>>>> + int i = 0; >>>>>> >>>>>> dev_dbg(dev, "%s\n", __func__); >>>>>> >>>>>> - ret = pm_generic_runtime_suspend(dev); >>>>>> - if (ret) >>>>>> - return ret; >>>>>> - >>>>>> - ret = pm_clk_suspend(dev); >>>>>> + ret = pm_clk_create(dev); >>>>>> if (ret) { >>>>>> - pm_generic_runtime_resume(dev); >>>>>> - return ret; >>>>>> + dev_err(dev, "pm_clk_create failed %d\n", ret); >>>>>> + return; >>>>>> + }; >>>>>> + >>>>>> + while ((clk = of_clk_get(dev->of_node, i++)) && !IS_ERR(clk)) { >>>>>> + ret = pm_clk_add_clk(dev, clk); >>>>>> + if (ret) { >>>>>> + dev_err(dev, "pm_clk_add_clk failed %d\n", ret); >>>>>> + goto clk_err; >>>>>> + }; >>>>>> } >>>>>> >>>>>> - return 0; >>>>>> + if (!IS_ENABLED(CONFIG_PM_RUNTIME)) { >>>>> Can we not okkup two seperate callbacks instead of above check ? >>>>> I don't like this CONFIG check here. Its slightly better version of >>>>> ifdef in middle of the code. >>>> >>>> I've found more-less similar comment on patch >>>> "Re: [PATCH v3 1/3] power-domain: add power domain drivers for Rockchip platform" >>>> https://lkml.org/lkml/2014/10/17/257 >>>> >>>> So, Would you like me to create patch which will enable clocks in pm_clk_add/_clk() >>>> in case !IS_ENABLED(CONFIG_PM_RUNTIME) >>> >>> I am wondering whether we actually should/could do this, no matter of >>> CONFIG_PM_RUNTIME. >>> >>> Typically, for configurations that uses CONFIG_PM_RUNTIME, the PM >>> clocks through pm_clk_suspend(), will be gated once the device becomes >>> runtime PM suspended. Right? >> >> Doing it unconditionally means we'll have lots of unneeded clocks running >> for a short while. > As long as the pm_clk_add() is being invoked from the ->attach_dev() > callback, we are in the probe path. Certainly we would like to have > clocks enabled while probing, don't you think? > > If we wouldn't enable the clocks for CONFIG_PM_RUNTIME, when will > those be enabled? They will be enabled when the driver does pm_runtime_enable(dev); pm_runtime_get_sync(dev); in its .probe() method. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/