Re: [PATCH v5 01/16] dt-bindings: regulator: Document ROHM BD71282 regulator bindings

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2452 bytes --]

On Mon, Nov 18, 2019 at 06:03:42PM +0000, Vaittinen, Matti wrote:

> It's nice to hear from you again :) I hope you're enjoying all the new
> things :)

:)

> On Mon, 2019-11-18 at 16:25 +0000, Mark Brown wrote:
> > On Mon, Nov 18, 2019 at 08:53:57AM +0200, Matti Vaittinen wrote:

> > I don't think I saw this having the effect on set_voltage() that I'd
> > have expected in the driver?  

> The support for this is added in patch 12. I should've ordered the
> patch series so that all regulator patches were one after another.
> Sorry for that.
> The patch 12 adds the run-level support. Please see the functions
> get_runcontrolled_bucks_dt(),
> mark_regulator_runlvl_controlled() (sets the g->runlvl)
> and set_buck_runlvl_controlled() (called based on g->runlvl)
> which changes the ops to disallow setters and to get voltage based on
> current runlevel - and different ops depending on if runlevels are
> controlled by GPIO or I2C. Additionally set_buck_runlvl_controlled()
> adds DT parsing call-back for setting the initial voltages.

Ah, OK.  I didn't even notice that patch when I scanned the series.
I'll look out for this next time around but that sounds like it's
generally going in the right direction, especially if it's integrated
with the suspend mode regulator bindings that Chunyan did.

> > > +        minimum: 0
> > > +        maximum: 2000000
> > > +        maxItems: 4
> > > +        description:
> > > +          Array of voltages for run-levels. First value is for
> > > run-level 0,
> > > +          second for run-level 1 etc. Microvolts.

> > What's the mapping from array indexes to the names used elsewhere to
> > support runlevels?

> Hmm. Sorry Mark, I don't think I follow your question. Do you mean
> names like LPSR, SUSPEND, IDLE, RUN? If so, then I might need to
> rephrase this. The runlevels referred here are different from LPSR,
> SUSPEND, IDLE etc. They are actually 'sub-levels' for PMIC's RUN state.
> Eg, kind of a 'fast way' to change voltages for multiple power rails
> when SoC is at RUN state. The names I have seen are RUN0, RUN1, RUN2
> and RUN3. That mapping is described in description above.

Yes, I think this needs clarification as I completely failed to pick up
on this and did indeed read this as referring to the modes.  "Voltages
that can be set in RUN mode" or something?  I take it these voltages are
fixed and the OS can't change them?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]