Messages in this thread | | | From | Brian Norris <> | Date | Thu, 2 Sep 2021 15:41:02 -0700 | Subject | Re: [PATCH] regulator: core: resolve supply voltage deferral silently |
| |
On Thu, Sep 2, 2021 at 10:06 AM Mark Brown <broonie@kernel.org> wrote: > On Wed, Sep 01, 2021 at 01:06:28PM -0700, Brian Norris wrote: > > On Wed, Sep 1, 2021 at 8:09 AM Mark Brown <broonie@kernel.org> wrote: > > > > 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") > > That driver change is at most tangentially related to the code that's > being updated,
It introduced another case where we hit a spurious error log. And below, you admit that you didn't understand what this is fixing without that pointer. I guess we disagree.
> > 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). > > That's definitely an abuse of the API, the hardware design is pretty > much a gross hack anywhere as far as I remember. As Chen-Yu says I'd > only expect this to be possible in the case where the supply is in > bypass mode and hasn't got its own parent. In any case I can see why > it's happening now...
Well the hardware exists, the driver exists, and it all worked OK until somewhat recently (and now it works again, thanks to Chen-Yu). What should we do here, then? Just leave the "abuse" in place?
We *did* attempt some kind of alternative solution here, but it's really not that easy. AFAICT, there isn't a good way for one regulator to lock another, without exposing quite a bit more regulator-core features to drivers. I think either the driver would need to access to the |struct ww_acquire_ctx| in some way, or else we'd need to teach the regulator core about the vctrl dependency, such that regulator_lock_dependent() can handle the locking properly for us.
Brian
| |