Re: [RFC PATCH v3 10/15] regulator: bd71828: Add GPIO based run-level control for regulators

From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "linus.walleij@linaro.org" <linus.walleij@linaro.org>
Cc: "linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
	"dmurphy@ti.com" <dmurphy@ti.com>,
	"linux-rtc@vger.kernel.org" <linux-rtc@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"alexandre.belloni@bootlin.com" <alexandre.belloni@bootlin.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"mturquette@baylibre.com" <mturquette@baylibre.com>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"mazziesaccount@gmail.com" <mazziesaccount@gmail.com>,
	"a.zummo@towertech.it" <a.zummo@towertech.it>,
	"jacek.anaszewski@gmail.com" <jacek.anaszewski@gmail.com>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"bgolaszewski@baylibre.com" <bgolaszewski@baylibre.com>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
	"sboyd@kernel.org" <sboyd@kernel.org>,
	"lee.jones@linaro.org" <lee.jones@linaro.org>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"pavel@ucw.cz" <pavel@ucw.cz>
Subject: Re: [RFC PATCH v3 10/15] regulator: bd71828: Add GPIO based run-level control for regulators
Date: Mon, 4 Nov 2019 07:05:06 +0000	[thread overview]
Message-ID: <1550472ac1e105bd38da25803358cfbc0404bf38.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <CACRpkdZYw3QQcQ4h5y_C0UD6+4Wz9AdmQ0qSrrjfUweuJj8hyQ@mail.gmail.com>

Hello Linus,

On Sun, 2019-11-03 at 23:27 +0100, Linus Walleij wrote:
> On Fri, Nov 1, 2019 at 12:43 PM Matti Vaittinen
> <matti.vaittinen@fi.rohmeurope.com> wrote:
> 
> > Bucks 1,2,6 and 7 on ROHM BD71828 can be either controlled as
> > individual regulartors - or they can be grouped to a group of
> > regulators that are controlled by 'run levels'. This can be
> > done via I2C. Each regulator can be assigned a voltage and
> > enable/disable status for each run-level. These statuses are
> > also changeable via I2C.
> > 
> > Run-levels can then be changed either by I2C or GPIO. This
> > control mechanism is selected by data in one time programmable
> > area (during production) and can't be changed later.
> > 
> > Allow regulators to be controlled via run-levels and allow
> > getting/setting the current run-level also via GPIO.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> 
> I like the way you use the gpio API so FWIW:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Thanks. And I like the GPIO set multiple - that's required in order to
do some of the run-level changes without intermediate states. (Eg. both
DVS GPIOs need to be toggled via single register write).

> I do not understand the regulator parts of the patch.

I'm sorry. The patch is not clearest one what comes to the regulator
stuff. I can try splitting it to smaller and more logical changes if
you, Mark or other interested people hope to get it splitted. Or
perhaps it would be simplest to review if it was all in one patch? 

Rationale for splitting it in first place was that I hoped the basic
support (first two regulator patches) to be acceptable without huge
changes - whereas the follow up patches are more like question that how
the heck should I implement this :] I've not hit similar 'change bunch
of regulator states at one go' drivers/hardware before.

Br,
	Matti Vaittinen