Re: [PATCH v6 07/12] ARM: tegra: Don't enable PLLX while resuming from LP1 on Tegra30

From: Dmitry Osipenko <digetx@gmail.com>
To: Marcel Ziswiler <marcel@ziswiler.com>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Peter De Schrijver <pdeschrijver@nvidia.com>,
	Prashant Gaikwad <pgaikwad@nvidia.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Peter Geis <pgwipeout@gmail.com>,
	Nicolas Chauvet <kwizart@gmail.com>,
	linux-pm@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 07/12] ARM: tegra: Don't enable PLLX while resuming from LP1 on Tegra30
Date: Sun, 2 Feb 2020 04:08:12 +0300	[thread overview]
Message-ID: <31e4cf7e-9bdc-c9f8-7e6d-a972f43fc5be@gmail.com> (raw)
In-Reply-To: <334ac00a0c83e4178e3195cca5d77acaf4a3fae7.camel@ziswiler.com>

02.02.2020 01:57, Marcel Ziswiler пишет:
> Hi Dmitry
> 
> Finally, I came around giving this a try again on the Colibri T20 and
> Apalis/Colibri T30.
> 
> Overall, works like a charm and you may add the following to the entire
> series:
> 
> On Wed, 2019-12-18 at 23:21 +0300, Dmitry Osipenko wrote:
>> PLLX may be kept disabled if cpufreq driver selects some other clock
>> for
>> CPU. In that case PLLX will be disabled later in the resume path by
>> the
>> CLK driver, which also can enable PLLX if necessary by itself. Thus
>> there
>> is no need to enable PLLX early during resume. Tegra114/124 CLK
>> drivers do
>> not manage PLLX on resume and thus they are left untouched by this
>> patch.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> 
> Tested-by: Marcel Ziswiler <marcel@ziswiler.com>
> Tested-on: Colibri T20, Apalis/Colibri T30 on resp. EvalBoards

Hello Marcel,

Thank you very much for the testing! Very appreciate that!

Could you please clarify whether you tried to enable the CPU DVFS by
including OPPs/etc into device-tree files of the tested boards?

>> ---
>>  arch/arm/mach-tegra/sleep-tegra30.S | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-
>> tegra/sleep-tegra30.S
>> index 9a20c93abe48..4f073869b8ac 100644
>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>> @@ -358,7 +358,6 @@ _no_pll_iddq_exit:
>>  
>>  	pll_enable r1, r0, CLK_RESET_PLLM_BASE, CLK_RESET_PLLM_MISC
>>  	pll_enable r1, r0, CLK_RESET_PLLC_BASE, CLK_RESET_PLLC_MISC
>> -	pll_enable r1, r0, CLK_RESET_PLLX_BASE, CLK_RESET_PLLX_MISC
>>  
>>  _pll_m_c_x_done:
>>  	pll_enable r1, r0, CLK_RESET_PLLP_BASE, CLK_RESET_PLLP_MISC
>> @@ -368,8 +367,18 @@ _pll_m_c_x_done:
>>  	pll_locked r1, r0, CLK_RESET_PLLP_BASE
>>  	pll_locked r1, r0, CLK_RESET_PLLA_BASE
>>  	pll_locked r1, r0, CLK_RESET_PLLC_BASE
>> +
>> +	/*
>> +	 * CPUFreq driver could select other PLL for CPU. PLLX will be
>> +	 * enabled by the Tegra30 CLK driver on an as-needed basis, see
>> +	 * tegra30_cpu_clock_resume().
>> +	 */
>> +	cmp	r10, #TEGRA30
>> +	beq	_pll_m_c_x_locked
>> +
>>  	pll_locked r1, r0, CLK_RESET_PLLX_BASE
>>  
>> +_pll_m_c_x_locked:
>>  	mov32	r7, TEGRA_TMRUS_BASE
>>  	ldr	r1, [r7]
>>  	add	r1, r1, #LOCK_DELAY
> 
> Unfortunately, that one does no longer apply after the following patch
> recently got applied on Wed Jan 8:
> 
> commit 1a3388d506bf ("ARM: tegra: Enable PLLP bypass during Tegra124
> LP1")
> 
> Thanks!

Yes, I'm aware about it and going to send out the updated version of the
patches soon. Meanwhile you could find recent rebase of the patches here
[1].

[1] https://github.com/grate-driver/linux/commits/master