| Message ID | 20180830185404.7224-3-digetx@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | EMC fixes for Tegra30+ | expand |
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. 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. The other question I have, should we be restoring the refresh value here somewhere? Cheers Jon
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.
On 20.11.2018 1:09, Dmitry Osipenko wrote: > 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. Ah, T20 exit_selfrefresh_loop doesn't latch registers.. that's probably why it stayed unnoticed.
On 19/11/2018 22:09, Dmitry Osipenko wrote: > 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? No the more I look at this, I just think it is a badly describe register in the TRM. I think that your description is correct afterall. Cheers Jon
On 19/11/2018 22:32, Dmitry Osipenko wrote: > On 20.11.2018 1:09, Dmitry Osipenko wrote: >> 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. > > Ah, T20 exit_selfrefresh_loop doesn't latch registers.. that's probably why it stayed unnoticed. Good to know. Cheers Jon
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 Acked-by: Jon Hunter <jonathanh@nvidia.com> Tested-by: Jon Hunter <jonathanh@nvidia.com> Cheers Jon
On 20.11.2018 13:26, Jon Hunter wrote: > > On 19/11/2018 22:32, Dmitry Osipenko wrote: >> On 20.11.2018 1:09, Dmitry Osipenko wrote: >>> 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. >> >> Ah, T20 exit_selfrefresh_loop doesn't latch registers.. that's probably why it stayed unnoticed. > > Good to know. Thank you for pointing at it, will be fixed in v2.
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
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(-)