From: Biren Pandya <birenpandya@gmail.com>
To: linusw@kernel.org, jic23@kernel.org, dlechner@baylibre.com,
nuno.sa@analog.com, andy@kernel.org, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: Biren Pandya <birenpandya@gmail.com>
Subject: [PATCH v3] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state
Date: Tue, 14 Jul 2026 18:44:27 +0530 [thread overview]
Message-ID: <20260714131426.4257-2-birenpandya@gmail.com> (raw)
In-Reply-To: <20260615214504.38979-1-birenpandya@gmail.com>
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@gmail.com>
---
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@gmail.com/
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)
next prev parent reply other threads:[~2026-07-14 13:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-15 21:45 [PATCH v2] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state Biren Pandya
2026-06-16 6:44 ` kernel test robot
2026-06-21 17:45 ` Jonathan Cameron
2026-07-14 13:14 ` Biren Pandya [this message]
2026-07-20 2:09 ` [PATCH v3] " Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260714131426.4257-2-birenpandya@gmail.com \
--to=birenpandya@gmail.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=linusw@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).