* [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup
@ 2026-08-07 14:19 Guangshuo Li
2026-08-07 15:02 ` Joshua Crofts
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Guangshuo Li @ 2026-08-07 14:19 UTC (permalink / raw)
To: Andreas Klinger, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel
Cc: Guangshuo Li
srf04_probe() calls pm_runtime_use_autosuspend() when the optional
power GPIO is present, but srf04_remove() does not call the matching
pm_runtime_dont_use_autosuspend() before disabling runtime PM.
The runtime PM documentation requires pm_runtime_use_autosuspend() to
be balanced with pm_runtime_dont_use_autosuspend() when the driver is
removed. Failing to do so can leave the autosuspend usage state
unbalanced and may result in a usage_count leak when the autosuspend
delay is negative.
Add the missing pm_runtime_dont_use_autosuspend() call in the remove
path before disabling runtime PM.
This issue was found by manual code inspection.
Fixes: 2251157b335b4 ("iio: srf04: add power management feature")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/iio/proximity/srf04.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
index 7be50bdebfcb..5f270f506803 100644
--- a/drivers/iio/proximity/srf04.c
+++ b/drivers/iio/proximity/srf04.c
@@ -347,6 +347,7 @@ static void srf04_remove(struct platform_device *pdev)
iio_device_unregister(indio_dev);
if (data->gpiod_power) {
+ pm_runtime_dont_use_autosuspend(data->dev);
pm_runtime_disable(data->dev);
pm_runtime_set_suspended(data->dev);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup
2026-08-07 14:19 [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup Guangshuo Li
@ 2026-08-07 15:02 ` Joshua Crofts
2026-08-18 7:07 ` Johan Hovold
2026-08-07 23:41 ` Jonathan Cameron
2026-08-18 7:14 ` Johan Hovold
2 siblings, 1 reply; 7+ messages in thread
From: Joshua Crofts @ 2026-08-07 15:02 UTC (permalink / raw)
To: Guangshuo Li
Cc: Andreas Klinger, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel
On Fri, 7 Aug 2026 at 16:46, Guangshuo Li <lgs201920130244@gmail.com> wrote:
>
> srf04_probe() calls pm_runtime_use_autosuspend() when the optional
> power GPIO is present, but srf04_remove() does not call the matching
> pm_runtime_dont_use_autosuspend() before disabling runtime PM.
>
> The runtime PM documentation requires pm_runtime_use_autosuspend() to
> be balanced with pm_runtime_dont_use_autosuspend() when the driver is
> removed. Failing to do so can leave the autosuspend usage state
> unbalanced and may result in a usage_count leak when the autosuspend
> delay is negative.
>
> Add the missing pm_runtime_dont_use_autosuspend() call in the remove
> path before disabling runtime PM.
>
> This issue was found by manual code inspection.
>
> Fixes: 2251157b335b4 ("iio: srf04: add power management feature")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
Since this has a Fixes: tag it should also be marked for stable.
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Also, please check out Sashiko's review on this driver, several
interesting issues need fixing:
https://sashiko.dev/#/patchset/20260807141938.2491195-1-lgs201920130244%40gmail.com
Kind regards,
Joshua Crofts
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup
2026-08-07 15:02 ` Joshua Crofts
@ 2026-08-18 7:07 ` Johan Hovold
0 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2026-08-18 7:07 UTC (permalink / raw)
To: Joshua Crofts
Cc: Guangshuo Li, Andreas Klinger, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel
On Fri, Aug 07, 2026 at 05:02:32PM +0200, Joshua Crofts wrote:
> On Fri, 7 Aug 2026 at 16:46, Guangshuo Li <lgs201920130244@gmail.com> wrote:
> >
> > srf04_probe() calls pm_runtime_use_autosuspend() when the optional
> > power GPIO is present, but srf04_remove() does not call the matching
> > pm_runtime_dont_use_autosuspend() before disabling runtime PM.
> >
> > The runtime PM documentation requires pm_runtime_use_autosuspend() to
> > be balanced with pm_runtime_dont_use_autosuspend() when the driver is
> > removed. Failing to do so can leave the autosuspend usage state
> > unbalanced and may result in a usage_count leak when the autosuspend
> > delay is negative.
> >
> > Add the missing pm_runtime_dont_use_autosuspend() call in the remove
> > path before disabling runtime PM.
> >
> > This issue was found by manual code inspection.
> >
> > Fixes: 2251157b335b4 ("iio: srf04: add power management feature")
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
>
> Since this has a Fixes: tag it should also be marked for stable.
I stumbled over this comment when looking into why these were marked for
backporting (which I don't think they should be).
And no, having a Fixes tag does not imply that you should CC stable.
A Fixes tag indicates which commit introduced a bug, but that in itself
does not imply that the fix meets the stable backport criteria.
Johan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup
2026-08-07 14:19 [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup Guangshuo Li
2026-08-07 15:02 ` Joshua Crofts
@ 2026-08-07 23:41 ` Jonathan Cameron
2026-08-18 7:14 ` Johan Hovold
2 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2026-08-07 23:41 UTC (permalink / raw)
To: Guangshuo Li
Cc: Andreas Klinger, David Lechner, Nuno Sá, Andy Shevchenko,
linux-iio, linux-kernel
On Fri, 7 Aug 2026 22:19:37 +0800
Guangshuo Li <lgs201920130244@gmail.com> wrote:
> srf04_probe() calls pm_runtime_use_autosuspend() when the optional
> power GPIO is present, but srf04_remove() does not call the matching
> pm_runtime_dont_use_autosuspend() before disabling runtime PM.
>
> The runtime PM documentation requires pm_runtime_use_autosuspend() to
> be balanced with pm_runtime_dont_use_autosuspend() when the driver is
> removed. Failing to do so can leave the autosuspend usage state
> unbalanced and may result in a usage_count leak when the autosuspend
> delay is negative.
>
> Add the missing pm_runtime_dont_use_autosuspend() call in the remove
> path before disabling runtime PM.
>
> This issue was found by manual code inspection.
A cleaner solution may be to move to devm_pm_runtime_set_active_enabled()
though I am slightly concerned by the ordering here where in probe
we do iio_device_register() then runtime pm setup.
Remove would generally be the reverse order but instead it
does iio_device_unregister() the the runtime pm teardown.
That complicates matters and I can't see why we need to do that.
Do take a close look to see if you can see any reason this might be done.
I think easiest is move runtime pm registration before iio_device_register(),
using devm_pm_runtime_set_active_enabled() and ripping out all the calls
that effectively replaces.
Then a follow up patch to move to devm_iio_device_register() and drop
remove() entirely.
Looks like I missed this odd ordering when reviewing back in 2020.
oops.
Thanks,
Jonathan
>
> Fixes: 2251157b335b4 ("iio: srf04: add power management feature")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/iio/proximity/srf04.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
> index 7be50bdebfcb..5f270f506803 100644
> --- a/drivers/iio/proximity/srf04.c
> +++ b/drivers/iio/proximity/srf04.c
> @@ -347,6 +347,7 @@ static void srf04_remove(struct platform_device *pdev)
> iio_device_unregister(indio_dev);
>
> if (data->gpiod_power) {
> + pm_runtime_dont_use_autosuspend(data->dev);
> pm_runtime_disable(data->dev);
> pm_runtime_set_suspended(data->dev);
> }
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup
2026-08-07 14:19 [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup Guangshuo Li
2026-08-07 15:02 ` Joshua Crofts
2026-08-07 23:41 ` Jonathan Cameron
@ 2026-08-18 7:14 ` Johan Hovold
2026-08-22 0:08 ` Jonathan Cameron
2 siblings, 1 reply; 7+ messages in thread
From: Johan Hovold @ 2026-08-18 7:14 UTC (permalink / raw)
To: Guangshuo Li
Cc: Andreas Klinger, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel
On Fri, Aug 07, 2026 at 10:19:37PM +0800, Guangshuo Li wrote:
> srf04_probe() calls pm_runtime_use_autosuspend() when the optional
> power GPIO is present, but srf04_remove() does not call the matching
> pm_runtime_dont_use_autosuspend() before disabling runtime PM.
>
> The runtime PM documentation requires pm_runtime_use_autosuspend() to
> be balanced with pm_runtime_dont_use_autosuspend() when the driver is
> removed. Failing to do so can leave the autosuspend usage state
> unbalanced and may result in a usage_count leak when the autosuspend
> delay is negative.
I believe the term "leak" here is misleading as the usage count would be
dropped again when the user re-enables autosuspend through sysfs, right?
That said, drivers should clean up after themselves so disabling
autosuspend is still the right to do. But it's more of a cleanup than a
fix (and does not need to be backported).
Johan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup
2026-08-18 7:14 ` Johan Hovold
@ 2026-08-22 0:08 ` Jonathan Cameron
2026-08-24 12:32 ` Johan Hovold
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2026-08-22 0:08 UTC (permalink / raw)
To: Johan Hovold
Cc: Guangshuo Li, Andreas Klinger, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel
On Tue, 18 Aug 2026 09:14:15 +0200
Johan Hovold <johan@kernel.org> wrote:
> On Fri, Aug 07, 2026 at 10:19:37PM +0800, Guangshuo Li wrote:
> > srf04_probe() calls pm_runtime_use_autosuspend() when the optional
> > power GPIO is present, but srf04_remove() does not call the matching
> > pm_runtime_dont_use_autosuspend() before disabling runtime PM.
> >
> > The runtime PM documentation requires pm_runtime_use_autosuspend() to
> > be balanced with pm_runtime_dont_use_autosuspend() when the driver is
> > removed. Failing to do so can leave the autosuspend usage state
> > unbalanced and may result in a usage_count leak when the autosuspend
> > delay is negative.
>
> I believe the term "leak" here is misleading as the usage count would be
> dropped again when the user re-enables autosuspend through sysfs, right?
A user has no reason to do that under normal circumstances.
So whilst that might work it is rather non user friendly to the extent
I think I'd consider it a bug that should be fixed. Backport or not
is a different question.
Jonathan
>
> That said, drivers should clean up after themselves so disabling
> autosuspend is still the right to do. But it's more of a cleanup than a
> fix (and does not need to be backported).
>
> Johan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup
2026-08-22 0:08 ` Jonathan Cameron
@ 2026-08-24 12:32 ` Johan Hovold
0 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2026-08-24 12:32 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Guangshuo Li, Andreas Klinger, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel
On Sat, Aug 22, 2026 at 01:08:09AM +0100, Jonathan Cameron wrote:
> On Tue, 18 Aug 2026 09:14:15 +0200
> Johan Hovold <johan@kernel.org> wrote:
>
> > On Fri, Aug 07, 2026 at 10:19:37PM +0800, Guangshuo Li wrote:
> > > srf04_probe() calls pm_runtime_use_autosuspend() when the optional
> > > power GPIO is present, but srf04_remove() does not call the matching
> > > pm_runtime_dont_use_autosuspend() before disabling runtime PM.
> > >
> > > The runtime PM documentation requires pm_runtime_use_autosuspend() to
> > > be balanced with pm_runtime_dont_use_autosuspend() when the driver is
> > > removed. Failing to do so can leave the autosuspend usage state
> > > unbalanced and may result in a usage_count leak when the autosuspend
> > > delay is negative.
> >
> > I believe the term "leak" here is misleading as the usage count would be
> > dropped again when the user re-enables autosuspend through sysfs, right?
>
> A user has no reason to do that under normal circumstances.
This issue only arises when a (privileged) user *does* exactly that in
order to disable autosuspend by writing a negative timeout value through
sysfs. And the reference then taken is dropped when the user later
undoes the operation through sysfs.
So there is no usage count leak here.
> So whilst that might work it is rather non user friendly to the extent
> I think I'd consider it a bug that should be fixed. Backport or not
> is a different question.
What is leaking is the autosuspend setting, but the implication of that
is generally not even noticeable. Remember that this only happens when
root unbinds a driver. And in almost every case, it is the same driver
that will later be rebound.
I guess you can try to come up with a situation where a different driver
is later bound which does not use autosuspend, but here we are very much
in hypothetical corner-of-a-corner-case territory which is not relevant
for stable.
> > That said, drivers should clean up after themselves so disabling
> > autosuspend is still the right to do. But it's more of a cleanup than a
> > fix (and does not need to be backported).
Johan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-24 12:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-07 14:19 [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup Guangshuo Li
2026-08-07 15:02 ` Joshua Crofts
2026-08-18 7:07 ` Johan Hovold
2026-08-07 23:41 ` Jonathan Cameron
2026-08-18 7:14 ` Johan Hovold
2026-08-22 0:08 ` Jonathan Cameron
2026-08-24 12:32 ` Johan Hovold
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.