[PATCH v2 0/5] clk: implement sync_state support

[PATCH v2 0/5] 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:
- Adds a generic clk_sync_state() callback that's initially empty.
- Adds a generic qcom_cc_sync_state() for qcom SoCs that need to
  interact with the Interconnect framework.
- Converts the 7 qcom drivers from using icc_sync_state() to
  qcom_cc_sync_state().
- Implement the framework-level sync state via clk_sync_state().

This approach maintains bisectability.

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 (5):
      clk: introduce stub clk_sync_state()
      clk: qcom: common: introduce qcom_cc_sync_state()
      clk: qcom: convert from icc_sync_state() to qcom_cc_sync_state()
      clk: qcom: cbf-8996: add clk_sync_state() call
      clk: implement sync_state support

 drivers/clk/clk.c                | 76 +++++++++++++++++++++++++++++++++-------
 drivers/clk/qcom/apss-ipq5424.c  |  3 +-
 drivers/clk/qcom/clk-cbf-8996.c  |  8 +++--
 drivers/clk/qcom/common.c        |  9 +++++
 drivers/clk/qcom/common.h        |  1 +
 drivers/clk/qcom/gcc-ipq5332.c   |  3 +-
 drivers/clk/qcom/gcc-ipq5424.c   |  3 +-
 drivers/clk/qcom/gcc-ipq9574.c   |  3 +-
 drivers/clk/qcom/nsscc-ipq5424.c |  3 +-
 drivers/clk/qcom/nsscc-ipq9574.c |  3 +-
 include/linux/clk.h              | 14 ++++++++
 11 files changed, 100 insertions(+), 26 deletions(-)
---
base-commit: 08484c504b55a98bd100527fbe10a3caf55ff3ff
change-id: 20260602-clk-sync-state-c0539c5530f4

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

[PATCH v2 1/5] clk: introduce stub clk_sync_state()

[Brian Masney]

Add a stub clk_sync_state() since this will be needed for some clk
drivers that already have a sync_state callback. This allows introducing
all of the necessary changes into the drivers before actually adding the
real clk_sync_state() implementation. Doing it this way ensures that the
tree stays bisectable, and the individual changes are small patches.

Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
 drivers/clk/clk.c   |  6 ++++++
 include/linux/clk.h | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 048adfa86a5d..9cb2b42d1be4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1557,6 +1557,12 @@ static int __init clk_disable_unused(void)
 }
 late_initcall_sync(clk_disable_unused);
 
+void clk_sync_state(struct device *dev)
+{
+	/* Will fill in */
+}
+EXPORT_SYMBOL_GPL(clk_sync_state);
+
 static int clk_core_determine_round_nolock(struct clk_core *core,
 					   struct clk_rate_request *req)
 {
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

[PATCH v2 2/5] clk: qcom: common: introduce qcom_cc_sync_state()

[Brian Masney]

Several qcom clk providers currently have a sync_state helper set to
icc_sync_state(). With an upcoming change to the clk framework, if
sync_state is not defined for the device, then the clk framework sets it
to clk_sync_state().

Clk providers that require their own sync_state will need to call the
framework level clk_sync_state(). Let's introduce a new common helper
qcom_cc_sync_state() that calls icc_sync_state() and clk_sync_state().

Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
 drivers/clk/qcom/common.c | 9 +++++++++
 drivers/clk/qcom/common.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index eec369d2173b..31382c49c948 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -3,12 +3,14 @@
  * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
  */
 
+#include <linux/clk.h>
 #include <linux/export.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 #include <linux/platform_device.h>
 #include <linux/clk-provider.h>
 #include <linux/interconnect-clk.h>
+#include <linux/interconnect-provider.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset-controller.h>
 #include <linux/of.h>
@@ -464,5 +466,12 @@ int qcom_cc_probe_by_index(struct platform_device *pdev, int index,
 }
 EXPORT_SYMBOL_GPL(qcom_cc_probe_by_index);
 
+void qcom_cc_sync_state(struct device *dev)
+{
+	icc_sync_state(dev);
+	clk_sync_state(dev);
+}
+EXPORT_SYMBOL_GPL(qcom_cc_sync_state);
+
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("QTI Common Clock module");
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 6f2406f8839e..ee448163b1fc 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -88,5 +88,6 @@ extern int qcom_cc_probe(struct platform_device *pdev,
 			 const struct qcom_cc_desc *desc);
 extern int qcom_cc_probe_by_index(struct platform_device *pdev, int index,
 				  const struct qcom_cc_desc *desc);
+extern void qcom_cc_sync_state(struct device *dev);
 
 #endif

-- 
2.54.0

[PATCH v2 3/5] clk: qcom: convert from icc_sync_state() to qcom_cc_sync_state()

[Brian Masney]

Convert all of the qcom clk drivers that use qcom_cc_*() from
icc_sync_state() to qcom_cc_sync_state().

Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
 drivers/clk/qcom/apss-ipq5424.c  | 3 +--
 drivers/clk/qcom/gcc-ipq5332.c   | 3 +--
 drivers/clk/qcom/gcc-ipq5424.c   | 3 +--
 drivers/clk/qcom/gcc-ipq9574.c   | 3 +--
 drivers/clk/qcom/nsscc-ipq5424.c | 3 +--
 drivers/clk/qcom/nsscc-ipq9574.c | 3 +--
 6 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/qcom/apss-ipq5424.c b/drivers/clk/qcom/apss-ipq5424.c
index 1662c83058bc..70991c3a9a58 100644
--- a/drivers/clk/qcom/apss-ipq5424.c
+++ b/drivers/clk/qcom/apss-ipq5424.c
@@ -7,7 +7,6 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
-#include <linux/interconnect-provider.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -248,7 +247,7 @@ static struct platform_driver apss_ipq5424_driver = {
 	.driver = {
 		.name   = "apss-ipq5424-clk",
 		.of_match_table = apss_ipq5424_match_table,
-		.sync_state = icc_sync_state,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c
index 9246e97d785a..adec25aa9c27 100644
--- a/drivers/clk/qcom/gcc-ipq5332.c
+++ b/drivers/clk/qcom/gcc-ipq5332.c
@@ -4,7 +4,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/interconnect-provider.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -3307,7 +3306,7 @@ static struct platform_driver gcc_ipq5332_driver = {
 	.driver = {
 		.name = "gcc-ipq5332",
 		.of_match_table = gcc_ipq5332_match_table,
-		.sync_state = icc_sync_state,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
diff --git a/drivers/clk/qcom/gcc-ipq5424.c b/drivers/clk/qcom/gcc-ipq5424.c
index 35af6ffeeb85..9bf3bb6f8904 100644
--- a/drivers/clk/qcom/gcc-ipq5424.c
+++ b/drivers/clk/qcom/gcc-ipq5424.c
@@ -5,7 +5,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/interconnect-provider.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -3322,7 +3321,7 @@ static struct platform_driver gcc_ipq5424_driver = {
 	.driver = {
 		.name   = "qcom,gcc-ipq5424",
 		.of_match_table = gcc_ipq5424_match_table,
-		.sync_state = icc_sync_state,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
diff --git a/drivers/clk/qcom/gcc-ipq9574.c b/drivers/clk/qcom/gcc-ipq9574.c
index 6dc86e686de4..9edcfd8afb77 100644
--- a/drivers/clk/qcom/gcc-ipq9574.c
+++ b/drivers/clk/qcom/gcc-ipq9574.c
@@ -5,7 +5,6 @@
 
 #include <linux/clk-provider.h>
 #include <linux/interconnect-clk.h>
-#include <linux/interconnect-provider.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -4134,7 +4133,7 @@ static struct platform_driver gcc_ipq9574_driver = {
 	.driver = {
 		.name   = "qcom,gcc-ipq9574",
 		.of_match_table = gcc_ipq9574_match_table,
-		.sync_state = icc_sync_state,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 
diff --git a/drivers/clk/qcom/nsscc-ipq5424.c b/drivers/clk/qcom/nsscc-ipq5424.c
index 5893c7146180..253d625ebf35 100644
--- a/drivers/clk/qcom/nsscc-ipq5424.c
+++ b/drivers/clk/qcom/nsscc-ipq5424.c
@@ -6,7 +6,6 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
-#include <linux/interconnect-provider.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -1331,7 +1330,7 @@ static struct platform_driver nss_cc_ipq5424_driver = {
 		.name = "qcom,ipq5424-nsscc",
 		.of_match_table = nss_cc_ipq5424_match_table,
 		.pm = &nss_cc_ipq5424_pm_ops,
-		.sync_state = icc_sync_state,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 module_platform_driver(nss_cc_ipq5424_driver);
diff --git a/drivers/clk/qcom/nsscc-ipq9574.c b/drivers/clk/qcom/nsscc-ipq9574.c
index c8b11b04a7c2..be8fbf5edda4 100644
--- a/drivers/clk/qcom/nsscc-ipq9574.c
+++ b/drivers/clk/qcom/nsscc-ipq9574.c
@@ -7,7 +7,6 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
-#include <linux/interconnect-provider.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -3100,7 +3099,7 @@ static struct platform_driver nss_cc_ipq9574_driver = {
 		.name = "qcom,nsscc-ipq9574",
 		.of_match_table = nss_cc_ipq9574_match_table,
 		.pm = &nss_cc_ipq9574_pm_ops,
-		.sync_state = icc_sync_state,
+		.sync_state = qcom_cc_sync_state,
 	},
 };
 

-- 
2.54.0

[PATCH v2 4/5] clk: qcom: cbf-8996: add clk_sync_state() call

[Brian Masney]

This driver doesn't use qcom_cc_probe() in qcom/common.c, so it
shouldn't use the newly introduced qcom_cc_sync_state(). Let's go ahead
and add a driver-specific sync state call.

Signed-off-by: Brian Masney <bmasney@redhat.com>
---
 drivers/clk/qcom/clk-cbf-8996.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/clk-cbf-8996.c b/drivers/clk/qcom/clk-cbf-8996.c
index 0b40ed601f9a..020d511072e2 100644
--- a/drivers/clk/qcom/clk-cbf-8996.c
+++ b/drivers/clk/qcom/clk-cbf-8996.c
@@ -249,7 +249,11 @@ static void qcom_msm8996_cbf_icc_remove(struct platform_device *pdev)
 
 	icc_clk_unregister(provider);
 }
-#define qcom_msm8996_cbf_icc_sync_state icc_sync_state
+static void qcom_msm8996_cbf_icc_sync_state(struct device *dev)
+{
+	icc_sync_state(dev);
+	clk_sync_state(dev);
+}
 #else
 static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev,  struct clk_hw *cbf_hw)
 {
@@ -258,7 +262,7 @@ static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev,  struct c
 	return 0;
 }
 #define qcom_msm8996_cbf_icc_remove(pdev) { }
-#define qcom_msm8996_cbf_icc_sync_state NULL
+#define qcom_msm8996_cbf_icc_sync_state clk_sync_state
 #endif
 
 static int qcom_msm8996_cbf_probe(struct platform_device *pdev)

-- 
2.54.0

[PATCH v2 5/5] 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.308051] clk: Disabling unused clocks not associated with a device
    [    6.541069] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
    [    6.843310] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
    [    7.604556] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
    [    8.446161] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
    [    8.446293] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
    [    8.546067] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
    [    8.546203] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
    [    8.546254] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks
    [   15.436834] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
    [   15.436953] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
    [   15.723348] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
    [   21.063241] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
    [   21.081996] va_macro 3370000.codec: clk: Disabling unused clocks
    [   21.092740] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
    [   21.118261] wsa_macro 3240000.codec: clk: Disabling unused clocks
    [   21.128758] tx_macro 3220000.txmacro: clk: Disabling unused clocks

Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
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 | 72 +++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 59 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 9cb2b42d1be4..7a15cceec620 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,7 +1545,7 @@ 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;
@@ -1526,7 +1555,10 @@ static int __init clk_disable_unused(void)
 		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 +1570,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,11 +1587,16 @@ 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)
 {
-	/* Will fill in */
+	__clk_disable_unused(dev);
 }
 EXPORT_SYMBOL_GPL(clk_sync_state);
 
@@ -4345,8 +4382,17 @@ __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;
+
+		/*
+		 * If a clk provider sets their own sync_state, then it needs to
+		 * also call clk_sync_state(). dev_set_drv_sync_state() won't
+		 * overwrite the sync_state callback, and this call will fail
+		 * with -EBUSY.
+		 */
+		dev_set_drv_sync_state(dev, clk_sync_state);
+	}
 	core->hw = hw;
 	core->flags = init->flags;
 	core->num_parents = init->num_parents;

-- 
2.54.0

Re: [PATCH v2 0/5] clk: implement sync_state support

[Neil Armstrong]

On 6/16/26 23:09, 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:
> - Adds a generic clk_sync_state() callback that's initially empty.
> - Adds a generic qcom_cc_sync_state() for qcom SoCs that need to
>    interact with the Interconnect framework.
> - Converts the 7 qcom drivers from using icc_sync_state() to
>    qcom_cc_sync_state().
> - Implement the framework-level sync state via clk_sync_state().
> 
> This approach maintains bisectability.
> 
> 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 (5):
>        clk: introduce stub clk_sync_state()
>        clk: qcom: common: introduce qcom_cc_sync_state()
>        clk: qcom: convert from icc_sync_state() to qcom_cc_sync_state()
>        clk: qcom: cbf-8996: add clk_sync_state() call
>        clk: implement sync_state support
> 
>   drivers/clk/clk.c                | 76 +++++++++++++++++++++++++++++++++-------
>   drivers/clk/qcom/apss-ipq5424.c  |  3 +-
>   drivers/clk/qcom/clk-cbf-8996.c  |  8 +++--
>   drivers/clk/qcom/common.c        |  9 +++++
>   drivers/clk/qcom/common.h        |  1 +
>   drivers/clk/qcom/gcc-ipq5332.c   |  3 +-
>   drivers/clk/qcom/gcc-ipq5424.c   |  3 +-
>   drivers/clk/qcom/gcc-ipq9574.c   |  3 +-
>   drivers/clk/qcom/nsscc-ipq5424.c |  3 +-
>   drivers/clk/qcom/nsscc-ipq9574.c |  3 +-
>   include/linux/clk.h              | 14 ++++++++
>   11 files changed, 100 insertions(+), 26 deletions(-)
> ---
> base-commit: 08484c504b55a98bd100527fbe10a3caf55ff3ff
> change-id: 20260602-clk-sync-state-c0539c5530f4
> 
> Best regards,

Just ran this patchset on the Amlogic boards (AML-S905X-CC, AML-S805X-CC, BPI-M2S, BPI-M5) in my CI lab, and it behaved correctly.

Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on AML-S905X-CC, AML-S805X-CC, BPI-M2S, BPI-M5

Thanks,
Neil

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

[dongxuyang]

> 
> 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.308051] clk: Disabling unused clocks not associated with a device
>     [    6.541069] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
>     [    6.843310] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
>     [    7.604556] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
>     [    8.446161] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
>     [    8.446293] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
>     [    8.546067] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
>     [    8.546203] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
>     [    8.546254] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks
>     [   15.436834] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
>     [   15.436953] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
>     [   15.723348] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
>     [   21.063241] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
>     [   21.081996] va_macro 3370000.codec: clk: Disabling unused clocks
>     [   21.092740] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
>     [   21.118261] wsa_macro 3240000.codec: clk: Disabling unused clocks
>     [   21.128758] tx_macro 3220000.txmacro: clk: Disabling unused clocks
> 
> 
> Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> 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 | 72 ++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 59 insertions(+), 13 deletions(-)
> 
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 9cb2b42d1be4..7a15cceec620 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,7 +1545,7 @@ 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;
> @@ -1526,7 +1555,10 @@ static int __init clk_disable_unused(void)
>  		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 +1570,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,11 +1587,16 @@ 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)
>  {
> -	/* Will fill in */
> +	__clk_disable_unused(dev);
>  }
>  EXPORT_SYMBOL_GPL(clk_sync_state);
>  
> @@ -4345,8 +4382,17 @@ __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;
> +
> +		/*
> +		 * If a clk provider sets their own sync_state, then it needs to
> +		 * also call clk_sync_state(). dev_set_drv_sync_state() won't
> +		 * overwrite the sync_state callback, and this call will fail
> +		 * with -EBUSY.
> +		 */
> +		dev_set_drv_sync_state(dev, clk_sync_state);
> +	}
>  	core->hw = hw;
>  	core->flags = init->flags;
>  	core->num_parents = init->num_parents;

Hi Brian,

Thank you for the patch set. I tested this in my tree on Sifive HiFive 
Premier P550, and it behaved correctly.

Tested-by: Xuyang Dong <dongxuyang@eswincomputing.com> # hfp550

Best regards,
Xuyang Dong

Re: [PATCH v2 5/5] clk: implement sync_state support

[Ulf Hansson]

On Tue, Jun 16, 2026 at 11:09 PM Brian Masney <bmasney@redhat.com> 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.
>
> 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.308051] clk: Disabling unused clocks not associated with a device
>     [    6.541069] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
>     [    6.843310] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
>     [    7.604556] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
>     [    8.446161] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
>     [    8.446293] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
>     [    8.546067] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
>     [    8.546203] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
>     [    8.546254] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks
>     [   15.436834] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
>     [   15.436953] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
>     [   15.723348] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
>     [   21.063241] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
>     [   21.081996] va_macro 3370000.codec: clk: Disabling unused clocks
>     [   21.092740] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
>     [   21.118261] wsa_macro 3240000.codec: clk: Disabling unused clocks
>     [   21.128758] tx_macro 3220000.txmacro: clk: Disabling unused clocks
>
> Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> 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/

For future revisions, please add ulfh@kernel.org on to/cc.

> ---
>  drivers/clk/clk.c | 72 +++++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 59 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 9cb2b42d1be4..7a15cceec620 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c

[...]

>  void clk_sync_state(struct device *dev)
>  {
> -       /* Will fill in */
> +       __clk_disable_unused(dev);
>  }
>  EXPORT_SYMBOL_GPL(clk_sync_state);
>
> @@ -4345,8 +4382,17 @@ __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;
> +
> +               /*
> +                * If a clk provider sets their own sync_state, then it needs to
> +                * also call clk_sync_state(). dev_set_drv_sync_state() won't
> +                * overwrite the sync_state callback, and this call will fail
> +                * with -EBUSY.
> +                */
> +               dev_set_drv_sync_state(dev, clk_sync_state);

We have cases where a device node represents a provider for multiple
types of resources, like clocks, power-domains (genpds), resets, etc,
as in the qcom case, for example.

For power-domain provider drivers (genpd) we also try to assign the
->sync_state() callback, see of_genpd_add_provider_simple() and
of_genpd_add_provider_simple(). This means the above doesn't play well
with how genpd behaves, so we need to figure out a way to manage these
cases.

In this regard, we also have of_genpd_sync_state(), which allows a
genpd provider driver to explicitly call genpd's sync state function,
if/when needed.

Unfortunately I am not able to suggest a detailed solution for how to
move this forward at this point, as it requires some more thinking and
I am heading for some vacation very soon.

> +       }
>         core->hw = hw;
>         core->flags = init->flags;
>         core->num_parents = init->num_parents;
>
> --
> 2.54.0

Kind regards
Uffe

Re: [PATCH v2 5/5] clk: implement sync_state support

[Brian Masney]

Hi Ulf,

On Wed, Jun 17, 2026 at 04:24:05PM +0200, Ulf Hansson wrote:
> On Tue, Jun 16, 2026 at 11:09 PM Brian Masney <bmasney@redhat.com> 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.
> >
> > 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.308051] clk: Disabling unused clocks not associated with a device
> >     [    6.541069] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
> >     [    6.843310] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
> >     [    7.604556] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
> >     [    8.446161] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
> >     [    8.446293] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
> >     [    8.546067] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
> >     [    8.546203] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
> >     [    8.546254] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks
> >     [   15.436834] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
> >     [   15.436953] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
> >     [   15.723348] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
> >     [   21.063241] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
> >     [   21.081996] va_macro 3370000.codec: clk: Disabling unused clocks
> >     [   21.092740] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
> >     [   21.118261] wsa_macro 3240000.codec: clk: Disabling unused clocks
> >     [   21.128758] tx_macro 3220000.txmacro: clk: Disabling unused clocks
> >
> > Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> > 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/
> 
> For future revisions, please add ulfh@kernel.org on to/cc.
> 
> > ---
> >  drivers/clk/clk.c | 72 +++++++++++++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 59 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 9cb2b42d1be4..7a15cceec620 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> 
> [...]
> 
> >  void clk_sync_state(struct device *dev)
> >  {
> > -       /* Will fill in */
> > +       __clk_disable_unused(dev);
> >  }
> >  EXPORT_SYMBOL_GPL(clk_sync_state);
> >
> > @@ -4345,8 +4382,17 @@ __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;
> > +
> > +               /*
> > +                * If a clk provider sets their own sync_state, then it needs to
> > +                * also call clk_sync_state(). dev_set_drv_sync_state() won't
> > +                * overwrite the sync_state callback, and this call will fail
> > +                * with -EBUSY.
> > +                */
> > +               dev_set_drv_sync_state(dev, clk_sync_state);
> 
> We have cases where a device node represents a provider for multiple
> types of resources, like clocks, power-domains (genpds), resets, etc,
> as in the qcom case, for example.
> 
> For power-domain provider drivers (genpd) we also try to assign the
> ->sync_state() callback, see of_genpd_add_provider_simple() and
> of_genpd_add_provider_simple(). This means the above doesn't play well
> with how genpd behaves, so we need to figure out a way to manage these
> cases.
> 
> In this regard, we also have of_genpd_sync_state(), which allows a
> genpd provider driver to explicitly call genpd's sync state function,
> if/when needed.
> 
> Unfortunately I am not able to suggest a detailed solution for how to
> move this forward at this point, as it requires some more thinking and
> I am heading for some vacation very soon.

One approach I initially considered was to make it so that we can have a
list of sync_state callbacks that can be added to. I already did some
work on this, but I didn't think it was worth it for just the QC clk
drivers in isolation, but it would address the concern here.

Brian

Re: [PATCH v2 5/5] clk: implement sync_state support

[Ulf Hansson]

On Wed, Jun 17, 2026 at 5:02 PM Brian Masney <bmasney@redhat.com> wrote:
>
> Hi Ulf,
>
> On Wed, Jun 17, 2026 at 04:24:05PM +0200, Ulf Hansson wrote:
> > On Tue, Jun 16, 2026 at 11:09 PM Brian Masney <bmasney@redhat.com> 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.
> > >
> > > 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.308051] clk: Disabling unused clocks not associated with a device
> > >     [    6.541069] qcom_aoss_qmp c300000.power-management: clk: Disabling unused clocks
> > >     [    6.843310] qcom-qmp-pcie-phy 1c24000.phy: clk: Disabling unused clocks
> > >     [    7.604556] qcom-qmp-pcie-phy 1c14000.phy: clk: Disabling unused clocks
> > >     [    8.446161] qcom-qmp-usb-phy 88f1000.phy: clk: Disabling unused clocks
> > >     [    8.446293] qcom-qmp-usb-phy 88ef000.phy: clk: Disabling unused clocks
> > >     [    8.546067] qcom-qmp-combo-phy 88eb000.phy: clk: Disabling unused clocks
> > >     [    8.546203] qcom-qmp-combo-phy 8903000.phy: clk: Disabling unused clocks
> > >     [    8.546254] qcom-edp-phy aec5a00.phy: clk: Disabling unused clocks
> > >     [   15.436834] qcom-cpufreq-hw 18591000.cpufreq: clk: Disabling unused clocks
> > >     [   15.436953] clk-rpmh 18200000.rsc:clock-controller: clk: Disabling unused clocks
> > >     [   15.723348] qcom-qmp-pcie-phy 1c06000.phy: clk: Disabling unused clocks
> > >     [   21.063241] q6prm-lpass-clock 3000000.remoteproc:glink-edge:gpr:service@2:clock-controller: clk: Disabling unused clocks
> > >     [   21.081996] va_macro 3370000.codec: clk: Disabling unused clocks
> > >     [   21.092740] rx_macro 3200000.rxmacro: clk: Disabling unused clocks
> > >     [   21.118261] wsa_macro 3240000.codec: clk: Disabling unused clocks
> > >     [   21.128758] tx_macro 3220000.txmacro: clk: Disabling unused clocks
> > >
> > > Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> > > 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/
> >
> > For future revisions, please add ulfh@kernel.org on to/cc.
> >
> > > ---
> > >  drivers/clk/clk.c | 72 +++++++++++++++++++++++++++++++++++++++++++++----------
> > >  1 file changed, 59 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > index 9cb2b42d1be4..7a15cceec620 100644
> > > --- a/drivers/clk/clk.c
> > > +++ b/drivers/clk/clk.c
> >
> > [...]
> >
> > >  void clk_sync_state(struct device *dev)
> > >  {
> > > -       /* Will fill in */
> > > +       __clk_disable_unused(dev);
> > >  }
> > >  EXPORT_SYMBOL_GPL(clk_sync_state);
> > >
> > > @@ -4345,8 +4382,17 @@ __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;
> > > +
> > > +               /*
> > > +                * If a clk provider sets their own sync_state, then it needs to
> > > +                * also call clk_sync_state(). dev_set_drv_sync_state() won't
> > > +                * overwrite the sync_state callback, and this call will fail
> > > +                * with -EBUSY.
> > > +                */
> > > +               dev_set_drv_sync_state(dev, clk_sync_state);
> >
> > We have cases where a device node represents a provider for multiple
> > types of resources, like clocks, power-domains (genpds), resets, etc,
> > as in the qcom case, for example.
> >
> > For power-domain provider drivers (genpd) we also try to assign the
> > ->sync_state() callback, see of_genpd_add_provider_simple() and
> > of_genpd_add_provider_simple(). This means the above doesn't play well
> > with how genpd behaves, so we need to figure out a way to manage these
> > cases.
> >
> > In this regard, we also have of_genpd_sync_state(), which allows a
> > genpd provider driver to explicitly call genpd's sync state function,
> > if/when needed.
> >
> > Unfortunately I am not able to suggest a detailed solution for how to
> > move this forward at this point, as it requires some more thinking and
> > I am heading for some vacation very soon.
>
> One approach I initially considered was to make it so that we can have a
> list of sync_state callbacks that can be added to. I already did some
> work on this, but I didn't think it was worth it for just the QC clk
> drivers in isolation, but it would address the concern here.

Right, maybe that would work.

One more thing to somewhat consider, is the problem I have been trying
to address for power-domains providers [1]. At least for genpd
providers, we need a more fine grained sync state solution, which also
must be able to co-exists with other subsystems sync state support. I
am not sure if something like this is needed for clocks too?

Kind regards
Uffe

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