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

[Andrey Skvortsov]

On 26-04-27 10:04, Jonathan Cameron wrote:
> On Sun, 26 Apr 2026 23:25:42 +0300
> Andrey Skvortsov <andrej.skvortzov@xxxxxxxxx> wrote:
> 
> > On 26-04-26 15:43, Jonathan Cameron wrote:
> > > On Sun, 26 Apr 2026 14:23:32 +0300
> > > Andrey Skvortsov <andrej.skvortzov@xxxxxxxxx> 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@xxxxxxxxx/#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.
For me moving pm_runtime_set_active earlier looks better as well. Then
I'll change this back to the previous version in the next patchset version,
if nobody has something against that.

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

-- 
Best regards,
Andrey Skvortsov