Linux-Devicetree Archive mirror
 help / color / mirror / Atom feed
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
Date: Wed, 15 May 2024 15:38:30 +0800	[thread overview]
Message-ID: <20240515073830.GA12525@linuxcarl2.richtek.com> (raw)
In-Reply-To: <20240514-plunging-chair-803d9e342e6f@spud>

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

  reply	other threads:[~2024-05-15  7:38 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-10 12:06 [PATCH v3 0/6] Fix rtq2208 BUCK ramp_delay and LDO dvs setting Alina Yu
2024-05-10 12:06 ` [PATCH v3 1/6] regulator: rtq2208: Fix invalid memory access when devm_of_regulator_put_matches is called Alina Yu
2024-05-15 15:47   ` Mark Brown
2024-05-27  9:31     ` Alina Yu
2024-05-10 12:06 ` [PATCH v3 2/6] regulator: rtq2208: Fix LDO vsel setting Alina Yu
2024-05-10 12:06 ` [PATCH v3 3/6] regulator: rtq2208: Fix LDO discharge register Alina Yu
2024-05-10 12:06 ` [PATCH v3 4/6] regulator: rtq2208: Fix the BUCK ramp_delay range to maximum of 16mVstep/us Alina Yu
2024-05-10 12:06 ` [PATCH v3 5/6] regulator: rtq2208: Fix LDO to be compatible with both fixed and adjustable vout Alina Yu
2024-05-10 12:06 ` [PATCH v3 6/6] regulator: dt-bindings: rtq2208: Add property to get ldo of RTQ2208 is adjustable or not Alina Yu
2024-05-13 16:22   ` Conor Dooley
2024-05-14 10:34     ` Mark Brown
2024-05-14 18:01       ` Conor Dooley
2024-05-15  7:38         ` Alina Yu [this message]
2024-05-15  8:06           ` Conor Dooley
2024-05-15  9:02             ` Alina Yu
2024-05-15 15:51               ` Conor Dooley
2024-05-15 16:04                 ` Mark Brown
2024-05-15 16:16                   ` Conor Dooley
2024-05-15 12:10         ` Mark Brown
2024-05-15 15:48           ` Conor Dooley
2024-05-29 11:21 ` (subset) [PATCH v3 0/6] Fix rtq2208 BUCK ramp_delay and LDO dvs setting Mark Brown

Reply instructions:

You may reply publicly 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=20240515073830.GA12525@linuxcarl2.richtek.com \
    --to=alina_yu@richtek.com \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=conor@kernel.org \
    --cc=cy_huang@richtek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=johnny_lai@richtek.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).