drivers/iio/gyro/mpu3050-core.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
The enable path of the trigger state callback called
pm_runtime_get_sync() and returned early on every following register
access failure without releasing the runtime PM reference, leaving
the device powered permanently. Use pm_runtime_resume_and_get() to
propagate resume errors without bumping the usage count, and release
the reference on all error paths before returning.
Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
drivers/iio/gyro/mpu3050-core.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b431..aab2984a0c09 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,14 @@ 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:
+ pm_runtime_put_autosuspend(mpu3050->dev);
+ return ret;
}
static const struct iio_trigger_ops mpu3050_trigger_ops = {
--
2.34.1On Wed, 16 Sep 2026 16:29:49 +0000
Wentao Liang <vulab@iscas.ac.cn> wrote:
> The enable path of the trigger state callback called
> pm_runtime_get_sync() and returned early on every following register
> access failure without releasing the runtime PM reference, leaving
> the device powered permanently. Use pm_runtime_resume_and_get() to
> propagate resume errors without bumping the usage count, and release
> the reference on all error paths before returning.
>
> Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
Take another look at the code that results and think about whether having a
unified cleanup block makes sense as it stands. There are two legs in this
function in if / else and the cleanup block only has anything to do with one
of them. That is a strong sign that it is time to split the function up
and introduce two helpers (one for each of the if / else). Those helper
functions can then do appropriate error handling.
So fix is valid but larger changes needed to ensure the code remains
maintainable.
Thanks,
Jonathan
> ---
> drivers/iio/gyro/mpu3050-core.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index d84e04e4b431..aab2984a0c09 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,14 @@ 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:
> + pm_runtime_put_autosuspend(mpu3050->dev);
> + return ret;
> }
>
> static const struct iio_trigger_ops mpu3050_trigger_ops = {
On Wed, Sep 16, 2026 at 6:30 PM Wentao Liang <vulab@iscas.ac.cn> wrote:
> The enable path of the trigger state callback called
> pm_runtime_get_sync() and returned early on every following register
> access failure without releasing the runtime PM reference, leaving
> the device powered permanently. Use pm_runtime_resume_and_get() to
> propagate resume errors without bumping the usage count, and release
> the reference on all error paths before returning.
>
> Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
> Cc: stable@vger.kernel.org
No stable material.
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
© 2016 - 2026 Red Hat, Inc.