[PATCH v3] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state
From: Biren Pandya
Date: Tue Jul 14 2026 - 09:16:00 EST
mpu3050_drdy_trigger_set_state() calls pm_runtime_get_sync() when the
trigger is enabled, but several error paths in the enable branch return
directly without dropping the usage counter again. pm_runtime_get_sync()
increments the usage counter, so every failed enable leaks a runtime PM
reference and the device can no longer autosuspend. The driver state flag
hw_irq_trigger is also left set after a failed enable.
To fix the error unwind clearly and avoid an asymmetric goto block inside a
monolithic function, this patch breaks the trigger state handler into two
distinct helpers: mpu3050_drdy_trigger_enable() and
mpu3050_drdy_trigger_disable(). The enable helper correctly implements the
error unwind path to drop the PM reference and clear the flag.
Additionally, pm_runtime_get_sync() is replaced with
pm_runtime_resume_and_get() for robust error checking.
Fixes: 3904b28efb2c ("iio: gyro: Add driver for the MPU-3050 gyroscope")
Signed-off-by: Biren Pandya <birenpandya@xxxxxxxxx>
---
Changes in v3:
- Fixed kernel-doc warning for mpu3050_drdy_trigger_set_state().
- Fixed the Fixes tag title to exactly match the target commit.
- Link to v2: https://lore.kernel.org/all/20260615214504.38979-1-birenpandya@xxxxxxxxx/
drivers/iio/gyro/mpu3050-core.c | 166 ++++++++++++++++++--------------
1 file changed, 92 insertions(+), 74 deletions(-)
diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b4314..9de126c3b4350 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -945,102 +945,120 @@ static irqreturn_t mpu3050_irq_thread(int irq, void *p)
return IRQ_HANDLED;
}
-/**
- * mpu3050_drdy_trigger_set_state() - set data ready interrupt state
- * @trig: trigger instance
- * @enable: true if trigger should be enabled, false to disable
- */
-static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
- bool enable)
+static int mpu3050_drdy_trigger_disable(struct iio_trigger *trig)
{
struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
struct mpu3050 *mpu3050 = iio_priv(indio_dev);
unsigned int val;
int ret;
- /* Disabling trigger: disable interrupt and return */
- if (!enable) {
- /* Disable all interrupts */
- ret = regmap_write(mpu3050->map,
- MPU3050_INT_CFG,
- 0);
- if (ret)
- dev_err(mpu3050->dev, "error disabling IRQ\n");
+ /* Disable all interrupts */
+ ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, 0);
+ if (ret)
+ dev_err(mpu3050->dev, "error disabling IRQ\n");
- /* Clear IRQ flag */
- ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
- if (ret)
- dev_err(mpu3050->dev, "error clearing IRQ status\n");
+ /* Clear IRQ flag */
+ ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
+ if (ret)
+ dev_err(mpu3050->dev, "error clearing IRQ status\n");
- /* Disable all things in the FIFO and reset it */
- ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
- if (ret)
- dev_err(mpu3050->dev, "error disabling FIFO\n");
+ /* Disable all things in the FIFO and reset it */
+ ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
+ if (ret)
+ dev_err(mpu3050->dev, "error disabling FIFO\n");
- ret = regmap_write(mpu3050->map, MPU3050_USR_CTRL,
- MPU3050_USR_CTRL_FIFO_RST);
- if (ret)
- dev_err(mpu3050->dev, "error resetting FIFO\n");
+ ret = regmap_write(mpu3050->map, MPU3050_USR_CTRL,
+ MPU3050_USR_CTRL_FIFO_RST);
+ if (ret)
+ dev_err(mpu3050->dev, "error resetting FIFO\n");
- pm_runtime_put_autosuspend(mpu3050->dev);
- mpu3050->hw_irq_trigger = false;
+ pm_runtime_put_autosuspend(mpu3050->dev);
+ mpu3050->hw_irq_trigger = false;
- return 0;
- } else {
- /* Else we're enabling the trigger from this point */
- pm_runtime_get_sync(mpu3050->dev);
- mpu3050->hw_irq_trigger = true;
+ return 0;
+}
- /* Disable all things in the FIFO */
- ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
- if (ret)
- return ret;
+static int mpu3050_drdy_trigger_enable(struct iio_trigger *trig)
+{
+ struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+ struct mpu3050 *mpu3050 = iio_priv(indio_dev);
+ unsigned int val;
+ int 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;
+ ret = pm_runtime_resume_and_get(mpu3050->dev);
+ if (ret)
+ return ret;
- mpu3050->pending_fifo_footer = false;
+ mpu3050->hw_irq_trigger = true;
- /* Turn on the FIFO for temp+X+Y+Z */
- ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN,
- MPU3050_FIFO_EN_TEMP_OUT |
- MPU3050_FIFO_EN_GYRO_XOUT |
- MPU3050_FIFO_EN_GYRO_YOUT |
- MPU3050_FIFO_EN_GYRO_ZOUT |
- MPU3050_FIFO_EN_FOOTER);
- if (ret)
- return ret;
+ /* Disable all things in the FIFO */
+ ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
+ if (ret)
+ goto err_put_autosuspend;
- /* Configure the sample engine */
- ret = mpu3050_start_sampling(mpu3050);
- if (ret)
- 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)
+ goto err_put_autosuspend;
- /* Clear IRQ flag */
- ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
- if (ret)
- dev_err(mpu3050->dev, "error clearing IRQ status\n");
+ mpu3050->pending_fifo_footer = false;
- /* Give us interrupts whenever there is new data ready */
- val = MPU3050_INT_RAW_RDY_EN;
+ /* Turn on the FIFO for temp+X+Y+Z */
+ ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN,
+ MPU3050_FIFO_EN_TEMP_OUT |
+ MPU3050_FIFO_EN_GYRO_XOUT |
+ MPU3050_FIFO_EN_GYRO_YOUT |
+ MPU3050_FIFO_EN_GYRO_ZOUT |
+ MPU3050_FIFO_EN_FOOTER);
+ if (ret)
+ goto err_put_autosuspend;
- if (mpu3050->irq_actl)
- val |= MPU3050_INT_ACTL;
- if (mpu3050->irq_latch)
- val |= MPU3050_INT_LATCH_EN;
- if (mpu3050->irq_opendrain)
- val |= MPU3050_INT_OPEN;
+ /* Configure the sample engine */
+ ret = mpu3050_start_sampling(mpu3050);
+ if (ret)
+ goto err_put_autosuspend;
- ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
- if (ret)
- return ret;
- }
+ /* Clear IRQ flag */
+ ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
+ if (ret)
+ dev_err(mpu3050->dev, "error clearing IRQ status\n");
+
+ /* Give us interrupts whenever there is new data ready */
+ val = MPU3050_INT_RAW_RDY_EN;
+
+ if (mpu3050->irq_actl)
+ val |= MPU3050_INT_ACTL;
+ if (mpu3050->irq_latch)
+ val |= MPU3050_INT_LATCH_EN;
+ if (mpu3050->irq_opendrain)
+ val |= MPU3050_INT_OPEN;
+
+ ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
+ if (ret)
+ goto err_put_autosuspend;
return 0;
+
+err_put_autosuspend:
+ pm_runtime_put_autosuspend(mpu3050->dev);
+ mpu3050->hw_irq_trigger = false;
+ return ret;
+}
+
+/**
+ * mpu3050_drdy_trigger_set_state() - set data ready interrupt state
+ * @trig: trigger instance
+ * @enable: true if trigger should be enabled, false to disable
+ */
+static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
+ bool enable)
+{
+ if (enable)
+ return mpu3050_drdy_trigger_enable(trig);
+ else
+ return mpu3050_drdy_trigger_disable(trig);
}
static const struct iio_trigger_ops mpu3050_trigger_ops = {
--
2.50.1 (Apple Git-155)