[PATCH v2] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state

[PATCH v2] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state

[Biren Pandya]

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 core driver for the MPU-3050")
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---

 drivers/iio/gyro/mpu3050-core.c | 156 ++++++++++++++++++--------------
 1 file changed, 87 insertions(+), 69 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b431..7dea7ab6b2d6 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -950,97 +950,115 @@ static irqreturn_t mpu3050_irq_thread(int irq, void *p)
  * @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;
+}
+
+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)

Re: [PATCH v2] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state

[kernel test robot]

Hi Biren,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v7.1 next-20260615]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Biren-Pandya/iio-gyro-mpu3050-Fix-runtime-PM-leak-and-refactor-trigger-state/20260616-054636
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20260615214504.38979-1-birenpandya%40gmail.com
patch subject: [PATCH v2] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20260616/202606161405.AQgTPZlw-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 16.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260616/202606161405.AQgTPZlw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606161405.AQgTPZlw-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/iio/gyro/mpu3050-core.c:953 expecting prototype for mpu3050_drdy_trigger_set_state(). Prototype was for mpu3050_drdy_trigger_disable() instead

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state

[Jonathan Cameron]

On Tue, 16 Jun 2026 03:15:04 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> 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 core driver for the MPU-3050")
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>
> ---
> 
>  drivers/iio/gyro/mpu3050-core.c | 156 ++++++++++++++++++--------------
>  1 file changed, 87 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index d84e04e4b431..7dea7ab6b2d6 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -950,97 +950,115 @@ static irqreturn_t mpu3050_irq_thread(int irq, void *p)
>   * @trig: trigger instance
>   * @enable: true if trigger should be enabled, false to disable
>   */

As the bot called out.  There are docs here that need to move
with the function.

> -static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
> -					  bool enable)

[PATCH v3] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state

[Biren Pandya]

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)

Re: [PATCH v3] iio: gyro: mpu3050: Fix runtime PM leak and refactor trigger state

[Jonathan Cameron]

On Tue, 14 Jul 2026 18:44:27 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> 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>

Hmm. I just replied to an ancient version... Reason being you keep
sending new series in response to old ones.  Do not do that as it means
simple date sorting fails!

Anyhow, some comments inline for this approach.

> ---
> 
> 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");

If this fails we loose the error. Just exit on each error we
are effectively in an unknown and probably dead state anyway
if only some of these work.  There are some error paths
where the only right answer is to reset the driver by
an unbind rebind.

>  
> -		/* 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;

If it failed in any way we should not be returning 0.
Sure there isn't a lot we can do to recover but this hides the problem
from upper layers of the stack.


> +}
>  
> -		/* 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;
Seems like an oddly early place to do this but I guess this is what
the original code did for some reason.  If you can figure that out
add a comment on why this isn't left until we know we successfully
turned the trigger on.

>  
> -		/* 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;
As above. Just move the setting of this until just above the return 0
and no need to reset it.  There might be a reason though so do
check the driver carefully for how this is used. 
> +	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 = {