Re: [PATCH 1/3] regulator: core: fix boot-on regulators use_count usage

From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "broonie@kernel.org" <broonie@kernel.org>
Cc: "lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"dianders@chromium.org" <dianders@chromium.org>,
	"m.felsch@pengutronix.de" <m.felsch@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"zhang.chunyan@linaro.org" <zhang.chunyan@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"ckeepax@opensource.cirrus.com" <ckeepax@opensource.cirrus.com>
Subject: Re: [PATCH 1/3] regulator: core: fix boot-on regulators use_count usage
Date: Fri, 4 Oct 2019 12:03:12 +0000	[thread overview]
Message-ID: <054bc4c050f1b16988de057f812232b0feb707cb.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <20191004113234.GA4866@sirena.co.uk>

On Fri, 2019-10-04 at 12:32 +0100, Mark Brown wrote:
> On Fri, Oct 04, 2019 at 09:34:43AM +0300, Matti Vaittinen wrote:
> > On Tue, Oct 01, 2019 at 12:57:31PM -0700, Doug Anderson wrote:
> > > I don't think your fix is correct.  It sounds as if the intention
> > > of
> > > "regulator-boot-on" is to have the OS turn the regulator on at
> > > bootup
> > > and it keep an implicit reference until someone explicitly tells
> > > the
> > > OS to drop the reference.
> > Hmm.. What is the intended way to explicitly tell the OS to drop
> > the
> > reference? I would assume we should still use same logic as with
> > other
> > regulators - if last user calls regulator_disable() we should
> > disable
> > the regulator? (I may not understand all this well enough though)
> 
> Yes.
> 
> > > It's a fixed regulator controlled by a GPIO?  Presumably the GPIO
> > > can
> > > be read.  That would mean it ideally shouldn't be using
> > > "regulator-boot-on" since this is _not_ a regulator whose
> > > software
> > > state can't be read.  Just remove the property.
> > How should we handle cases where we want OS to enable regulator at
> > boot-up - possibly before consumer drivers can be load?
> 
> If you want the regulator to be on without any driver present then
> mark
> it always-on.  If you want the regulator to be enabled prior to the
> driver being loaded then the expectation is that the bootloader will
> do
> that, it's difficult to see what the benefit there is from having the
> kernel enable things when it starts prior to having a driver unless
> the
> intent is to keep the regulator always on.

I thought the regulator-boot-on could have been used for that. But as I
said - I don't really know all this so well =) And no, I am not opposed
to offloading this from kernel to boot, I was just trying to learn what
is the correct thing to do (tm). Thanks for educating me on this :) I
will suggest adding the enabling to boot code if (when) I get questions
concerning this. (always-on won't do for regulators which need to be
controlled for power saving or heating issues).

Br,
	Matti Vaittinen