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.schiffer@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));