Re: [PATCH 1/1] clk: clk-cdce925: Add regulator support

[Stephen Boyd <sboyd@kernel.org>]

Quoting Phil Reid (2019-06-25 18:06:29)
> On 26/06/2019 06:54, Stephen Boyd wrote:
> > Quoting Phil Reid (2019-06-13 20:52:43)
> >> +               return PTR_ERR(regulator);
> >> +
> >> +       err = regulator_enable(regulator);
> > 
> G'day Stephen,
> 
> Thanks for looking at this.
> > The regulator is never turned off though. Are these regulators really
> > just always on regulators that don't need to be managed by this driver?
> > 
> For our system the regulator needs to be enabled before we try talking to the chip.
> Funny that.
> Unloading the driver will disable the regulator thru the devm call to
> cdce925_regulator_disable

Ok. Is it a regulator that is expected to just always be on though? Or
does the datasheet for this device indicate that these supplies can be
turned on and off when the device isn't in use?

> 
> > +     return devm_add_action_or_reset(dev, cdce925_regulator_disable,
> > +                                     regulator);
> > +}
> 
> In the future suspend/resume support could be added to power the device down.
> The system I have doesn't support suspending thou.
> 
> > Also, is there an update to the DT binding somewhere?
> > 
> No I didn't update that.
> It seems a bit adhoc if supply reference are including in the DT docs.
> I can add something if your happy with the pathc in general.
> 

Yes, the binding needs an update to list out the supplies. If they're
not always going to be enabled because they're controlled supplies in
the design then it makes sense to me to add them to the binding and use
them from this driver so that things operate properly.