Re: [PATCH] staging: iio: ad5933: rework probe to use devm_ function variants

[Jonathan Cameron]

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