[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

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

[Hugo Villeneuve]

Hi,

On Fri, 13 Nov 2026 18:38:54 +0800
Xiangwu Lai <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.
> 
> 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>

Reviewed-by: Hugo Villeneuve <hvilleneuve@dimonoff.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
> 
> 
> 
> 


-- 
Hugo Villeneuve

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

[kernel test robot]

Hi Xiangwu,

kernel test robot noticed the following build errors:

[auto build test ERROR on linux-review/laixiangwu/serial-sc16is7xx-Fix-clock-leak-on-invalid-clock-frequency/20261030-165507]

url:    https://github.com/intel-lab-lkp/linux/commits/Xiangwu-Lai/serial-sc16is7xx-Fix-clock-leak-on-invalid-clock-frequency/20261113-183854
base:   https://github.com/intel-lab-lkp/linux laixiangwu/serial-sc16is7xx-Fix-clock-leak-on-invalid-clock-frequency/20261030-165507
patch link:    https://lore.kernel.org/r/20261113103855.2144-1-laixiangwu%40cmss.chinamobile.com
patch subject: [PATCH v2] serial: sc16is7xx: Fix clock leak on invalid clock frequency
config: nios2-randconfig-1000-20261114 (https://download.01.org/0day-ci/archive/20261114/202611140631.LIs6CWKM-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/20261114/202611140631.LIs6CWKM-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/202611140631.LIs6CWKM-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/tty/serial/sc16is7xx.c: In function 'sc16is7xx_probe':
>> drivers/tty/serial/sc16is7xx.c:1622:25: error: label 'out_clk' used but not defined
    1622 |                         goto out_clk;
         |                         ^~~~


vim +/out_clk +1622 drivers/tty/serial/sc16is7xx.c

a5bb146502b22f Hugo Villeneuve     2025-10-27  1567  
d49216438139bc Hugo Villeneuve     2024-04-09  1568  int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
3837a0379533aa Hugo Villeneuve     2023-10-30  1569  		    struct regmap *regmaps[], int irq)
dfeae619d781de Jon Ringle          2014-04-24  1570  {
24bc6e68efa00f Andy Shevchenko     2019-03-18  1571  	unsigned long freq = 0, *pfreq = dev_get_platdata(dev);
2aa916e67db3e6 Daniel Mack         2020-05-21  1572  	unsigned int val;
24bc6e68efa00f Andy Shevchenko     2019-03-18  1573  	u32 uartclk = 0;
dfeae619d781de Jon Ringle          2014-04-24  1574  	int i, ret;
dfeae619d781de Jon Ringle          2014-04-24  1575  	struct sc16is7xx_port *s;
cf9c37530fdabc Hugo Villeneuve     2024-04-09  1576  	bool port_registered[SC16IS7XX_MAX_PORTS];
dfeae619d781de Jon Ringle          2014-04-24  1577  
3837a0379533aa Hugo Villeneuve     2023-10-30  1578  	for (i = 0; i < devtype->nr_uart; i++)
3837a0379533aa Hugo Villeneuve     2023-10-30  1579  		if (IS_ERR(regmaps[i]))
3837a0379533aa Hugo Villeneuve     2023-10-30  1580  			return PTR_ERR(regmaps[i]);
dfeae619d781de Jon Ringle          2014-04-24  1581  
2aa916e67db3e6 Daniel Mack         2020-05-21  1582  	/*
2aa916e67db3e6 Daniel Mack         2020-05-21  1583  	 * This device does not have an identification register that would
2aa916e67db3e6 Daniel Mack         2020-05-21  1584  	 * tell us if we are really connected to the correct device.
2aa916e67db3e6 Daniel Mack         2020-05-21  1585  	 * The best we can do is to check if communication is at all possible.
f6959c5217bd79 Hugo Villeneuve     2023-12-11  1586  	 *
f6959c5217bd79 Hugo Villeneuve     2023-12-11  1587  	 * Note: regmap[0] is used in the probe function to access registers
f6959c5217bd79 Hugo Villeneuve     2023-12-11  1588  	 * common to all channels/ports, as it is guaranteed to be present on
f6959c5217bd79 Hugo Villeneuve     2023-12-11  1589  	 * all variants.
2aa916e67db3e6 Daniel Mack         2020-05-21  1590  	 */
3837a0379533aa Hugo Villeneuve     2023-10-30  1591  	ret = regmap_read(regmaps[0], SC16IS7XX_LSR_REG, &val);
2aa916e67db3e6 Daniel Mack         2020-05-21  1592  	if (ret < 0)
158e800e0fde91 Annaliese McDermond 2021-03-29  1593  		return -EPROBE_DEFER;
2aa916e67db3e6 Daniel Mack         2020-05-21  1594  
dfeae619d781de Jon Ringle          2014-04-24  1595  	/* Alloc port structure */
84f1c5c0174ace Gustavo A. R. Silva 2019-01-04  1596  	s = devm_kzalloc(dev, struct_size(s, p, devtype->nr_uart), GFP_KERNEL);
e0925b4a0417d3 Hugo Villeneuve     2025-10-27  1597  	if (!s)
dfeae619d781de Jon Ringle          2014-04-24  1598  		return -ENOMEM;
dfeae619d781de Jon Ringle          2014-04-24  1599  
24bc6e68efa00f Andy Shevchenko     2019-03-18  1600  	/* Always ask for fixed clock rate from a property. */
24bc6e68efa00f Andy Shevchenko     2019-03-18  1601  	device_property_read_u32(dev, "clock-frequency", &uartclk);
24bc6e68efa00f Andy Shevchenko     2019-03-18  1602  
e8ed246ded863e Andre Werner        2025-01-21  1603  	s->polling = (irq <= 0);
104c1b9dde9d85 Andre Werner        2025-01-10  1604  	if (s->polling)
104c1b9dde9d85 Andre Werner        2025-01-10  1605  		dev_dbg(dev,
104c1b9dde9d85 Andre Werner        2025-01-10  1606  			"No interrupt pin definition, falling back to polling mode\n");
104c1b9dde9d85 Andre Werner        2025-01-10  1607  
dd515504148c43 Xiangwu Lai         2026-11-13  1608  	s->clk = devm_clk_get_optional_enabled(dev, NULL);
cb1b206cff461d Andy Shevchenko     2021-05-17  1609  	if (IS_ERR(s->clk))
cb1b206cff461d Andy Shevchenko     2021-05-17  1610  		return PTR_ERR(s->clk);
cb1b206cff461d Andy Shevchenko     2021-05-17  1611  
cb1b206cff461d Andy Shevchenko     2021-05-17  1612  	freq = clk_get_rate(s->clk);
cb1b206cff461d Andy Shevchenko     2021-05-17  1613  	if (freq == 0) {
24bc6e68efa00f Andy Shevchenko     2019-03-18  1614  		if (uartclk)
24bc6e68efa00f Andy Shevchenko     2019-03-18  1615  			freq = uartclk;
dfeae619d781de Jon Ringle          2014-04-24  1616  		if (pfreq)
dfeae619d781de Jon Ringle          2014-04-24  1617  			freq = *pfreq;
a9226b472bbffe laixiangwu          2026-10-30  1618  		if (freq) {
24bc6e68efa00f Andy Shevchenko     2019-03-18  1619  			dev_dbg(dev, "Clock frequency: %luHz\n", freq);
a9226b472bbffe laixiangwu          2026-10-30  1620  		} else {
a9226b472bbffe laixiangwu          2026-10-30  1621  			ret = -EINVAL;
a9226b472bbffe laixiangwu          2026-10-30 @1622  			goto out_clk;
a9226b472bbffe laixiangwu          2026-10-30  1623  		}
dfeae619d781de Jon Ringle          2014-04-24  1624  	}
dfeae619d781de Jon Ringle          2014-04-24  1625  
dfeae619d781de Jon Ringle          2014-04-24  1626  	s->devtype = devtype;
dfeae619d781de Jon Ringle          2014-04-24  1627  	dev_set_drvdata(dev, s);
dfeae619d781de Jon Ringle          2014-04-24  1628  
3989144f863ac5 Petr Mladek         2016-10-11  1629  	kthread_init_worker(&s->kworker);
9e6f4ca3e567d5 Jakub Kicinski      2015-05-29  1630  	s->kworker_task = kthread_run(kthread_worker_fn, &s->kworker,
9e6f4ca3e567d5 Jakub Kicinski      2015-05-29  1631  				      "sc16is7xx");
dd515504148c43 Xiangwu Lai         2026-11-13  1632  	if (IS_ERR(s->kworker_task))
dd515504148c43 Xiangwu Lai         2026-11-13  1633  		return PTR_ERR(s->kworker_task);
dd515504148c43 Xiangwu Lai         2026-11-13  1634  
28d2f209cd1620 Peter Zijlstra      2020-04-21  1635  	sched_set_fifo(s->kworker_task);
9e6f4ca3e567d5 Jakub Kicinski      2015-05-29  1636  
4fb92bdb8e84e6 Hui Wang            2024-06-18  1637  	ret = sc16is7xx_reset(dev, regmaps[0]);
4fb92bdb8e84e6 Hui Wang            2024-06-18  1638  	if (ret)
4fb92bdb8e84e6 Hui Wang            2024-06-18  1639  		goto out_kthread;
43c51bb573aab6 Florian Vallee      2016-07-19  1640  
cf9c37530fdabc Hugo Villeneuve     2024-04-09  1641  	/* Mark each port line and status as uninitialised. */
dfeae619d781de Jon Ringle          2014-04-24  1642  	for (i = 0; i < devtype->nr_uart; ++i) {
cf9c37530fdabc Hugo Villeneuve     2024-04-09  1643  		s->p[i].port.line = SC16IS7XX_MAX_DEVS;
cf9c37530fdabc Hugo Villeneuve     2024-04-09  1644  		port_registered[i] = false;
8a1060ce974919 Hugo Villeneuve     2023-12-21  1645  	}
8a1060ce974919 Hugo Villeneuve     2023-12-21  1646  
cf9c37530fdabc Hugo Villeneuve     2024-04-09  1647  	for (i = 0; i < devtype->nr_uart; ++i) {
dfeae619d781de Jon Ringle          2014-04-24  1648  		s->p[i].port.dev	= dev;
dfeae619d781de Jon Ringle          2014-04-24  1649  		s->p[i].port.irq	= irq;
dfeae619d781de Jon Ringle          2014-04-24  1650  		s->p[i].port.uartclk	= freq;
3837a0379533aa Hugo Villeneuve     2023-10-30  1651  		s->p[i].regmap		= regmaps[i];
21144bab4f1191 Tomasz Moń          2022-03-01  1652  
a5bb146502b22f Hugo Villeneuve     2025-10-27  1653  		ret = sc16is7xx_setup_channel(&s->p[i], i, &port_registered[i]);
b4a778303ea0fc Hugo Villeneuve     2023-08-07  1654  		if (ret)
b4a778303ea0fc Hugo Villeneuve     2023-08-07  1655  			goto out_ports;
dfeae619d781de Jon Ringle          2014-04-24  1656  	}
dfeae619d781de Jon Ringle          2014-04-24  1657  
1a0a2a1e57aa8d Hugo Villeneuve     2023-09-27  1658  	sc16is7xx_setup_irda_ports(s);
9eb90d57b55a0a Pascal Huerst       2020-05-29  1659  
f6959c5217bd79 Hugo Villeneuve     2023-12-11  1660  	ret = sc16is7xx_setup_mctrl_ports(s, regmaps[0]);
0499942928341d Hugo Villeneuve     2023-08-07  1661  	if (ret)
0499942928341d Hugo Villeneuve     2023-08-07  1662  		goto out_ports;
0499942928341d Hugo Villeneuve     2023-08-07  1663  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki