On 20.11.2018 13:27, Jon Hunter wrote: > > On 19/11/2018 22:35, Dmitry Osipenko wrote: >> On 20.11.2018 1:00, Jon Hunter wrote: >>> >>> On 30/08/2018 19:54, Dmitry Osipenko wrote: >>>> Two interrupts are raised on resume from LP1 on Tegra30+: first is the >>>> clock change completed interrupt which is set after updating timing >>>> configuration, second is DLL alarm interrupt which is set when DLL >>>> starts re-calibration after being reset. Clear these two interrupts >>>> in the end of exiting from the self-refresh mode for consistency, that >>>> will also allow to not receive spurious interrupts in the EMC driver >>>> after resume from suspend. >>>> >>>> Signed-off-by: Dmitry Osipenko <[email protected]> >>>> --- >>>> arch/arm/mach-tegra/sleep-tegra30.S | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>>> >>>> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S >>>> b/arch/arm/mach-tegra/sleep-tegra30.S >>>> index 828f6c37afde..78c6e9fb56e7 100644 >>>> --- a/arch/arm/mach-tegra/sleep-tegra30.S >>>> +++ b/arch/arm/mach-tegra/sleep-tegra30.S >>>> @@ -26,6 +26,7 @@ >>>> #include "irammap.h" >>>> #include "sleep.h" >>>> >>>> +#define EMC_INTSTATUS 0x0 >>>> #define EMC_CFG 0xc >>>> #define EMC_ADR_CFG 0x10 >>>> #define EMC_TIMING_CONTROL 0x28 >>>> @@ -44,6 +45,9 @@ >>>> #define EMC_XM2VTTGENPADCTRL 0x310 >>>> #define EMC_XM2VTTGENPADCTRL2 0x314 >>>> >>>> +#define EMC_CLKCHANGE_COMPLETE_INT (1 << 4) >>>> +#define EMC_DLL_ALARM_INT (1 << 7) >>>> + >>>> #define MC_EMEM_ARB_CFG 0x90 >>>> >>>> #define PMC_CTRL 0x0 >>>> @@ -539,6 +543,9 @@ zcal_done: >>>> >>>> emc_timing_update r1, r0 >>>> >>>> + mov r1, #(EMC_CLKCHANGE_COMPLETE_INT | EMC_DLL_ALARM_INT) >>>> + str r1, [r0, #EMC_INTSTATUS] @ clear interrupts >>>> + >>>> /* Tegra114 had dual EMC channel, now config the other one */ >>>> cmp r10, #TEGRA114 >>>> bne __no_dual_emc_chanl >>>> >>> >>> Where are these interrupts enabled? I did not see where they are >>> enabled. I see that the Tegra24 EMC driver does poll these, but it did >>> not look like they were enabled. If they are enabled, I wondering if >>> they should be masked on entering self-refresh? >> >> EMC interrupt is not enabled on Tegra124. IIRC, it doesn't use the interrupt >> at all in the driver. Indeed, T124 EMC driver polls the interrupt status, >> but it clears the status before starting to poll. Probably I was just >> thinking about to write T30 EMC driver at that time and to utilize the >> interrupt.. >> >> Seems it should be okay to drop this patch for now, but maybe then we will >> have to return to it sometime later. Up to you to decide. > > I would be tempted to drop for now. If you have such a driver maybe the > driver should mask any interrupts it uses on suspending.
I don't have such driver yet. Masking interrupts on suspend and clearing status on resume within the driver should result in possibility to miss some trouble that happened between "CPU wake"->"EMC driver resuming", but probably that's not too important and unlikely to happen.

