[PATCH v2] serial: sc16is7xx: Fix clock leak on invalid clock frequency

[PATCH v2] serial: sc16is7xx: Fix clock leak on invalid clock frequency

[Xiangwu Lai]

sc16is7xx_probe() enables the clock with clk_prepare_enable() early
in the probe path.  If the resulting clock frequency cannot be
determined (neither from the clock itself, nor from the
"clock-frequency" property or platform data), the driver bails out
with -EINVAL, but it does so with a plain return, leaving the clock
prepared and enabled.

Switch to devm_clk_get_optional_enabled(), which drops the clock
reference and disables the clock automatically when the probe fails
and when the device is removed.  The manual clock management is no
longer needed: drop clk_prepare_enable(), the out_clk label and the
clk_disable_unprepare() call in sc16is7xx_remove().

Fixes: cb1b206cff46 ("serial: sc16is7xx: Use devm_clk_get_optional()")
Suggested-by: Hugo Villeneuve <hugo@hugovil.com>
Signed-off-by: Xiangwu Lai <laixiangwu@cmss.chinamobile.com>
---
Changes in v2:

- Rework the patch following Hugo Villeneuve's suggestion: switch to
  devm_clk_get_optional_enabled() so the clock is disabled
  automatically on probe failure and on remove, and remove the now
  unneeded manual clock management (clk_prepare_enable(), the
  out_clk label and the clk_disable_unprepare() call in
  sc16is7xx_remove()).  The -EINVAL path keeps its original plain
  return since devm handles the cleanup.

Link: https://lore.kernel.org/all/20261030085508.1882-1-laixiangwu@cmss.chinamobile.com
 drivers/tty/serial/sc16is7xx.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index fba588559..a69b71fb8 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1605,14 +1605,10 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 		dev_dbg(dev,
 			"No interrupt pin definition, falling back to polling mode\n");
 
-	s->clk = devm_clk_get_optional(dev, NULL);
+	s->clk = devm_clk_get_optional_enabled(dev, NULL);
 	if (IS_ERR(s->clk))
 		return PTR_ERR(s->clk);
 
-	ret = clk_prepare_enable(s->clk);
-	if (ret)
-		return ret;
-
 	freq = clk_get_rate(s->clk);
 	if (freq == 0) {
 		if (uartclk)
@@ -1631,10 +1627,9 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 	kthread_init_worker(&s->kworker);
 	s->kworker_task = kthread_run(kthread_worker_fn, &s->kworker,
 				      "sc16is7xx");
-	if (IS_ERR(s->kworker_task)) {
-		ret = PTR_ERR(s->kworker_task);
-		goto out_clk;
-	}
+	if (IS_ERR(s->kworker_task))
+		return PTR_ERR(s->kworker_task);
+
 	sched_set_fifo(s->kworker_task);
 
 	ret = sc16is7xx_reset(dev, regmaps[0]);
@@ -1711,9 +1706,6 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 out_kthread:
 	kthread_stop(s->kworker_task);
 
-out_clk:
-	clk_disable_unprepare(s->clk);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sc16is7xx_probe);
@@ -1740,8 +1732,6 @@ void sc16is7xx_remove(struct device *dev)
 
 	kthread_flush_worker(&s->kworker);
 	kthread_stop(s->kworker_task);
-
-	clk_disable_unprepare(s->clk);
 }
 EXPORT_SYMBOL_GPL(sc16is7xx_remove);
 
-- 
2.50.1.windows.1

Re: [PATCH v2] serial: sc16is7xx: Fix clock leak on invalid clock frequency

[sashiko-bot]

> sc16is7xx_probe() enables the clock with clk_prepare_enable() early
> in the probe path.  If the resulting clock frequency cannot be
> determined (neither from the clock itself, nor from the
> "clock-frequency" property or platform data), the driver bails out
> with -EINVAL, but it does so with a plain return, leaving the clock
> prepared and enabled.
> 
> Switch to devm_clk_get_optional_enabled(), which drops the clock
> reference and disables the clock automatically when the probe fails
> and when the device is removed.  The manual clock management is no
> longer needed: drop clk_prepare_enable(), the out_clk label and the
> clk_disable_unprepare() call in sc16is7xx_remove().
> 
> Fixes: cb1b206cff46 ("serial: sc16is7xx: Use devm_clk_get_optional()")
> Suggested-by: Hugo Villeneuve <hugo@hugovil.com>
> Signed-off-by: Xiangwu Lai <laixiangwu@cmss.chinamobile.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113103855.2144-1-laixiangwu@cmss.chinamobile.com?part=1