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

[Andrey Skvortsov <andrej.skvortzov@gmail.com>]

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