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

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

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