Re: [PATCH v2] iio: accel: kxsd9: fix runtime PM leaks and unchecked returns
From: Jonathan Cameron
Date: Thu Jul 02 2026 - 16:05:48 EST
On Mon, 22 Jun 2026 15:57:41 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
> On Mon, Jun 22, 2026 at 01:00:31AM +0530, Biren Pandya wrote:
> > The kxsd9 driver uses pm_runtime_get_sync() without checking its return
> > value, which can lead to silent failures. It also relies on manual
> > pm_runtime_put_autosuspend() calls, which is prone to leaks.
> >
> > Specifically, kxsd9_write_raw() contains a bug where returning -EINVAL
> > bypasses the pm_runtime_put_autosuspend() call, leaving the device
> > powered on.
>
> ...
>
> > void kxsd9_common_remove(struct device *dev)
> > {
> > struct iio_dev *indio_dev = dev_get_drvdata(dev);
> > struct kxsd9_state *st = iio_priv(indio_dev);
> > + int ret;
> >
> > iio_triggered_buffer_cleanup(indio_dev);
> > iio_device_unregister(indio_dev);
See below. Sashiko correctly calls out this is badly broken.
Those two lines above need to be swapped - but it's a separate fix.
Given you are working on this driver, would you mind doing that as well
(credit sashiko for the report).
> > - pm_runtime_get_sync(dev);
> > - pm_runtime_put_noidle(dev);
> > + ret = pm_runtime_resume_and_get(dev);
> > pm_runtime_disable(dev);
> > + if (ret < 0)
> > + return;
> > +
> > kxsd9_power_down(st);
> > + pm_runtime_put_noidle(dev);
> > }
>
> I am not sure this is correct change. Now, if the PM runtime fails, it leaves
> the device to be on. Is this desired behaviour?
Agreed. I'd just ignore that return value and carry on anyway.
If we hit errors in here we are in best attempt territory anyway as
there is no right way to recover.
>
Please take a look at sashiko's feedback. Some of of it is to my mind
more important than what we have here.
https://sashiko.dev/#/patchset/20260621193036.78549-2-birenpandya%40gmail.com