Messages in this thread | | | From | Brian Norris <> | Date | Wed, 1 Sep 2021 13:06:28 -0700 | Subject | Re: [PATCH] regulator: core: resolve supply voltage deferral silently |
| |
On Wed, Sep 1, 2021 at 8:09 AM Mark Brown <broonie@kernel.org> 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
| |