Message-ID: <72a1f4c5768b8c08c2669ea01e60d1b614095a43.camel@fi.rohmeurope.com>
Date:   Mon, 2 Dec 2019 14:02:41 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "broonie@...nel.org" <broonie@...nel.org>
CC:     "corbet@....net" <corbet@....net>, "pavel@....cz" <pavel@....cz>,
        "linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>,
        "dmurphy@...com" <dmurphy@...com>,
        "linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
        "jeffrey.t.kirsher@...el.com" <jeffrey.t.kirsher@...el.com>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "mchehab+samsung@...nel.org" <mchehab+samsung@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
        "mturquette@...libre.com" <mturquette@...libre.com>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "jacek.anaszewski@...il.com" <jacek.anaszewski@...il.com>,
        "mazziesaccount@...il.com" <mazziesaccount@...il.com>,
        "a.zummo@...ertech.it" <a.zummo@...ertech.it>,
        "hkallweit1@...il.com" <hkallweit1@...il.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "hofrat@...dl.org" <hofrat@...dl.org>,
        "wsa+renesas@...g-engineering.com" <wsa+renesas@...g-engineering.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
        "bgolaszewski@...libre.com" <bgolaszewski@...libre.com>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "phil.edworthy@...esas.com" <phil.edworthy@...esas.com>,
        "lee.jones@...aro.org" <lee.jones@...aro.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "sboyd@...nel.org" <sboyd@...nel.org>
Subject: Re: [PATCH v5 01/16] dt-bindings: regulator: Document ROHM BD71282
 regulator bindings


On Mon, 2019-12-02 at 13:11 +0000, Mark Brown wrote:
> On Mon, Dec 02, 2019 at 07:57:13AM +0000, Vaittinen, Matti wrote:
> > On Fri, 2019-11-29 at 12:09 +0000, Mark Brown wrote:
> > > The regulator driver has a bunch fo set_suspend_ operations.
> > Hmm. I saw these. But unless I am mistaken linux only knows one
> > 'suspend' state whereas the PMIC has a few separate states I can
> > see as
> > 'suspend' states. As far as I understood the set_suspend_voltage
> > does
> > not allow setting separate suspend voltages depending on the "type
> > of
> > suspend" (as there is only one 'suspend' state).
> 
> No, look at the bindings - we support a bunch of different
> suspend states matching the different suspend states that the
> kernel as a whole supports.  We don't assume that the device will
> know this but you can always use the current suspend we're going
> for to decide where to update.

Hm. So if I understand this correctly, you mean user should set the
suspend 'target' - and then call the set_suspend_voltage for this
state. To set voltages for all states one should do loop

get_current_mode()

for_all_modes() {
	set_mode()
	set_voltage()
}

restore_original_mode()

am I on a right track? I'll try to see if I can find some examples of
this - thanks.

> 
> > > > (RUN0, ... RUN3) could be mapped to regulator modes
> > > > REGULATOR_MODE_FAST, REGULATOR_MODE_NORMAL, REGULATOR_MODE_IDLE
> > > > and 
> > > > REGULATOR_MODE_STANDBY. But regulators which are controlled by
> > > > these
> > > That doesn't make sense at all, the modes affect the quality of
> > > regulation not the voltage that is set.
> > Thanks. I misunderstood this. I thought these states could be used
> > for
> > some adaptive voltages. My understanding is that the RUN0,...RUN3
> > are
> > designed for that - but I didn't know if regulator framework is
> > designed for this.
> 
> The framework doesn't care how a device is controlled, that's up
> to the device.  Like I said I recommend figuring out what
> voltages are useful to have quick access to at runtime, for
> example it's likely that it's good to have quick access to the
> highest voltage that's been set (and/or the top of the
> constraints).

Problem is that the run-level controlled regulator can't be
individually controlled (unless it is only regulator in the group). I
misunderstood these REGULATOR_MODE_FAST, ...,REGULATOR_MODE_STANDBY to
be global 'states' rather than states of individual regulators. And I
thought these were also designed for voltage scaling. But as I said, I
misunderstood them - so thanks for correcting me on this.

> 
> > > The cpufreq code is all there in kernel - drivers/cpufreq.  I
> > > can't
> > > remember if Android still has a custom governor in their trees
> > > but it
> > > doesn't really make much difference in terms of how it interacts
> > > with
> > > the regulator drivers.
> > Right. I guess your answers mean that there is no "regulator group
> > control" for "adaptive voltage changes" supported by regulator
> 
> I can't parse the above, sorry.  What is "regulator group
> control"?

I mean bundling the regulators in a group - and changing state for all
of the bundled regulators in one go. The thing I mentioned earlier -
and I guess you did already confirm it's not doable. I think you said
that only 'mass operation' or 'group operation' is the suspend.

But just to confirm, I meant for example assigning bucks 1,2,6 and 7
into a group which 'state' is changed via GPIO line. Say 'states' are
RUN0, RUN1. For each of these bucks we can define a voltage and
enable/disable status which is to be used on RUN0, and another
voltage/state tuple for RUN1.

When certain 'trigger' is detected (I assume CPU load here and adaptive
voltage scaling - but this is just my assumption of the use-case for
now) the PMIC state can be quickly changed via this GPIO toggle.

In realty, we have two GPIOs and 4 states - but that does not change
the princible. I don't think there is any 'de-facto' mechanism to
control such groups.


Br,
	Matti Vaittinen

Powered by blists - more mailing lists