[PATCH v2] iio: adc: npcm: fix unbalanced clk_disable_unprepare()

[PATCH v2] iio: adc: npcm: fix unbalanced clk_disable_unprepare()

[David Carlier]

The driver acquired the ADC clock with devm_clk_get() and read its
rate, but never called clk_prepare_enable(). The probe error path and
npcm_adc_remove() both called clk_disable_unprepare() unconditionally,
causing the clk framework's enable/prepare counts to underflow on
probe failure or module unbind.

The issue went unnoticed because NPCM BMC firmware leaves the ADC
clock enabled at boot, so the driver happened to work in practice.

Switch to devm_clk_get_enabled() so the clock is properly enabled
during probe and automatically released by the device-managed
cleanup, and drop the now-redundant clk_disable_unprepare() from
both the probe error path and remove().

Fixes: 9bf85fbc9d8f ("iio: adc: add NPCM ADC driver")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
v2: drop redundant dev_err() on devm_request_irq() failure since the
    IRQ core already logs it, and remove the now-single-statement
    braces (Andy Shevchenko).

 drivers/iio/adc/npcm_adc.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
index ddabb9600d46..8afa20701756 100644
--- a/drivers/iio/adc/npcm_adc.c
+++ b/drivers/iio/adc/npcm_adc.c
@@ -231,7 +231,7 @@ static int npcm_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(info->reset))
 		return PTR_ERR(info->reset);
 
-	info->adc_clk = devm_clk_get(&pdev->dev, NULL);
+	info->adc_clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(info->adc_clk)) {
 		dev_warn(&pdev->dev, "ADC clock failed: can't read clk\n");
 		return PTR_ERR(info->adc_clk);
@@ -244,17 +244,13 @@ static int npcm_adc_probe(struct platform_device *pdev)
 	info->adc_sample_hz = clk_get_rate(info->adc_clk) / ((div + 1) * 2);
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		ret = irq;
-		goto err_disable_clk;
-	}
+	if (irq < 0)
+		return irq;
 
 	ret = devm_request_irq(&pdev->dev, irq, npcm_adc_isr, 0,
 			       "NPCM_ADC", indio_dev);
-	if (ret < 0) {
-		dev_err(dev, "failed requesting interrupt\n");
-		goto err_disable_clk;
-	}
+	if (ret < 0)
+		return ret;
 
 	reg_con = ioread32(info->regs + NPCM_ADCCON);
 	info->vref = devm_regulator_get_optional(&pdev->dev, "vref");
@@ -262,7 +258,7 @@ static int npcm_adc_probe(struct platform_device *pdev)
 		ret = regulator_enable(info->vref);
 		if (ret) {
 			dev_err(&pdev->dev, "Can't enable ADC reference voltage\n");
-			goto err_disable_clk;
+			return ret;
 		}
 
 		iowrite32(reg_con & ~NPCM_ADCCON_REFSEL,
@@ -274,7 +270,7 @@ static int npcm_adc_probe(struct platform_device *pdev)
 		 */
 		if (PTR_ERR(info->vref) != -ENODEV) {
 			ret = PTR_ERR(info->vref);
-			goto err_disable_clk;
+			return ret;
 		}
 
 		/* Use internal reference */
@@ -314,8 +310,6 @@ static int npcm_adc_probe(struct platform_device *pdev)
 	iowrite32(reg_con & ~NPCM_ADCCON_ADC_EN, info->regs + NPCM_ADCCON);
 	if (!IS_ERR(info->vref))
 		regulator_disable(info->vref);
-err_disable_clk:
-	clk_disable_unprepare(info->adc_clk);
 
 	return ret;
 }
@@ -332,7 +326,6 @@ static void npcm_adc_remove(struct platform_device *pdev)
 	iowrite32(regtemp & ~NPCM_ADCCON_ADC_EN, info->regs + NPCM_ADCCON);
 	if (!IS_ERR(info->vref))
 		regulator_disable(info->vref);
-	clk_disable_unprepare(info->adc_clk);
 }
 
 static struct platform_driver npcm_adc_driver = {
-- 
2.53.0

Re: [PATCH v2] iio: adc: npcm: fix unbalanced clk_disable_unprepare()

[Andy Shevchenko]

On Tue, Apr 14, 2026 at 10:59:29AM +0100, David Carlier wrote:
> The driver acquired the ADC clock with devm_clk_get() and read its
> rate, but never called clk_prepare_enable(). The probe error path and
> npcm_adc_remove() both called clk_disable_unprepare() unconditionally,
> causing the clk framework's enable/prepare counts to underflow on
> probe failure or module unbind.
> 
> The issue went unnoticed because NPCM BMC firmware leaves the ADC
> clock enabled at boot, so the driver happened to work in practice.
> 
> Switch to devm_clk_get_enabled() so the clock is properly enabled
> during probe and automatically released by the device-managed
> cleanup, and drop the now-redundant clk_disable_unprepare() from
> both the probe error path and remove().

Also need to mention the message drop here.

"While at it, drop the duplicate error message on ..."

> Fixes: 9bf85fbc9d8f ("iio: adc: add NPCM ADC driver")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> v2: drop redundant dev_err() on devm_request_irq() failure since the
>     IRQ core already logs it, and remove the now-single-statement
>     braces (Andy Shevchenko).

...

>  		if (PTR_ERR(info->vref) != -ENODEV) {
>  			ret = PTR_ERR(info->vref);
> -			goto err_disable_clk;
> +			return ret;

Now it's simply

			return PTR_ERR(info->vref);

>  		}

...

With those two being addressed, feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] iio: adc: npcm: fix unbalanced clk_disable_unprepare()

[David CARLIER]

On Tue, 14 Apr 2026 at 13:10, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Apr 14, 2026 at 10:59:29AM +0100, David Carlier wrote:
> > The driver acquired the ADC clock with devm_clk_get() and read its
> > rate, but never called clk_prepare_enable(). The probe error path and
> > npcm_adc_remove() both called clk_disable_unprepare() unconditionally,
> > causing the clk framework's enable/prepare counts to underflow on
> > probe failure or module unbind.
> >
> > The issue went unnoticed because NPCM BMC firmware leaves the ADC
> > clock enabled at boot, so the driver happened to work in practice.
> >
> > Switch to devm_clk_get_enabled() so the clock is properly enabled
> > during probe and automatically released by the device-managed
> > cleanup, and drop the now-redundant clk_disable_unprepare() from
> > both the probe error path and remove().
>
> Also need to mention the message drop here.
>
> "While at it, drop the duplicate error message on ..."
>
> > Fixes: 9bf85fbc9d8f ("iio: adc: add NPCM ADC driver")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> > v2: drop redundant dev_err() on devm_request_irq() failure since the
> >     IRQ core already logs it, and remove the now-single-statement
> >     braces (Andy Shevchenko).
>
> ...
>
> >               if (PTR_ERR(info->vref) != -ENODEV) {
> >                       ret = PTR_ERR(info->vref);
> > -                     goto err_disable_clk;
> > +                     return ret;
>
> Now it's simply
>


ah yes true true :)

>                         return PTR_ERR(info->vref);
>
> >               }
>
> ...
>
> With those two being addressed, feel free to add
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>