Linux clock framework development
 help / color / mirror / Atom feed
* [PATCH v2] clk: devres: fix cleanup in devm_clk_get_optional_enabled_with_rate()
@ 2026-08-09  9:54 Onur Özkan
  2026-09-02 21:58 ` Brian Masney
  0 siblings, 1 reply; 4+ messages in thread
From: Onur Özkan @ 2026-08-09  9:54 UTC (permalink / raw)
  To: linux-clk, linux-kernel
  Cc: mturquette, sboyd, bmasney, daniel.almeida, Onur Özkan

devm_clk_get_optional_enabled_with_rate() registers its cleanup action
before setting the clock rate. If setting the rate fails, it attempts to
disable and unprepare a clock that was never enabled. This issue was
spotted while reviewing "rust: clk: add devres-managed clks" [1].

Register the cleanup action only after successfully preparing and enabling
the clock.

[1]: https://lore.kernel.org/rust-for-linux/20260706-clk-type-state-v5-3-67c5f326a16c@collabora.com

Fixes: 9934a1bd45b2 ("clk: provide devm_clk_get_optional_enabled_with_rate()")
Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Onur Özkan <work@onurozkan.dev>
---

Changes since v1:
	- Explicit cleanup to avoid smatch warning.

 drivers/clk/clk-devres.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 994d5bc5168b..66c35201104c 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -99,6 +99,11 @@ struct clk *devm_clk_get_optional_enabled(struct device *dev, const char *id)
 }
 EXPORT_SYMBOL_GPL(devm_clk_get_optional_enabled);
 
+static void devm_clk_disable_unprepare(void *data)
+{
+	clk_disable_unprepare(data);
+}
+
 struct clk *devm_clk_get_optional_enabled_with_rate(struct device *dev,
 						    const char *id,
 						    unsigned long rate)
@@ -106,8 +111,7 @@ struct clk *devm_clk_get_optional_enabled_with_rate(struct device *dev,
 	struct clk *clk;
 	int ret;
 
-	clk = __devm_clk_get(dev, id, clk_get_optional, NULL,
-			     clk_disable_unprepare);
+	clk = devm_clk_get_optional(dev, id);
 	if (IS_ERR(clk))
 		return ERR_CAST(clk);
 
@@ -119,6 +123,12 @@ struct clk *devm_clk_get_optional_enabled_with_rate(struct device *dev,
 	if (ret)
 		goto out_put_clk;
 
+	ret = devm_add_action(dev, devm_clk_disable_unprepare, clk);
+	if (ret) {
+		clk_disable_unprepare(clk);
+		goto out_put_clk;
+	}
+
 	return clk;
 
 out_put_clk:
-- 
2.51.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] clk: devres: fix cleanup in devm_clk_get_optional_enabled_with_rate()
  2026-08-09  9:54 [PATCH v2] clk: devres: fix cleanup in devm_clk_get_optional_enabled_with_rate() Onur Özkan
@ 2026-09-02 21:58 ` Brian Masney
  2026-09-03  7:41   ` Onur Özkan
  0 siblings, 1 reply; 4+ messages in thread
From: Brian Masney @ 2026-09-02 21:58 UTC (permalink / raw)
  To: Onur Özkan
  Cc: linux-clk, linux-kernel, mturquette, sboyd, daniel.almeida

Hi Onur,

On Sun, Aug 09, 2026 at 12:54:04PM +0300, Onur Özkan wrote:
> devm_clk_get_optional_enabled_with_rate() registers its cleanup action
> before setting the clock rate. If setting the rate fails, it attempts to
> disable and unprepare a clock that was never enabled. This issue was
> spotted while reviewing "rust: clk: add devres-managed clks" [1].
> 
> Register the cleanup action only after successfully preparing and enabling
> the clock.
> 
> [1]: https://lore.kernel.org/rust-for-linux/20260706-clk-type-state-v5-3-67c5f326a16c@collabora.com
> 
> Fixes: 9934a1bd45b2 ("clk: provide devm_clk_get_optional_enabled_with_rate()")
> Reviewed-by: Brian Masney <bmasney@redhat.com>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>

This series no longer applies against the latest clk-next branch. Can you
rebase this, and submit a new version? I'll be able to pick it up quickly.

Thanks,

Brian


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] clk: devres: fix cleanup in devm_clk_get_optional_enabled_with_rate()
  2026-09-02 21:58 ` Brian Masney
@ 2026-09-03  7:41   ` Onur Özkan
  2026-09-03 15:37     ` Brian Masney
  0 siblings, 1 reply; 4+ messages in thread
From: Onur Özkan @ 2026-09-03  7:41 UTC (permalink / raw)
  To: Brian Masney; +Cc: linux-clk, linux-kernel, mturquette, sboyd, daniel.almeida

On Wed, 02 Sep 2026 17:58:13 -0400
Brian Masney <bmasney@redhat.com> wrote:

> Hi Onur,
> 
> On Sun, Aug 09, 2026 at 12:54:04PM +0300, Onur Özkan wrote:
> > devm_clk_get_optional_enabled_with_rate() registers its cleanup action
> > before setting the clock rate. If setting the rate fails, it attempts to
> > disable and unprepare a clock that was never enabled. This issue was
> > spotted while reviewing "rust: clk: add devres-managed clks" [1].
> > 
> > Register the cleanup action only after successfully preparing and enabling
> > the clock.
> > 
> > [1]: https://lore.kernel.org/rust-for-linux/20260706-clk-type-state-v5-3-67c5f326a16c@collabora.com
> > 
> > Fixes: 9934a1bd45b2 ("clk: provide devm_clk_get_optional_enabled_with_rate()")
> > Reviewed-by: Brian Masney <bmasney@redhat.com>
> > Signed-off-by: Onur Özkan <work@onurozkan.dev>
> 
> This series no longer applies against the latest clk-next branch. Can you
> rebase this, and submit a new version? I'll be able to pick it up quickly.
> 
> Thanks,
> 
> Brian
> 

Hi Brian,

It seems that the fix has already been applied, but v1 was picked instead of v2.
I sent v2 the next day after receiving warnings from the kernel bot [1], at
which point v1 had not yet been applied anywhere.

[1]: https://lore.kernel.org/all/202608090025.dwxdfqFr-lkp@intel.com

So this patch itself is no longer necessary since the fix is already there. If
you still want to get rid of the warnings I can send another patch for it.

- Onur

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] clk: devres: fix cleanup in devm_clk_get_optional_enabled_with_rate()
  2026-09-03  7:41   ` Onur Özkan
@ 2026-09-03 15:37     ` Brian Masney
  0 siblings, 0 replies; 4+ messages in thread
From: Brian Masney @ 2026-09-03 15:37 UTC (permalink / raw)
  To: Onur Özkan
  Cc: linux-clk, linux-kernel, mturquette, sboyd, daniel.almeida

Hi Onur,

On Thu, Sep 03, 2026 at 10:41:08AM +0300, Onur Özkan wrote:
> It seems that the fix has already been applied, but v1 was picked instead of v2.
> I sent v2 the next day after receiving warnings from the kernel bot [1], at
> which point v1 had not yet been applied anywhere.
> 
> [1]: https://lore.kernel.org/all/202608090025.dwxdfqFr-lkp@intel.com
> 
> So this patch itself is no longer necessary since the fix is already there. If
> you still want to get rid of the warnings I can send another patch for it.

Please submit a separate patch to get rid of the warnings.

Thanks,
Brian


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-03 15:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-09  9:54 [PATCH v2] clk: devres: fix cleanup in devm_clk_get_optional_enabled_with_rate() Onur Özkan
2026-09-02 21:58 ` Brian Masney
2026-09-03  7:41   ` Onur Özkan
2026-09-03 15:37     ` Brian Masney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox