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

[Andreas Kemnade <andreas@kemnade.info>]

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