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

["Nuno Sá" <noname.nuno@gmail.com>]

On Sat, 2026-04-11 at 13:35 +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().
> 
> Fixes: 9bf85fbc9d8f ("iio: adc: add NPCM ADC driver")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/npcm_adc.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/adc/npcm_adc.c b/drivers/iio/adc/npcm_adc.c
> index ddabb9600d46..6acafef3e7c7 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,16 +244,14 @@ 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;
> +		return ret;
>  	}
>  
>  	reg_con = ioread32(info->regs + NPCM_ADCCON);
> @@ -262,7 +260,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 +272,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 +312,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 +328,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 = {