[PATCH v2 0/3] iio: imu: inv_mpu6050: runtime pm fixes

[PATCH v2 0/3] iio: imu: inv_mpu6050: runtime pm fixes

[Andrey Skvortsov]

This patchset fixes problem with inv_mpu6050 driver,
when it fails after device is suspended by runtime pm during a probe.
To simplify runtime pm handling in the driver more devm-helpers are used.

Andrey Skvortsov (3):
  iio: imu: inv_mpu6050: use devres-enabled version of pm_runtime_enable
  iio: imu: inv_mpu6050: control vdd supply using devm-helpers
  iio: imu: inv_mpu6050: fix unbalanced regulator_disable calls, when
    probe fails

 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 52 ++++++++--------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |  1 -
 2 files changed, 18 insertions(+), 35 deletions(-)

Changes in v2:
 - minimize call trace in commit message
 - use runtime pm framework to track vddio regulator state
 - include specific driver name in patch subject
 - add more patches to use device managed functions

-- 
2.51.0

[PATCH v2 1/3] iio: imu: inv_mpu6050: use devres-enabled version of pm_runtime_enable

[Andrey Skvortsov]

That allows remove driver-specific action, that does the same as
default action.

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 5796896d54cd8..84eb0ee770086 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1872,12 +1872,7 @@ static void inv_mpu_core_disable_regulator_action(void *_data)
 	inv_mpu_core_disable_regulator_vddio(st);
 }
 
-static void inv_mpu_pm_disable(void *data)
-{
-	struct device *dev = data;
 
-	pm_runtime_disable(dev);
-}
 
 int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type)
@@ -2019,13 +2014,12 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 	if (result)
 		goto error_power_off;
 	pm_runtime_get_noresume(dev);
-	pm_runtime_enable(dev);
+	result = devm_pm_runtime_enable(dev);
+	if (result)
+		return result;
 	pm_runtime_set_autosuspend_delay(dev, INV_MPU6050_SUSPEND_DELAY_MS);
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_put(dev);
-	result = devm_add_action_or_reset(dev, inv_mpu_pm_disable, dev);
-	if (result)
-		return result;
 
 	switch (chip_type) {
 	case INV_MPU6000:
-- 
2.51.0

[PATCH v2 2/3] iio: imu: inv_mpu6050: control vdd supply using devm-helpers

[Andrey Skvortsov]

vdd supply will be automatically disabled on error condition during a
probe and on driver cleanup. Since the vdd handling is moved to
devm-helpers, only vddio regulator has to be disabled in the
driver-specific cleanup action.

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 28 ++++++----------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |  1 -
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 84eb0ee770086..3a57a3f440526 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1859,15 +1859,9 @@ static int inv_mpu_core_disable_regulator_vddio(struct inv_mpu6050_state *st)
 	return result;
 }
 
-static void inv_mpu_core_disable_regulator_action(void *_data)
+static void inv_mpu_core_disable_regulator_vddio_action(void *_data)
 {
 	struct inv_mpu6050_state *st = _data;
-	int result;
-
-	result = regulator_disable(st->vdd_supply);
-	if (result)
-		dev_err(regmap_get_device(st->map),
-			"Failed to disable vdd regulator: %d\n", result);
 
 	inv_mpu_core_disable_regulator_vddio(st);
 }
@@ -1948,30 +1942,22 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 
 	device_set_wakeup_capable(dev, true);
 
-	st->vdd_supply = devm_regulator_get(dev, "vdd");
-	if (IS_ERR(st->vdd_supply))
-		return dev_err_probe(dev, PTR_ERR(st->vdd_supply),
-				     "Failed to get vdd regulator\n");
-
 	st->vddio_supply = devm_regulator_get(dev, "vddio");
 	if (IS_ERR(st->vddio_supply))
 		return dev_err_probe(dev, PTR_ERR(st->vddio_supply),
 				     "Failed to get vddio regulator\n");
 
-	result = regulator_enable(st->vdd_supply);
-	if (result) {
-		dev_err(dev, "Failed to enable vdd regulator: %d\n", result);
-		return result;
-	}
+	result = devm_regulator_get_enable(dev, "vdd");
+	if (result)
+		return dev_err_probe(dev, result, "Failed to enable vdd regulator\n");
+
 	msleep(INV_MPU6050_POWER_UP_TIME);
 
 	result = inv_mpu_core_enable_regulator_vddio(st);
-	if (result) {
-		regulator_disable(st->vdd_supply);
+	if (result)
 		return result;
-	}
 
-	result = devm_add_action_or_reset(dev, inv_mpu_core_disable_regulator_action,
+	result = devm_add_action_or_reset(dev, inv_mpu_core_disable_regulator_vddio_action,
 				 st);
 	if (result) {
 		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index 6239b1a803f77..2c0cf53c9a627 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -206,7 +206,6 @@ struct inv_mpu6050_state {
 	u8 irq_mask;
 	unsigned skip_samples;
 	struct inv_sensors_timestamp timestamp;
-	struct regulator *vdd_supply;
 	struct regulator *vddio_supply;
 	bool magn_disabled;
 	s32 magn_raw_to_gauss[3];
-- 
2.51.0

[PATCH v2 3/3] iio: imu: inv_mpu6050: fix unbalanced regulator_disable calls, when probe fails

[Andrey Skvortsov]

During a probe functions after all regulators are enabled, runtime pm
is enabled. Before probe function finishes, runtime pm triggers and
disables vddio regulator. When probe function fails after that,
inv_mpu_core_disable_regulator_action tries to disable already
disabled by runtime pm vddio regulator causing following backtrace:

  inv-mpu6050-i2c 1-0068: trigger probe fail -19
  WARNING: drivers/regulator/core.c:3244 at _regulator_disable+0x2ac/0x600
  Call trace:
   _regulator_disable+0x2ac/0x600 (P)
   regulator_disable+0xac/0x148
   inv_mpu_core_disable_regulator_vddio_action+0x3c/0xb0 [inv_mpu6050]
   devm_action_release+0x4c/0x88
   release_nodes+0xd8/0x178
   devres_release_group+0x214/0x3c8
   i2c_device_probe+0x6fc/0x9b0
  ...
  inv-mpu6050-i2c 1-0068: Failed to disable vddio regulator: -5

Fix the issue by checking runtime suspend status, when vddio regulator
is disabled. To handle this correctly pm_runtime active status has to
be set early before vddio regulator setup.

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
Fixes: 07c12b1c007c ("iio: imu: mpu6050: add support for regulator framework")
---
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 3a57a3f440526..86e4d42c4d3d1 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1862,8 +1862,10 @@ static int inv_mpu_core_disable_regulator_vddio(struct inv_mpu6050_state *st)
 static void inv_mpu_core_disable_regulator_vddio_action(void *_data)
 {
 	struct inv_mpu6050_state *st = _data;
+	struct device *dev = regmap_get_device(st->map);
 
-	inv_mpu_core_disable_regulator_vddio(st);
+	if (!pm_runtime_status_suspended(dev))
+		inv_mpu_core_disable_regulator_vddio(st);
 }
 
 
@@ -1953,6 +1955,11 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 
 	msleep(INV_MPU6050_POWER_UP_TIME);
 
+	/* set pm_runtime active early for disable vddio resource cleanup */
+	result = pm_runtime_set_active(dev);
+	if (result)
+		return result;
+
 	result = inv_mpu_core_enable_regulator_vddio(st);
 	if (result)
 		return result;
@@ -1996,9 +2003,6 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 	}
 
 	/* chip init is done, turning on runtime power management */
-	result = pm_runtime_set_active(dev);
-	if (result)
-		goto error_power_off;
 	pm_runtime_get_noresume(dev);
 	result = devm_pm_runtime_enable(dev);
 	if (result)
-- 
2.51.0

Re: [PATCH v2 1/3] iio: imu: inv_mpu6050: use devres-enabled version of pm_runtime_enable

[Andy Shevchenko]

On Wed, Apr 01, 2026 at 11:27:35AM +0300, Andrey Skvortsov wrote:
> That allows remove driver-specific action, that does the same as
> default action.

Don't use the Subject as the first sentence in the explanation. Id est
always start a commit message as no Subject was ever existed.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] iio: imu: inv_mpu6050: control vdd supply using devm-helpers

[Andy Shevchenko]

On Wed, Apr 01, 2026 at 11:27:36AM +0300, Andrey Skvortsov wrote:
> vdd supply will be automatically disabled on error condition during a
> probe and on driver cleanup. Since the vdd handling is moved to
> devm-helpers, only vddio regulator has to be disabled in the
> driver-specific cleanup action.

...

> -static void inv_mpu_core_disable_regulator_action(void *_data)
> +static void inv_mpu_core_disable_regulator_vddio_action(void *_data)

You can rename now _data --> st...

>  {
>  	struct inv_mpu6050_state *st = _data;

...and drop this.

>  	inv_mpu_core_disable_regulator_vddio(st);
>  }

...

> -	st->vdd_supply = devm_regulator_get(dev, "vdd");
> -	if (IS_ERR(st->vdd_supply))
> -		return dev_err_probe(dev, PTR_ERR(st->vdd_supply),
> -				     "Failed to get vdd regulator\n");
> -
>  	st->vddio_supply = devm_regulator_get(dev, "vddio");
>  	if (IS_ERR(st->vddio_supply))
>  		return dev_err_probe(dev, PTR_ERR(st->vddio_supply),
>  				     "Failed to get vddio regulator\n");
>  
> -	result = regulator_enable(st->vdd_supply);
> -	if (result) {
> -		dev_err(dev, "Failed to enable vdd regulator: %d\n", result);
> -		return result;
> -	}
> +	result = devm_regulator_get_enable(dev, "vdd");
> +	if (result)
> +		return dev_err_probe(dev, result, "Failed to enable vdd regulator\n");

I'm not that expert in regulator framework, but this change might be sensitive.
I dunno if regulator is exclusive or shared and if order of requesting may affect
anything. Just needs careful review from somebody more familiar with that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] iio: imu: inv_mpu6050: control vdd supply using devm-helpers

[Andrey Skvortsov]

On 26-04-01 18:10, Andy Shevchenko wrote:
> On Wed, Apr 01, 2026 at 11:27:36AM +0300, Andrey Skvortsov wrote:
> > vdd supply will be automatically disabled on error condition during a
> > probe and on driver cleanup. Since the vdd handling is moved to
> > devm-helpers, only vddio regulator has to be disabled in the
> > driver-specific cleanup action.
> 
> ...
> 
> > -static void inv_mpu_core_disable_regulator_action(void *_data)
> > +static void inv_mpu_core_disable_regulator_vddio_action(void *_data)
> 
> You can rename now _data --> st...
> 
> >  {
> >  	struct inv_mpu6050_state *st = _data;
> 
> ...and drop this.
> 
> >  	inv_mpu_core_disable_regulator_vddio(st);
> >  }

Hi, Andy,

thanks for the review.
You are right, but this variable will be added in patch 3. Do you
think it's still worth removing variable here and add it back in patch
3? If yes, I'll change it in v2.

-- 
Best regards,
Andrey Skvortsov

Re: [PATCH v2 2/3] iio: imu: inv_mpu6050: control vdd supply using devm-helpers

[Andy Shevchenko]

On Mon, Apr 06, 2026 at 09:53:14AM +0300, Andrey Skvortsov wrote:
> On 26-04-01 18:10, Andy Shevchenko wrote:
> > On Wed, Apr 01, 2026 at 11:27:36AM +0300, Andrey Skvortsov wrote:

...

> > > -static void inv_mpu_core_disable_regulator_action(void *_data)
> > > +static void inv_mpu_core_disable_regulator_vddio_action(void *_data)
> > 
> > You can rename now _data --> st...
> > 
> > >  {
> > >  	struct inv_mpu6050_state *st = _data;
> > 
> > ...and drop this.
> > 
> > >  	inv_mpu_core_disable_regulator_vddio(st);
> > >  }
> 
> thanks for the review.
> You are right, but this variable will be added in patch 3. Do you
> think it's still worth removing variable here and add it back in patch
> 3? If yes, I'll change it in v2.

Yes, we do not add a dead code. Add it when it's really being used.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] iio: imu: inv_mpu6050: use devres-enabled version of pm_runtime_enable

[David Lechner]

On 4/1/26 3:27 AM, Andrey Skvortsov wrote:
> That allows remove driver-specific action, that does the same as
> default action.
> 
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 5796896d54cd8..84eb0ee770086 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -1872,12 +1872,7 @@ static void inv_mpu_core_disable_regulator_action(void *_data)
>  	inv_mpu_core_disable_regulator_vddio(st);
>  }
>  
> -static void inv_mpu_pm_disable(void *data)
> -{
> -	struct device *dev = data;
>  
> -	pm_runtime_disable(dev);
> -}
>  

I think this leaves too many blank lines.

Re: [PATCH v2 3/3] iio: imu: inv_mpu6050: fix unbalanced regulator_disable calls, when probe fails

[Nuno Sá]

On Wed, 2026-04-01 at 11:27 +0300, Andrey Skvortsov wrote:
> During a probe functions after all regulators are enabled, runtime pm
> is enabled. Before probe function finishes, runtime pm triggers and
> disables vddio regulator. When probe function fails after that,
> inv_mpu_core_disable_regulator_action tries to disable already
> disabled by runtime pm vddio regulator causing following backtrace:
> 
>   inv-mpu6050-i2c 1-0068: trigger probe fail -19
>   WARNING: drivers/regulator/core.c:3244 at _regulator_disable+0x2ac/0x600
>   Call trace:
>    _regulator_disable+0x2ac/0x600 (P)
>    regulator_disable+0xac/0x148
>    inv_mpu_core_disable_regulator_vddio_action+0x3c/0xb0 [inv_mpu6050]
>    devm_action_release+0x4c/0x88
>    release_nodes+0xd8/0x178
>    devres_release_group+0x214/0x3c8
>    i2c_device_probe+0x6fc/0x9b0
>   ...
>   inv-mpu6050-i2c 1-0068: Failed to disable vddio regulator: -5
> 
> Fix the issue by checking runtime suspend status, when vddio regulator
> is disabled. To handle this correctly pm_runtime active status has to
> be set early before vddio regulator setup.

The above looks a bit hacky. We're playing with runtime status for things to work.
Also, setting the active state makes sense logically when the device is up and
running.

Maybe we should just move enabling runtime to the end of the probe function?

Also, fixes should come first in the series in case we want to backport them.

- Nuno Sá
> 
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> Fixes: 07c12b1c007c ("iio: imu: mpu6050: add support for regulator framework")
> ---
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 3a57a3f440526..86e4d42c4d3d1 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -1862,8 +1862,10 @@ static int inv_mpu_core_disable_regulator_vddio(struct
> inv_mpu6050_state *st)
>  static void inv_mpu_core_disable_regulator_vddio_action(void *_data)
>  {
>  	struct inv_mpu6050_state *st = _data;
> +	struct device *dev = regmap_get_device(st->map);
>  
> -	inv_mpu_core_disable_regulator_vddio(st);
> +	if (!pm_runtime_status_suspended(dev))
> +		inv_mpu_core_disable_regulator_vddio(st);
>  }
>  
>  
> @@ -1953,6 +1955,11 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const
> char *name,
>  
>  	msleep(INV_MPU6050_POWER_UP_TIME);
>  
> +	/* set pm_runtime active early for disable vddio resource cleanup */
> +	result = pm_runtime_set_active(dev);
> +	if (result)
> +		return result;
> +
>  	result = inv_mpu_core_enable_regulator_vddio(st);
>  	if (result)
>  		return result;
> @@ -1996,9 +2003,6 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const
> char *name,
>  	}
>  
>  	/* chip init is done, turning on runtime power management */
> -	result = pm_runtime_set_active(dev);
> -	if (result)
> -		goto error_power_off;
>  	pm_runtime_get_noresume(dev);
>  	result = devm_pm_runtime_enable(dev);
>  	if (result)

Re: [PATCH v2 3/3] iio: imu: inv_mpu6050: fix unbalanced regulator_disable calls, when probe fails

[Andrey Skvortsov]

Hi,

On 26-04-11 14:22, Nuno Sá wrote:
> On Wed, 2026-04-01 at 11:27 +0300, Andrey Skvortsov wrote:
> > During a probe functions after all regulators are enabled, runtime pm
> > is enabled. Before probe function finishes, runtime pm triggers and
> > disables vddio regulator. When probe function fails after that,
> > inv_mpu_core_disable_regulator_action tries to disable already
> > disabled by runtime pm vddio regulator causing following backtrace:
> > 
> >   inv-mpu6050-i2c 1-0068: trigger probe fail -19
> >   WARNING: drivers/regulator/core.c:3244 at _regulator_disable+0x2ac/0x600
> >   Call trace:
> >    _regulator_disable+0x2ac/0x600 (P)
> >    regulator_disable+0xac/0x148
> >    inv_mpu_core_disable_regulator_vddio_action+0x3c/0xb0 [inv_mpu6050]
> >    devm_action_release+0x4c/0x88
> >    release_nodes+0xd8/0x178
> >    devres_release_group+0x214/0x3c8
> >    i2c_device_probe+0x6fc/0x9b0
> >   ...
> >   inv-mpu6050-i2c 1-0068: Failed to disable vddio regulator: -5
> > 
> > Fix the issue by checking runtime suspend status, when vddio regulator
> > is disabled. To handle this correctly pm_runtime active status has to
> > be set early before vddio regulator setup.
> 

> The above looks a bit hacky. We're playing with runtime status for things to work.
> Also, setting the active state makes sense logically when the device is up and
> running.
> 
Thanks for the review. Approach with setting active state just before
regulator is enabled was taken from recent commit in inv_icm4500
driver [1].

Another solution could be that pm_runtime_set_active(dev) is left,
where it's now, where pm runtime is initialized. So there is no
playing with runtime status. And just after that devm_add_action_or_reset(dev,
inv_mpu_core_disable_regulator_action, st) is called.

And if probe fails before inv_mpu_core_disable_regulator_action action
is set, than regulators will be disabled explicitly in error paths
(goto error_vddio_power_off).

Is this approach ok?

1. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/iio/imu/inv_icm45600?id=2617595538be8a2f270ad13fccb9f56007b292d7

> Maybe we should just move enabling runtime to the end of the probe
> function?
The problem is not that pm runtime is enabled so early during a probe,
but that action to disable regulators has to be aware of changes made
to regulators by runtime pm. Action is called, when probe fails (for
example, before pm runtime is enabled) and when driver is unloaded. 
To work correctly action has to be set after pm_runtime_set_active.

> 
> Also, fixes should come first in the series in case we want to backport them.
Thanks, I'll fix that in the next version.

-- 
Best regards,
Andrey Skvortsov