Re: [PATCH 2/2] pmdomain: core: Initialize state_idx to deepest state for OFF domains

On Tue, Aug 11, 2026 at 7:50 AM Maulik Shah
<[email protected]> wrote:
>
> pm_genpd_init() sets genpd->status from is_off but never touches state_idx,
> which stays at its kzalloc'd value of 0 (dt_idle_pd_alloc()) regardless of
> is_off. A domain that starts OFF without ever having gone through an actual
> power-off sequence is therefore left looking like it's parked at its
> shallowest idle state (state_idx=0) instead of its deepest one, even though
> it is fully powered down.
>
> This is visible on hamoa, where cluster0/1/2 each expose two idle states,
> CL4 (state_idx=0) and CL5 (state_idx=1, the deepest). When CPU-cluster
> genpds are initialised directly into GENPD_STATE_OFF under OSI mode
> ("cpuidle: psci: Start CPU-cluster pmdomain OFF when OSI is used"),
> clusters whose CPUs are excluded at boot (e.g. maxcpus=4, leaving clusters
> 1 and 2 untouched) never go through a power-off path, so their state_idx
> remains 0 instead of the deepest 1. debugfs reports this as "off-0",
> meaning the domain is off but parked at idle-state 0 (CL4) rather than
> "off-1", idle-state 1 (CL5):
>
> genpd_power_off() and genpd_sync_power_off() both gate parent power-off on
> every child being at its deepest state index:
>
>   if (child->state_idx < child->state_count - 1)
>       return;
>
> so the parent's check treats these fully-off children as "not yet at
> deepest state" and refuses to power off, blocking the deepest system-level
> low-power mode (SS3) from ever being entered.
>
> Without this change:
>
>   $ cat .../power-domain-cpu-cluster1/current_state
>   off-0
>
>   $ cat /sys/kernel/debug/qcom_stats/apss
>   Count: 0
>   Last Entered At: 0
>   Last Exited At: 0
>   Accumulated Duration: 0
>
> Fix this at the source: when a domain is initialised OFF, set its state_idx
> to its deepest state (state_count - 1) instead of leaving it at 0. This
> makes genpd's reported state consistent with reality for every is_off=true
> caller of pm_genpd_init(), not just the PSCI CPU-cluster case, and requires
> no special-casing in the parent power-off checks.
>
> With this change, the domain correctly reports its deepest state and SS3
> is entered normally:
>
>   $ cat .../power-domain-cpu-cluster1/current_state
>   off-1
>
>   $ cat /sys/kernel/debug/qcom_stats/apss
>   Count: 218
>   Last Entered At: 726792712
>   Last Exited At: 726950687
>   Accumulated Duration: 199888773
>
> Fixes: e7d90cfac551 ("PM: domains: Prevent power off for parent unless child is in deepest state")
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Maulik Shah <[email protected]>
> ---
>  drivers/pmdomain/core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 842c4169e290..5d96d9eabc1f 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -2409,6 +2409,8 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
>         INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
>         atomic_set(&genpd->sd_count, 0);
>         genpd->status = is_off ? GENPD_STATE_OFF : GENPD_STATE_ON;
> +       if (is_off && genpd->state_count)
> +               genpd->state_idx = genpd->state_count - 1;

Even if this is probably the most sane default value, I would prefer
to keep the flexibility.

That said, please make this change on the genpd provider driver
instead of in the core.

>         genpd_set_stay_on(genpd, is_off);
>         genpd->sync_state = GENPD_SYNC_STATE_OFF;
>         genpd->device_count = 0;
>
> --
> 2.43.0
>

Kind regards
Uffe