[PATCH 0/2] ARM: omap: cpuidle34xx: Fix unbalanced cpu_pm_enter()/exit() and cleanup

[PATCH 0/2] ARM: omap: cpuidle34xx: Fix unbalanced cpu_pm_enter()/exit() and cleanup

[Andreas Kemnade]

Fix unbalanced CPU pm calls and delegate the question whether the
cx pointer contents could change to the compiler.
During research for some spurios FPU issues, this oddity come to
light.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Andreas Kemnade (2):
      ARM: omap: cpuidle34xx: balance cpu_pm_enter/exit()
      ARM: omap: cpuidle34xx: Constify state info

 arch/arm/mach-omap2/cpuidle34xx.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
---
base-commit: 5fd8fdf561e0e6093305b6c993ee9b7c25bc8b51
change-id: 20261028-susp2-e2a9a2c9e71f

Best regards,
--  
Andreas Kemnade <andreas@kemnade.info>

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

[Andreas Kemnade]

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>
---
 arch/arm/mach-omap2/cpuidle34xx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 2ab5dcbfb7f6..e42dcf8c03a3 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -139,8 +139,7 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	 * Call idle CPU PM enter notifier chain to restore
 	 * VFP context.
 	 */
-	if (cx->mpu_state == PWRDM_POWER_OFF &&
-	    pwrdm_read_prev_pwrst(mpu_pd) == PWRDM_POWER_OFF)
+	if (cx->mpu_state == PWRDM_POWER_OFF)
 		cpu_pm_exit();
 
 out_clkdm_set:

-- 
2.47.3

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

[Andreas Kemnade]

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()")

> ---
>  arch/arm/mach-omap2/cpuidle34xx.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 2ab5dcbfb7f6..e42dcf8c03a3 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -139,8 +139,7 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
>  	 * Call idle CPU PM enter notifier chain to restore
>  	 * VFP context.
>  	 */
> -	if (cx->mpu_state == PWRDM_POWER_OFF &&
> -	    pwrdm_read_prev_pwrst(mpu_pd) == PWRDM_POWER_OFF)
> +	if (cx->mpu_state == PWRDM_POWER_OFF)
>  		cpu_pm_exit();
>  
>  out_clkdm_set:
> 

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

[Kevin Hilman]

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);

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

[Andreas Kemnade]

On Tue, 10 Nov 2026 11:48:47 -0800
Kevin Hilman <khilman@baylibre.com> wrote:

> 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.
> 
There seem to be different problems, cases where MPU sticks to INA
but does not go OFF (reported on N900), and that spurios vfp crash
I have seen on the GTA04.

> 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.
> 
sound same. I also looked at the wakeup dep, but only in the running system.

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

Then this ordeal comes to an end, I first thought that these are hw
issues. But then maybe it is time to reinit my automatic testing stuff.

> I'm still doing a few more tests, but will sent a proper patch with
> changelog after some more testing.
>
I will do that too. And then time to do some bisect to catch some omapdrm
bug. 8250_DMA also smells.

Regards,
Andreas
 
> 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);
> 
> 
> 
> 

[PATCH 2/2] ARM: omap: cpuidle34xx: Constify state info

[Andreas Kemnade]

omap3_idle_statedata is only static read only data,
so have that fact cleary visible.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 arch/arm/mach-omap2/cpuidle34xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index e42dcf8c03a3..16d2e1f08b48 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -58,7 +58,7 @@ static struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
  * Prevent PER OFF if CORE is not in RETention or OFF as this would
  * disable PER wakeups completely.
  */
-static struct omap3_idle_statedata omap3_idle_data[] = {
+static const struct omap3_idle_statedata omap3_idle_data[] = {
 	{
 		.mpu_state = PWRDM_POWER_ON,
 		.core_state = PWRDM_POWER_ON,
@@ -108,7 +108,7 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 			    struct cpuidle_driver *drv,
 			    int index)
 {
-	struct omap3_idle_statedata *cx = &omap3_idle_data[index];
+	const struct omap3_idle_statedata *cx = &omap3_idle_data[index];
 	int error;
 
 	if (omap_irq_pending() || need_resched())
@@ -168,7 +168,7 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 static int next_valid_state(struct cpuidle_device *dev,
 			    struct cpuidle_driver *drv, int index)
 {
-	struct omap3_idle_statedata *cx = &omap3_idle_data[index];
+	const struct omap3_idle_statedata *cx = &omap3_idle_data[index];
 	u32 mpu_deepest_state = PWRDM_POWER_RET;
 	u32 core_deepest_state = PWRDM_POWER_RET;
 	int idx;
@@ -221,7 +221,7 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 {
 	int new_state_idx, ret;
 	u8 per_next_state, per_saved_state;
-	struct omap3_idle_statedata *cx;
+	const struct omap3_idle_statedata *cx;
 
 	/*
 	 * Use only C1 if CAM is active.

-- 
2.47.3

Re: [PATCH 0/2] ARM: omap: cpuidle34xx: Fix unbalanced cpu_pm_enter()/exit() and cleanup

[Kevin Hilman]

On Wed, 28 Oct 2026 20:15:10 +0100, Andreas Kemnade wrote:
> Fix unbalanced CPU pm calls and delegate the question whether the
> cx pointer contents could change to the compiler.
> During research for some spurios FPU issues, this oddity come to
> light.

Applied, thanks!

[1/2] ARM: omap: cpuidle34xx: balance cpu_pm_enter/exit()
      commit: 97e0f6eb6112788e7a46376998e797b4d7d6f524
[2/2] ARM: omap: cpuidle34xx: Constify state info
      commit: 6d6f135fb1c38fc833e4189e1db47eb7060c0b1f

Best regards,
-- 
Kevin Hilman (TI) <khilman@baylibre.com>