[PATCH 0/4] clk: implement sync_state support

[PATCH 0/4] clk: implement sync_state support

[Brian Masney]

The existing support for disabling unused clks runs in the late initcall
stage, and it has been known for a long time that this is broken since
it runs too early in the boot up process. It doesn't work for kernel
modules, and it also doesn't work if all of the consumers haven't fully
probed yet. Folks have long recommended to boot certain platforms with
clk_ignore_unused to work around issues with disabling unused clks.

This series fixes this by adding support for sync_state to the clk
subsystem.

Changes in v3:
- Allow multiple sync_state callbacks in the driver core via a new
  helper dev_add_sync_state(). This allows dropping changes to the QC
  drivers that already have a sync_state callback since everything can
  coexist without changes. This also makes it so that this can coexist
  with the pmdomain subsystem.
- Only show "clk: Not disabling unused clocks" message once.
- Add Tested-by from Neil and Xuyang
- Link to v2: https://lore.kernel.org/linux-clk/20260616-clk-sync-state-v2-0-15f82c64d95c@redhat.com/

Changes in v2:
- Split out clk-cbf-8996.c into it's own patch, and don't call
  qcom_cc_sync_state().
- Clarify comment above call to dev_set_drv_sync_state() about
  the -EBUSY.
- Added Tested-by from Jens
- Link to v1: https://lore.kernel.org/r/20260603-clk-sync-state-v1-0-457120eed200@redhat.com

Signed-off-by: Brian Masney <bmasney@redhat.com>
---
Brian Masney (4):
      driver: core: introduce dev_add_sync_state()
      pmdomain: core: migrate to dev_add_sync_state()
      driver: core: remove dev_set_drv_sync_state()
      clk: implement sync_state support

 drivers/base/base.h     |  7 +++++
 drivers/base/core.c     | 29 ++++++++++++++++++
 drivers/clk/clk.c       | 78 ++++++++++++++++++++++++++++++++++++++++---------
 drivers/pmdomain/core.c |  4 +--
 include/linux/clk.h     | 14 +++++++++
 include/linux/device.h  | 21 +++++++------
 6 files changed, 127 insertions(+), 26 deletions(-)
---
base-commit: 6c94b38b83a04c43ea49004275f0391404051093
change-id: 20260626-clk-sync-state-ad008829a689

Best regards,
-- 
Brian Masney <bmasney@redhat.com>

[PATCH 1/4] driver: core: introduce dev_add_sync_state()

[Brian Masney]

We have cases where a device node represents a provider for multiple
types of resources, like clocks, power-domains, resets, etc. We
currently have dev_set_drv_sync_state() where a framework or driver
can set the sync_state callback for a device node, however it currently
only supports a single sync_state callback.

The pmdomain subsystem currently sets up a sync_state callback in the
core framework, and the clk subsystem will setup it's own separate
sync_state callback in the core framework. These can collide with each
other on some types of devices that have multiple types of resources.
Additionally, some clk drivers already have their own separate
sync_state callback already defined.

Let's introduce support for allowing drivers and frameworks to add their
own sync_state callback via a new function dev_add_sync_state() so that
multiple sync_state callbacks can coexist.

Link: https://lore.kernel.org/linux-clk/CAPx+jO9JiV16ePLk59hTQzEMnA96Va6Ns4jqJbwyZ6oTT0AjXA@mail.gmail.com/
Signed-off-by: Brian Masney <bmasney@redhat.com>
Assisted-by: Claude:claude-opus-4-6
---
 drivers/base/base.h    |  7 +++++++
 drivers/base/core.c    | 29 +++++++++++++++++++++++++++++
 include/linux/device.h | 11 +++++++++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index a5b7abc10ff0..339db4afbeb4 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -178,6 +178,8 @@ static inline bool dev_has_sync_state(struct device *dev)
 
 	if (!dev)
 		return false;
+	if (!list_empty(&dev->sync_state_list))
+		return true;
 	drv = READ_ONCE(dev->driver);
 	if (drv && drv->sync_state)
 		return true;
@@ -188,10 +190,15 @@ static inline bool dev_has_sync_state(struct device *dev)
 
 static inline void dev_sync_state(struct device *dev)
 {
+	struct sync_state_entry *entry;
+
 	if (dev->bus->sync_state)
 		dev->bus->sync_state(dev);
 	else if (dev->driver && dev->driver->sync_state)
 		dev->driver->sync_state(dev);
+
+	list_for_each_entry(entry, &dev->sync_state_list, node)
+		entry->fn(dev);
 }
 
 int driver_add_groups(const struct device_driver *drv,
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 4d026682944f..acc12f402dd3 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2612,6 +2612,7 @@ EXPORT_SYMBOL_GPL(device_show_string);
 static void device_release(struct kobject *kobj)
 {
 	struct device *dev = kobj_to_dev(kobj);
+	struct sync_state_entry *entry, *tmp;
 	struct device_private *p = dev->p;
 
 	/*
@@ -2625,6 +2626,11 @@ static void device_release(struct kobject *kobj)
 	 */
 	devres_release_all(dev);
 
+	list_for_each_entry_safe(entry, tmp, &dev->sync_state_list, node) {
+		list_del(&entry->node);
+		kfree(entry);
+	}
+
 	kfree(dev->dma_range_map);
 	kfree(dev->driver_override.name);
 
@@ -3239,12 +3245,35 @@ 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->sync_state_list);
 	dev->links.status = DL_DEV_NO_DRIVER;
 	dev_assign_dma_coherent(dev, dma_default_coherent);
 	swiotlb_dev_init(dev);
 }
 EXPORT_SYMBOL_GPL(device_initialize);
 
+int dev_add_sync_state(struct device *dev,
+			   void (*fn)(struct device *dev))
+{
+	struct sync_state_entry *entry;
+
+	if (!dev || !dev->driver)
+		return 0;
+
+	list_for_each_entry(entry, &dev->sync_state_list, node)
+		if (entry->fn == fn)
+			return 0;
+
+	entry = kmalloc_obj(*entry);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->fn = fn;
+	list_add_tail(&entry->node, &dev->sync_state_list);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dev_add_sync_state);
+
 struct kobject *virtual_device_parent(void)
 {
 	static struct kobject *virtual_dir = NULL;
diff --git a/include/linux/device.h b/include/linux/device.h
index 7b2baffdd2f5..b7a3dd4b56ed 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -642,6 +642,8 @@ enum struct_device_flags {
  * @driver_override: Driver name to force a match.  Do not touch directly; use
  *		     device_set_driver_override() instead.
  * @links:	Links to suppliers and consumers of this device.
+ * @sync_state_list: List of sync_state callbacks added by subsystem
+ *		frameworks (e.g. clk, genpd) via dev_add_sync_state().
  * @power:	For device power management.
  *		See Documentation/driver-api/pm/devices.rst for details.
  * @pm_domain:	Provide callbacks that are executed during system suspend,
@@ -723,6 +725,7 @@ struct device {
 					 */
 
 	struct dev_links_info	links;
+	struct list_head	sync_state_list;
 	struct dev_pm_info	power;
 	struct dev_pm_domain	*pm_domain;
 
@@ -1137,6 +1140,14 @@ static inline int dev_set_drv_sync_state(struct device *dev,
 	return 0;
 }
 
+struct sync_state_entry {
+	struct list_head	node;
+	void			(*fn)(struct device *dev);
+};
+
+int dev_add_sync_state(struct device *dev,
+			   void (*fn)(struct device *dev));
+
 static inline void dev_set_removable(struct device *dev,
 				     enum device_removable removable)
 {

-- 
2.54.0

[PATCH 2/4] pmdomain: core: migrate to dev_add_sync_state()

[Brian Masney]

We have cases where a device node represents a provider for multiple
types of resources, like clocks, power-domains, resets, etc. Having just
a single sync_state on the device is not sufficient since other
frameworks will want to add their own sync_state callback.

Migrate the pmdomain core code from dev_set_drv_sync_state() to
dev_add_sync_state() so that this works nicely with other frameworks
that want to add their own sync_state callback.

Signed-off-by: Brian Masney <bmasney@redhat.com>
Assisted-by: Claude:claude-opus-4-6
---
 drivers/pmdomain/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
index 842c4169e290..22791353249c 100644
--- a/drivers/pmdomain/core.c
+++ b/drivers/pmdomain/core.c
@@ -2697,7 +2697,7 @@ int of_genpd_add_provider_simple(struct device_node *np,
 		genpd->sync_state = GENPD_SYNC_STATE_SIMPLE;
 		device_set_node(&genpd->dev, fwnode);
 	} else {
-		dev_set_drv_sync_state(dev, genpd_sync_state);
+		dev_add_sync_state(dev, genpd_sync_state);
 	}
 
 	put_device(dev);
@@ -2771,7 +2771,7 @@ int of_genpd_add_provider_onecell(struct device_node *np,
 	if (!dev)
 		sync_state = true;
 	else
-		dev_set_drv_sync_state(dev, genpd_sync_state);
+		dev_add_sync_state(dev, genpd_sync_state);
 
 	put_device(dev);
 

-- 
2.54.0

[PATCH 3/4] driver: core: remove dev_set_drv_sync_state()

[Brian Masney]

dev_set_drv_sync_state() is no longer used since all users have been
migrated to dev_add_sync_state(), so let's go ahead and remove it.

Signed-off-by: Brian Masney <bmasney@redhat.com>
Assisted-by: Claude:claude-opus-4-6
---
 include/linux/device.h | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index b7a3dd4b56ed..7f71217ecd48 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1128,18 +1128,6 @@ static inline void device_lock_assert(struct device *dev)
 	lockdep_assert_held(&dev->mutex);
 }
 
-static inline int dev_set_drv_sync_state(struct device *dev,
-					 void (*fn)(struct device *dev))
-{
-	if (!dev || !dev->driver)
-		return 0;
-	if (dev->driver->sync_state && dev->driver->sync_state != fn)
-		return -EBUSY;
-	if (!dev->driver->sync_state)
-		dev->driver->sync_state = fn;
-	return 0;
-}
-
 struct sync_state_entry {
 	struct list_head	node;
 	void			(*fn)(struct device *dev);

-- 
2.54.0

[PATCH 4/4] clk: implement sync_state support

[Brian Masney]

The existing support for disabling unused clks runs in the late initcall
stage, and it has been known for a long time that this is broken since
it runs too early in the boot up process. It doesn't work for kernel
modules, and it also doesn't work if all of the consumers haven't fully
probed yet. Folks have long recommended to boot certain platforms with
clk_ignore_unused to work around issues with disabling unused clks.

Let's go ahead and add a framework-level sync_state callback for the clk
subsystem. If a driver doesn't have a sync_state callback configured,
which is the 99+% use case today, then let's set it up to use the
clk_sync_state() introduced in this commit so that no driver changes
are needed.

At the time of this writing, there are currently only 7 clk drivers that
implement sync_state, and all are Qualcomm SoCs where they interact with
the interconnect framework via icc_sync_state(). A shared helper has
been created for this platform that calls clk_sync_state(). It is
expected that any new clk drivers that want to implement their own
sync_state will also need to call clk_sync_state() at the end of their
custom sync_state callback.

There will be several stages of disabling unused clks:

- The first phase will be executed at late_initcall and it will only
  disable unused clks that do not have a struct dev.
- The sync_state callback will be invoked for each clk driver once all
  consumers have probed.

This is based on previous attempts by Saravana Kannan and Abel Vesa
that are linked below.

This change was tested on a Thinkpad x13s laptop.

    [    0.366737] clk: Disabling unused clocks not associated with a device
    [    0.367232] PM: genpd: Disabling unused power domains
    [    7.791413] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
    [    7.799702] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
    [    8.548820] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
    [    9.121849] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
    [    9.121985] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
    [    9.122691] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks
    [    9.122760] disp_cc-sc8280xp af00000.clock-controller: clk: Disabling unused clocks
    [    9.142121] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
    [    9.169149] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
    [   16.057997] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
    [   16.058149] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
    [   16.334879] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
    [   16.706113] camcc-sc8280xp ad00000.clock-controller: clk: Disabling unused clocks
    [   21.565731] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
    [   21.597069] va_macro 3370000.codec: clk: Disabling unused clocks
    [   21.605039] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
    [   21.630313] wsa_macro 3240000.codec: clk: Disabling unused clocks
    [   21.635069] tx_macro 3220000.txmacro: clk: Disabling unused clocks

Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on AML-S905X-CC, AML-S805X-CC, BPI-M2S, BPI-M5
Tested-by: Xuyang Dong <dongxuyang@eswincomputing.com> # hfp550
Signed-off-by: Brian Masney <bmasney@redhat.com>
Link: https://www.youtube.com/watch?v=tXYzM8yLIQA
Link: https://lore.kernel.org/all/20210407034456.516204-1-saravanak@google.com/
Link: https://lore.kernel.org/all/20221227204528.1899863-1-abel.vesa@linaro.org/
---
 drivers/clk/clk.c   | 78 ++++++++++++++++++++++++++++++++++++++++++++---------
 include/linux/clk.h | 14 ++++++++++
 2 files changed, 79 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 048adfa86a5d..2b6fae54702a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1442,14 +1442,39 @@ static void clk_core_disable_unprepare(struct clk_core *core)
 	clk_core_unprepare_lock(core);
 }
 
-static void __init clk_unprepare_unused_subtree(struct clk_core *core)
+/*
+ * Returns true if @core should be skipped during an unused-clock sweep for
+ * @dev.  When @dev is NULL the sweep is the global late_initcall pass; when
+ * @dev is non-NULL the sweep is a per-device sync_state pass.
+ */
+static bool clk_core_skip_unused(struct clk_core *core, struct device *dev)
+{
+	/*
+	 * At late_initcall, skip clocks that belong to a device — they will be
+	 * handled at sync_state time.
+	 */
+	if (!dev && core->dev)
+		return true;
+
+	/* When called from sync_state, only process clocks for this device. */
+	if (dev && core->dev != dev)
+		return true;
+
+	return false;
+}
+
+static void clk_unprepare_unused_subtree(struct clk_core *core,
+					 struct device *dev)
 {
 	struct clk_core *child;
 
 	lockdep_assert_held(&prepare_lock);
 
 	hlist_for_each_entry(child, &core->children, child_node)
-		clk_unprepare_unused_subtree(child);
+		clk_unprepare_unused_subtree(child, dev);
+
+	if (clk_core_skip_unused(core, dev))
+		return;
 
 	if (core->prepare_count)
 		return;
@@ -1467,7 +1492,8 @@ static void __init clk_unprepare_unused_subtree(struct clk_core *core)
 	}
 }
 
-static void __init clk_disable_unused_subtree(struct clk_core *core)
+static void clk_disable_unused_subtree(struct clk_core *core,
+				       struct device *dev)
 {
 	struct clk_core *child;
 	unsigned long flags;
@@ -1475,7 +1501,10 @@ static void __init clk_disable_unused_subtree(struct clk_core *core)
 	lockdep_assert_held(&prepare_lock);
 
 	hlist_for_each_entry(child, &core->children, child_node)
-		clk_disable_unused_subtree(child);
+		clk_disable_unused_subtree(child, dev);
+
+	if (clk_core_skip_unused(core, dev))
+		return;
 
 	if (core->flags & CLK_OPS_PARENT_ENABLE)
 		clk_core_prepare_enable(core->parent);
@@ -1508,7 +1537,7 @@ static void __init clk_disable_unused_subtree(struct clk_core *core)
 		clk_core_disable_unprepare(core->parent);
 }
 
-static bool clk_ignore_unused __initdata;
+static bool clk_ignore_unused;
 static int __init clk_ignore_unused_setup(char *__unused)
 {
 	clk_ignore_unused = true;
@@ -1516,17 +1545,27 @@ static int __init clk_ignore_unused_setup(char *__unused)
 }
 __setup("clk_ignore_unused", clk_ignore_unused_setup);
 
-static int __init clk_disable_unused(void)
+static int __clk_disable_unused(struct device *dev)
 {
 	struct clk_core *core;
 	int ret;
 
 	if (clk_ignore_unused) {
-		pr_warn("clk: Not disabling unused clocks\n");
+		/*
+		 * Only show this message once for the providers that are not
+		 * associated with a device, which is the first pass of
+		 * disabling unused clocks. Otherwise it'll show up for each
+		 * clk provider as well.
+		 */
+		if (!dev)
+			pr_warn("clk: Not disabling unused clocks\n");
 		return 0;
 	}
 
-	pr_info("clk: Disabling unused clocks\n");
+	if (dev)
+		dev_info(dev, "clk: Disabling unused clocks\n");
+	else
+		pr_info("clk: Disabling unused clocks not associated with a device\n");
 
 	ret = clk_pm_runtime_get_all();
 	if (ret)
@@ -1538,16 +1577,16 @@ static int __init clk_disable_unused(void)
 	clk_prepare_lock();
 
 	hlist_for_each_entry(core, &clk_root_list, child_node)
-		clk_disable_unused_subtree(core);
+		clk_disable_unused_subtree(core, dev);
 
 	hlist_for_each_entry(core, &clk_orphan_list, child_node)
-		clk_disable_unused_subtree(core);
+		clk_disable_unused_subtree(core, dev);
 
 	hlist_for_each_entry(core, &clk_root_list, child_node)
-		clk_unprepare_unused_subtree(core);
+		clk_unprepare_unused_subtree(core, dev);
 
 	hlist_for_each_entry(core, &clk_orphan_list, child_node)
-		clk_unprepare_unused_subtree(core);
+		clk_unprepare_unused_subtree(core, dev);
 
 	clk_prepare_unlock();
 
@@ -1555,8 +1594,19 @@ static int __init clk_disable_unused(void)
 
 	return 0;
 }
+
+static int __init clk_disable_unused(void)
+{
+	return __clk_disable_unused(NULL);
+}
 late_initcall_sync(clk_disable_unused);
 
+void clk_sync_state(struct device *dev)
+{
+	__clk_disable_unused(dev);
+}
+EXPORT_SYMBOL_GPL(clk_sync_state);
+
 static int clk_core_determine_round_nolock(struct clk_core *core,
 					   struct clk_rate_request *req)
 {
@@ -4339,8 +4389,10 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 	core->dev = dev;
 	clk_pm_runtime_init(core);
 	core->of_node = np;
-	if (dev && dev->driver)
+	if (dev && dev->driver) {
 		core->owner = dev->driver->owner;
+		dev_add_sync_state(dev, clk_sync_state);
+	}
 	core->hw = hw;
 	core->flags = init->flags;
 	core->num_parents = init->num_parents;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 998ba3f261da..31a0c9224c46 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -846,6 +846,20 @@ void devm_clk_put(struct device *dev, struct clk *clk);
  */
 
 
+/*
+ * clk_sync_state - sync_state callback to disable unused clocks
+ * @dev: the clock provider device whose unused clocks should be disabled
+ *
+ * It is called by the driver core once all consumers of @dev have probed,
+ * and disables any clocks belonging to @dev that are unused at that point.
+ *
+ * If a clock provider doesn't have a sync_state callback, then the framework
+ * will set up clk_sync_state() on your drivers behalf. If your driver needs
+ * a sync_state callback, then that callback also needs to call
+ * clk_sync_state().
+ */
+void clk_sync_state(struct device *dev);
+
 /**
  * clk_round_rate - adjust a rate to the exact rate a clock can provide
  * @clk: clock source

-- 
2.54.0

Re: [PATCH 0/4] clk: implement sync_state support

[Konrad Dybcio]

On 6/26/26 6:32 PM, Brian Masney wrote:
> The existing support for disabling unused clks runs in the late initcall
> stage, and it has been known for a long time that this is broken since
> it runs too early in the boot up process. It doesn't work for kernel
> modules, and it also doesn't work if all of the consumers haven't fully
> probed yet. Folks have long recommended to boot certain platforms with
> clk_ignore_unused to work around issues with disabling unused clks.
> 
> This series fixes this by adding support for sync_state to the clk
> subsystem.
> 
> Changes in v3:

The version tag is missing from the subjects of the patches you sent
By the trailers, it seems like you used b4.. did you play with
prep --force-revision by chance?

Konrad

Re: [PATCH 1/4] driver: core: introduce dev_add_sync_state()

[Konrad Dybcio]

On 6/26/26 6:32 PM, Brian Masney wrote:
> We have cases where a device node represents a provider for multiple
> types of resources, like clocks, power-domains, resets, etc. We
> currently have dev_set_drv_sync_state() where a framework or driver
> can set the sync_state callback for a device node, however it currently
> only supports a single sync_state callback.
> 
> The pmdomain subsystem currently sets up a sync_state callback in the
> core framework, and the clk subsystem will setup it's own separate
> sync_state callback in the core framework. These can collide with each
> other on some types of devices that have multiple types of resources.
> Additionally, some clk drivers already have their own separate
> sync_state callback already defined.
> 
> Let's introduce support for allowing drivers and frameworks to add their
> own sync_state callback via a new function dev_add_sync_state() so that
> multiple sync_state callbacks can coexist.
> 
> Link: https://lore.kernel.org/linux-clk/CAPx+jO9JiV16ePLk59hTQzEMnA96Va6Ns4jqJbwyZ6oTT0AjXA@mail.gmail.com/
> Signed-off-by: Brian Masney <bmasney@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---

[...]

> +int dev_add_sync_state(struct device *dev,
> +			   void (*fn)(struct device *dev))
> +{
> +	struct sync_state_entry *entry;
> +
> +	if (!dev || !dev->driver)
> +		return 0;
> +
> +	list_for_each_entry(entry, &dev->sync_state_list, node)
> +		if (entry->fn == fn)
> +			return 0;

Do we expect this to be a valid call, i.e. should we WARN_ON here?

Konrad

Re: [PATCH 2/4] pmdomain: core: migrate to dev_add_sync_state()

[Konrad Dybcio]

On 6/26/26 6:32 PM, Brian Masney wrote:
> We have cases where a device node represents a provider for multiple
> types of resources, like clocks, power-domains, resets, etc. Having just
> a single sync_state on the device is not sufficient since other
> frameworks will want to add their own sync_state callback.
> 
> Migrate the pmdomain core code from dev_set_drv_sync_state() to
> dev_add_sync_state() so that this works nicely with other frameworks
> that want to add their own sync_state callback.
> 
> Signed-off-by: Brian Masney <bmasney@redhat.com>
> Assisted-by: Claude:claude-opus-4-6
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH 4/4] clk: implement sync_state support

[Konrad Dybcio]

On 6/26/26 6:32 PM, Brian Masney wrote:
> The existing support for disabling unused clks runs in the late initcall
> stage, and it has been known for a long time that this is broken since
> it runs too early in the boot up process. It doesn't work for kernel
> modules, and it also doesn't work if all of the consumers haven't fully
> probed yet. Folks have long recommended to boot certain platforms with
> clk_ignore_unused to work around issues with disabling unused clks.

[...]

>     [    0.366737] clk: Disabling unused clocks not associated with a device
>     [    0.367232] PM: genpd: Disabling unused power domains
>     [    7.791413] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
>     [    7.799702] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
>     [    8.548820] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
>     [    9.121849] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
>     [    9.121985] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
>     [    9.122691] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks

Many of these drivers only register fixed "virtual" clocks that
don't lead to any registers being altered and only exist to convey
what the clocks look like on the hw level.. but I don't think we
have great infra to skip that..

>     [    9.122760] disp_cc-sc8280xp af00000.clock-controller: clk: Disabling unused clocks
>     [    9.142121] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
>     [    9.169149] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
>     [   16.057997] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
>     [   16.058149] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
>     [   16.334879] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
>     [   16.706113] camcc-sc8280xp ad00000.clock-controller: clk: Disabling unused clocks
>     [   21.565731] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
>     [   21.597069] va_macro 3370000.codec: clk: Disabling unused clocks
>     [   21.605039] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
>     [   21.630313] wsa_macro 3240000.codec: clk: Disabling unused clocks
>     [   21.635069] tx_macro 3220000.txmacro: clk: Disabling unused clocks

This is a bit noisy, but then it's necessary for debugging the
related hangs. Maybe if it turns out to be a huge issue, we can
hide it behind a _dbg() in the future.

I was hoping/expecting that sync_state would completely replace the
late initcall (which would also simplify this diff), but I'm surprised
to learn that there's a whole bunch of clk_register(dev=NULL)
calls in the kernel (are most of them doing it for no good reason
by chance?)

Konrad

Re: [PATCH 1/4] driver: core: introduce dev_add_sync_state()

[Brian Masney]

On Mon, Jun 29, 2026 at 12:11:18PM +0200, Konrad Dybcio wrote:
> On 6/26/26 6:32 PM, Brian Masney wrote:
> > We have cases where a device node represents a provider for multiple
> > types of resources, like clocks, power-domains, resets, etc. We
> > currently have dev_set_drv_sync_state() where a framework or driver
> > can set the sync_state callback for a device node, however it currently
> > only supports a single sync_state callback.
> > 
> > The pmdomain subsystem currently sets up a sync_state callback in the
> > core framework, and the clk subsystem will setup it's own separate
> > sync_state callback in the core framework. These can collide with each
> > other on some types of devices that have multiple types of resources.
> > Additionally, some clk drivers already have their own separate
> > sync_state callback already defined.
> > 
> > Let's introduce support for allowing drivers and frameworks to add their
> > own sync_state callback via a new function dev_add_sync_state() so that
> > multiple sync_state callbacks can coexist.
> > 
> > Link: https://lore.kernel.org/linux-clk/CAPx+jO9JiV16ePLk59hTQzEMnA96Va6Ns4jqJbwyZ6oTT0AjXA@mail.gmail.com/
> > Signed-off-by: Brian Masney <bmasney@redhat.com>
> > Assisted-by: Claude:claude-opus-4-6
> > ---
> 
> [...]
> 
> > +int dev_add_sync_state(struct device *dev,
> > +			   void (*fn)(struct device *dev))
> > +{
> > +	struct sync_state_entry *entry;
> > +
> > +	if (!dev || !dev->driver)
> > +		return 0;
> > +
> > +	list_for_each_entry(entry, &dev->sync_state_list, node)
> > +		if (entry->fn == fn)
> > +			return 0;
> 
> Do we expect this to be a valid call, i.e. should we WARN_ON here?

That's a good point about adding a WARN_ON() here. I kept this logic
from when I converted from dev_set_drv_sync_state().

Brian

Re: [PATCH 0/4] clk: implement sync_state support

[Brian Masney]

On Mon, Jun 29, 2026 at 12:06:19PM +0200, Konrad Dybcio wrote:
> On 6/26/26 6:32 PM, Brian Masney wrote:
> > The existing support for disabling unused clks runs in the late initcall
> > stage, and it has been known for a long time that this is broken since
> > it runs too early in the boot up process. It doesn't work for kernel
> > modules, and it also doesn't work if all of the consumers haven't fully
> > probed yet. Folks have long recommended to boot certain platforms with
> > clk_ignore_unused to work around issues with disabling unused clks.
> > 
> > This series fixes this by adding support for sync_state to the clk
> > subsystem.
> > 
> > Changes in v3:
> 
> The version tag is missing from the subjects of the patches you sent
> By the trailers, it seems like you used b4.. did you play with
> prep --force-revision by chance?

Oops, yea that was my bad. I mistakenly got too aggressive with rebasing
my branch, and I dropped the commit that had the cover letter / b4
metadata the previous day. I didn't know the old SHA, so I had to
start over with a new b4 managed branch, and cherry pick my patches on
top. I forgot to run b4 prep --force-revision to force it to v3.

Brian

Re: [PATCH 4/4] clk: implement sync_state support

[Brian Masney]

On Mon, Jun 29, 2026 at 03:44:26PM +0200, Konrad Dybcio wrote:
> On 6/26/26 6:32 PM, Brian Masney wrote:
> > The existing support for disabling unused clks runs in the late initcall
> > stage, and it has been known for a long time that this is broken since
> > it runs too early in the boot up process. It doesn't work for kernel
> > modules, and it also doesn't work if all of the consumers haven't fully
> > probed yet. Folks have long recommended to boot certain platforms with
> > clk_ignore_unused to work around issues with disabling unused clks.
> 
> [...]
> 
> >     [    0.366737] clk: Disabling unused clocks not associated with a device
> >     [    0.367232] PM: genpd: Disabling unused power domains
> >     [    7.791413] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
> >     [    7.799702] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
> >     [    8.548820] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
> >     [    9.121849] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
> >     [    9.121985] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
> >     [    9.122691] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks
> 
> Many of these drivers only register fixed "virtual" clocks that
> don't lead to any registers being altered and only exist to convey
> what the clocks look like on the hw level.. but I don't think we
> have great infra to skip that..
> 
> >     [    9.122760] disp_cc-sc8280xp af00000.clock-controller: clk: Disabling unused clocks
> >     [    9.142121] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
> >     [    9.169149] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
> >     [   16.057997] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
> >     [   16.058149] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
> >     [   16.334879] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
> >     [   16.706113] camcc-sc8280xp ad00000.clock-controller: clk: Disabling unused clocks
> >     [   21.565731] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
> >     [   21.597069] va_macro 3370000.codec: clk: Disabling unused clocks
> >     [   21.605039] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
> >     [   21.630313] wsa_macro 3240000.codec: clk: Disabling unused clocks
> >     [   21.635069] tx_macro 3220000.txmacro: clk: Disabling unused clocks
> 
> This is a bit noisy, but then it's necessary for debugging the
> related hangs. Maybe if it turns out to be a huge issue, we can
> hide it behind a _dbg() in the future.

Agreed. Personally I think we have this go in with dev_info() initially,
then after it's been there for one release, move it to dev_dbg().

> I was hoping/expecting that sync_state would completely replace the
> late initcall (which would also simplify this diff), but I'm surprised
> to learn that there's a whole bunch of clk_register(dev=NULL)
> calls in the kernel (are most of them doing it for no good reason
> by chance?)

Yes, correct there are a ton of clk drivers that use
clk_register(dev=NULL) to register clks from __init early in the boot.
My understanding of one use case is that this is needed to setup arch
timers that have a dependency on a clock.

My understanding is that lots of drivers that register clks in __init
don't need to do it from there.

Brian