[PATCH v3 00/13] driver core / pmdomain: Add support for fined grained sync_state

[PATCH v3 00/13] driver core / pmdomain: Add support for fined grained sync_state

[Ulf Hansson]

Changes in v3:
	- Dropped tested-by tags from Geert, due to changes in patch2.
	- Addressed the comments from Danilo and Geert, more information in the
	changelog for each patch.

Assuming this is accepted, the best merge strategy would be for Danilo to pick
up the driver core patches (1 to 4) and host a branch that I can pull into my
pmdomain tree so I can take the rest of the series. Danilo, please let me know
what you prefer.

Since the introduction [1] of the common sync_state support for pmdomains
(genpd), we have encountered a lot of various interesting problems. In most
cases the new behaviour of genpd triggered some weird platform specific bugs.

That said, in LPC in Tokyo me and Saravana hosted a session to walk through the
remaining limitations that we have found for genpd's sync state support. In
particular, we discussed the problems we have for the so-called onecell power
domain providers, where a single provider typically provides multiple
independent power domains, all with their own set of consumers.

Note that, onecell power domain providers are very common. It's being used by
many SoCs/platforms/technologies. To name a few:
SCMI, Qualcomm, NXP, Mediatek, Renesas, TI, etc.

Anyway, in these cases, the generic sync_state mechanism with fw_devlink isn't
fine grained enough, as we end up waiting for all consumers for all power
domains before the ->sync_callback gets called for the supplier/provider. In
other words, we may end up keeping unused power domains powered-on, for no good
reasons.

The series intends to fix this problem. Please have a look at the commit
messages for more details and help review/test!

Kind regards
Ulf Hansson

[1]
https://lore.kernel.org/all/20250701114733.636510-1-ulf.hansson@linaro.org/


Ulf Hansson (13):
  Revert "driver core: move dev_has_sync_state() to drivers/base/base.h"
  driver core: Enable suppliers to implement fine grained sync_state
    support
  driver core: Add documentation for dev_set_drv_sync_state()
  driver core: Add dev_set_drv_queue_sync_state()
  pmdomain: core: Move genpd_get_from_provider()
  pmdomain: core: Add initial fine grained sync_state support
  pmdomain: core: Extend fine grained sync_state to more onecell
    providers
  pmdomain: core: Export a common function for ->queue_sync_state()
  pmdomain: renesas: rcar-gen4-sysc: Drop GENPD_FLAG_NO_STAY_ON
  pmdomain: renesas: rcar-sysc: Drop GENPD_FLAG_NO_STAY_ON
  pmdomain: renesas: rmobile-sysc: Drop GENPD_FLAG_NO_STAY_ON
  pmdomain: core: Avoid an unnecessary power off at sync_state
  pmdomain: core: Add a couple of debug prints for sync_state

 drivers/base/base.h                       |  22 ++-
 drivers/base/core.c                       |  77 ++++++--
 drivers/base/driver.c                     |   7 +
 drivers/pmdomain/core.c                   | 227 ++++++++++++++++++----
 drivers/pmdomain/renesas/rcar-gen4-sysc.c |   1 -
 drivers/pmdomain/renesas/rcar-sysc.c      |   1 -
 drivers/pmdomain/renesas/rmobile-sysc.c   |   3 +-
 include/linux/device.h                    |  44 +++++
 include/linux/device/driver.h             |   7 +
 include/linux/pm_domain.h                 |   3 +
 10 files changed, 318 insertions(+), 74 deletions(-)

-- 
2.43.0

[PATCH v3 01/13] Revert "driver core: move dev_has_sync_state() to drivers/base/base.h"

[Ulf Hansson]

It turns out that the dev_has_sync_state() is needed outside the driver
core. A subsequent change to the pmdomain subsystem starts making use of
it.

Fixes: 9db268212e0d ("driver core: move dev_has_sync_state() to drivers/base/base.h")
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- New patch.

---
 drivers/base/base.h    | 14 --------------
 include/linux/device.h | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0ed1e278b957..30b416588617 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -188,20 +188,6 @@ static inline int driver_match_device(const struct device_driver *drv,
 	return drv->bus->match ? drv->bus->match(dev, drv) : 1;
 }
 
-static inline bool dev_has_sync_state(struct device *dev)
-{
-	struct device_driver *drv;
-
-	if (!dev)
-		return false;
-	drv = READ_ONCE(dev->driver);
-	if (drv && drv->sync_state)
-		return true;
-	if (dev->bus && dev->bus->sync_state)
-		return true;
-	return false;
-}
-
 static inline void dev_sync_state(struct device *dev)
 {
 	if (dev->bus->sync_state)
diff --git a/include/linux/device.h b/include/linux/device.h
index d54c86d77764..56a96e41d2c9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1061,6 +1061,20 @@ static inline void device_lock_assert(struct device *dev)
 	lockdep_assert_held(&dev->mutex);
 }
 
+static inline bool dev_has_sync_state(struct device *dev)
+{
+	struct device_driver *drv;
+
+	if (!dev)
+		return false;
+	drv = READ_ONCE(dev->driver);
+	if (drv && drv->sync_state)
+		return true;
+	if (dev->bus && dev->bus->sync_state)
+		return true;
+	return false;
+}
+
 static inline int dev_set_drv_sync_state(struct device *dev,
 					 void (*fn)(struct device *dev))
 {
-- 
2.43.0

Re: [PATCH v3 01/13] Revert "driver core: move dev_has_sync_state() to drivers/base/base.h"

[Danilo Krummrich]

On Fri May 8, 2026 at 2:38 PM CEST, Ulf Hansson wrote:
> It turns out that the dev_has_sync_state() is needed outside the driver
> core. A subsequent change to the pmdomain subsystem starts making use of
> it.
>
> Fixes: 9db268212e0d ("driver core: move dev_has_sync_state() to drivers/base/base.h")

Hm...the commit was correct, it's just that you now want to use it outside of
the driver-core code, so I'd drop this tag.

> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

[PATCH v3 02/13] driver core: Enable suppliers to implement fine grained sync_state support

[Ulf Hansson]

The common sync_state support isn't fine grained enough for some types of
suppliers, like power domains for example. Especially when a supplier
provides multiple independent power domains, each with their own set of
consumers. In these cases we need to wait for all consumers for all the
provided power domains before invoking the supplier's ->sync_state().

To allow a more fine grained sync_state support to be implemented on per
supplier's driver basis, let's add a new optional callback. As soon as
there is an update worth to consider in regards to managing sync_state for
a supplier device, __device_links_queue_sync_state() queues the device in a
list, allowing the new callback to be invoked when flushing the list in
device_links_flush_sync_list().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- Re-worked the approach to use a list to queue/flush devices for
	->queue_sync_state(). This should make sure the device lock is being
	held when it's needed, as pointed out by Danilo.

---
 drivers/base/base.h           | 18 ++++++++
 drivers/base/core.c           | 77 ++++++++++++++++++++++++++---------
 drivers/base/driver.c         |  7 ++++
 include/linux/device.h        |  2 +
 include/linux/device/driver.h |  7 ++++
 5 files changed, 91 insertions(+), 20 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 30b416588617..c8be24af92c3 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -196,6 +196,24 @@ static inline void dev_sync_state(struct device *dev)
 		dev->driver->sync_state(dev);
 }
 
+static inline bool dev_has_queue_sync_state(struct device *dev)
+{
+	struct device_driver *drv;
+
+	if (!dev)
+		return false;
+	drv = READ_ONCE(dev->driver);
+	if (drv && drv->queue_sync_state)
+		return true;
+	return false;
+}
+
+static inline void dev_queue_sync_state(struct device *dev)
+{
+	if (dev->driver && dev->driver->queue_sync_state)
+		dev->driver->queue_sync_state(dev);
+}
+
 int driver_add_groups(const struct device_driver *drv, const struct attribute_group **groups);
 void driver_remove_groups(const struct device_driver *drv, const struct attribute_group **groups);
 void device_driver_detach(struct device *dev);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index d49420e066de..f1f95b3c81e5 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1101,15 +1101,18 @@ int device_links_check_suppliers(struct device *dev)
 /**
  * __device_links_queue_sync_state - Queue a device for sync_state() callback
  * @dev: Device to call sync_state() on
- * @list: List head to queue the @dev on
+ * @s_list: List head for the sync_state to queue the @dev on
+ * @q_list: List head for the queue_sync_state to queue the @dev on
  *
  * Queues a device for a sync_state() callback when the device links write lock
  * isn't held. This allows the sync_state() execution flow to use device links
  * APIs.  The caller must ensure this function is called with
- * device_links_write_lock() held.
+ * device_links_write_lock() held.  Note, if the optional queue_sync_state()
+ * callback has been assigned too, the device is queued for that list to allow a
+ * more fine grained support to be implemented on per supplier basis.
  *
  * This function does a get_device() to make sure the device is not freed while
- * on this list.
+ * on the corresponding list.
  *
  * So the caller must also ensure that device_links_flush_sync_list() is called
  * as soon as the caller releases device_links_write_lock().  This is necessary
@@ -1117,7 +1120,8 @@ int device_links_check_suppliers(struct device *dev)
  * put_device() is called on this device.
  */
 static void __device_links_queue_sync_state(struct device *dev,
-					    struct list_head *list)
+					    struct list_head *s_list,
+					    struct list_head *q_list)
 {
 	struct device_link *link;
 
@@ -1129,8 +1133,14 @@ static void __device_links_queue_sync_state(struct device *dev,
 	list_for_each_entry(link, &dev->links.consumers, s_node) {
 		if (!device_link_test(link, DL_FLAG_MANAGED))
 			continue;
-		if (link->status != DL_STATE_ACTIVE)
+		if (link->status != DL_STATE_ACTIVE) {
+			if (dev_has_queue_sync_state(dev) &&
+			    list_empty(&dev->links.queue_sync)) {
+				get_device(dev);
+				list_add_tail(&dev->links.queue_sync, q_list);
+			}
 			return;
+		}
 	}
 
 	/*
@@ -1144,25 +1154,28 @@ static void __device_links_queue_sync_state(struct device *dev,
 		return;
 
 	get_device(dev);
-	list_add_tail(&dev->links.defer_sync, list);
+	list_add_tail(&dev->links.defer_sync, s_list);
 }
 
 /**
- * device_links_flush_sync_list - Call sync_state() on a list of devices
- * @list: List of devices to call sync_state() on
+ * device_links_flush_sync_list - Call sync_state callbacks for the devices
+ * @s_list: List of devices to call sync_state() on
+ * @q_list: List of devices to call queue_sync_state() on
  * @dont_lock_dev: Device for which lock is already held by the caller
  *
- * Calls sync_state() on all the devices that have been queued for it. This
- * function is used in conjunction with __device_links_queue_sync_state(). The
- * @dont_lock_dev parameter is useful when this function is called from a
- * context where a device lock is already held.
+ * Calls sync_state() and queue_sync_state() on all the devices that have been
+ * queued for it. This function is used in conjunction with
+ * __device_links_queue_sync_state(). The @dont_lock_dev parameter is useful
+ * when this function is called from a context where a device lock is already
+ * held.
  */
-static void device_links_flush_sync_list(struct list_head *list,
+static void device_links_flush_sync_list(struct list_head *s_list,
+					 struct list_head *q_list,
 					 struct device *dont_lock_dev)
 {
 	struct device *dev, *tmp;
 
-	list_for_each_entry_safe(dev, tmp, list, links.defer_sync) {
+	list_for_each_entry_safe(dev, tmp, s_list, links.defer_sync) {
 		list_del_init(&dev->links.defer_sync);
 
 		if (dev != dont_lock_dev)
@@ -1175,6 +1188,25 @@ static void device_links_flush_sync_list(struct list_head *list,
 
 		put_device(dev);
 	}
+
+	if (!q_list)
+		return;
+
+	list_for_each_entry_safe(dev, tmp, q_list, links.queue_sync) {
+		list_del_init(&dev->links.queue_sync);
+
+		if (dev != dont_lock_dev)
+			device_lock(dev);
+
+		device_links_write_lock();
+		dev_queue_sync_state(dev);
+		device_links_write_unlock();
+
+		if (dev != dont_lock_dev)
+			device_unlock(dev);
+
+		put_device(dev);
+	}
 }
 
 void device_links_supplier_sync_state_pause(void)
@@ -1188,6 +1220,7 @@ void device_links_supplier_sync_state_resume(void)
 {
 	struct device *dev, *tmp;
 	LIST_HEAD(sync_list);
+	LIST_HEAD(queue_list);
 
 	device_links_write_lock();
 	if (!defer_sync_state_count) {
@@ -1204,12 +1237,12 @@ void device_links_supplier_sync_state_resume(void)
 		 * sync_list because defer_sync is used for both lists.
 		 */
 		list_del_init(&dev->links.defer_sync);
-		__device_links_queue_sync_state(dev, &sync_list);
+		__device_links_queue_sync_state(dev, &sync_list, &queue_list);
 	}
 out:
 	device_links_write_unlock();
 
-	device_links_flush_sync_list(&sync_list, NULL);
+	device_links_flush_sync_list(&sync_list, &queue_list, NULL);
 }
 
 static int sync_state_resume_initcall(void)
@@ -1296,6 +1329,7 @@ void device_links_driver_bound(struct device *dev)
 {
 	struct device_link *link, *ln;
 	LIST_HEAD(sync_list);
+	LIST_HEAD(queue_list);
 
 	/*
 	 * If a device binds successfully, it's expected to have created all
@@ -1351,7 +1385,7 @@ void device_links_driver_bound(struct device *dev)
 	if (defer_sync_state_count)
 		__device_links_supplier_defer_sync(dev);
 	else
-		__device_links_queue_sync_state(dev, &sync_list);
+		__device_links_queue_sync_state(dev, &sync_list, &queue_list);
 
 	list_for_each_entry_safe(link, ln, &dev->links.suppliers, c_node) {
 		struct device *supplier;
@@ -1393,14 +1427,15 @@ void device_links_driver_bound(struct device *dev)
 		if (defer_sync_state_count)
 			__device_links_supplier_defer_sync(supplier);
 		else
-			__device_links_queue_sync_state(supplier, &sync_list);
+			__device_links_queue_sync_state(supplier, &sync_list,
+							&queue_list);
 	}
 
 	dev->links.status = DL_DEV_DRIVER_BOUND;
 
 	device_links_write_unlock();
 
-	device_links_flush_sync_list(&sync_list, dev);
+	device_links_flush_sync_list(&sync_list, &queue_list, dev);
 }
 
 /**
@@ -1516,6 +1551,7 @@ void device_links_driver_cleanup(struct device *dev)
 	}
 
 	list_del_init(&dev->links.defer_sync);
+	list_del_init(&dev->links.queue_sync);
 	__device_links_no_driver(dev);
 
 	device_links_write_unlock();
@@ -1808,7 +1844,7 @@ void fw_devlink_probing_done(void)
 	class_for_each_device(&devlink_class, NULL, &sync_list,
 			      fw_devlink_dev_sync_state);
 	device_links_write_unlock();
-	device_links_flush_sync_list(&sync_list, NULL);
+	device_links_flush_sync_list(&sync_list, NULL, NULL);
 }
 
 /**
@@ -3169,6 +3205,7 @@ void device_initialize(struct device *dev)
 	INIT_LIST_HEAD(&dev->links.consumers);
 	INIT_LIST_HEAD(&dev->links.suppliers);
 	INIT_LIST_HEAD(&dev->links.defer_sync);
+	INIT_LIST_HEAD(&dev->links.queue_sync);
 	dev->links.status = DL_DEV_NO_DRIVER;
 	dev_assign_dma_coherent(dev, dma_default_coherent);
 	swiotlb_dev_init(dev);
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 8ab010ddf709..b8f4d08bbd58 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -239,6 +239,13 @@ int driver_register(struct device_driver *drv)
 		pr_warn("Driver '%s' needs updating - please use "
 			"bus_type methods\n", drv->name);
 
+	if (drv->queue_sync_state && !drv->sync_state &&
+	    !drv->bus->sync_state) {
+		pr_err("Driver '%s' or its bus_type needs ->sync_state()",
+		       drv->name);
+		return -EINVAL;
+	}
+
 	other = driver_find(drv->name, drv->bus);
 	if (other) {
 		pr_err("Error: Driver '%s' is already registered, "
diff --git a/include/linux/device.h b/include/linux/device.h
index 56a96e41d2c9..6848b0a2c2d9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -414,12 +414,14 @@ enum device_removable {
  * @suppliers: List of links to supplier devices.
  * @consumers: List of links to consumer devices.
  * @defer_sync: Hook to global list of devices that have deferred sync_state.
+ * @defer_sync: Hook to global list of devices scheduled for queue_sync_state.
  * @status: Driver status information.
  */
 struct dev_links_info {
 	struct list_head suppliers;
 	struct list_head consumers;
 	struct list_head defer_sync;
+	struct list_head queue_sync;
 	enum dl_dev_state status;
 };
 
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index bbc67ec513ed..bc9ae1cbe03c 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -68,6 +68,12 @@ enum probe_type {
  *		be called at late_initcall_sync level. If the device has
  *		consumers that are never bound to a driver, this function
  *		will never get called until they do.
+ * @queue_sync_state: Similar to the ->sync_state() callback, but called to
+ *		allow syncing device state to software state in a more fine
+ *		grained way. It is called when there is an updated state that
+ *		may be worth to consider for any of the consumers linked to
+ *		this device. If implemented, the ->sync_state() callback is
+ *		required too.
  * @remove:	Called when the device is removed from the system to
  *		unbind a device from this driver.
  * @shutdown:	Called at shut-down time to quiesce the device.
@@ -110,6 +116,7 @@ struct device_driver {
 
 	int (*probe) (struct device *dev);
 	void (*sync_state)(struct device *dev);
+	void (*queue_sync_state)(struct device *dev);
 	int (*remove) (struct device *dev);
 	void (*shutdown) (struct device *dev);
 	int (*suspend) (struct device *dev, pm_message_t state);
-- 
2.43.0

Re: [PATCH v3 02/13] driver core: Enable suppliers to implement fine grained sync_state support

[Danilo Krummrich]

On Fri May 8, 2026 at 2:38 PM CEST, Ulf Hansson wrote:
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 30b416588617..c8be24af92c3 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -196,6 +196,24 @@ static inline void dev_sync_state(struct device *dev)
>  		dev->driver->sync_state(dev);
>  }
>  
> +static inline bool dev_has_queue_sync_state(struct device *dev)
> +{
> +	struct device_driver *drv;
> +
> +	if (!dev)
> +		return false;
> +	drv = READ_ONCE(dev->driver);
> +	if (drv && drv->queue_sync_state)
> +		return true;
> +	return false;
> +}
> +
> +static inline void dev_queue_sync_state(struct device *dev)
> +{

Let's add device_lock_assert(dev), as this function requires it to be held.

> +	if (dev->driver && dev->driver->queue_sync_state)
> +		dev->driver->queue_sync_state(dev);
> +}

<snip>

> @@ -1175,6 +1188,25 @@ static void device_links_flush_sync_list(struct list_head *list,
>  
>  		put_device(dev);
>  	}
> +
> +	if (!q_list)
> +		return;
> +
> +	list_for_each_entry_safe(dev, tmp, q_list, links.queue_sync) {
> +		list_del_init(&dev->links.queue_sync);

Hm...this is called without the device_links_write_lock() held, so this looks
like this could this race with the list_del_init() from
device_links_driver_cleanup() and create an infinite loop.

(The other list iterator above might have the same issue.)

> +
> +		if (dev != dont_lock_dev)

This is pre-existing, but I think it would be good to add a device_lock_assert()
call for this as well.

> +			device_lock(dev);
> +
> +		device_links_write_lock();
> +		dev_queue_sync_state(dev);
> +		device_links_write_unlock();
> +
> +		if (dev != dont_lock_dev)
> +			device_unlock(dev);
> +
> +		put_device(dev);
> +	}
>  }

[PATCH v3 03/13] driver core: Add documentation for dev_set_drv_sync_state()

[Ulf Hansson]

As dev_set_drv_sync_state() is an exported function, let's add some
documentation of it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- New patch.

---
 include/linux/device.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 6848b0a2c2d9..209feea8050e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1077,6 +1077,14 @@ static inline bool dev_has_sync_state(struct device *dev)
 	return false;
 }
 
+/**
+ * dev_set_drv_sync_state - Assign the ->sync_state callback for a device.
+ * @dev: The device.
+ * @fn: The callback.
+ *
+ * This function dynamically tries to assign the driver's ->sync_state()
+ * callback for the corresponding @dev.
+ */
 static inline int dev_set_drv_sync_state(struct device *dev,
 					 void (*fn)(struct device *dev))
 {
-- 
2.43.0

[PATCH v3 04/13] driver core: Add dev_set_drv_queue_sync_state()

[Ulf Hansson]

Similar to the dev_set_drv_sync_state() helper, let's add another one to
allow subsystem level code to set the ->queue_sync_state() callback for a
driver that has not already set it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- Added documentation.

---
 include/linux/device.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 209feea8050e..e32f85e8d80c 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1097,6 +1097,26 @@ static inline int dev_set_drv_sync_state(struct device *dev,
 	return 0;
 }
 
+/**
+ * dev_set_drv_sync_state - Assign the ->queue_sync_state callback for a device.
+ * @dev: The device.
+ * @fn: The callback.
+ *
+ * This function dynamically tries to assign the driver's ->queue_sync_state()
+ * callback for the corresponding @dev.
+ */
+static inline int dev_set_drv_queue_sync_state(struct device *dev,
+					       void (*fn)(struct device *dev))
+{
+	if (!dev || !dev->driver)
+		return 0;
+	if (dev->driver->queue_sync_state && dev->driver->queue_sync_state != fn)
+		return -EBUSY;
+	if (!dev->driver->queue_sync_state)
+		dev->driver->queue_sync_state = fn;
+	return 0;
+}
+
 static inline void dev_set_removable(struct device *dev,
 				     enum device_removable removable)
 {
-- 
2.43.0

[PATCH v3 05/13] pmdomain: core: Move genpd_get_from_provider()

[Ulf Hansson]

To prepare for subsequent changes and to avoid an unnecessary function
declaration, let's move genpd_get_from_provider() a bit earlier in the
code.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- No change.

---
 drivers/pmdomain/core.c | 70 ++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 4d32fc676aaf..ad57846f02a3 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2664,6 +2664,41 @@ static bool genpd_present(const struct generic_pm_domain *genpd)
 	return ret;
 }
 
+/**
+ * genpd_get_from_provider() - Look-up PM domain
+ * @genpdspec: OF phandle args to use for look-up
+ *
+ * Looks for a PM domain provider under the node specified by @genpdspec and if
+ * found, uses xlate function of the provider to map phandle args to a PM
+ * domain.
+ *
+ * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
+ * on failure.
+ */
+static struct generic_pm_domain *genpd_get_from_provider(
+					const struct of_phandle_args *genpdspec)
+{
+	struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
+	struct of_genpd_provider *provider;
+
+	if (!genpdspec)
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&of_genpd_mutex);
+
+	/* Check if we have such a provider in our array */
+	list_for_each_entry(provider, &of_genpd_providers, link) {
+		if (provider->node == genpdspec->np)
+			genpd = provider->xlate(genpdspec, provider->data);
+		if (!IS_ERR(genpd))
+			break;
+	}
+
+	mutex_unlock(&of_genpd_mutex);
+
+	return genpd;
+}
+
 static void genpd_sync_state(struct device *dev)
 {
 	return of_genpd_sync_state(dev->of_node);
@@ -2889,41 +2924,6 @@ void of_genpd_del_provider(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_genpd_del_provider);
 
-/**
- * genpd_get_from_provider() - Look-up PM domain
- * @genpdspec: OF phandle args to use for look-up
- *
- * Looks for a PM domain provider under the node specified by @genpdspec and if
- * found, uses xlate function of the provider to map phandle args to a PM
- * domain.
- *
- * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
- * on failure.
- */
-static struct generic_pm_domain *genpd_get_from_provider(
-					const struct of_phandle_args *genpdspec)
-{
-	struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
-	struct of_genpd_provider *provider;
-
-	if (!genpdspec)
-		return ERR_PTR(-EINVAL);
-
-	mutex_lock(&of_genpd_mutex);
-
-	/* Check if we have such a provider in our array */
-	list_for_each_entry(provider, &of_genpd_providers, link) {
-		if (provider->node == genpdspec->np)
-			genpd = provider->xlate(genpdspec, provider->data);
-		if (!IS_ERR(genpd))
-			break;
-	}
-
-	mutex_unlock(&of_genpd_mutex);
-
-	return genpd;
-}
-
 /**
  * of_genpd_add_device() - Add a device to an I/O PM domain
  * @genpdspec: OF phandle args to use for look-up PM domain
-- 
2.43.0

[PATCH v3 06/13] pmdomain: core: Add initial fine grained sync_state support

[Ulf Hansson]

A onecell (#power-domain-cells = <1 or 2>; in DT) power domain provider
typically provides multiple independent power domains, each with their own
corresponding consumers. In these cases we have to wait for all consumers
for all the provided power domains before the ->sync_state() callback gets
called for the supplier.

In a first step to improve this, let's implement support for fine grained
sync_state support a per genpd basis by using the ->queue_sync_state()
callback. To take step by step, let's initially limit the improvement to
the internal genpd provider driver and to its corresponding genpd devices
for onecell providers.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- Addressed some cosmetic comments from Geert.

---
 drivers/pmdomain/core.c   | 124 ++++++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h |   1 +
 2 files changed, 125 insertions(+)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index ad57846f02a3..c01a9a96e5c2 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2699,6 +2699,119 @@ static struct generic_pm_domain *genpd_get_from_provider(
 	return genpd;
 }
 
+static bool genpd_should_wait_for_consumer(struct device_node *np)
+{
+	struct generic_pm_domain *genpd;
+	bool should_wait = false;
+
+	mutex_lock(&gpd_list_lock);
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+		if (genpd->provider == of_fwnode_handle(np)) {
+			genpd_lock(genpd);
+
+			/* Clear the previous state before reevaluating. */
+			genpd->wait_for_consumer = false;
+
+			/*
+			 * Unless there is at least one genpd for the provider
+			 * that is being kept powered-on, we don't have to care
+			 * about waiting for consumers.
+			 */
+			if (genpd->stay_on)
+				should_wait = true;
+
+			genpd_unlock(genpd);
+		}
+	}
+	mutex_unlock(&gpd_list_lock);
+
+	return should_wait;
+}
+
+static void genpd_parse_for_consumer(struct device_node *sup,
+				     struct device_node *con)
+{
+	struct generic_pm_domain *genpd;
+
+	for (unsigned int i = 0; ; i++) {
+		struct of_phandle_args pd_args;
+
+		if (of_parse_phandle_with_args(con, "power-domains",
+					       "#power-domain-cells",
+					       i, &pd_args))
+			break;
+
+		/*
+		 * The phandle must correspond to the supplier's genpd provider
+		 * to be relevant else let's move to the next index.
+		 */
+		if (sup != pd_args.np) {
+			of_node_put(pd_args.np);
+			continue;
+		}
+
+		mutex_lock(&gpd_list_lock);
+		genpd = genpd_get_from_provider(&pd_args);
+		if (!IS_ERR(genpd)) {
+			genpd_lock(genpd);
+			genpd->wait_for_consumer = true;
+			genpd_unlock(genpd);
+		}
+		mutex_unlock(&gpd_list_lock);
+
+		of_node_put(pd_args.np);
+	}
+}
+
+static void _genpd_queue_sync_state(struct device_node *np)
+{
+	struct generic_pm_domain *genpd;
+
+	mutex_lock(&gpd_list_lock);
+	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
+		if (genpd->provider == of_fwnode_handle(np)) {
+			genpd_lock(genpd);
+			if (genpd->stay_on && !genpd->wait_for_consumer) {
+				genpd->stay_on = false;
+				genpd_queue_power_off_work(genpd);
+			}
+			genpd_unlock(genpd);
+		}
+	}
+	mutex_unlock(&gpd_list_lock);
+}
+
+static void genpd_queue_sync_state(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_link *link;
+
+	if (!genpd_should_wait_for_consumer(np))
+		return;
+
+	list_for_each_entry(link, &dev->links.consumers, s_node) {
+		struct device *consumer = link->consumer;
+
+		if (!device_link_test(link, DL_FLAG_MANAGED))
+			continue;
+
+		if (link->status == DL_STATE_ACTIVE)
+			continue;
+
+		if (!consumer->of_node)
+			continue;
+
+		/*
+		 * A consumer device has not been probed yet. Let's parse its
+		 * device node for the power-domains property, to find out the
+		 * genpds it may belong to and then prevent sync state for them.
+		 */
+		genpd_parse_for_consumer(np, consumer->of_node);
+	}
+
+	_genpd_queue_sync_state(np);
+}
+
 static void genpd_sync_state(struct device *dev)
 {
 	return of_genpd_sync_state(dev->of_node);
@@ -3531,6 +3644,16 @@ static int genpd_provider_probe(struct device *dev)
 	return 0;
 }
 
+static void genpd_provider_queue_sync_state(struct device *dev)
+{
+	struct generic_pm_domain *genpd = container_of(dev, struct generic_pm_domain, dev);
+
+	if (genpd->sync_state != GENPD_SYNC_STATE_ONECELL)
+		return;
+
+	genpd_queue_sync_state(dev);
+}
+
 static void genpd_provider_sync_state(struct device *dev)
 {
 	struct generic_pm_domain *genpd = container_of(dev, struct generic_pm_domain, dev);
@@ -3559,6 +3682,7 @@ static struct device_driver genpd_provider_drv = {
 	.name = "genpd_provider",
 	.bus = &genpd_provider_bus_type,
 	.probe = genpd_provider_probe,
+	.queue_sync_state = genpd_provider_queue_sync_state,
 	.sync_state = genpd_provider_sync_state,
 	.suppress_bind_attrs = true,
 };
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index b299dc0128d6..7aa49721cde5 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -215,6 +215,7 @@ struct generic_pm_domain {
 	cpumask_var_t cpus;		/* A cpumask of the attached CPUs */
 	bool synced_poweroff;		/* A consumer needs a synced poweroff */
 	bool stay_on;			/* Stay powered-on during boot. */
+	bool wait_for_consumer;		/* Consumers awaits to be probed. */
 	enum genpd_sync_state sync_state; /* How sync_state is managed. */
 	int (*power_off)(struct generic_pm_domain *domain);
 	int (*power_on)(struct generic_pm_domain *domain);
-- 
2.43.0

[PATCH v3 07/13] pmdomain: core: Extend fine grained sync_state to more onecell providers

[Ulf Hansson]

A onecell power domain provider driver that we can assign a common
->sync_state() callback for, should be able to benefit from the improved
fine grained sync_state support in genpd. Therefore, let's also assign the
->queue_sync_state() callback for these types of provider drivers.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- No changes.

---
 drivers/pmdomain/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index c01a9a96e5c2..b2b5cc30c7fc 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2917,10 +2917,12 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 
 	fwnode = of_fwnode_handle(np);
 	dev = get_dev_from_fwnode(fwnode);
-	if (!dev)
+	if (!dev) {
 		sync_state = true;
-	else
+	} else if (!dev_has_sync_state(dev)) {
 		dev_set_drv_sync_state(dev, genpd_sync_state);
+		dev_set_drv_queue_sync_state(dev, genpd_queue_sync_state);
+	}
 
 	put_device(dev);
 
-- 
2.43.0

[PATCH v3 08/13] pmdomain: core: Export a common function for ->queue_sync_state()

[Ulf Hansson]

Along with of_genpd_sync_state() that genpd provider drivers may use to
manage sync_state, let's add and export of_genpd_queue_sync_state() for
those that may need it. It's expected that the genpd provider driver
assigns it's own ->queue_sync_state() callback and invoke the new helper
from there.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- No changes.

---
 drivers/pmdomain/core.c   | 14 +++++++++-----
 include/linux/pm_domain.h |  2 ++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index b2b5cc30c7fc..b3ec3dec3f10 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2763,7 +2763,7 @@ static void genpd_parse_for_consumer(struct device_node *sup,
 	}
 }
 
-static void _genpd_queue_sync_state(struct device_node *np)
+static void genpd_queue_sync_state(struct device_node *np)
 {
 	struct generic_pm_domain *genpd;
 
@@ -2781,11 +2781,14 @@ static void _genpd_queue_sync_state(struct device_node *np)
 	mutex_unlock(&gpd_list_lock);
 }
 
-static void genpd_queue_sync_state(struct device *dev)
+void of_genpd_queue_sync_state(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
 	struct device_link *link;
 
+	if (!np)
+		return;
+
 	if (!genpd_should_wait_for_consumer(np))
 		return;
 
@@ -2809,8 +2812,9 @@ static void genpd_queue_sync_state(struct device *dev)
 		genpd_parse_for_consumer(np, consumer->of_node);
 	}
 
-	_genpd_queue_sync_state(np);
+	genpd_queue_sync_state(np);
 }
+EXPORT_SYMBOL_GPL(of_genpd_queue_sync_state);
 
 static void genpd_sync_state(struct device *dev)
 {
@@ -2921,7 +2925,7 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 		sync_state = true;
 	} else if (!dev_has_sync_state(dev)) {
 		dev_set_drv_sync_state(dev, genpd_sync_state);
-		dev_set_drv_queue_sync_state(dev, genpd_queue_sync_state);
+		dev_set_drv_queue_sync_state(dev, of_genpd_queue_sync_state);
 	}
 
 	put_device(dev);
@@ -3653,7 +3657,7 @@ static void genpd_provider_queue_sync_state(struct device *dev)
 	if (genpd->sync_state != GENPD_SYNC_STATE_ONECELL)
 		return;
 
-	genpd_queue_sync_state(dev);
+	of_genpd_queue_sync_state(dev);
 }
 
 static void genpd_provider_sync_state(struct device *dev)
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 7aa49721cde5..d428dd805c46 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -467,6 +467,7 @@ int of_genpd_remove_subdomain(const struct of_phandle_args *parent_spec,
 struct generic_pm_domain *of_genpd_remove_last(struct device_node *np);
 int of_genpd_parse_idle_states(struct device_node *dn,
 			       struct genpd_power_state **states, int *n);
+void of_genpd_queue_sync_state(struct device *dev);
 void of_genpd_sync_state(struct device_node *np);
 
 int genpd_dev_pm_attach(struct device *dev);
@@ -513,6 +514,7 @@ static inline int of_genpd_parse_idle_states(struct device_node *dn,
 	return -ENODEV;
 }
 
+static inline void of_genpd_queue_sync_state(struct device *dev) {}
 static inline void of_genpd_sync_state(struct device_node *np) {}
 
 static inline int genpd_dev_pm_attach(struct device *dev)
-- 
2.43.0

[PATCH v3 09/13] pmdomain: renesas: rcar-gen4-sysc: Drop GENPD_FLAG_NO_STAY_ON

[Ulf Hansson]

Due to the new fine grained sync_state support for onecell genpd provider
drivers, we should no longer need use the legacy behaviour. Therefore,
let's drop GENPD_FLAG_NO_STAY_ON.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- No changes.

---
 drivers/pmdomain/renesas/rcar-gen4-sysc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pmdomain/renesas/rcar-gen4-sysc.c b/drivers/pmdomain/renesas/rcar-gen4-sysc.c
index 0c6c639a91d0..81b154da725f 100644
--- a/drivers/pmdomain/renesas/rcar-gen4-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-gen4-sysc.c
@@ -251,7 +251,6 @@ static int __init rcar_gen4_sysc_pd_setup(struct rcar_gen4_sysc_pd *pd)
 		genpd->detach_dev = cpg_mssr_detach_dev;
 	}
 
-	genpd->flags |= GENPD_FLAG_NO_STAY_ON;
 	genpd->power_off = rcar_gen4_sysc_pd_power_off;
 	genpd->power_on = rcar_gen4_sysc_pd_power_on;
 
-- 
2.43.0

[PATCH v3 10/13] pmdomain: renesas: rcar-sysc: Drop GENPD_FLAG_NO_STAY_ON

[Ulf Hansson]

Due to the new fine grained sync_state support for onecell genpd provider
drivers, we should no longer need use the legacy behaviour. Therefore,
let's drop GENPD_FLAG_NO_STAY_ON.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- No changes.

---
 drivers/pmdomain/renesas/rcar-sysc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pmdomain/renesas/rcar-sysc.c b/drivers/pmdomain/renesas/rcar-sysc.c
index bd7bb9cbd9da..e4608c657629 100644
--- a/drivers/pmdomain/renesas/rcar-sysc.c
+++ b/drivers/pmdomain/renesas/rcar-sysc.c
@@ -241,7 +241,6 @@ static int __init rcar_sysc_pd_setup(struct rcar_sysc_pd *pd)
 		}
 	}
 
-	genpd->flags |= GENPD_FLAG_NO_STAY_ON;
 	genpd->power_off = rcar_sysc_pd_power_off;
 	genpd->power_on = rcar_sysc_pd_power_on;
 
-- 
2.43.0

[PATCH v3 11/13] pmdomain: renesas: rmobile-sysc: Drop GENPD_FLAG_NO_STAY_ON

[Ulf Hansson]

Rmobile-sysc is not a onecell provider and didn't really needed
the GENPD_FLAG_NO_STAY_ON flag in the first place. Let's drop it.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- No changes.

---
 drivers/pmdomain/renesas/rmobile-sysc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pmdomain/renesas/rmobile-sysc.c b/drivers/pmdomain/renesas/rmobile-sysc.c
index 93103ff33d6e..e36f5d763c91 100644
--- a/drivers/pmdomain/renesas/rmobile-sysc.c
+++ b/drivers/pmdomain/renesas/rmobile-sysc.c
@@ -100,8 +100,7 @@ static void rmobile_init_pm_domain(struct rmobile_pm_domain *rmobile_pd)
 	struct generic_pm_domain *genpd = &rmobile_pd->genpd;
 	struct dev_power_governor *gov = rmobile_pd->gov;
 
-	genpd->flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP |
-		GENPD_FLAG_NO_STAY_ON;
+	genpd->flags |= GENPD_FLAG_PM_CLK | GENPD_FLAG_ACTIVE_WAKEUP;
 	genpd->attach_dev = cpg_mstp_attach_dev;
 	genpd->detach_dev = cpg_mstp_detach_dev;
 
-- 
2.43.0

[PATCH v3 12/13] pmdomain: core: Avoid an unnecessary power off at sync_state

[Ulf Hansson]

If the PM domain was not powered on during initialization, genpd will not
prevent it from being powered off during boot. In these cases there is no
need to call genpd_power_off() at sync_state, hence let's avoid it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- New patch.

---
 drivers/pmdomain/core.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index b3ec3dec3f10..b5d90ddfd698 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -3636,8 +3636,10 @@ void of_genpd_sync_state(struct device_node *np)
 	list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
 		if (genpd->provider == of_fwnode_handle(np)) {
 			genpd_lock(genpd);
-			genpd->stay_on = false;
-			genpd_power_off(genpd, false, 0);
+			if (genpd->stay_on) {
+				genpd->stay_on = false;
+				genpd_power_off(genpd, false, 0);
+			}
 			genpd_unlock(genpd);
 		}
 	}
@@ -3674,8 +3676,10 @@ static void genpd_provider_sync_state(struct device *dev)
 
 	case GENPD_SYNC_STATE_SIMPLE:
 		genpd_lock(genpd);
-		genpd->stay_on = false;
-		genpd_power_off(genpd, false, 0);
+		if (genpd->stay_on) {
+			genpd->stay_on = false;
+			genpd_power_off(genpd, false, 0);
+		}
 		genpd_unlock(genpd);
 		break;
 
-- 
2.43.0

[PATCH v3 13/13] pmdomain: core: Add a couple of debug prints for sync_state

[Ulf Hansson]

To inform the developer of what is happening during sync_state transitions
in genpd, let's add a couple of debug prints for it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v3:
	- New patch.

---
 drivers/pmdomain/core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index b5d90ddfd698..3d3e0f38815b 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2756,6 +2756,9 @@ static void genpd_parse_for_consumer(struct device_node *sup,
 			genpd_lock(genpd);
 			genpd->wait_for_consumer = true;
 			genpd_unlock(genpd);
+
+			pr_debug("%s: wait for consumer %pOF\n",
+				 dev_name(&genpd->dev), con);
 		}
 		mutex_unlock(&gpd_list_lock);
 
@@ -2772,6 +2775,9 @@ static void genpd_queue_sync_state(struct device_node *np)
 		if (genpd->provider == of_fwnode_handle(np)) {
 			genpd_lock(genpd);
 			if (genpd->stay_on && !genpd->wait_for_consumer) {
+				pr_debug("%s: queuing sync state\n",
+					 dev_name(&genpd->dev));
+
 				genpd->stay_on = false;
 				genpd_queue_power_off_work(genpd);
 			}
@@ -3637,6 +3643,9 @@ void of_genpd_sync_state(struct device_node *np)
 		if (genpd->provider == of_fwnode_handle(np)) {
 			genpd_lock(genpd);
 			if (genpd->stay_on) {
+				pr_debug("%s: sync state\n",
+					 dev_name(&genpd->dev));
+
 				genpd->stay_on = false;
 				genpd_power_off(genpd, false, 0);
 			}
@@ -3677,6 +3686,8 @@ static void genpd_provider_sync_state(struct device *dev)
 	case GENPD_SYNC_STATE_SIMPLE:
 		genpd_lock(genpd);
 		if (genpd->stay_on) {
+			pr_debug("%s: sync state\n", dev_name(&genpd->dev));
+
 			genpd->stay_on = false;
 			genpd_power_off(genpd, false, 0);
 		}
-- 
2.43.0