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

David CARLIER
2026-04-14T12:11:24Z

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
>
>