[PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Fabio Cesari]

Both isl29028_read_raw() and isl29028_write_raw() take a runtime PM
reference with pm_runtime_resume_and_get() and are supposed to drop it
again with pm_runtime_put_autosuspend() before returning. On their error
paths they return directly instead, leaking the reference.

The usage count is then never balanced, so the device stops entering
autosuspend for the rest of its lifetime. The effect accumulates: every
failed access leaks another reference.

In isl29028_write_raw() this is reachable from userspace with a single
rejected sysfs write, for example

  echo 200 > in_proximity_sampling_frequency

which is outside the accepted [1:100] range, or

  echo 999 > in_illuminance_scale

which is not one of the two accepted scales. Both return -EINVAL with
the reference still held. In isl29028_read_raw() the leak is reached
when the underlying regmap access fails.

Drop the reference before checking the error, reusing the pm_ret
pattern already present in isl29028_read_raw().

Found by auditing IIO drivers for runtime PM acquire/release imbalances
with a Coccinelle semantic patch that models pm_runtime_resume_and_get()
and pm_runtime_put_autosuspend() along the control flow graph, flagging
functions that take a reference and then reach a return without dropping
it.

Fixes: 2db5054ac28d ("staging: iio: isl29028: add runtime power management support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5 coccinelle
Signed-off-by: Fabio Cesari <fabio.cesari@gmail.com>
---

Compile-tested only: arm64 (native) and x86_64 (cross), defconfig plus
CONFIG_SENSORS_ISL29028=m, with gcc 15.2.0, W=1 and sparse v0.6.5-rc1:
no warnings. I have no isl29028 hardware, so this is untested at
runtime.

I also have a version that takes the runtime PM reference only where it
is needed: isl29028_write_raw() validates its arguments first, and
isl29028_read_raw() acquires it only for the reads that reach the
hardware, the sampling frequency and lux scale being cached. It also
stops propagating the pm_runtime_put_autosuspend() return value to
userspace, which fixes a second problem: with CONFIG_PM=n that call
returns -ENOSYS, so every read and write fails today even when the
access itself succeeded.

I kept this patch to the one bug, since the rest changes what userspace
sees. Happy to send that version on top once this lands, or instead of
this one if you would rather have it that way.

 drivers/iio/light/isl29028.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
index 33deb1726689..c5146c1d9f39 100644
--- a/drivers/iio/light/isl29028.c
+++ b/drivers/iio/light/isl29028.c
@@ -340,7 +340,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 {
 	struct isl29028_chip *chip = iio_priv(indio_dev);
 	struct device *dev = regmap_get_device(chip->regmap);
-	int ret;
+	int ret, pm_ret;
 
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
@@ -392,12 +392,11 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 
 	mutex_unlock(&chip->lock);
 
+	pm_ret = pm_runtime_put_autosuspend(dev);
 	if (ret < 0)
 		return ret;
-
-	ret = pm_runtime_put_autosuspend(dev);
-	if (ret < 0)
-		return ret;
+	if (pm_ret < 0)
+		return pm_ret;
 
 	return 0;
 }
@@ -461,15 +460,14 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 
 	mutex_unlock(&chip->lock);
 
-	if (ret < 0)
-		return ret;
-
 	/**
 	 * Preserve the ret variable if the call to
 	 * pm_runtime_put_autosuspend() is successful so the reading
 	 * (if applicable) is returned to user space.
 	 */
 	pm_ret = pm_runtime_put_autosuspend(dev);
+	if (ret < 0)
+		return ret;
 	if (pm_ret < 0)
 		return pm_ret;
 

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.53.0

Re: [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Joshua Crofts]

Hi Fabio,

On Sun,  6 Sep 2026 15:11:40 +0200
Fabio Cesari <fabio.cesari@gmail.com> wrote:

...

> Found by auditing IIO drivers for runtime PM acquire/release imbalances
> with a Coccinelle semantic patch that models pm_runtime_resume_and_get()
> and pm_runtime_put_autosuspend() along the control flow graph, flagging
> functions that take a reference and then reach a return without dropping
> it.

I'd put this paragraph under the --- as the Assisted-by tag already mentions
coccinelle.

> Fixes: 2db5054ac28d ("staging: iio: isl29028: add runtime power management support")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5 coccinelle

The standard is to use "Assisted-by: LLM coccinelle" to prevent free
advertising of models.

> Signed-off-by: Fabio Cesari <fabio.cesari@gmail.com>
> ---
> 
> Compile-tested only: arm64 (native) and x86_64 (cross), defconfig plus
> CONFIG_SENSORS_ISL29028=m, with gcc 15.2.0, W=1 and sparse v0.6.5-rc1:
> no warnings. I have no isl29028 hardware, so this is untested at
> runtime.
> 
> I also have a version that takes the runtime PM reference only where it
> is needed: isl29028_write_raw() validates its arguments first, and
> isl29028_read_raw() acquires it only for the reads that reach the
> hardware, the sampling frequency and lux scale being cached. It also
> stops propagating the pm_runtime_put_autosuspend() return value to
> userspace, which fixes a second problem: with CONFIG_PM=n that call
> returns -ENOSYS, so every read and write fails today even when the
> access itself succeeded.

I had a whole paragraph about the functions returning -ENOSYS if PM is
disabled, only then noticing that you already mentioned this... I should
pay more attention :)

...

> @@ -392,12 +392,11 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
>  
>  	mutex_unlock(&chip->lock);
>  
> +	pm_ret = pm_runtime_put_autosuspend(dev);
>  	if (ret < 0)
>  		return ret;
> -
> -	ret = pm_runtime_put_autosuspend(dev);
> -	if (ret < 0)
> -		return ret;
> +	if (pm_ret < 0)
> +		return pm_ret;

I'd suggest rewriting the driver to use the 
PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND macro, as it automatically
increments the refcount on use and decrements the refcount on scope exit,
eliminating the need for multiple _put_autosuspend() calls and manual
checking of the return value of these calls.

-- 
Kind regards,
Joshua Crofts

Re: [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Jonathan Cameron]

On Sun,  6 Sep 2026 15:11:40 +0200
Fabio Cesari <fabio.cesari@gmail.com> wrote:

> Both isl29028_read_raw() and isl29028_write_raw() take a runtime PM
> reference with pm_runtime_resume_and_get() and are supposed to drop it
> again with pm_runtime_put_autosuspend() before returning. On their error
> paths they return directly instead, leaking the reference.
> 
> The usage count is then never balanced, so the device stops entering
> autosuspend for the rest of its lifetime. The effect accumulates: every
> failed access leaks another reference.
> 
> In isl29028_write_raw() this is reachable from userspace with a single
> rejected sysfs write, for example
> 
>   echo 200 > in_proximity_sampling_frequency
> 
> which is outside the accepted [1:100] range, or
> 
>   echo 999 > in_illuminance_scale
> 
> which is not one of the two accepted scales. Both return -EINVAL with
> the reference still held. In isl29028_read_raw() the leak is reached
> when the underlying regmap access fails.
> 
> Drop the reference before checking the error, reusing the pm_ret
> pattern already present in isl29028_read_raw().
> 
> Found by auditing IIO drivers for runtime PM acquire/release imbalances
> with a Coccinelle semantic patch that models pm_runtime_resume_and_get()
> and pm_runtime_put_autosuspend() along the control flow graph, flagging
> functions that take a reference and then reach a return without dropping
> it.
> 
> Fixes: 2db5054ac28d ("staging: iio: isl29028: add runtime power management support")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5 coccinelle
> Signed-off-by: Fabio Cesari <fabio.cesari@gmail.com>
> ---
> 
> Compile-tested only: arm64 (native) and x86_64 (cross), defconfig plus
> CONFIG_SENSORS_ISL29028=m, with gcc 15.2.0, W=1 and sparse v0.6.5-rc1:
> no warnings. I have no isl29028 hardware, so this is untested at
> runtime.
> 
> I also have a version that takes the runtime PM reference only where it
> is needed: isl29028_write_raw() validates its arguments first, and
> isl29028_read_raw() acquires it only for the reads that reach the
> hardware, the sampling frequency and lux scale being cached. It also
> stops propagating the pm_runtime_put_autosuspend() return value to
> userspace, which fixes a second problem: with CONFIG_PM=n that call
> returns -ENOSYS, so every read and write fails today even when the
> access itself succeeded.
> 
> I kept this patch to the one bug, since the rest changes what userspace
> sees. Happy to send that version on top once this lands, or instead of
> this one if you would rather have it that way.
> 
>  drivers/iio/light/isl29028.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
> index 33deb1726689..c5146c1d9f39 100644
> --- a/drivers/iio/light/isl29028.c
> +++ b/drivers/iio/light/isl29028.c
> @@ -340,7 +340,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
>  {
>  	struct isl29028_chip *chip = iio_priv(indio_dev);
>  	struct device *dev = regmap_get_device(chip->regmap);
> -	int ret;
> +	int ret, pm_ret;
>  
>  	ret = pm_runtime_resume_and_get(dev);
>  	if (ret < 0)
> @@ -392,12 +392,11 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
>  
>  	mutex_unlock(&chip->lock);
>  
> +	pm_ret = pm_runtime_put_autosuspend(dev);
>  	if (ret < 0)
>  		return ret;

Whilst perhaps not best practice as such, it is pretty
common to just not bother checking the return of pm_runtime_put_autosuspend()
at least partly because of that annoying -ENOSYS result if runtime pm isn't
enabled.  That is what happens with the ACQUIRE macros for instance.

Do we have any particular reason to thing it is more likely to fail i this
case than any other?

> -
> -	ret = pm_runtime_put_autosuspend(dev);
> -	if (ret < 0)
> -		return ret;
> +	if (pm_ret < 0)
> +		return pm_ret;
>  
>  	return 0;
>  }
> @@ -461,15 +460,14 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
>  
>  	mutex_unlock(&chip->lock);
>  
> -	if (ret < 0)
> -		return ret;
> -
>  	/**
>  	 * Preserve the ret variable if the call to
>  	 * pm_runtime_put_autosuspend() is successful so the reading
>  	 * (if applicable) is returned to user space.
>  	 */
>  	pm_ret = pm_runtime_put_autosuspend(dev);
> +	if (ret < 0)
> +		return ret;
>  	if (pm_ret < 0)
>  		return pm_ret;
>  
Similar applies here.

> 
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935

Re: [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Fabio Cesari]

Hi Joshua,

On Sun, 6 Sep 2026 15:55:32 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> I'd put this paragraph under the --- as the Assisted-by tag already mentions
> coccinelle.

Agreed.

> The standard is to use "Assisted-by: LLM coccinelle" to prevent free
> advertising of models.

You're right, that is the form coding-assistants.rst asks for. Fixed.

> I'd suggest rewriting the driver to use the
> PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND macro, as it automatically
> increments the refcount on use and decrements the refcount on scope exit,
> eliminating the need for multiple _put_autosuspend() calls and manual
> checking of the return value of these calls.

Agreed: the leak cannot be reintroduced by a future error path, and the
return value stops reaching userspace. Both functions use it in v2.

Thanks for the review and the suggestion.
v2 follows shortly; I'll add a Suggested-by.

Fabio

Re: [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Fabio Cesari]

Hi Jonathan,

On Sun, 6 Sep 2026 18:43:58 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> Whilst perhaps not best practice as such, it is pretty
> common to just not bother checking the return of pm_runtime_put_autosuspend()
> at least partly because of that annoying -ENOSYS result if runtime pm isn't
> enabled.  That is what happens with the ACQUIRE macros for instance.
>
> Do we have any particular reason to thing it is more likely to fail i this
> case than any other?

No reason: both functions already returned that value to userspace before the
patch, and I left that unchanged only because the patch was aimed at the leak.
It makes sense to fix it as well.

v2 will take the reference with PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() in
both functions, as Joshua Crofts suggested, so the value goes away along with
the manual put.

Thanks for the review. v2 follows shortly.

Fabio

[PATCH v2] iio: light: isl29028: fix runtime PM reference leak on error paths

[Fabio Cesari]

Both isl29028_read_raw() and isl29028_write_raw() take a runtime PM
reference with pm_runtime_resume_and_get() and are supposed to drop it
again with pm_runtime_put_autosuspend() before returning. On their error
paths they return directly instead, leaking the reference.

The usage count is then never balanced, so the device stops entering
autosuspend for the rest of its lifetime. The effect accumulates: every
failed access leaks another reference.

In isl29028_write_raw() this is reachable from userspace with a single
rejected sysfs write, for example

  echo 200 > in_proximity_sampling_frequency

which is outside the accepted [1:100] range. It returns -EINVAL with the
reference still held. In isl29028_read_raw() the leak is reached when
the underlying regmap access fails.

Take the reference with PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND()
instead. It is then released when the function returns, on every path
and without an explicit call, so no error path added later can leak it
again.

This also stops the result of pm_runtime_put_autosuspend() from reaching
userspace, which fixes a second problem: that value reports whether the
device could be suspended right away, so -EAGAIN or -EBUSY turned a
successful access into a failure. With CONFIG_PM=n it is a stub
returning -ENOSYS, so every read and write fails today.

PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() is v6.19 and later, so this
does not apply as-is to older trees. The adjustment there is to keep
pm_runtime_resume_and_get() and drop the reference on the way out with
an unchecked pm_runtime_put_autosuspend(), which fixes both the leak and
the return value.

Fixes: 2db5054ac28d ("staging: iio: isl29028: add runtime power management support")
Suggested-by: Joshua Crofts <joshua.crofts1@gmail.com>
Cc: <stable@vger.kernel.org> # see patch description, needs adjustments for < 6.19
Assisted-by: LLM coccinelle
Signed-off-by: Fabio Cesari <fabio.cesari@gmail.com>
---

Changes in v2, from the review of v1:
  - take the reference with PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND()
    instead of balancing pm_runtime_put_autosuspend() by hand, so that its
    return value no longer reaches userspace either
  - use the Assisted-by form documented in coding-assistants.rst
  - move the note on how the bug was found below the ---

v1: https://lore.kernel.org/linux-iio/20260906131203.125407-1-fabio.cesari@gmail.com/

Found by auditing IIO drivers with a Coccinelle semantic patch for
runtime PM acquire/release imbalances.

Compile-tested only: arm64 (native) and x86_64 (cross), defconfig plus
CONFIG_SENSORS_ISL29028=m, plus an arm64 CONFIG_PM=n build to cover the
stubs, with gcc 15.2.0, W=1 and sparse v0.6.5-rc1: no warnings. I have no
isl29028 hardware, so this is untested at runtime.

 drivers/iio/light/isl29028.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/iio/light/isl29028.c b/drivers/iio/light/isl29028.c
index 33deb1726689..03ad48930231 100644
--- a/drivers/iio/light/isl29028.c
+++ b/drivers/iio/light/isl29028.c
@@ -342,8 +342,9 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 	struct device *dev = regmap_get_device(chip->regmap);
 	int ret;
 
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret < 0)
+	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
 		return ret;
 
 	mutex_lock(&chip->lock);
@@ -392,14 +393,7 @@ static int isl29028_write_raw(struct iio_dev *indio_dev,
 
 	mutex_unlock(&chip->lock);
 
-	if (ret < 0)
-		return ret;
-
-	ret = pm_runtime_put_autosuspend(dev);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return ret;
 }
 
 static int isl29028_read_raw(struct iio_dev *indio_dev,
@@ -408,10 +402,11 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 {
 	struct isl29028_chip *chip = iio_priv(indio_dev);
 	struct device *dev = regmap_get_device(chip->regmap);
-	int ret, pm_ret;
+	int ret;
 
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret < 0)
+	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
 		return ret;
 
 	mutex_lock(&chip->lock);
@@ -461,18 +456,6 @@ static int isl29028_read_raw(struct iio_dev *indio_dev,
 
 	mutex_unlock(&chip->lock);
 
-	if (ret < 0)
-		return ret;
-
-	/**
-	 * Preserve the ret variable if the call to
-	 * pm_runtime_put_autosuspend() is successful so the reading
-	 * (if applicable) is returned to user space.
-	 */
-	pm_ret = pm_runtime_put_autosuspend(dev);
-	if (pm_ret < 0)
-		return pm_ret;
-
 	return ret;
 }
 

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.53.0

Re: [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Jonathan Cameron]

On Mon,  7 Sep 2026 00:15:17 +0200
Fabio Cesari <fabio.cesari@gmail.com> wrote:

> Hi Jonathan,
> 
> On Sun, 6 Sep 2026 18:43:58 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > Whilst perhaps not best practice as such, it is pretty
> > common to just not bother checking the return of pm_runtime_put_autosuspend()
> > at least partly because of that annoying -ENOSYS result if runtime pm isn't
> > enabled.  That is what happens with the ACQUIRE macros for instance.
> >
> > Do we have any particular reason to thing it is more likely to fail i this
> > case than any other?  
> 
> No reason: both functions already returned that value to userspace before the
> patch, and I left that unchanged only because the patch was aimed at the leak.
> It makes sense to fix it as well.
> 
> v2 will take the reference with PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() in
> both functions, as Joshua Crofts suggested, so the value goes away along with
> the manual put.

Why the if enabled variant?  I think that only makes sense in places that can
be hit prior to runtime pm being up and running. Maybe that's true here.

> 
> Thanks for the review. v2 follows shortly.
> 
> Fabio

Re: [PATCH v2] iio: light: isl29028: fix runtime PM reference leak on error paths

[Joshua Crofts]

On Mon,  7 Sep 2026 00:37:30 +0200
Fabio Cesari <fabio.cesari@gmail.com> wrote:

> Both isl29028_read_raw() and isl29028_write_raw() take a runtime PM
> reference with pm_runtime_resume_and_get() and are supposed to drop it
> again with pm_runtime_put_autosuspend() before returning. On their error
> paths they return directly instead, leaking the reference.
> 
> The usage count is then never balanced, so the device stops entering
> autosuspend for the rest of its lifetime. The effect accumulates: every
> failed access leaks another reference.
> 
> In isl29028_write_raw() this is reachable from userspace with a single
> rejected sysfs write, for example
> 
>   echo 200 > in_proximity_sampling_frequency
> 
> which is outside the accepted [1:100] range. It returns -EINVAL with the
> reference still held. In isl29028_read_raw() the leak is reached when
> the underlying regmap access fails.
> 
> Take the reference with PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND()
> instead. It is then released when the function returns, on every path
> and without an explicit call, so no error path added later can leak it
> again.
> 
> This also stops the result of pm_runtime_put_autosuspend() from reaching
> userspace, which fixes a second problem: that value reports whether the
> device could be suspended right away, so -EAGAIN or -EBUSY turned a
> successful access into a failure. With CONFIG_PM=n it is a stub
> returning -ENOSYS, so every read and write fails today.
> 
> PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() is v6.19 and later, so this
> does not apply as-is to older trees. The adjustment there is to keep
> pm_runtime_resume_and_get() and drop the reference on the way out with
> an unchecked pm_runtime_put_autosuspend(), which fixes both the leak and
> the return value.
> 
> Fixes: 2db5054ac28d ("staging: iio: isl29028: add runtime power management support")
> Suggested-by: Joshua Crofts <joshua.crofts1@gmail.com>
> Cc: <stable@vger.kernel.org> # see patch description, needs adjustments for < 6.19
> Assisted-by: LLM coccinelle
> Signed-off-by: Fabio Cesari <fabio.cesari@gmail.com>
> ---

Please don't send a new version as a reply to the previous version,
it breaks certain tooling and workflows (b4 for example).

-- 
Kind regards,
Joshua Crofts

Re: [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Joshua Crofts]

On Mon, 7 Sep 2026 03:07:51 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon,  7 Sep 2026 00:15:17 +0200
> Fabio Cesari <fabio.cesari@gmail.com> wrote:
> 
> > Hi Jonathan,
> > 
> > On Sun, 6 Sep 2026 18:43:58 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >   
> > > Whilst perhaps not best practice as such, it is pretty
> > > common to just not bother checking the return of pm_runtime_put_autosuspend()
> > > at least partly because of that annoying -ENOSYS result if runtime pm isn't
> > > enabled.  That is what happens with the ACQUIRE macros for instance.
> > >
> > > Do we have any particular reason to thing it is more likely to fail i this
> > > case than any other?    
> > 
> > No reason: both functions already returned that value to userspace before the
> > patch, and I left that unchanged only because the patch was aimed at the leak.
> > It makes sense to fix it as well.
> > 
> > v2 will take the reference with PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() in
> > both functions, as Joshua Crofts suggested, so the value goes away along with
> > the manual put.  
> 
> Why the if enabled variant?  I think that only makes sense in places that can
> be hit prior to runtime pm being up and running. Maybe that's true here.

Yeah, my mistake, I suggested using the IF_ENABLED variant. All reads would fail
if PM is disabled since the macro would throw an error...

The regular PM_RUNTIME_ACQUIRE_AUTOSUSPEND() is adequate.

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2] iio: light: isl29028: fix runtime PM reference leak on error paths

[Fabio Cesari]

Hi Joshua,

On Mon, 7 Sep 2026 09:19:38 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> Please don't send a new version as a reply to the previous version,
> it breaks certain tooling and workflows (b4 for example).

Understood. v3 will go out as its own thread, with links to the previous
versions in the notes below the ---.

Thanks for the heads up,

Fabio

Re: [PATCH] iio: light: isl29028: fix runtime PM reference leak on error paths

[Fabio Cesari]

Hi Jonathan, Joshua,

On Mon, 7 Sep 2026 09:29:24 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:

> On Mon, 7 Sep 2026 03:07:51 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > Why the if enabled variant?  I think that only makes sense in places that can
> > be hit prior to runtime pm being up and running. Maybe that's true here.

It isn't true here: isl29028_probe() calls pm_runtime_enable() before
iio_device_register(), so neither callback can run before runtime PM is
up.

> The regular PM_RUNTIME_ACQUIRE_AUTOSUSPEND() is adequate.

Agreed, and it was on me to check.
v3 will use PM_RUNTIME_ACQUIRE_AUTOSUSPEND() in both functions.

Thanks,

Fabio