Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755284AbbDKLAm (ORCPT ); Sat, 11 Apr 2015 07:00:42 -0400 Received: from mail.kapsi.fi ([217.30.184.167]:59961 "EHLO mail.kapsi.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754587AbbDKLAj (ORCPT ); Sat, 11 Apr 2015 07:00:39 -0400 Message-ID: <5528FEC8.3080103@kapsi.fi> Date: Sat, 11 Apr 2015 14:00:24 +0300 From: Mikko Perttunen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Michael Turquette , swarren@wwwdotorg.org, thierry.reding@gmail.com, gnurou@gmail.com, pdeschrijver@nvidia.com, rjw@rjwysocki.net, viresh.kumar@linaro.org CC: pwalmsley@nvidia.com, vinceh@nvidia.com, pgaikwad@nvidia.com, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, linux-tegra@vger.kernel.org, linux-arm-kernel@lists.infradead.org, tuomas.tynkkynen@iki.fi, Tomeu Vizoso Subject: Re: [PATCH v8 10/18] clk: tegra: Initialize PLL_X before CCLK_G to ensure it has a parent References: <1425213881-5262-1-git-send-email-mikko.perttunen@kapsi.fi> <1425213881-5262-11-git-send-email-mikko.perttunen@kapsi.fi> <20150410210832.14369.65901@quantum> In-Reply-To: <20150410210832.14369.65901@quantum> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2001:708:30:12d0:beee:7bff:fe5b:f272 X-SA-Exim-Mail-From: mikko.perttunen@kapsi.fi X-SA-Exim-Scanned: No (on mail.kapsi.fi); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4269 Lines: 119 On 04/11/2015 12:08 AM, Michael Turquette wrote: > Quoting Mikko Perttunen (2015-03-01 04:44:33) >> This patch moves the initialization of PLL_X to be slightly before >> that of CCLK_G. This ensures that at boot, CCLK_G will immediately >> have a parent and the common clock framework can determine its >> clock rate correctly. >> >> Without this patch, calling clk_put on CCLK_G could cause the CCF >> to set its rate to zero, hanging the system. > > Hi Mikko, > > Patch looks fine to me but I wanted to get more info on the behavior you > mentioned above about clk_put. Is there some special circumstance that > causes this for you? Why does calling clk_put adjust the rate of your > clock? > > Thanks, > Mike Hi Mike, this is the chain of events: - CCLK_G is registered. CCF stores its current rate, but since it doesn't have a parent at this point, the rate is assumed zero. - tegra cpufreq driver tries to probe, and clk_gets CCLK_G - tegra dfll driver tries to probe, but fails - tegra cpufreq driver's probe fails, and during unwinding clk_puts CCLK_G - CCF attempts to restore CCLK_G's rate to what it was prior to the clk_get (to revert possible changes due to clock constraints) - the stored rate was zero, so CCLK_G is set to zero. We did discuss it a bit on IRC with Tomeu and Peter and agreed that some fix in CCF should be done, but we didn't get much further than that. Mikko > >> >> Signed-off-by: Mikko Perttunen >> --- >> v8: >> - Added >> >> drivers/clk/tegra/clk-tegra-super-gen4.c | 46 ++++++++++++++++++-------------- >> 1 file changed, 26 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c >> index f1f4410..c5ea9ee 100644 >> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c >> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c >> @@ -104,6 +104,32 @@ void __init tegra_super_clk_gen4_init(void __iomem *clk_base, >> struct clk *clk; >> struct clk **dt_clk; >> >> + /* >> + * Register PLL_X first so that CCLK_G has a parent at registration >> + * time. This ensures that the common clock framework knows CCLK_G's >> + * rate. >> + */ >> + >> +#if defined(CONFIG_ARCH_TEGRA_114_SOC) || defined(CONFIG_ARCH_TEGRA_124_SOC) >> + /* PLLX */ >> + dt_clk = tegra_lookup_dt_id(tegra_clk_pll_x, tegra_clks); >> + if (!dt_clk) >> + return; >> + >> + clk = tegra_clk_register_pllxc("pll_x", "pll_ref", clk_base, >> + pmc_base, CLK_IGNORE_UNUSED, params, NULL); >> + *dt_clk = clk; >> + >> + /* PLLX_OUT0 */ >> + >> + dt_clk = tegra_lookup_dt_id(tegra_clk_pll_x_out0, tegra_clks); >> + if (!dt_clk) >> + return; >> + clk = clk_register_fixed_factor(NULL, "pll_x_out0", "pll_x", >> + CLK_SET_RATE_PARENT, 1, 2); >> + *dt_clk = clk; >> +#endif >> + >> /* CCLKG */ >> dt_clk = tegra_lookup_dt_id(tegra_clk_cclk_g, tegra_clks); >> if (dt_clk) { >> @@ -127,25 +153,5 @@ void __init tegra_super_clk_gen4_init(void __iomem *clk_base, >> } >> >> tegra_sclk_init(clk_base, tegra_clks); >> - >> -#if defined(CONFIG_ARCH_TEGRA_114_SOC) || defined(CONFIG_ARCH_TEGRA_124_SOC) >> - /* PLLX */ >> - dt_clk = tegra_lookup_dt_id(tegra_clk_pll_x, tegra_clks); >> - if (!dt_clk) >> - return; >> - >> - clk = tegra_clk_register_pllxc("pll_x", "pll_ref", clk_base, >> - pmc_base, CLK_IGNORE_UNUSED, params, NULL); >> - *dt_clk = clk; >> - >> - /* PLLX_OUT0 */ >> - >> - dt_clk = tegra_lookup_dt_id(tegra_clk_pll_x_out0, tegra_clks); >> - if (!dt_clk) >> - return; >> - clk = clk_register_fixed_factor(NULL, "pll_x_out0", "pll_x", >> - CLK_SET_RATE_PARENT, 1, 2); >> - *dt_clk = clk; >> -#endif >> } >> >> -- >> 2.3.0 >> -- 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/