Re: [PATCH] regulator: core: resolve supply voltage deferral silently

[Brian Norris]

On Wed, Sep 1, 2021 at 8:09 AM Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> On Thu, Aug 26, 2021 at 12:40:17PM -0700, Brian Norris wrote:
>
> >               if (current_uV < 0) {
> > -                     rdev_err(rdev,
> > -                              "failed to get the current voltage: %pe\n",
> > -                              ERR_PTR(current_uV));
> > +                     if (current_uV != -EPROBE_DEFER)
> > +                             rdev_err(rdev,
> > +                                      "failed to get the current voltage: %pe\n",
> > +                                      ERR_PTR(current_uV));
>
> This doesn't make sense to me.  Why are we getting as far as trying to
> read the voltage if we've been told to defer probe?  This suggests that
> we ought to be doing this earlier on.  I see that the logic is already
> there to handle a deferral being generated here but it looks off.

Take a look at the commit this "Fixes":

21e39809fd7c ("regulator: vctrl: Avoid lockdep warning in enable/disable ops")

The target |rdev| hasn't deferred probe, but it's telling the
regulator core to DEFER because the supply (which is required for
|rdev| to "get" its present voltage) isn't yet resolved. So the probe
deferral isn't really about the device framework, but about the
regulator framework.

If this were a device framework deferral, then agreed, this would be
bad -- we can't guarantee, for one, that the second try will not also
defer. But in this case, vctrl_probe() has already ensured that its
supply regualator is there (devm_regulator_get(..., "ctrl")) -- it
just isn't wired up into |rdev->supply| yet.

Frankly, I'm not sure if we're abusing regulator framework features
(particularly, around use of ->supply) in commit 21e39809fd7c, or if
this is just a lacking area in the framework. I'm interested in
whether you have thoughts on doing this Better(TM).

Brian