On Fri, 8 May 2020 13:57:46 +0100 Mark Brown <broonie@xxxxxxxxxx> wrote: > On Fri, May 08, 2020 at 01:43:07PM +0100, Jonathan Cameron wrote: > > Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > > > It feels like we should just make a devm_ version of regulator_enable(). > > > Or potentially this is more complicated than it seems, but in that case > > > probably adding devm_add_action_or_reset() is more complicated than it > > > seems as well. > > > It has been a while since that was last proposed. At the time the > > counter argument was that you should almost always be doing some form > > of PM and hence the regulator shouldn't have the same lifetime as the > > driver. Reality is that a lot of simple drivers either don't do > > PM or have elected to not turn the regulator off so as to retain state > > etc. > > Same issue as before - I fear it's far too error prone in conjunction > with runtime PM, and if the driver really is just doing an enable and > disable at probe and remove then that seems fairly trivial anyway. I > am constantly finding abuses of things like regulator_get_optional() > (which we do actually need) in drivers and it's not like I can review > all the users, I don't have much confidence in this stuff especially > when practically speaking few regulators ever change state at runtime so > issues don't manifest so often. > Fair enough. We'll carry on doing it with devm_add_action_or_reset which forces us to take a close look at why we always want the lifetime to match that of the device. Note the key thing here is we don't have a remove in these drivers. Everything is managed. Mixing and matching between managed and unmanaged causes more subtle race conditions... Jonathan
