From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Subject: Re: [PATCH] clk: tegra: Mark APB clock as critical To: Peter De Schrijver Cc: Jon Hunter , Prashant Gaikwad , Michael Turquette , Stephen Boyd , Thierry Reding , linux-clk@vger.kernel.org, linux-tegra@vger.kernel.org References: <95c14859-a2c7-1c61-adba-bd6c16155c01@gmail.com> <9d479a38-f40b-0e58-09c3-d06e9ee32a25@nvidia.com> <6264cf17-0850-37dd-ca92-9362521d2db6@gmail.com> <236aa250-3b9b-94de-0978-0fb8546d504d@nvidia.com> <20171201084818.GI32106@tbergstrom-lnx.Nvidia.com> <803890bc-e3fe-134a-2127-3363187d04f2@gmail.com> <20171205090635.GL32106@tbergstrom-lnx.Nvidia.com> From: Dmitry Osipenko Message-ID: <0e593ef4-8f22-92c7-d119-2aec7a7009f1@gmail.com> Date: Tue, 5 Dec 2017 21:22:57 +0300 MIME-Version: 1.0 In-Reply-To: <20171205090635.GL32106@tbergstrom-lnx.Nvidia.com> Content-Type: text/plain; charset=utf-8 List-ID: On 05.12.2017 12:06, Peter De Schrijver wrote: > On Sat, Dec 02, 2017 at 03:47:32PM +0300, Dmitry Osipenko wrote: >> On 01.12.2017 11:48, Peter De Schrijver wrote: >>> On Thu, Nov 30, 2017 at 04:39:22PM +0000, Jon Hunter wrote: >>>> >>>> On 30/11/17 13:24, Dmitry Osipenko wrote: >>>>> On 30.11.2017 14:31, Jon Hunter wrote: >>>>>> >>>>>> On 29/11/17 23:13, Dmitry Osipenko wrote: >>>>>>> On 30.11.2017 01:55, Jon Hunter wrote: >>>>>> >>>>>> ... >>>>>> >>>>>>> I've asked you to re-test Tegra114/124 or whatever was failing for you with the >>>>>>> PLL_M being marked as critical instead of PCLK. Maybe it was PLL_M that actually >>>>>>> caused trouble on Tegra114/124. >>>>>> >>>>>> Please share the exact change you would like me to test and I will. >>>>> >>>>> Please try this: >>>> >>>> I tried the patch, but this does not work for Tegra124 it still hangs. >>>> Tracing the clk calls the last thing I see is ... >>>> >>>> [ 2.687846] tegra124-dfll 70110000.clock: couldn't get vdd_cpu regulator >>>> [ 2.694403] clk_prepare: hclk_div >>>> [ 2.695929] clk_prepare_complete: hclk_div >>>> [ 2.700027] clk_prepare: hclk >>>> [ 2.702947] clk_prepare_complete: hclk >>>> [ 2.706673] clk_prepare: pclk_div >>>> [ 2.709986] clk_prepare_complete: pclk_div >>>> [ 2.714039] clk_prepare: pclk >>>> [ 2.716985] clk_prepare_complete: pclk >>>> [ 2.720739] clk_prepare: apbdma >>>> [ 2.723833] clk_prepare_complete: apbdma >>>> [ 2.727736] clk_enable: hclk_div >>>> [ 2.730940] clk_enable_complete: hclk_div >>>> [ 2.734926] clk_enable: hclk >>>> [ 2.737788] clk_enable_complete: hclk >>>> [ 2.741426] clk_enable: pclk_div >>>> [ 2.744633] clk_enable_complete: pclk_div >>>> [ 2.748619] clk_enable: pclk >>>> [ 2.751481] clk_enable_complete: pclk >>>> [ 2.755120] clk_enable: apbdma >>>> [ 2.758153] clk_enable_complete: apbdma >>>> [ 2.762390] clk_disable: apbdma >>>> [ 2.765088] clk_disable_complete: apbdma >>>> [ 2.768986] clk >>>> >>>> So I believe this change is correct and that Peter's analysis on IRC >>>> seems correct, that this change has exposed another issue with the clock >>>> driver. >>>> >>>> Peter, can you summarise which other clocks should be made critical? >>> >>> The following clocks should be critical: >>> >>> pclk >>> hclk >>> sclk >>> emc >>> mc >>> pll_p >>> >>> I think that's it. >> >> Could you please elaborate why pll_p should be critical? Is it applicable to all >> Tegra generations? > > There are quite many clocks relying on pll_p as their source. In theory we > might get away with making sure all thsoe clocks are critical if needed, but > it's usually easier to just make sure we never turn off pll_p. We should > certainly never try to change the rate of pll_p. Obviously also osc and clk32k > should never be turned off during runtime, but there is no hw which allows you > to do this. Okay, I see it is explicitly documented in TRM that "modules take in an additional fix PLL clock source for a portion of their logic" and indeed most of periperals use pll_p and its dividers for the "portion of logic".