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

[Jonathan Cameron <jic23@kernel.org>]

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