Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30

From: Dmitry Osipenko <digetx@gmail.com>
To: Jon Hunter <jonathanh@nvidia.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 2/4] ARM: tegra: Fix DRAM refresh-interval clobbering on resume from LP1 on Tegra30
Date: Tue, 20 Nov 2018 01:09:20 +0300	[thread overview]
Message-ID: <cee4398b-edb8-4c9d-7830-6f52fffd6e1c@gmail.com> (raw)
In-Reply-To: <4f61bf5f-0aa8-df6e-109b-194b08f3374e@nvidia.com>

On 20.11.2018 0:34, Jon Hunter wrote:
> 
> On 30/08/2018 19:54, Dmitry Osipenko wrote:
>> The DRAM refresh-interval is getting erroneously set to "1" on exiting
>> from memory self-refreshing mode. The clobbered interval causes the
>> "refresh request overflow timeout" error raised by the External Memory
>> Controller on exiting from LP1 on Tegra30.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  arch/arm/mach-tegra/sleep-tegra30.S | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>> index 801fe58978ae..99ac9c6dcf7c 100644
>> --- a/arch/arm/mach-tegra/sleep-tegra30.S
>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S
>> @@ -29,7 +29,6 @@
>>  #define EMC_CFG				0xc
>>  #define EMC_ADR_CFG			0x10
>>  #define EMC_TIMING_CONTROL		0x28
>> -#define EMC_REFRESH			0x70
>>  #define EMC_NOP				0xdc
>>  #define EMC_SELF_REF			0xe0
>>  #define EMC_MRW				0xe8
>> @@ -459,7 +458,6 @@ emc_wait_auto_cal_onetime:
>>  	cmp	r10, #TEGRA30
>>  	streq	r1, [r0, #EMC_NOP]
>>  	streq	r1, [r0, #EMC_NOP]
>> -	streq	r1, [r0, #EMC_REFRESH]
>>  
>>  	emc_device_mask r1, r0
> 
> This does look incorrect and it appears Tegra20 has the same bug.

Indeed.. somehow this doesn't cause any problems on T20. Maybe this affects only specific timing configurations and it's just a luck that "refresh overflow" isn't getting raised.

> However, looking at the EMC_REFRESH register it appears that bits 5:0
> are the REFRESH_LO and bits 15:6 are the refresh interval. So this seems
> to imply the interval is set to 0 and not 1. So maybe the commit message
> needs to be fixed up.

Do you mean that EMC_REFRESH is a fractional value?

> The other question I have, should we be restoring the refresh value here
> somewhere?

The EMC_REFRESH value isn't altered on enter/exit self-refresh, at least on Tegra30.. very likely it should be the same for other gens.