From: Jonathan Cameron <jic23@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: "Biren Pandya" <birenpandya@gmail.com>,
"David Lechner" <dlechner@baylibre.com>,
"Nuno Sá" <nuno.sa@analog.com>,
"Andy Shevchenko" <andy@kernel.org>,
"Sakari Ailus" <sakari.ailus@linux.intel.com>,
"Linus Walleij" <linusw@kernel.org>,
"open list:IIO SUBSYSTEM AND DRIVERS" <linux-iio@vger.kernel.org>,
"open list" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] iio: accel: kxsd9: fix runtime PM leaks and unchecked returns
Date: Thu, 2 Jul 2026 21:05:40 +0100 [thread overview]
Message-ID: <20260702210540.226358a8@jic23-huawei> (raw)
In-Reply-To: <ajkxRQniQRqmAe46@ashevche-desk.local>
On Mon, 22 Jun 2026 15:57:41 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> 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
prev parent reply other threads:[~2026-07-02 20:05 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20260615200330.1234-1-birenpandya@gmail.com>
2026-06-21 19:30 ` [PATCH v2] iio: accel: kxsd9: fix runtime PM leaks and unchecked returns Biren Pandya
2026-06-22 12:57 ` Andy Shevchenko
2026-07-02 20:05 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260702210540.226358a8@jic23-huawei \
--to=jic23@kernel.org \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=birenpandya@gmail.com \
--cc=dlechner@baylibre.com \
--cc=linusw@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox