linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dhruva G <goledhruva@gmail.com>
To: Ulf Hansson <ulf.hansson@oss.qualcomm.com>,
	Sudeep Holla <sudeep.holla@kernel.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Daniel Lezcano <daniel.lezcano@kernel.org>,
	linux-pm@vger.kernel.org
Cc: Abel Vesa <abel.vesa@oss.qualcomm.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Christian Loehle <christian.loehle@arm.com>,
	Maulik Shah <maulik.shah@oss.qualcomm.com>,
	Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>,
	Sneh Mankad <sneh.mankad@oss.qualcomm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/5] cpuidle: psci: Initialize the PM domains in powered off state for OSI
Date: Fri, 11 Sep 2026 22:04:42 +0530	[thread overview]
Message-ID: <327e3afc-0803-4ca4-9c76-249077ab9b2e@gmail.com> (raw)
In-Reply-To: <20260907111659.263324-5-ulf.hansson@oss.qualcomm.com>

On 07-09-2026 16:46, Ulf Hansson wrote:
> At the point when the PM domain and the topology are registered through the
> genpd subsystem, it's not really known whether corresponding CPUs are
> online and thus if the PM domain should be initialized as powered on or
> not. Instead this information becomes available when the CPU devices gets
> attached to their respective PM domain through dt_idle_attach_cpu().
> 
> This is a problem when using PSCI OS-initiated mode, as we may end up with
> a PM domain that has the genpd's status indicating it to be powered on,
> while it in fact may not be the case. In the less severe scenario, this
> leads to selecting a shallower domain idle state for the PM domain than
> necessary. A more critical problem is when a non-CPU device shares the PM
> domain, leading to their corresponding drivers not being able to trust the
> status of it.
> 
> Let's fix these problems by initializing the state for the genpd's to be
> powered off and in the deepest possible domain idle state, when using
> OS-initiated mode. The support for ->sync_state() is maintained by setting
> the GENPD_FLAG_POWER_UNKNOWN for the genpds in question.
> 
> Reported-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> Link: https://lore.kernel.org/all/20260811-domain_off_ss3-v1-0-6a0a0fc023f5@oss.qualcomm.com/
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Tested-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@oss.qualcomm.com>
> ---
> 
> Changes in v3:
> 	- None.
> Changes in v2:
> 	- Fix a bug in the call to pm_genpd_init().
> 
> ---
>  drivers/cpuidle/cpuidle-psci-domain.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c
> index b9e4ad7d43a3..4d8c63d329c2 100644
> --- a/drivers/cpuidle/cpuidle-psci-domain.c
> +++ b/drivers/cpuidle/cpuidle-psci-domain.c
> @@ -68,7 +68,8 @@ static int psci_pd_init(struct device_node *np, bool use_osi)
>  	 */
>  	if (use_osi) {
>  		pd->power_off = psci_pd_power_off;
> -		pd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
> +		pd->flags |= GENPD_FLAG_ACTIVE_WAKEUP | GENPD_FLAG_POWER_UNKNOWN;
> +		pd->state_idx = pd->state_count ? pd->state_count - 1 : 0;
>  		if (IS_ENABLED(CONFIG_PREEMPT_RT))
>  			pd->flags |= GENPD_FLAG_RPM_ALWAYS_ON;
>  	} else {
> @@ -78,7 +79,7 @@ static int psci_pd_init(struct device_node *np, bool use_osi)
>  	/* Use governor for CPU PM domains if it has some states to manage. */
>  	pd_gov = pd->states ? &pm_domain_cpu_gov : NULL;
>  
> -	ret = pm_genpd_init(pd, pd_gov, false);
> +	ret = pm_genpd_init(pd, pd_gov, use_osi);

If CONFIG_PREEMPT_RT=y,

psci_pd_init(use_osi=true)
  -> sets GENPD_FLAG_POWER_UNKNOWN
  -> sets GENPD_FLAG_RPM_ALWAYS_ON
  -> pm_genpd_init(..., is_off=true)
  -> genpd->status = GENPD_STATE_OFF
  -> RPM_ALWAYS_ON + OFF is rejected (pmdomain/core.c: pm_genpd_init() 
				      rejects an RPM_ALWAYS_ON domain whose initial state is OFF)
  -> return -EINVAL

Therefore, on a PREEMPT_RT platform using OSI and hierarchical PSCI domains, psci_cpuidle_domain_probe()
should fail while initializing the first domain. The genpd providers are then unavailable, so 
dt_idle_attach_cpu() fails during PSCI cpuidle initialization and the driver rolls back its CPU
registrations.

I don't have a device on me to test this path, perhaps one of the QC devices + RT config
can reproduce this?

Should PREEMPT_RT case instead initialize these domains as ON?

This might be safer here atleast:

ret = pm_genpd_init(pd, pd_gov,
		    use_osi && !IS_ENABLED(CONFIG_PREEMPT_RT));

Regards,
Dhruva

  reply	other threads:[~2026-09-11 16:34 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 11:16 [PATCH v3 0/5] pmdomain/cpuidle-psci: Fix behaviours for CPU PM domains Ulf Hansson
2026-09-07 11:16 ` [PATCH v3 1/5] pmdomain: core: Rename genpd_status_on() Ulf Hansson
2026-09-07 11:16 ` [PATCH v3 2/5] pmdomain: core: Allow a non-CPU device in a CPU PM domain to do power on Ulf Hansson
2026-09-10 18:31   ` Dhruva G
2026-09-11  9:06     ` Ulf Hansson
2026-09-11 10:16       ` Ulf Hansson
2026-09-11 15:33         ` Dhruva G
2026-09-07 11:16 ` [PATCH v3 3/5] pmdomain: core: Add a genpd config to support unknown initial status Ulf Hansson
2026-09-11 16:13   ` Dhruva G
2026-09-07 11:16 ` [PATCH v3 4/5] cpuidle: psci: Initialize the PM domains in powered off state for OSI Ulf Hansson
2026-09-11 16:34   ` Dhruva G [this message]
2026-09-14 11:28     ` Ulf Hansson
2026-09-07 11:16 ` [PATCH v3 5/5] cpuidle: psci: Move initialization a bit earlier in the boot sequence Ulf Hansson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=327e3afc-0803-4ca4-9c76-249077ab9b2e@gmail.com \
    --to=goledhruva@gmail.com \
    --cc=abel.vesa@oss.qualcomm.com \
    --cc=christian.loehle@arm.com \
    --cc=daniel.lezcano@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=maulik.shah@oss.qualcomm.com \
    --cc=rafael@kernel.org \
    --cc=sneh.mankad@oss.qualcomm.com \
    --cc=sudeep.holla@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=ulf.hansson@oss.qualcomm.com \
    --cc=yuanfang.zhang@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).