On Tue, 2019-11-19 at 18:13 +0000, Mark Brown wrote: > On Mon, Nov 18, 2019 at 06:03:42PM +0000, Vaittinen, Matti wrote: > > 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. Probably it is not as I am not familiar with Chunyan's work. I'll try looking what has been done on that front :) And I am pretty sure you might not be happy with that patch - but perhaps you can give me a nudge to better direction... > > > > + 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? Unfortunately they are not. Voltages and enable/disable statuses for each run-level (and individually for each run-level capable buck) can be changed at runtime via I2C. And a customer requested me also to support this - hence the in-kernel API - but I am sure you have some nice words when you check the patch 12. :] Br, Matti Vaittinen