Received-SPF: pass (google.com: domain of linux-kernel+bounces-179592-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 15 May 2024 15:38:30 +0800
From: Alina Yu <alina_yu@richtek.com>
To: Conor Dooley <conor@kernel.org>
CC: Mark Brown <broonie@kernel.org>, <lgirdwood@gmail.com>,
	<robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
	<conor+dt@kernel.org>, <linux-kernel@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <johnny_lai@richtek.com>,
	<cy_huang@richtek.com>
Subject: Re: [PATCH v3 6/6] regulator: dt-bindings: rtq2208: Add property to
 get ldo of RTQ2208 is adjustable or not
Message-ID: <20240515073830.GA12525@linuxcarl2.richtek.com>
References: <cover.1715340537.git.alina_yu@richtek.com>
 <6a3a90d9aa2022dfb92e124e417f3e72c2f28b0b.1715340537.git.alina_yu@richtek.com>
 <20240513-tissue-repave-13d2e3bf88fd@spud>
 <d97752ed-4032-4681-b28f-17f149fdc3d4@sirena.org.uk>
 <20240514-plunging-chair-803d9e342e6f@spud>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240514-plunging-chair-803d9e342e6f@spud>
User-Agent: Mutt/1.5.21 (2010-09-15)

On Tue, May 14, 2024 at 07:01:21PM +0100, Conor Dooley wrote:
> On Tue, May 14, 2024 at 11:34:29AM +0100, Mark Brown wrote:
> > On Mon, May 13, 2024 at 05:22:54PM +0100, Conor Dooley wrote:
> > > On Fri, May 10, 2024 at 08:06:25PM +0800, Alina Yu wrote:
> > 
> > > > +            richtek,fixed-microvolt = <1200000>;
> > > >              regulator-min-microvolt = <1200000>;
> > > >              regulator-max-microvolt = <1200000>;
> > 
> > > I'm dumb and this example seemed odd to me. Can you explain to me why
> > > it is not sufficient to set min-microvolt == max-microvolt to achieve
> > > the same thing?
> > 
> > This is for a special mode where the voltage being configured is out of
> > the range usually supported by the regulator, requiring a hardware
> > design change to achieve.  The separate property is because otherwise we
> > can't distinguish the case where the mode is in use from the case where
> > the constraints are nonsense, and we need to handle setting a fixed
> > voltage on a configurable regulator differently to there being a
> > hardware fixed voltage on a normally configurable regulator.
> 
> Cool, I think an improved comment message and description would be
> helpful then to describe the desired behaviour that you mention here.
> The commit message in particular isn't great:
> | Since there is no way to check is ldo is adjustable or not.
> | As discussing in v2 series, 'richtek,fixed-microvolt' is added for that.
> | user is supposed to know whether vout of ldo is adjustable.
> 
> It also doesn't seem like this sort of behaviour would be limited to
> Richtek either, should this actually be a common property in
> regulator.yaml w/o the vendor prefix?
> 
> Cheers,
> Conor.


Hi Conor,


Should I update v4 to fix the commit message ?
I will modify it as follows.


There are two types of LDO VOUT: fixed voltage mode and adjustable voltage mode.

As the fixed voltage for the LDO is outside the range of the adjustable voltage mode,
the constraints for this scenario are not suitable to represent both modes.

In version 3, a property has been added to specify the fixed voltage.


Thanks,
Alina