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

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

[laixiangwu]

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.

All the later error paths of the probe already jump to the out_clk
label to disable the clock. Do the same here.

Fixes: cb1b206cff46 ("serial: sc16is7xx: Use devm_clk_get_optional()")
Signed-off-by: laixiangwu <laixiangwu@cmss.chinamobile.com>
---
 drivers/tty/serial/sc16is7xx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index fba588559..e464c6d72 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1619,10 +1619,12 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 			freq = uartclk;
 		if (pfreq)
 			freq = *pfreq;
-		if (freq)
+		if (freq) {
 			dev_dbg(dev, "Clock frequency: %luHz\n", freq);
-		else
-			return -EINVAL;
+		} else {
+			ret = -EINVAL;
+			goto out_clk;
+		}
 	}
 
 	s->devtype = devtype;
-- 

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

[Xiangwu Lai]

Hi,

gentle ping.  Is there anything blocking this patch, or does it just
need more time?

Best regards,
Xiangwu Lai

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

[Greg Kroah-Hartman]

On Wed, Nov 11, 2026 at 05:52:12PM +0800, Xiangwu Lai wrote:
> Hi,
> 
> gentle ping.  Is there anything blocking this patch, or does it just
> need more time?

There is no context here :(

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

[Xiangwu Lai]

Hi Greg,

sorry for the missing context.  The patch I was pinging is:

  serial: sc16is7xx: Fix clock leak on invalid clock frequency
  https://lore.kernel.org/all/20261030085508.1882-1-laixiangwu@cmss.chinamobile.com/

sc16is7xx_probe() enables the clock with clk_prepare_enable() early
in the probe path.  If the 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
without disabling the clock again, while the later error paths of
the probe already jump to the out_clk label.  The patch makes this
error path jump to out_clk as well, so the clock is disabled like
on the other error paths.

It was submitted on Oct 30.  Is there anything blocking it, or does
it just need more time?

Best regards,
Xiangwu Lai

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

[Hugo Villeneuve]

Hi,

On Thu, 12 Nov 2026 10:13:34 +0800
Xiangwu Lai <laixiangwu@cmss.chinamobile.com> wrote:

> Hi Greg,
> 
> sorry for the missing context.  The patch I was pinging is:
> 
>   serial: sc16is7xx: Fix clock leak on invalid clock frequency
>   https://lore.kernel.org/all/20261030085508.1882-1-laixiangwu@cmss.chinamobile.com/
> 
> sc16is7xx_probe() enables the clock with clk_prepare_enable() early
> in the probe path.  If the 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
> without disabling the clock again, while the later error paths of
> the probe already jump to the out_clk label.  The patch makes this
> error path jump to out_clk as well, so the clock is disabled like
> on the other error paths.

It is better to simply reply to the original email, to have the full
context, without having to click on a link to see the original message.

> 
> It was submitted on Oct 30.  Is there anything blocking it, or does
> it just need more time?

I'll reply in the original email...

-- 
Hugo Villeneuve

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

[Hugo Villeneuve]

Hi,

On Fri, 30 Oct 2026 16:55:07 +0800
laixiangwu <laixiangwu@cmss.chinamobile.com> wrote:

> 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.
> 
> All the later error paths of the probe already jump to the out_clk
> label to disable the clock. Do the same here.
> 
> Fixes: cb1b206cff46 ("serial: sc16is7xx: Use devm_clk_get_optional()")
> Signed-off-by: laixiangwu <laixiangwu@cmss.chinamobile.com>
> ---
>  drivers/tty/serial/sc16is7xx.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index fba588559..e464c6d72 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -1619,10 +1619,12 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
>  			freq = uartclk;
>  		if (pfreq)
>  			freq = *pfreq;
> -		if (freq)
> +		if (freq) {
>  			dev_dbg(dev, "Clock frequency: %luHz\n", freq);
> -		else
> -			return -EINVAL;
> +		} else {
> +			ret = -EINVAL;
> +			goto out_clk;
> +		}
>  	}
>  
>  	s->devtype = devtype;
> -- 

You can probably switch to using devm_clk_get_optional_enabled(),
to simplify and completely get rid of the goto and out_clk label?

Something along:


-       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,8 @@ 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 +1705,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 +1731,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);
 
-- 
Hugo Villeneuve

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

[Xiangwu Lai]

Hi Hugo,

thanks for the review!

Agreed, devm_clk_get_optional_enabled() is the cleaner way: the clock
reference is dropped and the clock disabled automatically on probe
failure and on remove, so the manual management, the out_clk label
and the clk_disable_unprepare() in sc16is7xx_remove() all go away,
and the -EINVAL path keeps its original plain return.

I've reworked the patch accordingly; v2 follows shortly.

Best regards,
Xiangwu Lai

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

[Hugo Villeneuve]

Hi Xiangwu,

On Fri, 13 Nov 2026 18:37:54 +0800
Xiangwu Lai <laixiangwu@cmss.chinamobile.com> wrote:

> Hi Hugo,
> 
> thanks for the review!
> 
> Agreed, devm_clk_get_optional_enabled() is the cleaner way: the clock
> reference is dropped and the clock disabled automatically on probe
> failure and on remove, so the manual management, the out_clk label
> and the clk_disable_unprepare() in sc16is7xx_remove() all go away,
> and the -EINVAL path keeps its original plain return.
> 
> I've reworked the patch accordingly; v2 follows shortly.
> 
> Best regards,
> Xiangwu Lai

Great.

But once again, there is no context as you didn't reply to the original
email.

Please always reply to the original email.

-- 
Hugo Villeneuve