On Mon, 2022-09-26 at 10:31 +0200, Matthias Schiffer wrote: > The e10133 workaround was broken in two places: > > - The code intended to temporarily mask all interrupts in GPC_IMRx_CORE0. > While the old register values were saved, the actual masking was > missing. > - imx_udelay() expects the system counter to run at its base frequency, > but the system counter is switched to a lower frequency earlier in > psci_system_suspend(), leading to a much longer delay than intended. > Replace the call with an equivalent loop (linux-imx 5.15 does the same) > > This fixes the SoC hanging forever when there was already a wakeup IRQ > pending while suspending. > > Fixes: 57b620255e ("imx: mx7: add system suspend/resume support") > Signed-off-by: Matthias Schiffer <matthias.schif...@ew.tq-group.com>
Ping, any feedback on this? I'm not entirely sure anymore if this solution is adequate, as the duration of the loop depends on the CPU clock frequency and cache enable/disable state. Maybe a modified imx_udelay() that accounts for the changed system counter configuration would be necessary after all? > --- > arch/arm/mach-imx/mx7/psci-mx7.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/mach-imx/mx7/psci-mx7.c > b/arch/arm/mach-imx/mx7/psci-mx7.c > index f32945ea37..699a2569cb 100644 > --- a/arch/arm/mach-imx/mx7/psci-mx7.c > +++ b/arch/arm/mach-imx/mx7/psci-mx7.c > @@ -643,8 +643,10 @@ __secure void psci_system_suspend(u32 __always_unused > function_id, > /* disable GIC distributor */ > writel(0, GIC400_ARB_BASE_ADDR + GIC_DIST_OFFSET); > > - for (i = 0; i < 4; i++) > + for (i = 0; i < 4; i++) { > gpc_mask[i] = readl(GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); > + writel(~0, GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); > + } > > /* > * enable the RBC bypass counter here > @@ -668,7 +670,7 @@ __secure void psci_system_suspend(u32 __always_unused > function_id, > writel(gpc_mask[i], GPC_IPS_BASE_ADDR + GPC_IMR1_CORE0 + i * 4); > > /* > - * now delay for a short while (3usec) > + * now delay for a short while (~3usec) > * ARM is at 1GHz at this point > * so a short loop should be enough. > * this delay is required to ensure that > @@ -677,7 +679,8 @@ __secure void psci_system_suspend(u32 __always_unused > function_id, > * or in case an interrupt arrives just > * as ARM is about to assert DSM_request. > */ > - imx_udelay(3); > + for (i = 0; i < 2000; i++) > + asm volatile(""); > > /* save resume entry and sp in CPU0 GPR registers */ > asm volatile("mov %0, sp" : "=r" (val));