Re: [PATCH 1/2] ARM: omap: cpuidle34xx: balance cpu_pm_enter/exit()

[Kevin Hilman <khilman@baylibre.com>]

Andreas Kemnade <andreas@kemnade.info> writes:

> On Wed, 28 Oct 2026 20:15:11 +0100
> Andreas Kemnade <andreas@kemnade.info> wrote:
>
>> These calls are usually done in a balanced way, so allow notifiers to
>> assume that these calls are balanced under all circumstances.
>> Seen some spurios vfp issues probably connected to this.
>> 
>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
>
> Looked again at some discussions with Sicelo testing such things:
> He had observed a NEON off count 1 but MPU 0 in /sys/kernel/debug/pm_debug/count.
> Probably more involved, but lets not freak out things here.
> So that is definitively a real problem fixed by this. vfp_pm_resume()
> would not be called to restore the state without a patch like this.
>
> So this is
>
> Fixes: 1cd96478cf150 ("ARM: OMAP3xxx: CPUIdle: optimize __omap3_enter_idle()")

Even with this fix, I found on OMAP3 (beagle and beagle-xm) that there
are still intermittent failures, and the NEON vs. MPU count mismatch was
a good indicator to dig a little deeper.

I think the fix here is to add the neon_clkdm -> mpu_clkdm wakeup
dependency before the clockdomains are allowed to idle.  Otherwise NEON
could idle immediately.

With the patch below, I don't see any intermittent VFP crashes anymore
with early testing.

I'm still doing a few more tests, but will sent a proper patch with
changelog after some more testing.

Kevin


diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 9992549c7336..d11967c2f4bf 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -510,6 +510,20 @@ int __init omap3_pm_init(void)
 		goto err3;
 	}
 
+	neon_clkdm = clkdm_lookup("neon_clkdm");
+	mpu_clkdm = clkdm_lookup("mpu_clkdm");
+	per_clkdm = clkdm_lookup("per_clkdm");
+	wkup_clkdm = clkdm_lookup("wkup_clkdm");
+
+	/*
+	 * Add the NEON wakeup dependency on the MPU before the clockdomains
+	 * are allowed to idle: once pwrdms_setup() has programmed NEON's next
+	 * state to OFF/RET, an idle NEON clockdomain otherwise lets NEON power
+	 * down while the MPU keeps running, and VFP instructions then trap as
+	 * undefined until NEON is woken.
+	 */
+	clkdm_add_wkdep(neon_clkdm, mpu_clkdm);
+
 	(void) clkdm_for_each(omap_pm_clkdms_setup, NULL);
 
 	mpu_pwrdm = pwrdm_lookup("mpu_pwrdm");
@@ -523,11 +537,6 @@ int __init omap3_pm_init(void)
 	per_pwrdm = pwrdm_lookup("per_pwrdm");
 	core_pwrdm = pwrdm_lookup("core_pwrdm");
 
-	neon_clkdm = clkdm_lookup("neon_clkdm");
-	mpu_clkdm = clkdm_lookup("mpu_clkdm");
-	per_clkdm = clkdm_lookup("per_clkdm");
-	wkup_clkdm = clkdm_lookup("wkup_clkdm");
-
 	omap_common_suspend_init(omap3_pm_suspend);
 
 	arm_pm_idle = omap3_pm_idle;
@@ -562,7 +571,6 @@ int __init omap3_pm_init(void)
 	if (IS_PM34XX_ERRATUM(PM_PER_MEMORIES_ERRATUM_i582))
 		clkdm_add_wkdep(per_clkdm, wkup_clkdm);
 
-	clkdm_add_wkdep(neon_clkdm, mpu_clkdm);
 	if (omap_type() != OMAP2_DEVICE_TYPE_GP) {
 		omap3_secure_ram_storage =
 			kmalloc(OMAP3_SAVE_SECURE_RAM_SZ, GFP_KERNEL);