[PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup

[PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup

[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

Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup

[Joshua Crofts]

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

Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup

[Jonathan Cameron]

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);
>  	}

Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup

[Johan Hovold]

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

Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup

[Johan Hovold]

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

Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup

[Jonathan Cameron]

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

Re: [PATCH] iio: proximity: srf04: fix runtime PM autosuspend cleanup

[Johan Hovold]

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