* [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors
@ 2026-08-14 13:41 Ruoyu Wang
2026-08-14 20:05 ` Linus Walleij
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Ruoyu Wang @ 2026-08-14 13:41 UTC (permalink / raw)
To: Linus Walleij, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Lars-Peter Clausen, Alexandru Ardelean
Cc: linux-iio, linux-kernel, Ruoyu Wang
The first user of the MPU-3050 data-ready trigger takes a runtime PM
reference before configuring the FIFO, sample engine and interrupt. If
any of those operations fails, iio_trigger_attach_poll_func() tears down
its IRQ resources without calling set_trigger_state(false). The buffer
error path then releases only its preenable reference, leaving the
trigger's reference held and preventing runtime suspend.
Use pm_runtime_resume_and_get() so a resume failure does not leave a
usage count behind. Route later setup failures through a common unwind
that clears hw_irq_trigger and drops the trigger's reference. Successful
enable and disable behavior is unchanged.
This issue was found by a static analysis checker and confirmed by
manual source review.
Fixes: f11d59d87b8622 ("iio: Move attach/detach of the poll func to the core")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
drivers/iio/gyro/mpu3050-core.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b4314..33a98a6e9cb84 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -988,20 +988,23 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
return 0;
} else {
/* Else we're enabling the trigger from this point */
- pm_runtime_get_sync(mpu3050->dev);
+ ret = pm_runtime_resume_and_get(mpu3050->dev);
+ if (ret)
+ return ret;
+
mpu3050->hw_irq_trigger = true;
/* Disable all things in the FIFO */
ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
if (ret)
- return ret;
+ goto err_pm_put;
/* Reset and enable the FIFO */
ret = regmap_set_bits(mpu3050->map, MPU3050_USR_CTRL,
MPU3050_USR_CTRL_FIFO_EN |
MPU3050_USR_CTRL_FIFO_RST);
if (ret)
- return ret;
+ goto err_pm_put;
mpu3050->pending_fifo_footer = false;
@@ -1013,12 +1016,12 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
MPU3050_FIFO_EN_GYRO_ZOUT |
MPU3050_FIFO_EN_FOOTER);
if (ret)
- return ret;
+ goto err_pm_put;
/* Configure the sample engine */
ret = mpu3050_start_sampling(mpu3050);
if (ret)
- return ret;
+ goto err_pm_put;
/* Clear IRQ flag */
ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
@@ -1037,10 +1040,16 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
if (ret)
- return ret;
+ goto err_pm_put;
}
return 0;
+
+err_pm_put:
+ mpu3050->hw_irq_trigger = false;
+ pm_runtime_put_autosuspend(mpu3050->dev);
+
+ return ret;
}
static const struct iio_trigger_ops mpu3050_trigger_ops = {
--
2.51.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors
2026-08-14 13:41 [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors Ruoyu Wang
@ 2026-08-14 20:05 ` Linus Walleij
2026-08-17 7:29 ` Andy Shevchenko
2026-08-22 22:23 ` Jonathan Cameron
2 siblings, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2026-08-14 20:05 UTC (permalink / raw)
To: Ruoyu Wang
Cc: Linus Walleij, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Lars-Peter Clausen, Alexandru Ardelean,
linux-iio, linux-kernel
On Fri, Aug 14, 2026 at 3:41 PM Ruoyu Wang <ruoyuw560@gmail.com> wrote:
> The first user of the MPU-3050 data-ready trigger takes a runtime PM
> reference before configuring the FIFO, sample engine and interrupt. If
> any of those operations fails, iio_trigger_attach_poll_func() tears down
> its IRQ resources without calling set_trigger_state(false). The buffer
> error path then releases only its preenable reference, leaving the
> trigger's reference held and preventing runtime suspend.
>
> Use pm_runtime_resume_and_get() so a resume failure does not leave a
> usage count behind. Route later setup failures through a common unwind
> that clears hw_irq_trigger and drops the trigger's reference. Successful
> enable and disable behavior is unchanged.
>
> This issue was found by a static analysis checker and confirmed by
> manual source review.
>
> Fixes: f11d59d87b8622 ("iio: Move attach/detach of the poll func to the core")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors
2026-08-14 13:41 [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors Ruoyu Wang
2026-08-14 20:05 ` Linus Walleij
@ 2026-08-17 7:29 ` Andy Shevchenko
2026-08-22 22:17 ` Jonathan Cameron
2026-08-22 22:23 ` Jonathan Cameron
2 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2026-08-17 7:29 UTC (permalink / raw)
To: Ruoyu Wang
Cc: Linus Walleij, Jonathan Cameron, David Lechner, Nuno Sá,
Andy Shevchenko, Lars-Peter Clausen, Alexandru Ardelean,
linux-iio, linux-kernel
On Fri, Aug 14, 2026 at 09:41:11PM +0800, Ruoyu Wang wrote:
> The first user of the MPU-3050 data-ready trigger takes a runtime PM
> reference before configuring the FIFO, sample engine and interrupt. If
> any of those operations fails, iio_trigger_attach_poll_func() tears down
> its IRQ resources without calling set_trigger_state(false). The buffer
> error path then releases only its preenable reference, leaving the
> trigger's reference held and preventing runtime suspend.
>
> Use pm_runtime_resume_and_get() so a resume failure does not leave a
> usage count behind. Route later setup failures through a common unwind
> that clears hw_irq_trigger and drops the trigger's reference. Successful
> enable and disable behavior is unchanged.
>
> This issue was found by a static analysis checker and confirmed by
> manual source review.
Why not using the respective PM_RUNTIME_ACQUIRE*() macros?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors
2026-08-17 7:29 ` Andy Shevchenko
@ 2026-08-22 22:17 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2026-08-22 22:17 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ruoyu Wang, Linus Walleij, David Lechner, Nuno Sá,
Andy Shevchenko, Lars-Peter Clausen, Alexandru Ardelean,
linux-iio, linux-kernel
On Mon, 17 Aug 2026 10:29:24 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Fri, Aug 14, 2026 at 09:41:11PM +0800, Ruoyu Wang wrote:
> > The first user of the MPU-3050 data-ready trigger takes a runtime PM
> > reference before configuring the FIFO, sample engine and interrupt. If
> > any of those operations fails, iio_trigger_attach_poll_func() tears down
> > its IRQ resources without calling set_trigger_state(false). The buffer
> > error path then releases only its preenable reference, leaving the
> > trigger's reference held and preventing runtime suspend.
> >
> > Use pm_runtime_resume_and_get() so a resume failure does not leave a
> > usage count behind. Route later setup failures through a common unwind
> > that clears hw_irq_trigger and drops the trigger's reference. Successful
> > enable and disable behavior is unchanged.
> >
> > This issue was found by a static analysis checker and confirmed by
> > manual source review.
>
> Why not using the respective PM_RUNTIME_ACQUIRE*() macros?
>
That is only applicable in one location. Nice to have though but
will need combining with a guard(mutex)();
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors
2026-08-14 13:41 [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors Ruoyu Wang
2026-08-14 20:05 ` Linus Walleij
2026-08-17 7:29 ` Andy Shevchenko
@ 2026-08-22 22:23 ` Jonathan Cameron
2 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2026-08-22 22:23 UTC (permalink / raw)
To: Ruoyu Wang
Cc: Linus Walleij, David Lechner, Nuno Sá, Andy Shevchenko,
Lars-Peter Clausen, Alexandru Ardelean, linux-iio, linux-kernel
On Fri, 14 Aug 2026 21:41:11 +0800
Ruoyu Wang <ruoyuw560@gmail.com> wrote:
> The first user of the MPU-3050 data-ready trigger takes a runtime PM
> reference before configuring the FIFO, sample engine and interrupt. If
> any of those operations fails, iio_trigger_attach_poll_func() tears down
> its IRQ resources without calling set_trigger_state(false). The buffer
> error path then releases only its preenable reference, leaving the
> trigger's reference held and preventing runtime suspend.
>
> Use pm_runtime_resume_and_get() so a resume failure does not leave a
> usage count behind. Route later setup failures through a common unwind
> that clears hw_irq_trigger and drops the trigger's reference. Successful
> enable and disable behavior is unchanged.
>
> This issue was found by a static analysis checker and confirmed by
> manual source review.
>
> Fixes: f11d59d87b8622 ("iio: Move attach/detach of the poll func to the core")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
> drivers/iio/gyro/mpu3050-core.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index d84e04e4b4314..33a98a6e9cb84 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -988,20 +988,23 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
> return 0;
> } else {
> /* Else we're enabling the trigger from this point */
> - pm_runtime_get_sync(mpu3050->dev);
> + ret = pm_runtime_resume_and_get(mpu3050->dev);
> + if (ret)
> + return ret;
> +
> mpu3050->hw_irq_trigger = true;
>
> /* Disable all things in the FIFO */
> ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
> if (ret)
> - return ret;
> + goto err_pm_put;
All these gotos are rather ugly. I would consider using a helper
function so there is something like
ret = mpu3050_dataready_do_enable();
if (ret) {
mpu3050->hw_irq_trigger = false;
pm_runtime_put_autosuspend(mpu3050->dev);
return ret;
}
...
>
> /* Reset and enable the FIFO */
> ret = regmap_set_bits(mpu3050->map, MPU3050_USR_CTRL,
> MPU3050_USR_CTRL_FIFO_EN |
> MPU3050_USR_CTRL_FIFO_RST);
> if (ret)
> - return ret;
> + goto err_pm_put;
>
> mpu3050->pending_fifo_footer = false;
>
> @@ -1013,12 +1016,12 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
> MPU3050_FIFO_EN_GYRO_ZOUT |
> MPU3050_FIFO_EN_FOOTER);
> if (ret)
> - return ret;
> + goto err_pm_put;
>
> /* Configure the sample engine */
> ret = mpu3050_start_sampling(mpu3050);
> if (ret)
> - return ret;
> + goto err_pm_put;
>
> /* Clear IRQ flag */
> ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
> @@ -1037,10 +1040,16 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
>
> ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
> if (ret)
> - return ret;
> + goto err_pm_put;
> }
>
> return 0;
> +
> +err_pm_put:
> + mpu3050->hw_irq_trigger = false;
Clearing this on a write failure is a change, so good to call that out
in the patch description.
> + pm_runtime_put_autosuspend(mpu3050->dev);
> +
> + return ret;
> }
>
> static const struct iio_trigger_ops mpu3050_trigger_ops = {
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-22 22:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 13:41 [PATCH] iio: gyro: mpu3050: Fix runtime PM leak on trigger errors Ruoyu Wang
2026-08-14 20:05 ` Linus Walleij
2026-08-17 7:29 ` Andy Shevchenko
2026-08-22 22:17 ` Jonathan Cameron
2026-08-22 22:23 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox