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

[PATCH v3 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: fix unbalanced regulator_disable calls, when
    probe fails
  iio: imu: inv_mpu6050: use devres-enabled version of pm_runtime_enable
  iio: imu: inv_mpu6050: control vdd supply using devm-helpers

 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 61 +++++++---------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |  1 -
 2 files changed, 20 insertions(+), 42 deletions(-)

Changes in v3:
 - make patch "fix unbalanced regulator_disable calls, when probe fails"
   with Fixes to be the first patch of the patchset for easier backporting
 - don't move pm_runtime_set_active, move devm_add_action_or_reset for
   inv_mpu_core_disable_regulator_action instead
 - update commit message in "fix unbalanced regulator_disable calls,
   when probe fails" to fix Andy's comment
 - remove blank lines in "use devres-enabled version of pm_runtime_enable"
 - no dead code in inv_mpu_core_disable_regulator_action anymore since
   st variable is used to get 'struct device' now

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

[PATCH v3 1/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

vddio state is handled in two places: pm_runtime and
inv_mpu_core_disable_regulator_vddio_action devm action.
inv_mpu_core_disable_regulator_vddio_action has to check, whether
regulator is disabled by pm_runtime already. But this information is
available only after pm_runtime state is initialized by
pm_runtime_set_active during a probe. So
inv_mpu_core_disable_regulator_vddio_action has to be called only
after pm_runtime is properly initialized. To handle cases, when probe
fails before pm_runtime is enabled, explicitly disable regulators
in error paths.

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
Fixes: 07c12b1c007c ("iio: imu: mpu6050: add support for regulator framework")
---

Changes in v3:
 - make patch to be the first patch of the patchset for easier backporting
 - update commit message to fix Andy's comment
 - don't move pm_runtime_set_active from pm_runtime initialization,
   move activation of inv_mpu_core_disable_regulator_action instead

Changes in v2:
 - minimize call trace in commit message
 - include specific driver name in patch subject

 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 25 ++++++++++++----------
 1 file changed, 14 insertions(+), 11 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..49fdf33adec99 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1862,6 +1862,7 @@ static int inv_mpu_core_disable_regulator_vddio(struct inv_mpu6050_state *st)
 static void inv_mpu_core_disable_regulator_action(void *_data)
 {
 	struct inv_mpu6050_state *st = _data;
+	struct device *dev = regmap_get_device(st->map);
 	int result;
 
 	result = regulator_disable(st->vdd_supply);
@@ -1869,7 +1870,8 @@ static void inv_mpu_core_disable_regulator_action(void *_data)
 		dev_err(regmap_get_device(st->map),
 			"Failed to disable vdd regulator: %d\n", result);
 
-	inv_mpu_core_disable_regulator_vddio(st);
+	if (!pm_runtime_status_suspended(dev))
+		inv_mpu_core_disable_regulator_vddio(st);
 }
 
 static void inv_mpu_pm_disable(void *data)
@@ -1976,23 +1978,15 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		return result;
 	}
 
-	result = devm_add_action_or_reset(dev, inv_mpu_core_disable_regulator_action,
-				 st);
-	if (result) {
-		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
-			result);
-		return result;
-	}
-
 	/* fill magnetometer orientation */
 	result = inv_mpu_magn_set_orient(st);
 	if (result)
-		return result;
+		goto error_vddio_off;
 
 	/* power is turned on inside check chip type*/
 	result = inv_check_and_setup_chip(st);
 	if (result)
-		return result;
+		goto error_vddio_off;
 
 	result = inv_mpu6050_init_config(indio_dev);
 	if (result) {
@@ -2018,6 +2012,12 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 	result = pm_runtime_set_active(dev);
 	if (result)
 		goto error_power_off;
+
+	result = devm_add_action_or_reset(dev, inv_mpu_core_disable_regulator_action,
+					  st);
+	if (result)
+		return dev_err_probe(dev, result,
+				     "Failed to setup regulator cleanup action\n");
 	pm_runtime_get_noresume(dev);
 	pm_runtime_enable(dev);
 	pm_runtime_set_autosuspend_delay(dev, INV_MPU6050_SUSPEND_DELAY_MS);
@@ -2114,6 +2114,9 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 
 error_power_off:
 	inv_mpu6050_set_power_itg(st, false);
+error_vddio_off:
+	inv_mpu_core_disable_regulator_vddio(st);
+	regulator_disable(st->vdd_supply);
 	return result;
 }
 EXPORT_SYMBOL_NS_GPL(inv_mpu_core_probe, "IIO_MPU6050");
-- 
2.53.0

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

[Andrey Skvortsov]

Usage of devm_pm_runtime_enable allows to remove driver-specific
action, that does the same as default action.

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---

Changes in v3:
 - remove extra blank lines in "use devres-enabled version of pm_runtime_enable"

 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 49fdf33adec99..fc786aaeac5e5 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1874,13 +1874,6 @@ 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 +2012,12 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		return dev_err_probe(dev, result,
 				     "Failed to setup regulator cleanup action\n");
 	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.53.0

[PATCH v3 3/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.

Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---

Changes in v3:
 - no dead code in inv_mpu_core_disable_regulator_action anymore since st variable
   is used to get 'struct device' now

 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 24 ++++------------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |  1 -
 2 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index fc786aaeac5e5..880bb7b8b83b1 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -1863,12 +1863,6 @@ static void inv_mpu_core_disable_regulator_action(void *_data)
 {
 	struct inv_mpu6050_state *st = _data;
 	struct device *dev = regmap_get_device(st->map);
-	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);
 
 	if (!pm_runtime_status_suspended(dev))
 		inv_mpu_core_disable_regulator_vddio(st);
@@ -1948,28 +1942,19 @@ 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;
-	}
 
 	/* fill magnetometer orientation */
 	result = inv_mpu_magn_set_orient(st);
@@ -2108,7 +2093,6 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 	inv_mpu6050_set_power_itg(st, false);
 error_vddio_off:
 	inv_mpu_core_disable_regulator_vddio(st);
-	regulator_disable(st->vdd_supply);
 	return result;
 }
 EXPORT_SYMBOL_NS_GPL(inv_mpu_core_probe, "IIO_MPU6050");
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.53.0

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

[Jonathan Cameron]

On Sun, 26 Apr 2026 14:23:32 +0300
Andrey Skvortsov <andrej.skvortzov@gmail.com> 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
> 
> vddio state is handled in two places: pm_runtime and
> inv_mpu_core_disable_regulator_vddio_action devm action.
> inv_mpu_core_disable_regulator_vddio_action has to check, whether
> regulator is disabled by pm_runtime already. But this information is
> available only after pm_runtime state is initialized by
> pm_runtime_set_active during a probe. So
> inv_mpu_core_disable_regulator_vddio_action has to be called only
> after pm_runtime is properly initialized. To handle cases, when probe
> fails before pm_runtime is enabled, explicitly disable regulators
> in error paths.
Why not drag pm_runtime_set_active() earlier?  Whilst we currently
check for an error on that call I'm not sure there is any reason
to do so.  It's a narrow set of conditions than can cause such an
error. 

If you have that between the regulator enable and registering the
devm action I think that would mean you don't need to manually clean
up the regulators.

Maybe I'm missing something.

J

> 
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> Fixes: 07c12b1c007c ("iio: imu: mpu6050: add support for regulator framework")
> ---
> 
> Changes in v3:
>  - make patch to be the first patch of the patchset for easier backporting
>  - update commit message to fix Andy's comment
>  - don't move pm_runtime_set_active from pm_runtime initialization,
>    move activation of inv_mpu_core_disable_regulator_action instead
> 
> Changes in v2:
>  - minimize call trace in commit message
>  - include specific driver name in patch subject
> 
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 25 ++++++++++++----------
>  1 file changed, 14 insertions(+), 11 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..49fdf33adec99 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -1862,6 +1862,7 @@ static int inv_mpu_core_disable_regulator_vddio(struct inv_mpu6050_state *st)
>  static void inv_mpu_core_disable_regulator_action(void *_data)
>  {
>  	struct inv_mpu6050_state *st = _data;
> +	struct device *dev = regmap_get_device(st->map);
>  	int result;
>  
>  	result = regulator_disable(st->vdd_supply);

> @@ -1869,7 +1870,8 @@ static void inv_mpu_core_disable_regulator_action(void *_data)
>  		dev_err(regmap_get_device(st->map),
>  			"Failed to disable vdd regulator: %d\n", result);
>  
> -	inv_mpu_core_disable_regulator_vddio(st);
> +	if (!pm_runtime_status_suspended(dev))
> +		inv_mpu_core_disable_regulator_vddio(st);
>  }
>  
>  static void inv_mpu_pm_disable(void *data)
> @@ -1976,23 +1978,15 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>  		return result;
>  	}
>  
> -	result = devm_add_action_or_reset(dev, inv_mpu_core_disable_regulator_action,
> -				 st);
> -	if (result) {
> -		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
> -			result);
> -		return result;
> -	}
> -
>  	/* fill magnetometer orientation */
>  	result = inv_mpu_magn_set_orient(st);
>  	if (result)
> -		return result;
> +		goto error_vddio_off;
>  
>  	/* power is turned on inside check chip type*/
>  	result = inv_check_and_setup_chip(st);
>  	if (result)
> -		return result;
> +		goto error_vddio_off;
>  
>  	result = inv_mpu6050_init_config(indio_dev);
>  	if (result) {
> @@ -2018,6 +2012,12 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>  	result = pm_runtime_set_active(dev);
>  	if (result)
>  		goto error_power_off;
> +
> +	result = devm_add_action_or_reset(dev, inv_mpu_core_disable_regulator_action,
> +					  st);
> +	if (result)
> +		return dev_err_probe(dev, result,
> +				     "Failed to setup regulator cleanup action\n");
>  	pm_runtime_get_noresume(dev);
>  	pm_runtime_enable(dev);
>  	pm_runtime_set_autosuspend_delay(dev, INV_MPU6050_SUSPEND_DELAY_MS);
> @@ -2114,6 +2114,9 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>  
>  error_power_off:
>  	inv_mpu6050_set_power_itg(st, false);
> +error_vddio_off:
> +	inv_mpu_core_disable_regulator_vddio(st);
> +	regulator_disable(st->vdd_supply);
>  	return result;
>  }
>  EXPORT_SYMBOL_NS_GPL(inv_mpu_core_probe, "IIO_MPU6050");

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

[Jonathan Cameron]

On Sun, 26 Apr 2026 14:23:34 +0300
Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:

> vdd supply will be automatically disabled on error condition during a
> probe and on driver cleanup.
> 
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> ---
Looks good to me.

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

[Andrey Skvortsov]

On 26-04-26 15:43, Jonathan Cameron wrote:
> On Sun, 26 Apr 2026 14:23:32 +0300
> Andrey Skvortsov <andrej.skvortzov@gmail.com> 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
> > 
> > vddio state is handled in two places: pm_runtime and
> > inv_mpu_core_disable_regulator_vddio_action devm action.
> > inv_mpu_core_disable_regulator_vddio_action has to check, whether
> > regulator is disabled by pm_runtime already. But this information is
> > available only after pm_runtime state is initialized by
> > pm_runtime_set_active during a probe. So
> > inv_mpu_core_disable_regulator_vddio_action has to be called only
> > after pm_runtime is properly initialized. To handle cases, when probe
> > fails before pm_runtime is enabled, explicitly disable regulators
> > in error paths.
> Why not drag pm_runtime_set_active() earlier?

Do you mean something like proposed in v2? [1]

1. https://lore.kernel.org/linux-iio/20260401082737.781018-4-andrej.skvortzov@gmail.com/#t

> Whilst we currently
> check for an error on that call I'm not sure there is any reason
> to do so.  It's a narrow set of conditions than can cause such an
> error. 
> 
> If you have that between the regulator enable and registering the
> devm action I think that would mean you don't need to manually clean
> up the regulators.
> 
> Maybe I'm missing something.
> 
> J
> 
> > 
> > Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> > Fixes: 07c12b1c007c ("iio: imu: mpu6050: add support for regulator framework")
> > ---
> > 
> > Changes in v3:
> >  - make patch to be the first patch of the patchset for easier backporting
> >  - update commit message to fix Andy's comment
> >  - don't move pm_runtime_set_active from pm_runtime initialization,
> >    move activation of inv_mpu_core_disable_regulator_action instead
> > 
> > Changes in v2:
> >  - minimize call trace in commit message
> >  - include specific driver name in patch subject
> > 

-- 
Best regards,
Andrey Skvortsov

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

[Andy Shevchenko]

On Sun, Apr 26, 2026 at 02:23:32PM +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

Please, use () when we refer to the functions or callbacks:
 — func()
 — .callback()

> disabled by runtime pm vddio regulator causing following backtrace:

PM

>   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
> 
> vddio state is handled in two places: pm_runtime and
> inv_mpu_core_disable_regulator_vddio_action devm action.
> inv_mpu_core_disable_regulator_vddio_action has to check, whether
> regulator is disabled by pm_runtime already. But this information is
> available only after pm_runtime state is initialized by
> pm_runtime_set_active during a probe. So
> inv_mpu_core_disable_regulator_vddio_action has to be called only
> after pm_runtime is properly initialized. To handle cases, when probe
> fails before pm_runtime is enabled, explicitly disable regulators
> in error paths.

As per previous comments.

...

> +	result = devm_add_action_or_reset(dev, inv_mpu_core_disable_regulator_action,
> +					  st);

I would go with a single line despite being long.


> +	if (result)
> +		return dev_err_probe(dev, result,
> +				     "Failed to setup regulator cleanup action\n");

-- 
With Best Regards,
Andy Shevchenko

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

[Jonathan Cameron]

On Sun, 26 Apr 2026 23:25:42 +0300
Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote:

> On 26-04-26 15:43, Jonathan Cameron wrote:
> > On Sun, 26 Apr 2026 14:23:32 +0300
> > Andrey Skvortsov <andrej.skvortzov@gmail.com> 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
> > > 
> > > vddio state is handled in two places: pm_runtime and
> > > inv_mpu_core_disable_regulator_vddio_action devm action.
> > > inv_mpu_core_disable_regulator_vddio_action has to check, whether
> > > regulator is disabled by pm_runtime already. But this information is
> > > available only after pm_runtime state is initialized by
> > > pm_runtime_set_active during a probe. So
> > > inv_mpu_core_disable_regulator_vddio_action has to be called only
> > > after pm_runtime is properly initialized. To handle cases, when probe
> > > fails before pm_runtime is enabled, explicitly disable regulators
> > > in error paths.  
> > Why not drag pm_runtime_set_active() earlier?  
> 
> Do you mean something like proposed in v2? [1]
> 
> 1. https://lore.kernel.org/linux-iio/20260401082737.781018-4-andrej.skvortzov@gmail.com/#t
Ah yes. Sorry, I missed that discussion.

To me moving this earlier is less hacky.  We are brining the runtime
state inline with what we have set the power to at the point of doing
so.

Nuno, having seen the complexity that moving that state change late
causes, I don't suppose you are convinced that v2 solution is the way
to go?

Thanks,

Jonathan

> 
> > Whilst we currently
> > check for an error on that call I'm not sure there is any reason
> > to do so.  It's a narrow set of conditions than can cause such an
> > error. 
> > 
> > If you have that between the regulator enable and registering the
> > devm action I think that would mean you don't need to manually clean
> > up the regulators.
> > 
> > Maybe I'm missing something.
> > 
> > J
> >   
> > > 
> > > Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> > > Fixes: 07c12b1c007c ("iio: imu: mpu6050: add support for regulator framework")
> > > ---
> > > 
> > > Changes in v3:
> > >  - make patch to be the first patch of the patchset for easier backporting
> > >  - update commit message to fix Andy's comment
> > >  - don't move pm_runtime_set_active from pm_runtime initialization,
> > >    move activation of inv_mpu_core_disable_regulator_action instead
> > > 
> > > Changes in v2:
> > >  - minimize call trace in commit message
> > >  - include specific driver name in patch subject
> > >   
>