Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "broonie@kernel.org" <broonie@kernel.org>
Cc: "corbet@lwn.net" <corbet@lwn.net>,
	"hkallweit1@gmail.com" <hkallweit1@gmail.com>,
	"phil.edworthy@renesas.com" <phil.edworthy@renesas.com>,
	"dmurphy@ti.com" <dmurphy@ti.com>,
	"linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
	"jeffrey.t.kirsher@intel.com" <jeffrey.t.kirsher@intel.com>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"mchehab+samsung@kernel.org" <mchehab+samsung@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
	"mturquette@baylibre.com" <mturquette@baylibre.com>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"jacek.anaszewski@gmail.com" <jacek.anaszewski@gmail.com>,
	"mazziesaccount@gmail.com" <mazziesaccount@gmail.com>,
	"a.zummo@towertech.it" <a.zummo@towertech.it>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"wsa+renesas@sang-engineering.com"
	<wsa+renesas@sang-engineering.com>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>,
	"hofrat@osadl.org" <hofrat@osadl.org>,
	"linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"m.szyprowski@samsung.com" <m.szyprowski@samsung.com>,
	"bgolaszewski@baylibre.com" <bgolaszewski@baylibre.com>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
	"lee.jones@linaro.org" <lee.jones@linaro.org>,
	"sboyd@kernel.org" <sboyd@kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"pavel@ucw.cz" <pavel@ucw.cz>
Subject: Re: [PATCH v5 01/16] dt-bindings: regulator: Document ROHM BD71282 regulator bindings
Date: Mon, 2 Dec 2019 07:57:13 +0000
Message-ID: <297fa021fb243072dbbb7bca455e57c13e8c6843.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <20191129120925.GA5747@sirena.org.uk>

Hello Mark!

On Fri, 2019-11-29 at 12:09 +0000, Mark Brown wrote:
> On Fri, Nov 29, 2019 at 07:48:13AM +0000, Vaittinen, Matti wrote:
> > On Tue, 2019-11-19 at 19:36 +0000, Mark Brown wrote:
> > > The driver interface was added in "regulator: add PM suspend and
> > > resume
> > > hooks".
> > I looked through the set but didn't spot any new interface towards
> > the
> > regulator driver (which accesses the HW). I saw interface towards
> > regulator consumer driver which can be used to set the constrains
> > though.
> 
> 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).

For example, from CPU point of view the BD71828 PMIC states HIBERNATE
and LPSR are probably both just "suspend" - but the PMIC could set
different voltages or ON/OFF states for some regulators depending on
the 'suspend' target (LPSR or HIBERNATE or STANDBY).

Yet, as I said, I haven't seen how these states are used by real
devices - we don't currently have any in-tree SoCs which use BD71828
and utilize these states (as far as I know). Hence I can't really
figure out how to add support for these PMIC features if there is no
'de-facto' mechanism in place :(
> 
> > Specifically, I don't see voltage setting callback for different
> > run-
> > modes. Nor do I see voltage setting (or differentiation) of more
> > than
> > one suspend state.
> 
> set_suspend_voltage.

Yes. But this does only allow setting one suspend voltage for
regulator, not own voltage for HIBERNATE, LPSR, STANDBY etc - or am I
mistaken?

> > To explain it further - my assumption is that the BD71828 'run-
> > levels'
> > (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.

> > run-levels, can't be individually controlled. If state for one is
> > changed, the state is changed for all of them. The DVS bucks 1,2,6
> > and
> 
> We don't really have anything that'd only work for group
> configuration
> except for the suspend modes.

Thanks. As I said, I thought the 'quality of regulation' states could
have been supporting also changing the voltages.

> 
> > > Ah, that's actually better.  It opens up possiblities for making
> > > use
> > > of
> > > the feature without encoding voltages in DT.  For example, you
> > > can
> > > cache
> > > the last however many voltages that were set and jump quickly to
> > > them
> > > or
> > > do something like put the top of the constraints in to help with
> > > governors like ondemand.  I'd recommend trying for something like
> > > that
> > > rather than encoding in DT, it'll probably be more robust with
> > > things
> > > like cpufreq changing.
> > I wish I was working with the full product so that I could see and
> > learn a proper example on how the cpufreq actually uses these
> > interfaces :) I'd really like to understand this much better. Maybe
> > this could be a topic for you to present in some Linux conference
> > ;)
> > Just please ping me when you are doing that and I'll be listening
> > there
> > for sure ;)
> 
> 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
framework / cpufreq - as of now. Furthermore, I have understood that it
is different story depending on PMIC and CPU/SoC capabilities. (Like
PMIC inbuilt states and state change mechanisms, SoC voltage scaling
support etc.)

>  Anyways, my idea was to set the inital voltage values for these
> states
> > via DT - but allow the voltages to be changed at run-time too (I
> > guess
> > this idea is visible in the patch 12).
> 
> It'd be much better if you could avoid putting the voltages in the
> binding if they're not strictly required.

Hmm.. I guess I can omit that from in-tree driver for now. I can try
maintaining own set of patches for existing customers for now. I think
adding these to bindings later is not impossible :) Removing them would
probably be harder.

Thanks again Mark. I truly appreciate your help and guidance :)

Br,
	Matti



  reply index

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-18  6:53 [PATCH v5 00/16] Support ROHM BD71828 PMIC Matti Vaittinen
2019-11-18  6:53 ` [PATCH v5 01/16] dt-bindings: regulator: Document ROHM BD71282 regulator bindings Matti Vaittinen
2019-11-18 16:25   ` Mark Brown
2019-11-18 18:03     ` Vaittinen, Matti
2019-11-19 18:13       ` Mark Brown
2019-11-19 18:51         ` Vaittinen, Matti
2019-11-19 19:36           ` Mark Brown
2019-11-29  7:48             ` Vaittinen, Matti
2019-11-29 12:09               ` Mark Brown
2019-12-02  7:57                 ` Vaittinen, Matti [this message]
2019-12-02 13:11                   ` Mark Brown
2019-12-02 14:02                     ` Vaittinen, Matti
2019-12-04 12:47                       ` Mark Brown
2019-12-04 13:13                         ` Vaittinen, Matti
2019-12-04 14:14                           ` Mark Brown
2019-11-22 22:48   ` Rob Herring
2019-11-18  6:54 ` [PATCH v5 02/16] dt-bindings: leds: ROHM BD71282 PMIC LED driver Matti Vaittinen
2019-11-22 23:00   ` Rob Herring
2019-11-18  6:54 ` [PATCH v5 03/16] dt-bindings: mfd: Document ROHM BD71828 bindings Matti Vaittinen
2019-11-22 23:05   ` Rob Herring
2019-11-18  6:55 ` [PATCH v5 04/16] mfd: rohm PMICs - use platform_device_id to match MFD sub-devices Matti Vaittinen
2019-11-18  6:55 ` [PATCH v5 05/16] mfd: bd71828: Support ROHM BD71828 PMIC - core Matti Vaittinen
2019-11-18  6:56 ` [PATCH v5 06/16] mfd: input: bd71828: Add power-key support Matti Vaittinen
2019-11-18  6:56 ` [PATCH v5 07/16] clk: bd718x7: Support ROHM BD71828 clk block Matti Vaittinen
2019-11-18  6:57 ` [PATCH v5 08/16] regulator: bd718x7: Split driver to common and bd718x7 specific parts Matti Vaittinen
2019-11-18  6:57 ` [PATCH v5 09/16] regulator: bd71828: Basic support for ROHM bd71828 PMIC regulators Matti Vaittinen
2019-11-18 16:20   ` Mark Brown
2019-11-19  9:12     ` Vaittinen, Matti
2019-11-18  6:58 ` [PATCH v5 10/16] gpio: devres: Add devm_gpiod_get_parent_array Matti Vaittinen
2019-11-19 14:43   ` Linus Walleij
2019-11-19 17:54     ` Vaittinen, Matti
2019-11-21 14:13       ` Linus Walleij
2019-11-18  6:59 ` [PATCH v5 11/16] docs: driver-model: Add missing managed GPIO array get functions Matti Vaittinen
2019-11-18  6:59 ` [PATCH v5 12/16] regulator: bd71828: Add GPIO based run-level control for regulators Matti Vaittinen
2019-11-18  7:00 ` [PATCH v5 13/16] rtc: bd70528 add BD71828 support Matti Vaittinen
2019-11-18  7:01 ` [PATCH v5 14/16] gpio: bd71828: Initial support for ROHM BD71828 PMIC GPIOs Matti Vaittinen
2019-11-18  9:22   ` Bartosz Golaszewski
2019-11-18  7:03 ` [PATCH v5 15/16] leds: Add common LED binding parsing support to LED class/core Matti Vaittinen
2019-11-18 21:55   ` Jacek Anaszewski
2019-11-19  7:21     ` Vaittinen, Matti
2019-11-19 19:30       ` Jacek Anaszewski
2019-11-20  7:31         ` Vaittinen, Matti
2019-11-19 14:23   ` Vaittinen, Matti
2019-11-18  7:03 ` [PATCH v5 16/16] led: bd71828: Support LED outputs on ROHM BD71828 PMIC Matti Vaittinen

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=297fa021fb243072dbbb7bca455e57c13e8c6843.camel@fi.rohmeurope.com \
    --to=matti.vaittinen@fi.rohmeurope.com \
    --cc=a.zummo@towertech.it \
    --cc=alexandre.belloni@bootlin.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=broonie@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=dmurphy@ti.com \
    --cc=hkallweit1@gmail.com \
    --cc=hofrat@osadl.org \
    --cc=jacek.anaszewski@gmail.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mark.rutland@arm.com \
    --cc=mazziesaccount@gmail.com \
    --cc=mchehab+samsung@kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=pavel@ucw.cz \
    --cc=phil.edworthy@renesas.com \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git