* [PATCH] rtc: pcf85063: disable the clkout output by default
@ 2026-08-24 8:07 A. Sverdlin
2026-08-24 8:19 ` sashiko-bot
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: A. Sverdlin @ 2026-08-24 8:07 UTC (permalink / raw)
To: linux-rtc; +Cc: Alexander Sverdlin, Alexandre Belloni, linux-kernel
From: Alexander Sverdlin <alexander.sverdlin@siemens.com>
The PCF85063 powers up with its CLKOUT pin driving the 32.768 kHz. While
it's possible to request a specific default frequency in the DT via
assigned-clocks/assigned-clock-rates on the RTC node itself, even without
a real consumer referencing the clock, there is no such possibility to
disable the clock output by default.
Therefore gate CLKOUT in the driver before registering the OF provider,
mirroring the pcf8563 and hym8563 siblings which already force an off state
at registration. Doing it before devm_of_clk_add_hw_provider() leaves the
real consumers in the device tree free to re-enable the output afterwards,
while an otherwise unused CLKOUT now stays off.
In practice it means that with CLKOUT disabled the RTC consumes around
250nA at 25°C and 6-10uA with CLKOUT enabled (5v Vdd). Which means days vs
hours on a supercapacitor.
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>
---
drivers/rtc/rtc-pcf85063.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
index 01e209d88f5f3..0b6d8083b6456 100644
--- a/drivers/rtc/rtc-pcf85063.c
+++ b/drivers/rtc/rtc-pcf85063.c
@@ -523,6 +523,12 @@ static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063)
/* optional override of the clockname */
of_property_read_string(node, "clock-output-names", &init.name);
+ /* power-on default is the 32768 Hz output on; gate it until claimed */
+ ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
+ PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF);
+ if (ret)
+ return ret;
+
/* register the clock */
clk = devm_clk_register(&pcf85063->rtc->dev, &pcf85063->clkout_hw);
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-24 8:07 [PATCH] rtc: pcf85063: disable the clkout output by default A. Sverdlin @ 2026-08-24 8:19 ` sashiko-bot 2026-08-24 10:30 ` Sverdlin, Alexander 2026-08-24 8:24 ` Alexandre Belloni ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: sashiko-bot @ 2026-08-24 8:19 UTC (permalink / raw) To: A. Sverdlin; +Cc: linux-rtc, Alexandre Belloni Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: New issues: - [Critical] Unconditionally gating the CLKOUT clock during probe overrides the hardware power-on state and breaks Device Tree ABI backward compatibility. - [Low] The patch introduces a compilation failure because the variable `ret` is undeclared, and returning it directly causes a type mismatch. Pre-existing issues: - [High] The driver's clock implementation violates the Common Clock Framework (CCF) contract by implicitly enabling the clock during set_rate and discarding the configured rate after unprepare. - [High] The clock provider registered via `of_clk_add_provider` is never removed, causing a memory leak and a dangling pointer on driver unbind, despite the commit message claiming otherwise. -- --- Patch [1]: [PATCH] rtc: pcf85063: disable the clkout output by default --- commit f0b8f2118b75da5ed86252e825d0385a7e1fe816 Author: Alexander Sverdlin <alexander.sverdlin@siemens.com> rtc: pcf85063: disable the clkout output by default This patch gates the PCF85063 CLKOUT pin by default before registering the OF provider to save power, unless a consumer explicitly claims it. > Therefore gate CLKOUT in the driver before registering the OF provider, > mirroring the pcf8563 and hym8563 siblings which already force an off state > at registration. Doing it before devm_of_clk_add_hw_provider() leaves the > real consumers in the device tree free to re-enable the output afterwards, > while an otherwise unused CLKOUT now stays off. [Severity: High] This is a pre-existing issue, but the commit message mentions using devm_of_clk_add_hw_provider(), which contradicts the actual code. The code still uses of_clk_add_provider() without a corresponding removal, which leads to a dangling pointer and memory leak if the driver is unbound: drivers/rtc/rtc-pcf85063.c:pcf85063_clkout_register_clk() { ... if (!IS_ERR(clk)) of_clk_add_provider(node, of_clk_src_simple_get, clk); ... } Since devm_clk_register() automatically frees the clock allocation on unbind, but of_clk_del_provider() is never called, the global of_clk_providers list will contain a pointer to freed memory. Should the driver be updated to actually use devm_of_clk_add_hw_provider() to ensure proper cleanup? > diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c > index 8cb9ffc73f6d9..9ccfc893be8d3 100644 > --- a/drivers/rtc/rtc-pcf85063.c > +++ b/drivers/rtc/rtc-pcf85063.c > @@ -523,6 +523,12 @@ static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063) > /* optional override of the clockname */ > of_property_read_string(node, "clock-output-names", &init.name); > > + /* power-on default is the 32768 Hz output on; gate it until claimed */ > + ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, > + PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF); [Severity: Critical] Does unconditionally gating the CLKOUT clock during probe break Device Tree ABI backward compatibility? Legacy hardware platforms might rely on the default active 32.768 kHz clock output without explicitly defining a clock consumer in their device tree. Will those systems lose their clock signal when booting a kernel with this patch? > + if (ret) > + return ret; [Severity: Low] Does this code compile with CONFIG_COMMON_CLK enabled? It looks like the variable ret is undeclared in pcf85063_clkout_register_clk(). Also, since the function returns a struct clk pointer, wouldn't returning an integer error code directly cause a type mismatch? [Severity: High] This isn't a bug introduced by this patch, but does the current clock implementation violate the Common Clock Framework contract? Looking at pcf85063_clkout_set_rate(), setting a rate implicitly enables the clock by modifying the hardware register directly: drivers/rtc/rtc-pcf85063.c:pcf85063_clkout_set_rate() { ... return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, PCF85063_REG_CLKO_F_MASK, i); } Furthermore, if a consumer disables the clock and later re-enables it, pcf85063_clkout_control() unconditionally resets the rate to 32768 Hz, discarding any previously configured custom rates: drivers/rtc/rtc-pcf85063.c:pcf85063_clkout_control() { ... if (enable) { if (buf == PCF85063_REG_CLKO_F_OFF) buf = PCF85063_REG_CLKO_F_32768HZ; ... } Will this cause custom clock rates set by consumers or device tree to be silently lost across enable/disable cycles? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260824080801.709764-1-alexander.sverdlin@siemens.com?part=1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-24 8:19 ` sashiko-bot @ 2026-08-24 10:30 ` Sverdlin, Alexander 0 siblings, 0 replies; 10+ messages in thread From: Sverdlin, Alexander @ 2026-08-24 10:30 UTC (permalink / raw) To: sashiko-reviews@lists.linux.dev Cc: linux-rtc@vger.kernel.org, alexandre.belloni@bootlin.com Hi all, On Mon, 2026-08-24 at 08:19 +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: > > New issues: > - [Critical] Unconditionally gating the CLKOUT clock during probe overrides the hardware power-on state and breaks Device Tree ABI backward compatibility. > - [Low] The patch introduces a compilation failure because the variable `ret` is undeclared, and returning it directly causes a type mismatch. > > Pre-existing issues: > - [High] The driver's clock implementation violates the Common Clock Framework (CCF) contract by implicitly enabling the clock during set_rate and discarding the configured rate after unprepare. > - [High] The clock provider registered via `of_clk_add_provider` is never removed, causing a memory leak and a dangling pointer on driver unbind, despite the commit message claiming otherwise. > -- > > --- Patch [1]: [PATCH] rtc: pcf85063: disable the clkout output by default --- > commit f0b8f2118b75da5ed86252e825d0385a7e1fe816 > Author: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > rtc: pcf85063: disable the clkout output by default > > This patch gates the PCF85063 CLKOUT pin by default before registering the > OF provider to save power, unless a consumer explicitly claims it. > > > Therefore gate CLKOUT in the driver before registering the OF provider, > > mirroring the pcf8563 and hym8563 siblings which already force an off state > > at registration. Doing it before devm_of_clk_add_hw_provider() leaves the > > real consumers in the device tree free to re-enable the output afterwards, > > while an otherwise unused CLKOUT now stays off. > > [Severity: High] > This is a pre-existing issue, but the commit message mentions using > devm_of_clk_add_hw_provider(), which contradicts the actual code. The code > still uses of_clk_add_provider() without a corresponding removal, which > leads to a dangling pointer and memory leak if the driver is unbound: > > drivers/rtc/rtc-pcf85063.c:pcf85063_clkout_register_clk() { > ... > if (!IS_ERR(clk)) > of_clk_add_provider(node, of_clk_src_simple_get, clk); > ... > } > > Since devm_clk_register() automatically frees the clock allocation on > unbind, but of_clk_del_provider() is never called, the global > of_clk_providers list will contain a pointer to freed memory. Should the > driver be updated to actually use devm_of_clk_add_hw_provider() to ensure > proper cleanup? Initially I had a fix for it prepared for another series, but taking into account this patch actually depended on the mentioned fix because of "ret" variable below... seems that I would need to just pack all the patches into one series. > > diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c > > index 8cb9ffc73f6d9..9ccfc893be8d3 100644 > > --- a/drivers/rtc/rtc-pcf85063.c > > +++ b/drivers/rtc/rtc-pcf85063.c > > @@ -523,6 +523,12 @@ static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063) > > /* optional override of the clockname */ > > of_property_read_string(node, "clock-output-names", &init.name); > > > > + /* power-on default is the 32768 Hz output on; gate it until claimed */ > > + ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, > > + PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF); > > [Severity: Critical] > Does unconditionally gating the CLKOUT clock during probe break Device Tree > ABI backward compatibility? Legacy hardware platforms might rely on the > default active 32.768 kHz clock output without explicitly defining a clock > consumer in their device tree. Will those systems lose their clock signal > when booting a kernel with this patch? Yes. But this would have been relying on something not part of a contract, because some drivers do define the state, some don't (keep the HW default). Now the DT allows for different ways of enabling the clock, but no way exists currently to explicitly disable the clock, which leaves us with only option to disable the clock on probe. > > > + if (ret) > > + return ret; > > [Severity: Low] > Does this code compile with CONFIG_COMMON_CLK enabled? It looks like the > variable ret is undeclared in pcf85063_clkout_register_clk(). Also, since > the function returns a struct clk pointer, wouldn't returning an integer > error code directly cause a type mismatch? Nice catch! This patch actually depends on another patch in my queue and I was wrong trying to send it separately. Next time I'll resend them all together. > [Severity: High] > This isn't a bug introduced by this patch, but does the current clock > implementation violate the Common Clock Framework contract? > > Looking at pcf85063_clkout_set_rate(), setting a rate implicitly enables > the clock by modifying the hardware register directly: > > drivers/rtc/rtc-pcf85063.c:pcf85063_clkout_set_rate() { > ... > return regmap_update_bits(pcf85063->regmap, > PCF85063_REG_CTRL2, > PCF85063_REG_CLKO_F_MASK, i); > } > > Furthermore, if a consumer disables the clock and later re-enables it, > pcf85063_clkout_control() unconditionally resets the rate to 32768 Hz, > discarding any previously configured custom rates: > > drivers/rtc/rtc-pcf85063.c:pcf85063_clkout_control() { > ... > if (enable) { > if (buf == PCF85063_REG_CLKO_F_OFF) > buf = PCF85063_REG_CLKO_F_32768HZ; > ... > } > > Will this cause custom clock rates set by consumers or device tree to be > silently lost across enable/disable cycles? -- Alexander Sverdlin Siemens AG www.siemens.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-24 8:07 [PATCH] rtc: pcf85063: disable the clkout output by default A. Sverdlin 2026-08-24 8:19 ` sashiko-bot @ 2026-08-24 8:24 ` Alexandre Belloni 2026-08-24 9:58 ` Sverdlin, Alexander 2026-08-25 21:48 ` kernel test robot 2026-08-25 22:10 ` kernel test robot 3 siblings, 1 reply; 10+ messages in thread From: Alexandre Belloni @ 2026-08-24 8:24 UTC (permalink / raw) To: A. Sverdlin; +Cc: linux-rtc, linux-kernel On 24/08/2026 10:07:59+0200, A. Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > The PCF85063 powers up with its CLKOUT pin driving the 32.768 kHz. While > it's possible to request a specific default frequency in the DT via > assigned-clocks/assigned-clock-rates on the RTC node itself, even without > a real consumer referencing the clock, there is no such possibility to > disable the clock output by default. Are you sure about this? The CCF should disable the clock if it is not used. > > Therefore gate CLKOUT in the driver before registering the OF provider, > mirroring the pcf8563 and hym8563 siblings which already force an off state > at registration. Doing it before devm_of_clk_add_hw_provider() leaves the > real consumers in the device tree free to re-enable the output afterwards, > while an otherwise unused CLKOUT now stays off. > > In practice it means that with CLKOUT disabled the RTC consumes around > 250nA at 25°C and 6-10uA with CLKOUT enabled (5v Vdd). Which means days vs > hours on a supercapacitor. > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > --- > drivers/rtc/rtc-pcf85063.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c > index 01e209d88f5f3..0b6d8083b6456 100644 > --- a/drivers/rtc/rtc-pcf85063.c > +++ b/drivers/rtc/rtc-pcf85063.c > @@ -523,6 +523,12 @@ static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063) > /* optional override of the clockname */ > of_property_read_string(node, "clock-output-names", &init.name); > > + /* power-on default is the 32768 Hz output on; gate it until claimed */ > + ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, > + PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF); > + if (ret) > + return ret; > + > /* register the clock */ > clk = devm_clk_register(&pcf85063->rtc->dev, &pcf85063->clkout_hw); > > -- > 2.55.0 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-24 8:24 ` Alexandre Belloni @ 2026-08-24 9:58 ` Sverdlin, Alexander 2026-08-25 21:07 ` Alexandre Belloni 0 siblings, 1 reply; 10+ messages in thread From: Sverdlin, Alexander @ 2026-08-24 9:58 UTC (permalink / raw) To: alexandre.belloni@bootlin.com Cc: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Hi Alexandre, On Mon, 2026-08-24 at 10:24 +0200, Alexandre Belloni wrote: > > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > > > The PCF85063 powers up with its CLKOUT pin driving the 32.768 kHz. While > > it's possible to request a specific default frequency in the DT via > > assigned-clocks/assigned-clock-rates on the RTC node itself, even without > > a real consumer referencing the clock, there is no such possibility to > > disable the clock output by default. > > Are you sure about this? The CCF should disable the clock if it is not > used. There is late_initcall_sync(clk_disable_unused) (marked __init), but for the clock providers registered later, say, via modules, there is no such mechanism. And I double-checked this putting some debug prints into the driver, nothing from struct clk_ops is being called without a consumer in DT. > > Therefore gate CLKOUT in the driver before registering the OF provider, > > mirroring the pcf8563 and hym8563 siblings which already force an off state > > at registration. Doing it before devm_of_clk_add_hw_provider() leaves the > > real consumers in the device tree free to re-enable the output afterwards, > > while an otherwise unused CLKOUT now stays off. > > > > In practice it means that with CLKOUT disabled the RTC consumes around > > 250nA at 25°C and 6-10uA with CLKOUT enabled (5v Vdd). Which means days vs > > hours on a supercapacitor. > > > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > --- > > drivers/rtc/rtc-pcf85063.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c > > index 01e209d88f5f3..0b6d8083b6456 100644 > > --- a/drivers/rtc/rtc-pcf85063.c > > +++ b/drivers/rtc/rtc-pcf85063.c > > @@ -523,6 +523,12 @@ static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063) > > /* optional override of the clockname */ > > of_property_read_string(node, "clock-output-names", &init.name); > > > > + /* power-on default is the 32768 Hz output on; gate it until claimed */ > > + ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, > > + PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF); > > + if (ret) > > + return ret; > > + > > /* register the clock */ > > clk = devm_clk_register(&pcf85063->rtc->dev, &pcf85063->clkout_hw); > > > > -- > > 2.55.0 > > -- Alexander Sverdlin Siemens AG www.siemens.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-24 9:58 ` Sverdlin, Alexander @ 2026-08-25 21:07 ` Alexandre Belloni 2026-08-26 6:13 ` Sverdlin, Alexander 0 siblings, 1 reply; 10+ messages in thread From: Alexandre Belloni @ 2026-08-25 21:07 UTC (permalink / raw) To: Sverdlin, Alexander Cc: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 2808 bytes --] On 24/08/2026 09:58:16+0000, Sverdlin, Alexander wrote: > Hi Alexandre, > > On Mon, 2026-08-24 at 10:24 +0200, Alexandre Belloni wrote: > > > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > > > > > The PCF85063 powers up with its CLKOUT pin driving the 32.768 kHz. While > > > it's possible to request a specific default frequency in the DT via > > > assigned-clocks/assigned-clock-rates on the RTC node itself, even without > > > a real consumer referencing the clock, there is no such possibility to > > > disable the clock output by default. > > > > Are you sure about this? The CCF should disable the clock if it is not > > used. > > There is late_initcall_sync(clk_disable_unused) (marked __init), but for the > clock providers registered later, say, via modules, there is no such mechanism. > And I double-checked this putting some debug prints into the driver, > nothing from struct clk_ops is being called without a consumer in DT. > Right, so what about the patch attached which seems to work fine with my rv3032. > > > Therefore gate CLKOUT in the driver before registering the OF provider, > > > mirroring the pcf8563 and hym8563 siblings which already force an off state > > > at registration. Doing it before devm_of_clk_add_hw_provider() leaves the > > > real consumers in the device tree free to re-enable the output afterwards, > > > while an otherwise unused CLKOUT now stays off. > > > > > > In practice it means that with CLKOUT disabled the RTC consumes around > > > 250nA at 25°C and 6-10uA with CLKOUT enabled (5v Vdd). Which means days vs > > > hours on a supercapacitor. > > > > > > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > > --- > > > drivers/rtc/rtc-pcf85063.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c > > > index 01e209d88f5f3..0b6d8083b6456 100644 > > > --- a/drivers/rtc/rtc-pcf85063.c > > > +++ b/drivers/rtc/rtc-pcf85063.c > > > @@ -523,6 +523,12 @@ static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063) > > > /* optional override of the clockname */ > > > of_property_read_string(node, "clock-output-names", &init.name); > > > > > > + /* power-on default is the 32768 Hz output on; gate it until claimed */ > > > + ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, > > > + PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF); > > > + if (ret) > > > + return ret; > > > + > > > /* register the clock */ > > > clk = devm_clk_register(&pcf85063->rtc->dev, &pcf85063->clkout_hw); > > > > > > -- > > > 2.55.0 > > > > > -- > Alexander Sverdlin > Siemens AG > www.siemens.com -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com [-- Attachment #2: 0001-clk-disable-unused-clocks-registered-after-boot.patch --] [-- Type: text/plain, Size: 3713 bytes --] From 68b91c1d8b64b0b9fc88b9036f9a4340e7a0a98f Mon Sep 17 00:00:00 2001 From: Alexandre Belloni <alexandre.belloni@bootlin.com> Date: Tue, 25 Aug 2026 22:54:06 +0200 Subject: [PATCH] clk: disable unused clocks registered after boot 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 048adfa86a5d..817f916192ac 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, @@ -1442,7 +1447,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; @@ -1467,7 +1472,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; @@ -1516,21 +1521,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. @@ -1552,10 +1550,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) @@ -4366,8 +4385,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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-25 21:07 ` Alexandre Belloni @ 2026-08-26 6:13 ` Sverdlin, Alexander 2026-08-26 6:45 ` Alexandre Belloni 0 siblings, 1 reply; 10+ messages in thread From: Sverdlin, Alexander @ 2026-08-26 6:13 UTC (permalink / raw) To: alexandre.belloni@bootlin.com Cc: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org Hi Alexandre, On Tue, 2026-08-25 at 23:07 +0200, Alexandre Belloni wrote: > > > > The PCF85063 powers up with its CLKOUT pin driving the 32.768 kHz. While > > > > it's possible to request a specific default frequency in the DT via > > > > assigned-clocks/assigned-clock-rates on the RTC node itself, even without > > > > a real consumer referencing the clock, there is no such possibility to > > > > disable the clock output by default. > > > > > > Are you sure about this? The CCF should disable the clock if it is not > > > used. > > > > There is late_initcall_sync(clk_disable_unused) (marked __init), but for the > > clock providers registered later, say, via modules, there is no such mechanism. > > And I double-checked this putting some debug prints into the driver, > > nothing from struct clk_ops is being called without a consumer in DT. > > > > Right, so what about the patch attached which seems to work fine with my > rv3032. The patch would work for me as well, will you publish it officially? BTW, it's racy, if __clk_register() would be called between clk_disable_unused(); ... and... WRITE_ONCE(clk_disable_unused_done, true); ... the workqueue will not be scheduled. But overall it's a nice idea! Maybe we just need to drop late_initcall_sync() entirely, this would simplify the things a bit? -- Alexander Sverdlin Siemens AG www.siemens.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-26 6:13 ` Sverdlin, Alexander @ 2026-08-26 6:45 ` Alexandre Belloni 0 siblings, 0 replies; 10+ messages in thread From: Alexandre Belloni @ 2026-08-26 6:45 UTC (permalink / raw) To: Sverdlin, Alexander Cc: linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org On 26/08/2026 06:13:08+0000, Sverdlin, Alexander wrote: > Hi Alexandre, > > On Tue, 2026-08-25 at 23:07 +0200, Alexandre Belloni wrote: > > > > > The PCF85063 powers up with its CLKOUT pin driving the 32.768 kHz. While > > > > > it's possible to request a specific default frequency in the DT via > > > > > assigned-clocks/assigned-clock-rates on the RTC node itself, even without > > > > > a real consumer referencing the clock, there is no such possibility to > > > > > disable the clock output by default. > > > > > > > > Are you sure about this? The CCF should disable the clock if it is not > > > > used. > > > > > > There is late_initcall_sync(clk_disable_unused) (marked __init), but for the > > > clock providers registered later, say, via modules, there is no such mechanism. > > > And I double-checked this putting some debug prints into the driver, > > > nothing from struct clk_ops is being called without a consumer in DT. > > > > > > > Right, so what about the patch attached which seems to work fine with my > > rv3032. > > The patch would work for me as well, will you publish it officially? > Yes, that's the plan. > BTW, it's racy, if __clk_register() would be called between > > clk_disable_unused(); > > ... and... > > WRITE_ONCE(clk_disable_unused_done, true); > > ... the workqueue will not be scheduled. But overall it's a nice idea! I'm not sure you can actually load modules before late_initcall_sync finishes. > > Maybe we just need to drop late_initcall_sync() entirely, this would simplify > the things a bit? I'll let this to the (new) clk maintainers. -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-24 8:07 [PATCH] rtc: pcf85063: disable the clkout output by default A. Sverdlin 2026-08-24 8:19 ` sashiko-bot 2026-08-24 8:24 ` Alexandre Belloni @ 2026-08-25 21:48 ` kernel test robot 2026-08-25 22:10 ` kernel test robot 3 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2026-08-25 21:48 UTC (permalink / raw) To: A. Sverdlin, linux-rtc Cc: oe-kbuild-all, Alexander Sverdlin, Alexandre Belloni, linux-kernel Hi Sverdlin, kernel test robot noticed the following build errors: [auto build test ERROR on abelloni/rtc-next] [also build test ERROR on linus/master v7.2 next-20260824] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/A-Sverdlin/rtc-pcf85063-disable-the-clkout-output-by-default/20260824-100759 base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next patch link: https://lore.kernel.org/r/20260824080801.709764-1-alexander.sverdlin%40siemens.com patch subject: [PATCH] rtc: pcf85063: disable the clkout output by default config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20260826/202608260554.ivtsRrZ9-lkp@intel.com/config) compiler: nios2-linux-gcc (GCC) 11.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260826/202608260554.ivtsRrZ9-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202608260554.ivtsRrZ9-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/rtc/rtc-pcf85063.c: In function 'pcf85063_clkout_register_clk': >> drivers/rtc/rtc-pcf85063.c:527:9: error: 'ret' undeclared (first use in this function); did you mean 'net'? 527 | ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, | ^~~ | net drivers/rtc/rtc-pcf85063.c:527:9: note: each undeclared identifier is reported only once for each function it appears in vim +527 drivers/rtc/rtc-pcf85063.c 497 498 static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063) 499 { 500 struct clk *clk; 501 struct clk_init_data init = {}; 502 struct device_node *node = pcf85063->rtc->dev.parent->of_node; 503 struct device_node *fixed_clock; 504 505 fixed_clock = of_get_child_by_name(node, "clock"); 506 if (fixed_clock) { 507 /* 508 * skip registering square wave clock when a fixed 509 * clock has been registered. The fixed clock is 510 * registered automatically when being referenced. 511 */ 512 of_node_put(fixed_clock); 513 return NULL; 514 } 515 516 init.name = "pcf85063-clkout"; 517 init.ops = &pcf85063_clkout_ops; 518 init.flags = 0; 519 init.parent_names = NULL; 520 init.num_parents = 0; 521 pcf85063->clkout_hw.init = &init; 522 523 /* optional override of the clockname */ 524 of_property_read_string(node, "clock-output-names", &init.name); 525 526 /* power-on default is the 32768 Hz output on; gate it until claimed */ > 527 ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, 528 PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF); 529 if (ret) 530 return ret; 531 532 /* register the clock */ 533 clk = devm_clk_register(&pcf85063->rtc->dev, &pcf85063->clkout_hw); 534 535 if (!IS_ERR(clk)) 536 of_clk_add_provider(node, of_clk_src_simple_get, clk); 537 538 return clk; 539 } 540 #endif 541 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rtc: pcf85063: disable the clkout output by default 2026-08-24 8:07 [PATCH] rtc: pcf85063: disable the clkout output by default A. Sverdlin ` (2 preceding siblings ...) 2026-08-25 21:48 ` kernel test robot @ 2026-08-25 22:10 ` kernel test robot 3 siblings, 0 replies; 10+ messages in thread From: kernel test robot @ 2026-08-25 22:10 UTC (permalink / raw) To: A. Sverdlin, linux-rtc Cc: llvm, oe-kbuild-all, Alexander Sverdlin, Alexandre Belloni, linux-kernel Hi Sverdlin, kernel test robot noticed the following build errors: [auto build test ERROR on abelloni/rtc-next] [also build test ERROR on linus/master v7.2 next-20260824] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/A-Sverdlin/rtc-pcf85063-disable-the-clkout-output-by-default/20260824-100759 base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next patch link: https://lore.kernel.org/r/20260824080801.709764-1-alexander.sverdlin%40siemens.com patch subject: [PATCH] rtc: pcf85063: disable the clkout output by default config: arm-randconfig-003-20260826 (https://download.01.org/0day-ci/archive/20260826/202608260642.ydlr9qYE-lkp@intel.com/config) compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project 771bdfdab2d66a4eb85b8ea7673cfa30152718b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260826/202608260642.ydlr9qYE-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202608260642.ydlr9qYE-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/rtc/rtc-pcf85063.c:527:2: error: use of undeclared identifier 'ret' 527 | ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, | ^~~ drivers/rtc/rtc-pcf85063.c:529:6: error: use of undeclared identifier 'ret' 529 | if (ret) | ^~~ drivers/rtc/rtc-pcf85063.c:530:10: error: use of undeclared identifier 'ret' 530 | return ret; | ^~~ 3 errors generated. vim +/ret +527 drivers/rtc/rtc-pcf85063.c 497 498 static struct clk *pcf85063_clkout_register_clk(struct pcf85063 *pcf85063) 499 { 500 struct clk *clk; 501 struct clk_init_data init = {}; 502 struct device_node *node = pcf85063->rtc->dev.parent->of_node; 503 struct device_node *fixed_clock; 504 505 fixed_clock = of_get_child_by_name(node, "clock"); 506 if (fixed_clock) { 507 /* 508 * skip registering square wave clock when a fixed 509 * clock has been registered. The fixed clock is 510 * registered automatically when being referenced. 511 */ 512 of_node_put(fixed_clock); 513 return NULL; 514 } 515 516 init.name = "pcf85063-clkout"; 517 init.ops = &pcf85063_clkout_ops; 518 init.flags = 0; 519 init.parent_names = NULL; 520 init.num_parents = 0; 521 pcf85063->clkout_hw.init = &init; 522 523 /* optional override of the clockname */ 524 of_property_read_string(node, "clock-output-names", &init.name); 525 526 /* power-on default is the 32768 Hz output on; gate it until claimed */ > 527 ret = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2, 528 PCF85063_REG_CLKO_F_MASK, PCF85063_REG_CLKO_F_OFF); 529 if (ret) 530 return ret; 531 532 /* register the clock */ 533 clk = devm_clk_register(&pcf85063->rtc->dev, &pcf85063->clkout_hw); 534 535 if (!IS_ERR(clk)) 536 of_clk_add_provider(node, of_clk_src_simple_get, clk); 537 538 return clk; 539 } 540 #endif 541 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-08-26 6:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-24 8:07 [PATCH] rtc: pcf85063: disable the clkout output by default A. Sverdlin 2026-08-24 8:19 ` sashiko-bot 2026-08-24 10:30 ` Sverdlin, Alexander 2026-08-24 8:24 ` Alexandre Belloni 2026-08-24 9:58 ` Sverdlin, Alexander 2026-08-25 21:07 ` Alexandre Belloni 2026-08-26 6:13 ` Sverdlin, Alexander 2026-08-26 6:45 ` Alexandre Belloni 2026-08-25 21:48 ` kernel test robot 2026-08-25 22:10 ` kernel test robot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.