Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] pmdomain/cpuidle-psci: Fix behaviours for CPU PM domains
@ 2026-09-07 11:16 Ulf Hansson
  2026-09-07 11:16 ` [PATCH v3 1/5] pmdomain: core: Rename genpd_status_on() Ulf Hansson
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Ulf Hansson @ 2026-09-07 11:16 UTC (permalink / raw)
  To: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm
  Cc: Dhruva Gole, Abel Vesa, Lorenzo Pieralisi, Christian Loehle,
	Maulik Shah, Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose,
	Ulf Hansson, linux-arm-kernel, linux-arm-msm, linux-kernel,
	Ulf Hansson

From: Ulf Hansson <ulfh@kernel.org>

This series fixes a couple of different, but highly related problems, when
dealing with CPU PM domains. More details are available in the commit messages
for each patch.

Note that, this series is a follow up to fix problems that have been pointed
out in a series [1] from Yuanfang Zhang (Qcom), to add support for new
components for the arm coresight subsystem.

Changes in v3:
	- Minor updates to patch1 and patch2.
	- Added tested/reviewed-by tags.

Changes in v2:
	- Dropped the RFC.
	- See updates in the changelog for each patch.

Kind regards
Uffe

[1]
[PATCH v2 00/12] coresight: Add CPU cluster funnel/replicator/tmc support
https://lore.kernel.org/all/20251218-cpu_cluster_component_pm-v2-0-2335a6ae62a0@oss.qualcomm.com/#r

Ulf Hansson (5):
  pmdomain: core: Rename genpd_status_on()
  pmdomain: core: Allow a non-CPU device in a CPU PM domain to do power
    on
  pmdomain: core: Add a genpd config to support unknown initial status
  cpuidle: psci: Initialize the PM domains in powered off state for OSI
  cpuidle: psci: Move initialization a bit earlier in the boot sequence

 drivers/cpuidle/cpuidle-psci-domain.c |   5 +-
 drivers/cpuidle/cpuidle-psci.c        |   2 +-
 drivers/pmdomain/core.c               | 112 +++++++++++++++++++++-----
 include/linux/pm_domain.h             |   9 +++
 4 files changed, 106 insertions(+), 22 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/5] pmdomain: core: Rename genpd_status_on()
  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 ` 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
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Ulf Hansson @ 2026-09-07 11:16 UTC (permalink / raw)
  To: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm
  Cc: Dhruva Gole, Abel Vesa, Lorenzo Pieralisi, Christian Loehle,
	Maulik Shah, Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose,
	Ulf Hansson, linux-arm-kernel, linux-arm-msm, linux-kernel

Rename the genpd_status_on() to genpd_status_on_unlocked() to make the name
available for a lock taking wrapper introduced by a subsequent change.

Suggested-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Tested-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
Reviewed-by: Dhruva Gole <goledhruva@gmail.com>
Signed-off-by: Ulf Hansson <ulf.hansson@oss.qualcomm.com>
---

Changes in v3:
	- Updated a comment in the code and the commit message.

Changes in v2:
	- New patch.
---
 drivers/pmdomain/core.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 052ec478dfcc..6abe8b198949 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -173,7 +173,7 @@ static const struct genpd_lock_ops genpd_raw_spin_ops = {
 #define genpd_lock_interruptible(p)	p->lock_ops->lock_interruptible(p)
 #define genpd_unlock(p)			p->lock_ops->unlock(p)
 
-#define genpd_status_on(genpd)		(genpd->status == GENPD_STATE_ON)
+#define genpd_status_on_unlocked(genpd)	(genpd->status == GENPD_STATE_ON)
 #define genpd_is_irq_safe(genpd)	(genpd->flags & GENPD_FLAG_IRQ_SAFE)
 #define genpd_is_always_on(genpd)	(genpd->flags & GENPD_FLAG_ALWAYS_ON)
 #define genpd_is_active_wakeup(genpd)	(genpd->flags & GENPD_FLAG_ACTIVE_WAKEUP)
@@ -771,8 +771,7 @@ EXPORT_SYMBOL_GPL(dev_pm_genpd_rpm_always_on);
  * @dev: Device to get the current power status
  *
  * This function checks whether the generic power domain associated with the
- * given device is on or not by verifying if genpd_status_on equals
- * GENPD_STATE_ON.
+ * given device is on or not by checking if genpd->status equals GENPD_STATE_ON.
  *
  * Note: this function returns the power status of the genpd at the time of the
  * call. The power status may change after due to activity from other devices
@@ -791,7 +790,7 @@ bool dev_pm_genpd_is_on(struct device *dev)
 		return false;
 
 	genpd_lock(genpd);
-	is_on = genpd_status_on(genpd);
+	is_on = genpd_status_on_unlocked(genpd);
 	genpd_unlock(genpd);
 
 	return is_on;
@@ -963,7 +962,7 @@ static void genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 	 * The domain was on at boot and still need to stay on.
 	 * The domain has a subdomain being powered on.
 	 */
-	if (!genpd_status_on(genpd) || genpd->prepared_count > 0 ||
+	if (!genpd_status_on_unlocked(genpd) || genpd->prepared_count > 0 ||
 	    genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd) ||
 	    genpd->stay_on || atomic_read(&genpd->sd_count) > 0)
 		return;
@@ -1040,7 +1039,7 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
 	struct gpd_link *link;
 	int ret = 0;
 
-	if (genpd_status_on(genpd))
+	if (genpd_status_on_unlocked(genpd))
 		return 0;
 
 	/* Reflect over the entered idle-states residency for debugfs. */
@@ -1406,7 +1405,7 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
 {
 	struct gpd_link *link;
 
-	if (!genpd_status_on(genpd) || genpd_is_always_on(genpd))
+	if (!genpd_status_on_unlocked(genpd) || genpd_is_always_on(genpd))
 		return;
 
 	if (genpd->suspended_count != genpd->device_count
@@ -1472,7 +1471,7 @@ static void genpd_sync_power_on(struct generic_pm_domain *genpd, bool use_lock,
 {
 	struct gpd_link *link;
 
-	if (genpd_status_on(genpd))
+	if (genpd_status_on_unlocked(genpd))
 		return;
 
 	list_for_each_entry(link, &genpd->child_links, child_node) {
@@ -2170,7 +2169,8 @@ static int genpd_add_subdomain(struct generic_pm_domain *genpd,
 	genpd_lock(subdomain);
 	genpd_lock_nested(genpd, SINGLE_DEPTH_NESTING);
 
-	if (!genpd_status_on(genpd) && genpd_status_on(subdomain)) {
+	if (!genpd_status_on_unlocked(genpd) &&
+	    genpd_status_on_unlocked(subdomain)) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2186,7 +2186,7 @@ static int genpd_add_subdomain(struct generic_pm_domain *genpd,
 	list_add_tail(&link->parent_node, &genpd->parent_links);
 	link->child = subdomain;
 	list_add_tail(&link->child_node, &subdomain->child_links);
-	if (genpd_status_on(subdomain))
+	if (genpd_status_on_unlocked(subdomain))
 		genpd_sd_counter_inc(genpd);
 
  out:
@@ -2246,7 +2246,7 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 		list_del(&link->parent_node);
 		list_del(&link->child_node);
 		kfree(link);
-		if (genpd_status_on(subdomain))
+		if (genpd_status_on_unlocked(subdomain))
 			genpd_sd_counter_dec(genpd);
 
 		ret = 0;
@@ -2441,7 +2441,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
 
 	/* Always-on domains must be powered on at initialization. */
 	if ((genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd)) &&
-			!genpd_status_on(genpd)) {
+			!genpd_status_on_unlocked(genpd)) {
 		pr_err("always-on PM domain %s is not on\n", genpd->name);
 		return -EINVAL;
 	}
@@ -3839,7 +3839,7 @@ static int genpd_summary_one(struct seq_file *s,
 
 	if (WARN_ON(genpd->status >= ARRAY_SIZE(status_lookup)))
 		goto exit;
-	if (!genpd_status_on(genpd))
+	if (!genpd_status_on_unlocked(genpd))
 		snprintf(state, sizeof(state), "%s-%u",
 			 status_lookup[genpd->status], genpd->state_idx);
 	else
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/5] pmdomain: core: Allow a non-CPU device in a CPU PM domain to do power on
  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 ` Ulf Hansson
  2026-09-10 18:31   ` Dhruva G
  2026-09-07 11:16 ` [PATCH v3 3/5] pmdomain: core: Add a genpd config to support unknown initial status Ulf Hansson
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2026-09-07 11:16 UTC (permalink / raw)
  To: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm
  Cc: Dhruva Gole, Abel Vesa, Lorenzo Pieralisi, Christian Loehle,
	Maulik Shah, Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose,
	Ulf Hansson, linux-arm-kernel, linux-arm-msm, linux-kernel

A driver for a non-CPU device that is attached to a CPU PM domain (the
genpd has the GENPD_FLAG_CPU_DOMAIN configuration set), is currently not
able to power on the PM domain. More precisely, to power on a CPU PM domain
one of its corresponding CPUs needs to be woken up if they are idle.

The current support for a non-CPU device is that its driver can only
prevent an already powered on CPU PM domain from being powered off. This
leads to problems for a driver while probing its device or when it needs to
call pm_runtime_get_sync() to turn on the power for it. From the driver
point of view it looks like it all works fine, but when accessing the
device it may end up with various errors as the device may not be fully
powered on.

To fix the behavior for these types of devices, let's adjust the behaviour
in genpd_power_on() to wake up an idle CPU that belongs to it, in cases
when it's needed.

Link: https://lore.kernel.org/all/CAPx+jO-sCierYj8jnoKQHckJG16dOBxnNrsZVYO=38R2cLV8nw@mail.gmail.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:
	- Moved to atomic polling, pointed out by Dhruva.

Changes in v2:
	- Rename a function according to Abel's suggestion.
---
 drivers/pmdomain/core.c | 80 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 75 insertions(+), 5 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 6abe8b198949..b62d4e544bc5 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -10,6 +10,7 @@
 #include <linux/idr.h>
 #include <linux/kernel.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/platform_device.h>
 #include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
@@ -19,11 +20,14 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/sched.h>
+#include <linux/smp.h>
 #include <linux/suspend.h>
 #include <linux/export.h>
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
 
+#include <trace/events/ipi.h>
+
 /* Provides a unique ID for each genpd device */
 static DEFINE_IDA(genpd_ida);
 
@@ -32,7 +36,9 @@ static const struct bus_type genpd_provider_bus_type = {
 	.name		= "genpd_provider",
 };
 
-#define GENPD_RETRY_MAX_MS	250		/* Approximate */
+#define GENPD_RETRY_MAX_MS		250		/* Approximate */
+#define GENPD_CPU_ON_POLL_PERIOD_US	100		/* 100us */
+#define GENPD_CPU_ON_TIMEOUT_US		5000000		/* 5s */
 
 #define GENPD_DEV_CALLBACK(genpd, type, callback, dev)		\
 ({								\
@@ -1026,15 +1032,75 @@ static void genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 	}
 }
 
+static bool genpd_status_on(struct generic_pm_domain *genpd)
+{
+	bool is_on;
+
+	genpd_lock(genpd);
+	is_on = genpd_status_on_unlocked(genpd);
+	genpd_unlock(genpd);
+
+	return is_on;
+}
+
+static int genpd_wakeup_cpu(struct generic_pm_domain *genpd)
+{
+	unsigned int cpu;
+	bool is_on;
+	int ret;
+
+	/* Find the first online CPU in the genpd's cpumask. */
+	cpu = cpumask_first_and(genpd->cpus, cpu_online_mask);
+	if (cpu >= nr_cpu_ids)
+		return -EAGAIN;
+
+	genpd_unlock(genpd);
+
+	/* Send a IPI to wakeup the selected CPU. */
+	smp_send_reschedule(cpu);
+
+	/* Poll to wait for it to complete the power on sequence. */
+	ret = readx_poll_timeout_atomic(genpd_status_on, genpd, is_on, is_on,
+					GENPD_CPU_ON_POLL_PERIOD_US,
+					GENPD_CPU_ON_TIMEOUT_US);
+
+	genpd_lock(genpd);
+
+	/* Re-check the status as we have released the lock in between. */
+	if (ret || !genpd_status_on_unlocked(genpd))
+		return -EAGAIN;
+
+	return 0;
+}
+
+static bool genpd_need_alive_cpu(struct generic_pm_domain *genpd,
+				 struct device *dev)
+{
+	if (!genpd_is_cpu_domain(genpd))
+		return false;
+
+	/* This is not for CPU devices as those are managed differently. */
+	if (to_gpd_data(dev->power.subsys_data->domain_data)->cpu >= 0)
+		return false;
+
+	/*
+	 * If the current CPU doesn't belong to the genpd's cpumask, we need to
+	 * wake up one of those idle CPUs to power on the CPU domain correctly.
+	 */
+	return !cpumask_test_cpu(smp_processor_id(), genpd->cpus);
+}
+
 /**
  * genpd_power_on - Restore power to a given PM domain and its parents.
  * @genpd: PM domain to power up.
+ * @dev: The device that needs the PM domain to power on.
  * @depth: nesting count for lockdep.
  *
  * Restore power to @genpd and all of its parents so that it is possible to
  * resume a device belonging to it.
  */
-static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
+static int genpd_power_on(struct generic_pm_domain *genpd, struct device *dev,
+			  unsigned int depth)
 {
 	struct gpd_link *link;
 	int ret = 0;
@@ -1042,6 +1108,10 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
 	if (genpd_status_on_unlocked(genpd))
 		return 0;
 
+	/* Special case for a device attached to a CPU domain. */
+	if (genpd_need_alive_cpu(genpd, dev))
+		return genpd_wakeup_cpu(genpd);
+
 	/* Reflect over the entered idle-states residency for debugfs. */
 	genpd_reflect_residency(genpd);
 
@@ -1056,7 +1126,7 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
 		genpd_sd_counter_inc(parent);
 
 		genpd_lock_nested(parent, depth + 1);
-		ret = genpd_power_on(parent, depth + 1);
+		ret = genpd_power_on(parent, dev, depth + 1);
 		genpd_unlock(parent);
 
 		if (ret) {
@@ -1306,7 +1376,7 @@ static int genpd_runtime_resume(struct device *dev)
 
 	genpd_lock(genpd);
 	genpd_restore_performance_state(dev, gpd_data->rpm_pstate);
-	ret = genpd_power_on(genpd, 0);
+	ret = genpd_power_on(genpd, dev, 0);
 	genpd_unlock(genpd);
 
 	if (ret)
@@ -3410,7 +3480,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
 
 	if (power_on) {
 		genpd_lock(pd);
-		ret = genpd_power_on(pd, 0);
+		ret = genpd_power_on(pd, dev, 0);
 		genpd_unlock(pd);
 	}
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/5] pmdomain: core: Add a genpd config to support unknown initial status
  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-07 11:16 ` 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-07 11:16 ` [PATCH v3 5/5] cpuidle: psci: Move initialization a bit earlier in the boot sequence Ulf Hansson
  4 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2026-09-07 11:16 UTC (permalink / raw)
  To: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm
  Cc: Dhruva Gole, Abel Vesa, Lorenzo Pieralisi, Christian Loehle,
	Maulik Shah, Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose,
	Ulf Hansson, linux-arm-kernel, linux-arm-msm, linux-kernel

It's not always possible for a genpd provider to know the initial status
for its corresponding PM domain(s). To register the PM domain in a safe
state, the genpd provider driver may therefore have to initialize the
genpd's status to be powered off, as a way to prevent the PM domain from
being used when it actually could be powered off.

In these cases we may end up to power on a PM domain through the genpd
subsystem, while from the HW point of view it may already be powered on.
Under these circumstances, it may also be required to keep the PM domain
powered on until all the consumer devices have been probed. In other words,
until the ->sync_state() callback for the genpd provider in question have
been called.

To support this behaviour for a genpd provider, let's introduce a new genpd
configuration, GENPD_FLAG_POWER_UNKNOWN.

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:
	- Rename the config to GENPD_FLAG_POWER_UNKNOWN.
---
 drivers/pmdomain/core.c   | 6 +++++-
 include/linux/pm_domain.h | 9 +++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index b62d4e544bc5..5b31ed47d3f1 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -189,6 +189,7 @@ static const struct genpd_lock_ops genpd_raw_spin_ops = {
 #define genpd_is_dev_name_fw(genpd)	(genpd->flags & GENPD_FLAG_DEV_NAME_FW)
 #define genpd_is_no_sync_state(genpd)	(genpd->flags & GENPD_FLAG_NO_SYNC_STATE)
 #define genpd_is_no_stay_on(genpd)	(genpd->flags & GENPD_FLAG_NO_STAY_ON)
+#define genpd_is_power_unknown(genpd)	(genpd->flags & GENPD_FLAG_POWER_UNKNOWN)
 
 static inline bool irq_safe_dev_in_sleep_domain(struct device *dev,
 		const struct generic_pm_domain *genpd)
@@ -2445,7 +2446,10 @@ static void genpd_lock_init(struct generic_pm_domain *genpd)
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 static void genpd_set_stay_on(struct generic_pm_domain *genpd, bool is_off)
 {
-	genpd->stay_on = !genpd_is_no_stay_on(genpd) && !is_off;
+	if (genpd_is_power_unknown(genpd))
+		genpd->stay_on = is_off;
+	else
+		genpd->stay_on = !genpd_is_no_stay_on(genpd) && !is_off;
 }
 #else
 static void genpd_set_stay_on(struct generic_pm_domain *genpd, bool is_off)
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f925614aebdb..14e0e346c610 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -121,6 +121,14 @@ struct dev_pm_domain_list {
  *				powered-off until the ->sync_state() callback is
  *				invoked. This flag informs genpd to allow a
  *				power-off without waiting for ->sync_state().
+ *
+ * GENPD_FLAG_POWER_UNKNOWN:	Use this flag to inform	genpd that its initial
+ *				status for the PM domain is set to powered off,
+ *				which may not correctly reflect the state of the
+ *				HW, as it's unknown. If the PM domain becomes
+ *				powered on during boot, genpd will prevent it
+ *				from being powered off until the ->sync_state
+ *				callback is invoked for it.
  */
 #define GENPD_FLAG_PM_CLK	 (1U << 0)
 #define GENPD_FLAG_IRQ_SAFE	 (1U << 1)
@@ -133,6 +141,7 @@ struct dev_pm_domain_list {
 #define GENPD_FLAG_DEV_NAME_FW	 (1U << 8)
 #define GENPD_FLAG_NO_SYNC_STATE (1U << 9)
 #define GENPD_FLAG_NO_STAY_ON	 (1U << 10)
+#define GENPD_FLAG_POWER_UNKNOWN (1U << 11)
 
 enum gpd_status {
 	GENPD_STATE_ON = 0,	/* PM domain is on */
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/5] cpuidle: psci: Initialize the PM domains in powered off state for OSI
  2026-09-07 11:16 [PATCH v3 0/5] pmdomain/cpuidle-psci: Fix behaviours for CPU PM domains Ulf Hansson
                   ` (2 preceding siblings ...)
  2026-09-07 11:16 ` [PATCH v3 3/5] pmdomain: core: Add a genpd config to support unknown initial status Ulf Hansson
@ 2026-09-07 11:16 ` Ulf Hansson
  2026-09-11 16:34   ` Dhruva G
  2026-09-07 11:16 ` [PATCH v3 5/5] cpuidle: psci: Move initialization a bit earlier in the boot sequence Ulf Hansson
  4 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2026-09-07 11:16 UTC (permalink / raw)
  To: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm
  Cc: Dhruva Gole, Abel Vesa, Lorenzo Pieralisi, Christian Loehle,
	Maulik Shah, Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose,
	Ulf Hansson, linux-arm-kernel, linux-arm-msm, linux-kernel

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 (ret)
 		goto free_pd_prov;
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 5/5] cpuidle: psci: Move initialization a bit earlier in the boot sequence
  2026-09-07 11:16 [PATCH v3 0/5] pmdomain/cpuidle-psci: Fix behaviours for CPU PM domains Ulf Hansson
                   ` (3 preceding siblings ...)
  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-07 11:16 ` Ulf Hansson
  4 siblings, 0 replies; 12+ messages in thread
From: Ulf Hansson @ 2026-09-07 11:16 UTC (permalink / raw)
  To: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm
  Cc: Dhruva Gole, Abel Vesa, Lorenzo Pieralisi, Christian Loehle,
	Maulik Shah, Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose,
	Ulf Hansson, linux-arm-kernel, linux-arm-msm, linux-kernel

When using PSCI OS-initiated mode along with the cpuidle-psci-domain driver
it's of great benefit to attach the CPU devices to their CPU PM domains as
soon as possible, as other non-CPU devices sharing the same PM domains
depends on it to allow their devices to be probed.

Let's therefore move to a subsys_initcall, which should be soon enough
after the cpuidle-psci-domain have initialized the PM domain topology.

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:
	- New patch.
---
 drivers/cpuidle/cpuidle-psci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c
index b250d0dde760..0187d6a9096a 100644
--- a/drivers/cpuidle/cpuidle-psci.c
+++ b/drivers/cpuidle/cpuidle-psci.c
@@ -479,4 +479,4 @@ static int __init psci_idle_init(void)
 
 	return 0;
 }
-device_initcall(psci_idle_init);
+subsys_initcall(psci_idle_init);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/5] pmdomain: core: Allow a non-CPU device in a CPU PM domain to do power on
  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
  0 siblings, 1 reply; 12+ messages in thread
From: Dhruva G @ 2026-09-10 18:31 UTC (permalink / raw)
  To: Ulf Hansson, Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano,
	linux-pm
  Cc: Abel Vesa, Lorenzo Pieralisi, Christian Loehle, Maulik Shah,
	Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose, linux-arm-kernel,
	linux-arm-msm, linux-kernel

On 07-09-2026 16:46, Ulf Hansson wrote:
> A driver for a non-CPU device that is attached to a CPU PM domain (the
> genpd has the GENPD_FLAG_CPU_DOMAIN configuration set), is currently not
> able to power on the PM domain. More precisely, to power on a CPU PM domain
> one of its corresponding CPUs needs to be woken up if they are idle.
> 
> The current support for a non-CPU device is that its driver can only
> prevent an already powered on CPU PM domain from being powered off. This
> leads to problems for a driver while probing its device or when it needs to
> call pm_runtime_get_sync() to turn on the power for it. From the driver
> point of view it looks like it all works fine, but when accessing the
> device it may end up with various errors as the device may not be fully
> powered on.
> 
> To fix the behavior for these types of devices, let's adjust the behaviour
> in genpd_power_on() to wake up an idle CPU that belongs to it, in cases
> when it's needed.
> 
> Link: https://lore.kernel.org/all/CAPx+jO-sCierYj8jnoKQHckJG16dOBxnNrsZVYO=38R2cLV8nw@mail.gmail.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:
> 	- Moved to atomic polling, pointed out by Dhruva.

Thanks, but even in v3 we still have potential issues.
It does not fully address the consequences of polling for five seconds in that context,
nor the parent-lock nesting issue.

This seems to have been pointed out by this corresponding sashiko review as well [1]

[1] https://sashiko.dev/#/patchset/20260907111659.263324-1-ulf.hansson%40oss.qualcomm.com

> 
> Changes in v2:
> 	- Rename a function according to Abel's suggestion.
> ---
>  drivers/pmdomain/core.c | 80 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 75 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> index 6abe8b198949..b62d4e544bc5 100644
> --- a/drivers/pmdomain/core.c
> +++ b/drivers/pmdomain/core.c
> @@ -10,6 +10,7 @@
>  #include <linux/idr.h>
>  #include <linux/kernel.h>
>  #include <linux/io.h>
> +#include <linux/iopoll.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_opp.h>
>  #include <linux/pm_runtime.h>
> @@ -19,11 +20,14 @@
>  #include <linux/slab.h>
>  #include <linux/err.h>
>  #include <linux/sched.h>
> +#include <linux/smp.h>
>  #include <linux/suspend.h>
>  #include <linux/export.h>
>  #include <linux/cpu.h>
>  #include <linux/debugfs.h>
>  
> +#include <trace/events/ipi.h>
> +
>  /* Provides a unique ID for each genpd device */
>  static DEFINE_IDA(genpd_ida);
>  
> @@ -32,7 +36,9 @@ static const struct bus_type genpd_provider_bus_type = {
>  	.name		= "genpd_provider",
>  };
>  
> -#define GENPD_RETRY_MAX_MS	250		/* Approximate */
> +#define GENPD_RETRY_MAX_MS		250		/* Approximate */
> +#define GENPD_CPU_ON_POLL_PERIOD_US	100		/* 100us */
> +#define GENPD_CPU_ON_TIMEOUT_US		5000000		/* 5s */
>  
>  #define GENPD_DEV_CALLBACK(genpd, type, callback, dev)		\
>  ({								\
> @@ -1026,15 +1032,75 @@ static void genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
>  	}
>  }
>  
> +static bool genpd_status_on(struct generic_pm_domain *genpd)
> +{
> +	bool is_on;
> +
> +	genpd_lock(genpd);
> +	is_on = genpd_status_on_unlocked(genpd);
> +	genpd_unlock(genpd);
> +
> +	return is_on;
> +}
> +
> +static int genpd_wakeup_cpu(struct generic_pm_domain *genpd)
> +{
> +	unsigned int cpu;
> +	bool is_on;
> +	int ret;
> +
> +	/* Find the first online CPU in the genpd's cpumask. */
> +	cpu = cpumask_first_and(genpd->cpus, cpu_online_mask);
> +	if (cpu >= nr_cpu_ids)
> +		return -EAGAIN;
> +
> +	genpd_unlock(genpd);
> +
> +	/* Send a IPI to wakeup the selected CPU. */
> +	smp_send_reschedule(cpu);
> +
> +	/* Poll to wait for it to complete the power on sequence. */
> +	ret = readx_poll_timeout_atomic(genpd_status_on, genpd, is_on, is_on,
> +					GENPD_CPU_ON_POLL_PERIOD_US,
> +					GENPD_CPU_ON_TIMEOUT_US);
> +
> +	genpd_lock(genpd);
> +
> +	/* Re-check the status as we have released the lock in between. */
> +	if (ret || !genpd_status_on_unlocked(genpd))
> +		return -EAGAIN;
> +
> +	return 0;
> +}
> +
> +static bool genpd_need_alive_cpu(struct generic_pm_domain *genpd,
> +				 struct device *dev)
> +{
> +	if (!genpd_is_cpu_domain(genpd))
> +		return false;
> +
> +	/* This is not for CPU devices as those are managed differently. */
> +	if (to_gpd_data(dev->power.subsys_data->domain_data)->cpu >= 0)
> +		return false;
> +
> +	/*
> +	 * If the current CPU doesn't belong to the genpd's cpumask, we need to
> +	 * wake up one of those idle CPUs to power on the CPU domain correctly.
> +	 */
> +	return !cpumask_test_cpu(smp_processor_id(), genpd->cpus);
> +}
> +
>  /**
>   * genpd_power_on - Restore power to a given PM domain and its parents.
>   * @genpd: PM domain to power up.
> + * @dev: The device that needs the PM domain to power on.
>   * @depth: nesting count for lockdep.
>   *
>   * Restore power to @genpd and all of its parents so that it is possible to
>   * resume a device belonging to it.
>   */
> -static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
> +static int genpd_power_on(struct generic_pm_domain *genpd, struct device *dev,
> +			  unsigned int depth)
>  {
>  	struct gpd_link *link;
>  	int ret = 0;
> @@ -1042,6 +1108,10 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
>  	if (genpd_status_on_unlocked(genpd))
>  		return 0;
>  
> +	/* Special case for a device attached to a CPU domain. */
> +	if (genpd_need_alive_cpu(genpd, dev))
> +		return genpd_wakeup_cpu(genpd);
> +
>  	/* Reflect over the entered idle-states residency for debugfs. */
>  	genpd_reflect_residency(genpd);
>  
> @@ -1056,7 +1126,7 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
>  		genpd_sd_counter_inc(parent);
>  
>  		genpd_lock_nested(parent, depth + 1);
> -		ret = genpd_power_on(parent, depth + 1);
> +		ret = genpd_power_on(parent, dev, depth + 1);
>  		genpd_unlock(parent);
>  
>  		if (ret) {
> @@ -1306,7 +1376,7 @@ static int genpd_runtime_resume(struct device *dev)
>  
>  	genpd_lock(genpd);
>  	genpd_restore_performance_state(dev, gpd_data->rpm_pstate);
> -	ret = genpd_power_on(genpd, 0);
> +	ret = genpd_power_on(genpd, dev, 0);
>  	genpd_unlock(genpd);
>  
>  	if (ret)
> @@ -3410,7 +3480,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
>  
>  	if (power_on) {
>  		genpd_lock(pd);
> -		ret = genpd_power_on(pd, 0);
> +		ret = genpd_power_on(pd, dev, 0);
>  		genpd_unlock(pd);
>  	}
>  



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/5] pmdomain: core: Allow a non-CPU device in a CPU PM domain to do power on
  2026-09-10 18:31   ` Dhruva G
@ 2026-09-11  9:06     ` Ulf Hansson
  2026-09-11 10:16       ` Ulf Hansson
  0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2026-09-11  9:06 UTC (permalink / raw)
  To: Dhruva G
  Cc: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm,
	Abel Vesa, Lorenzo Pieralisi, Christian Loehle, Maulik Shah,
	Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose, linux-arm-kernel,
	linux-arm-msm, linux-kernel

On Thu, Sep 10, 2026 at 8:31 PM Dhruva G <goledhruva@gmail.com> wrote:
>
> On 07-09-2026 16:46, Ulf Hansson wrote:
> > A driver for a non-CPU device that is attached to a CPU PM domain (the
> > genpd has the GENPD_FLAG_CPU_DOMAIN configuration set), is currently not
> > able to power on the PM domain. More precisely, to power on a CPU PM domain
> > one of its corresponding CPUs needs to be woken up if they are idle.
> >
> > The current support for a non-CPU device is that its driver can only
> > prevent an already powered on CPU PM domain from being powered off. This
> > leads to problems for a driver while probing its device or when it needs to
> > call pm_runtime_get_sync() to turn on the power for it. From the driver
> > point of view it looks like it all works fine, but when accessing the
> > device it may end up with various errors as the device may not be fully
> > powered on.
> >
> > To fix the behavior for these types of devices, let's adjust the behaviour
> > in genpd_power_on() to wake up an idle CPU that belongs to it, in cases
> > when it's needed.
> >
> > Link: https://lore.kernel.org/all/CAPx+jO-sCierYj8jnoKQHckJG16dOBxnNrsZVYO=38R2cLV8nw@mail.gmail.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:
> >       - Moved to atomic polling, pointed out by Dhruva.
>
> Thanks, but even in v3 we still have potential issues.
> It does not fully address the consequences of polling for five seconds in that context,
> nor the parent-lock nesting issue.

Well, I assume we will not be polling for 5s, as it would be an error
and it means that we fail to wake up the CPU. But, I get your point,
5s is really an unnecessary long timeout.

Ideally the timeout should map towards the deepest domain idle state's
entry+exit-latency-us, but rather than looking at what is actually
available for the PM domain(s) in question, I think it's easier (and
good enough) if we just pick a common value. Usually these values are
in the range of a couple milliseconds and in some cases up to
~15-20ms. I suggest we decrease the timeout to 300ms and see how that
plays out.

Also note that, at this point I don't know of any use cases similar to
what you describe, where the device in question is in an irqsafe child
domain. Hence the polling would not be done in an atomic context at
all, so we should be safe. Anyway, if this doesn't work we would
simply have to limit the support to non irqsafe child domains.

In regards to the parent-lock nesting issue. I don't think it's a
problem as genpd_wakeup_cpu() is not being called recursively, but let
me double check this to be sure.

Kind regards
Uffe

>
> This seems to have been pointed out by this corresponding sashiko review as well [1]
>
> [1] https://sashiko.dev/#/patchset/20260907111659.263324-1-ulf.hansson%40oss.qualcomm.com
>
> >
> > Changes in v2:
> >       - Rename a function according to Abel's suggestion.
> > ---
> >  drivers/pmdomain/core.c | 80 ++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 75 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> > index 6abe8b198949..b62d4e544bc5 100644
> > --- a/drivers/pmdomain/core.c
> > +++ b/drivers/pmdomain/core.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/idr.h>
> >  #include <linux/kernel.h>
> >  #include <linux/io.h>
> > +#include <linux/iopoll.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_opp.h>
> >  #include <linux/pm_runtime.h>
> > @@ -19,11 +20,14 @@
> >  #include <linux/slab.h>
> >  #include <linux/err.h>
> >  #include <linux/sched.h>
> > +#include <linux/smp.h>
> >  #include <linux/suspend.h>
> >  #include <linux/export.h>
> >  #include <linux/cpu.h>
> >  #include <linux/debugfs.h>
> >
> > +#include <trace/events/ipi.h>
> > +
> >  /* Provides a unique ID for each genpd device */
> >  static DEFINE_IDA(genpd_ida);
> >
> > @@ -32,7 +36,9 @@ static const struct bus_type genpd_provider_bus_type = {
> >       .name           = "genpd_provider",
> >  };
> >
> > -#define GENPD_RETRY_MAX_MS   250             /* Approximate */
> > +#define GENPD_RETRY_MAX_MS           250             /* Approximate */
> > +#define GENPD_CPU_ON_POLL_PERIOD_US  100             /* 100us */
> > +#define GENPD_CPU_ON_TIMEOUT_US              5000000         /* 5s */
> >
> >  #define GENPD_DEV_CALLBACK(genpd, type, callback, dev)               \
> >  ({                                                           \
> > @@ -1026,15 +1032,75 @@ static void genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
> >       }
> >  }
> >
> > +static bool genpd_status_on(struct generic_pm_domain *genpd)
> > +{
> > +     bool is_on;
> > +
> > +     genpd_lock(genpd);
> > +     is_on = genpd_status_on_unlocked(genpd);
> > +     genpd_unlock(genpd);
> > +
> > +     return is_on;
> > +}
> > +
> > +static int genpd_wakeup_cpu(struct generic_pm_domain *genpd)
> > +{
> > +     unsigned int cpu;
> > +     bool is_on;
> > +     int ret;
> > +
> > +     /* Find the first online CPU in the genpd's cpumask. */
> > +     cpu = cpumask_first_and(genpd->cpus, cpu_online_mask);
> > +     if (cpu >= nr_cpu_ids)
> > +             return -EAGAIN;
> > +
> > +     genpd_unlock(genpd);
> > +
> > +     /* Send a IPI to wakeup the selected CPU. */
> > +     smp_send_reschedule(cpu);
> > +
> > +     /* Poll to wait for it to complete the power on sequence. */
> > +     ret = readx_poll_timeout_atomic(genpd_status_on, genpd, is_on, is_on,
> > +                                     GENPD_CPU_ON_POLL_PERIOD_US,
> > +                                     GENPD_CPU_ON_TIMEOUT_US);
> > +
> > +     genpd_lock(genpd);
> > +
> > +     /* Re-check the status as we have released the lock in between. */
> > +     if (ret || !genpd_status_on_unlocked(genpd))
> > +             return -EAGAIN;
> > +
> > +     return 0;
> > +}
> > +
> > +static bool genpd_need_alive_cpu(struct generic_pm_domain *genpd,
> > +                              struct device *dev)
> > +{
> > +     if (!genpd_is_cpu_domain(genpd))
> > +             return false;
> > +
> > +     /* This is not for CPU devices as those are managed differently. */
> > +     if (to_gpd_data(dev->power.subsys_data->domain_data)->cpu >= 0)
> > +             return false;
> > +
> > +     /*
> > +      * If the current CPU doesn't belong to the genpd's cpumask, we need to
> > +      * wake up one of those idle CPUs to power on the CPU domain correctly.
> > +      */
> > +     return !cpumask_test_cpu(smp_processor_id(), genpd->cpus);
> > +}
> > +
> >  /**
> >   * genpd_power_on - Restore power to a given PM domain and its parents.
> >   * @genpd: PM domain to power up.
> > + * @dev: The device that needs the PM domain to power on.
> >   * @depth: nesting count for lockdep.
> >   *
> >   * Restore power to @genpd and all of its parents so that it is possible to
> >   * resume a device belonging to it.
> >   */
> > -static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
> > +static int genpd_power_on(struct generic_pm_domain *genpd, struct device *dev,
> > +                       unsigned int depth)
> >  {
> >       struct gpd_link *link;
> >       int ret = 0;
> > @@ -1042,6 +1108,10 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
> >       if (genpd_status_on_unlocked(genpd))
> >               return 0;
> >
> > +     /* Special case for a device attached to a CPU domain. */
> > +     if (genpd_need_alive_cpu(genpd, dev))
> > +             return genpd_wakeup_cpu(genpd);
> > +
> >       /* Reflect over the entered idle-states residency for debugfs. */
> >       genpd_reflect_residency(genpd);
> >
> > @@ -1056,7 +1126,7 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
> >               genpd_sd_counter_inc(parent);
> >
> >               genpd_lock_nested(parent, depth + 1);
> > -             ret = genpd_power_on(parent, depth + 1);
> > +             ret = genpd_power_on(parent, dev, depth + 1);
> >               genpd_unlock(parent);
> >
> >               if (ret) {
> > @@ -1306,7 +1376,7 @@ static int genpd_runtime_resume(struct device *dev)
> >
> >       genpd_lock(genpd);
> >       genpd_restore_performance_state(dev, gpd_data->rpm_pstate);
> > -     ret = genpd_power_on(genpd, 0);
> > +     ret = genpd_power_on(genpd, dev, 0);
> >       genpd_unlock(genpd);
> >
> >       if (ret)
> > @@ -3410,7 +3480,7 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
> >
> >       if (power_on) {
> >               genpd_lock(pd);
> > -             ret = genpd_power_on(pd, 0);
> > +             ret = genpd_power_on(pd, dev, 0);
> >               genpd_unlock(pd);
> >       }
> >
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/5] pmdomain: core: Allow a non-CPU device in a CPU PM domain to do power on
  2026-09-11  9:06     ` Ulf Hansson
@ 2026-09-11 10:16       ` Ulf Hansson
  2026-09-11 15:33         ` Dhruva G
  0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2026-09-11 10:16 UTC (permalink / raw)
  To: Dhruva G
  Cc: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm,
	Abel Vesa, Lorenzo Pieralisi, Christian Loehle, Maulik Shah,
	Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose, linux-arm-kernel,
	linux-arm-msm, linux-kernel

On Fri, Sep 11, 2026 at 11:06 AM Ulf Hansson
<ulf.hansson@oss.qualcomm.com> wrote:
>
> On Thu, Sep 10, 2026 at 8:31 PM Dhruva G <goledhruva@gmail.com> wrote:
> >
> > On 07-09-2026 16:46, Ulf Hansson wrote:
> > > A driver for a non-CPU device that is attached to a CPU PM domain (the
> > > genpd has the GENPD_FLAG_CPU_DOMAIN configuration set), is currently not
> > > able to power on the PM domain. More precisely, to power on a CPU PM domain
> > > one of its corresponding CPUs needs to be woken up if they are idle.
> > >
> > > The current support for a non-CPU device is that its driver can only
> > > prevent an already powered on CPU PM domain from being powered off. This
> > > leads to problems for a driver while probing its device or when it needs to
> > > call pm_runtime_get_sync() to turn on the power for it. From the driver
> > > point of view it looks like it all works fine, but when accessing the
> > > device it may end up with various errors as the device may not be fully
> > > powered on.
> > >
> > > To fix the behavior for these types of devices, let's adjust the behaviour
> > > in genpd_power_on() to wake up an idle CPU that belongs to it, in cases
> > > when it's needed.
> > >
> > > Link: https://lore.kernel.org/all/CAPx+jO-sCierYj8jnoKQHckJG16dOBxnNrsZVYO=38R2cLV8nw@mail.gmail.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:
> > >       - Moved to atomic polling, pointed out by Dhruva.
> >
> > Thanks, but even in v3 we still have potential issues.
> > It does not fully address the consequences of polling for five seconds in that context,
> > nor the parent-lock nesting issue.
>
> Well, I assume we will not be polling for 5s, as it would be an error
> and it means that we fail to wake up the CPU. But, I get your point,
> 5s is really an unnecessary long timeout.
>
> Ideally the timeout should map towards the deepest domain idle state's
> entry+exit-latency-us, but rather than looking at what is actually
> available for the PM domain(s) in question, I think it's easier (and
> good enough) if we just pick a common value. Usually these values are
> in the range of a couple milliseconds and in some cases up to
> ~15-20ms. I suggest we decrease the timeout to 300ms and see how that
> plays out.
>
> Also note that, at this point I don't know of any use cases similar to
> what you describe, where the device in question is in an irqsafe child
> domain. Hence the polling would not be done in an atomic context at
> all, so we should be safe. Anyway, if this doesn't work we would
> simply have to limit the support to non irqsafe child domains.
>
> In regards to the parent-lock nesting issue. I don't think it's a
> problem as genpd_wakeup_cpu() is not being called recursively, but let
> me double check this to be sure.

Confirmed, I could not trigger any lockdep problems with my tests.

If you are fine with the above suggestion for the timeout, I am
thinking of amending the patch when applying to avoid posting a new
version of the series. Please let me know.

[...]

Kind regards
Uffe


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/5] pmdomain: core: Allow a non-CPU device in a CPU PM domain to do power on
  2026-09-11 10:16       ` Ulf Hansson
@ 2026-09-11 15:33         ` Dhruva G
  0 siblings, 0 replies; 12+ messages in thread
From: Dhruva G @ 2026-09-11 15:33 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano, linux-pm,
	Abel Vesa, Lorenzo Pieralisi, Christian Loehle, Maulik Shah,
	Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose, linux-arm-kernel,
	linux-arm-msm, linux-kernel

On 11-09-2026 15:46, Ulf Hansson wrote:
> On Fri, Sep 11, 2026 at 11:06 AM Ulf Hansson
> <ulf.hansson@oss.qualcomm.com> wrote:
>>
>> On Thu, Sep 10, 2026 at 8:31 PM Dhruva G <goledhruva@gmail.com> wrote:
>>>
>>> On 07-09-2026 16:46, Ulf Hansson wrote:
>>>> A driver for a non-CPU device that is attached to a CPU PM domain (the
>>>> genpd has the GENPD_FLAG_CPU_DOMAIN configuration set), is currently not
>>>> able to power on the PM domain. More precisely, to power on a CPU PM domain
>>>> one of its corresponding CPUs needs to be woken up if they are idle.
>>>>
>>>> The current support for a non-CPU device is that its driver can only
>>>> prevent an already powered on CPU PM domain from being powered off. This
>>>> leads to problems for a driver while probing its device or when it needs to
>>>> call pm_runtime_get_sync() to turn on the power for it. From the driver
>>>> point of view it looks like it all works fine, but when accessing the
>>>> device it may end up with various errors as the device may not be fully
>>>> powered on.
>>>>
>>>> To fix the behavior for these types of devices, let's adjust the behaviour
>>>> in genpd_power_on() to wake up an idle CPU that belongs to it, in cases
>>>> when it's needed.
>>>>
>>>> Link: https://lore.kernel.org/all/CAPx+jO-sCierYj8jnoKQHckJG16dOBxnNrsZVYO=38R2cLV8nw@mail.gmail.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:
>>>>       - Moved to atomic polling, pointed out by Dhruva.
>>>
>>> Thanks, but even in v3 we still have potential issues.
>>> It does not fully address the consequences of polling for five seconds in that context,
>>> nor the parent-lock nesting issue.
>>
>> Well, I assume we will not be polling for 5s, as it would be an error
>> and it means that we fail to wake up the CPU. But, I get your point,
>> 5s is really an unnecessary long timeout.
>>
>> Ideally the timeout should map towards the deepest domain idle state's
>> entry+exit-latency-us, but rather than looking at what is actually
>> available for the PM domain(s) in question, I think it's easier (and
>> good enough) if we just pick a common value. Usually these values are
>> in the range of a couple milliseconds and in some cases up to
>> ~15-20ms. I suggest we decrease the timeout to 300ms and see how that
>> plays out.
>>
>> Also note that, at this point I don't know of any use cases similar to
>> what you describe, where the device in question is in an irqsafe child
>> domain. Hence the polling would not be done in an atomic context at
>> all, so we should be safe. Anyway, if this doesn't work we would
>> simply have to limit the support to non irqsafe child domains.
>>
>> In regards to the parent-lock nesting issue. I don't think it's a
>> problem as genpd_wakeup_cpu() is not being called recursively, but let
>> me double check this to be sure.
> 
> Confirmed, I could not trigger any lockdep problems with my tests.

thanks!

> 
> If you are fine with the above suggestion for the timeout, I am
> thinking of amending the patch when applying to avoid posting a new
> version of the series. Please let me know.

Yeah I am fine with this, will monitor in future if I suspect anything arising.

Reviewed-by: Dhruva Gole <goledhruva@gmail.com>


> 
> [...]
> 
> Kind regards
> Uffe



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 3/5] pmdomain: core: Add a genpd config to support unknown initial status
  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
  0 siblings, 0 replies; 12+ messages in thread
From: Dhruva G @ 2026-09-11 16:13 UTC (permalink / raw)
  To: Ulf Hansson, Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano,
	linux-pm
  Cc: Abel Vesa, Lorenzo Pieralisi, Christian Loehle, Maulik Shah,
	Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose, linux-arm-kernel,
	linux-arm-msm, linux-kernel

On 07-09-2026 16:46, Ulf Hansson wrote:
> It's not always possible for a genpd provider to know the initial status
> for its corresponding PM domain(s). To register the PM domain in a safe
> state, the genpd provider driver may therefore have to initialize the
> genpd's status to be powered off, as a way to prevent the PM domain from
> being used when it actually could be powered off.
> 
> In these cases we may end up to power on a PM domain through the genpd
> subsystem, while from the HW point of view it may already be powered on.
> Under these circumstances, it may also be required to keep the PM domain
> powered on until all the consumer devices have been probed. In other words,
> until the ->sync_state() callback for the genpd provider in question have
> been called.
> 
> To support this behaviour for a genpd provider, let's introduce a new genpd
> configuration, GENPD_FLAG_POWER_UNKNOWN.
> 
> 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>
> ---

Reviewed-by: Dhruva Gole <goledhruva@gmail.com>

> 
> Changes in v3:
> 	- None.
> Changes in v2:
> 	- Rename the config to GENPD_FLAG_POWER_UNKNOWN.
> ---
>  drivers/pmdomain/core.c   | 6 +++++-
>  include/linux/pm_domain.h | 9 +++++++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
[...]


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 4/5] cpuidle: psci: Initialize the PM domains in powered off state for OSI
  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
  0 siblings, 0 replies; 12+ messages in thread
From: Dhruva G @ 2026-09-11 16:34 UTC (permalink / raw)
  To: Ulf Hansson, Sudeep Holla, Rafael J . Wysocki, Daniel Lezcano,
	linux-pm
  Cc: Abel Vesa, Lorenzo Pieralisi, Christian Loehle, Maulik Shah,
	Yuanfang Zhang, Sneh Mankad, Suzuki K Poulose, linux-arm-kernel,
	linux-arm-msm, linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-09-11 16:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-09-07 11:16 ` [PATCH v3 5/5] cpuidle: psci: Move initialization a bit earlier in the boot sequence Ulf Hansson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox