[PATCH] clk: disable unused clocks registered after boot

[PATCH] clk: disable unused clocks registered after boot

[alexandre.belloni]

From: Alexandre Belloni <alexandre.belloni@bootlin.com>

clk_disable_unused() only runs once as a late_initcall, so clocks
registered afterwards by loading a module are never checked and may be left
running needlessly.

So record when clk_disable_unused has run and afterwards, schedule a
delayed work to run the scan when new clocks are registered.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/clk/clk.c | 50 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index fef87167a60b..f455d2482a5a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -24,6 +24,7 @@
 #include <linux/spinlock.h>
 #include <linux/string.h>
 #include <linux/stringhash.h>
+#include <linux/workqueue.h>
 
 #include "clk.h"
 
@@ -47,6 +48,10 @@ static LIST_HEAD(clk_notifier_list);
 static HLIST_HEAD(clk_rpm_list);
 static DEFINE_MUTEX(clk_rpm_list_lock);
 
+static bool clk_disable_unused_done;
+
+#define CLK_DISABLE_UNUSED_DELAY	(HZ / 2)
+
 static const struct hlist_head *all_lists[] = {
 	&clk_root_list,
 	&clk_orphan_list,
@@ -1511,7 +1516,7 @@ 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)
+static void clk_unprepare_unused_subtree(struct clk_core *core)
 {
 	struct clk_core *child;
 
@@ -1536,7 +1541,7 @@ 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 clk_core *child;
 	unsigned long flags;
@@ -1585,21 +1590,14 @@ 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 void clk_disable_unused(void)
 {
 	struct clk_core *core;
 	int ret;
 
-	if (clk_ignore_unused) {
-		pr_warn("clk: Not disabling unused clocks\n");
-		return 0;
-	}
-
-	pr_info("clk: Disabling unused clocks\n");
-
 	ret = clk_pm_runtime_get_all();
 	if (ret)
-		return ret;
+		return;
 	/*
 	 * Grab the prepare lock to keep the clk topology stable while iterating
 	 * over clks.
@@ -1621,10 +1619,31 @@ static int __init clk_disable_unused(void)
 	clk_prepare_unlock();
 
 	clk_pm_runtime_put_all();
+}
+
+static void clk_disable_unused_workfn(struct work_struct *work)
+{
+	clk_disable_unused();
+}
+
+static DECLARE_DELAYED_WORK(clk_disable_unused_work, clk_disable_unused_workfn);
+
+static int __init clk_disable_unused_init(void)
+{
+	if (clk_ignore_unused) {
+		pr_warn("clk: Not disabling unused clocks\n");
+		return 0;
+	}
+
+	pr_info("clk: Disabling unused clocks\n");
+
+	clk_disable_unused();
+
+	WRITE_ONCE(clk_disable_unused_done, true);
 
 	return 0;
 }
-late_initcall_sync(clk_disable_unused);
+late_initcall_sync(clk_disable_unused_init);
 
 static int clk_core_determine_round_nolock(struct clk_core *core,
 					   struct clk_rate_request *req)
@@ -4462,8 +4481,13 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 	clk_core_link_consumer(core, hw->clk);
 
 	ret = __clk_core_init(core);
-	if (!ret)
+	if (!ret) {
+		if (READ_ONCE(clk_disable_unused_done))
+			mod_delayed_work(system_power_efficient_wq,
+					 &clk_disable_unused_work,
+					 CLK_DISABLE_UNUSED_DELAY);
 		return hw->clk;
+	}
 
 	clk_prepare_lock();
 	clk_core_unlink_consumer(hw->clk);
-- 
2.55.0

Re: [PATCH] clk: disable unused clocks registered after boot

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk: unused clocks can bypass disable due to TOCTOU race in registration
--

commit 757dd3574808273e528cadee01291d010712df6b
Author: Alexandre Belloni <alexandre.belloni@bootlin.com>

clk: disable unused clocks registered after boot

This commit introduces a delayed work task to periodically scan for and disable
unused clocks registered by dynamically loaded modules after boot. It uses a
flag, clk_disable_unused_done, to determine whether the initial late_initcall
scan has completed.

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index fef87167a60b5..f455d2482a5a5 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
[ ... ]
> @@ -1621,10 +1619,31 @@ static int __init clk_disable_unused(void)
>  	clk_prepare_unlock();
>  
>  	clk_pm_runtime_put_all();
> +}
> +
> +static void clk_disable_unused_workfn(struct work_struct *work)
> +{
> +	clk_disable_unused();
> +}
> +
> +static DECLARE_DELAYED_WORK(clk_disable_unused_work, clk_disable_unused_workfn);
> +
> +static int __init clk_disable_unused_init(void)
> +{
> +	if (clk_ignore_unused) {
> +		pr_warn("clk: Not disabling unused clocks\n");
> +		return 0;
> +	}
> +
> +	pr_info("clk: Disabling unused clocks\n");
> +
> +	clk_disable_unused();
> +
> +	WRITE_ONCE(clk_disable_unused_done, true);

[Severity: Medium]
Does setting clk_disable_unused_done after clk_disable_unused() returns and
drops prepare_lock open up a time-of-check to time-of-use race?

If a driver registering a clock (such as via an async probe thread) adds the
clock right after the boot scan completes, could it slip through before this
flag is updated?

>  
>  	return 0;
>  }
> -late_initcall_sync(clk_disable_unused);
> +late_initcall_sync(clk_disable_unused_init);
[ ... ]
> @@ -4462,8 +4481,13 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
>  	clk_core_link_consumer(core, hw->clk);
>  
>  	ret = __clk_core_init(core);
> -	if (!ret)
> +	if (!ret) {
> +		if (READ_ONCE(clk_disable_unused_done))
> +			mod_delayed_work(system_power_efficient_wq,
> +					 &clk_disable_unused_work,
> +					 CLK_DISABLE_UNUSED_DELAY);
>  		return hw->clk;
> +	}

[Severity: Medium]
Can a concurrent registration complete __clk_core_init() after the boot
scan finishes, but read clk_disable_unused_done as false?

In this sequence, __clk_core_init() takes the prepare_lock after
clk_disable_unused() has already dropped it. The clock is added to the
internal lists, but the boot scan has already passed it by.

Then, __clk_register() reads clk_disable_unused_done. If
clk_disable_unused_init() hasn't updated the flag yet, it will read false
and skip scheduling the delayed work.

Would this leave the newly registered unused clock permanently enabled since
it missed both the boot scan and the delayed work?

>  
>  	clk_prepare_lock();
>  	clk_core_unlink_consumer(hw->clk);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917144011.3014860-1-alexandre.belloni@bootlin.com?part=1

Re: [PATCH] clk: disable unused clocks registered after boot

[Brian Masney]

Hi Alexandre,

On Thu, Sep 17, 2026 at 04:40:10PM +0200, alexandre.belloni@bootlin.com wrote:
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
> clk_disable_unused() only runs once as a late_initcall, so clocks
> registered afterwards by loading a module are never checked and may be left
> running needlessly.
> 
> So record when clk_disable_unused has run and afterwards, schedule a
> delayed work to run the scan when new clocks are registered.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

We're actually in the process of redoing the way clk_disable_unused is
handled. I posted a series to add sync_state support to the clk
framework:

https://lore.kernel.org/linux-clk/20260626-clk-sync-state-v1-0-4156d8196dc8@redhat.com/

Will you be at Linux Plumbers in Prague in 2.5 weeks? If so, come to
this talk where we will talk about the clk subsystem.

    Evolving support for sync_state to subsystems beyond genpd 
    https://lpc.events/event/20/contributions/2504/

Brian

Re: [PATCH] clk: disable unused clocks registered after boot

[Alexandre Belloni]

Hello,

On 17/09/2026 12:01:42-0400, Brian Masney wrote:
> Hi Alexandre,
> 
> On Thu, Sep 17, 2026 at 04:40:10PM +0200, alexandre.belloni@bootlin.com wrote:
> > From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > 
> > clk_disable_unused() only runs once as a late_initcall, so clocks
> > registered afterwards by loading a module are never checked and may be left
> > running needlessly.
> > 
> > So record when clk_disable_unused has run and afterwards, schedule a
> > delayed work to run the scan when new clocks are registered.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
> We're actually in the process of redoing the way clk_disable_unused is
> handled. I posted a series to add sync_state support to the clk
> framework:
> 
> https://lore.kernel.org/linux-clk/20260626-clk-sync-state-v1-0-4156d8196dc8@redhat.com/
> 
> Will you be at Linux Plumbers in Prague in 2.5 weeks? If so, come to
> this talk where we will talk about the clk subsystem.
> 
>     Evolving support for sync_state to subsystems beyond genpd 
>     https://lpc.events/event/20/contributions/2504/
> 

I will be at plumbers and I'll try to attend, thanks!

For reference, this is the reason for the patch:
https://lore.kernel.org/all/2ce03d3b4cd9cce8e32fe1fe1ffbea5760bf9ef3.camel@siemens.com/

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] clk: disable unused clocks registered after boot

[Brian Masney]

Hi Alexandre,

On Thu, Sep 17, 2026 at 07:23:36PM +0200, Alexandre Belloni wrote:
> On 17/09/2026 12:01:42-0400, Brian Masney wrote:
> > On Thu, Sep 17, 2026 at 04:40:10PM +0200, alexandre.belloni@bootlin.com wrote:
> > > From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > > 
> > > clk_disable_unused() only runs once as a late_initcall, so clocks
> > > registered afterwards by loading a module are never checked and may be left
> > > running needlessly.
> > > 
> > > So record when clk_disable_unused has run and afterwards, schedule a
> > > delayed work to run the scan when new clocks are registered.
> > > 
> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > 
> > We're actually in the process of redoing the way clk_disable_unused is
> > handled. I posted a series to add sync_state support to the clk
> > framework:
> > 
> > https://lore.kernel.org/linux-clk/20260626-clk-sync-state-v1-0-4156d8196dc8@redhat.com/
> > 
> > Will you be at Linux Plumbers in Prague in 2.5 weeks? If so, come to
> > this talk where we will talk about the clk subsystem.
> > 
> >     Evolving support for sync_state to subsystems beyond genpd 
> >     https://lpc.events/event/20/contributions/2504/
> > 
> 
> I will be at plumbers and I'll try to attend, thanks!
> 
> For reference, this is the reason for the patch:
> https://lore.kernel.org/all/2ce03d3b4cd9cce8e32fe1fe1ffbea5760bf9ef3.camel@siemens.com/

Can you try running with my sync_state patch set before LPC and see if
it works as expected for you? Be sure to leave clk_ignore_unused off the
kernel command line.

https://lore.kernel.org/linux-clk/20260626-clk-sync-state-v1-0-4156d8196dc8@redhat.com/

Brian