Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 27 Sep 2022 17:13:37 -0400
From:   =?utf-8?B?TsOtY29sYXMgRi4gUi4gQS4=?= Prado 
        <nfraprado@collabora.com>
To:     Chen-Yu Tsai <wenst@chromium.org>
Cc:     Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>, kernel@collabora.com,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators
Message-ID: <20220927211337.idgcuawo7xsiexah@notapiano>
References: <20220908171153.670762-1-nfraprado@collabora.com>
 <20220908171153.670762-2-nfraprado@collabora.com>
 <CAGXv+5FErSBT-t6vz_2naApuPoC4PympWft-9Gd_MMPUTN+CsQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAGXv+5FErSBT-t6vz_2naApuPoC4PympWft-9Gd_MMPUTN+CsQ@mail.gmail.com>
Precedence: bulk

Hi Chen-Yu,

thank you for the insights on the power supply hierarchy. Just a couple further
questions below.

On Wed, Sep 21, 2022 at 10:20:43PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Fri, Sep 9, 2022 at 1:12 AM N?colas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
> >
> > Add the regulators present on the Asurada platform that are used to
> > power the internal and external displays.
> >
> > Signed-off-by: N?colas F. R. A. Prado <nfraprado@collabora.com>
> >
> > ---
> >
> >  .../boot/dts/mediatek/mt8192-asurada.dtsi     | 114 ++++++++++++++++++
> >  1 file changed, 114 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> > index 4b314435f8fd..1d99e470ea1a 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> > @@ -23,6 +23,42 @@ memory@40000000 {
> >                 reg = <0 0x40000000 0 0x80000000>;
> >         };
> >
> > +       pp1000_dpbrdg: regulator-1v0-dpbrdg {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "pp1000_dpbrdg";
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&pp1000_dpbrdg_en_pins>;
> > +               regulator-min-microvolt = <1000000>;
> > +               regulator-max-microvolt = <1000000>;
> 
> This is fed by a rail called PP1350_VS2, which is from the MT6359 PMIC.
> And this regulator is a proper LDO.

So, we should have an additional regulator node here called pp1350_vs2 which
will feed into pp1000_dpbrdg and that is itself fed in from mt6359_vs2_buck_reg
(from mt6359.dtsi). Is that right?

Also, is PP1350_VS2 just a simple switch or an LDO?

> 
[..]
> > +       pp1800_dpbrdg: regulator-1v8-dpbrdg {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "pp1800_dpbrdg";
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&pp1800_dpbrdg_en_pins>;
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <1800000>;
> 
> This regulator is only a power switch. Please drop the min/max properties.
> This is fed by a rail called PP1800_VIO18_U, which is from an LDO on the
> MT6359 PMIC.

Similarly, we should have a pp1800_vio18_u node that is fed in by
mt6359_vio18_ldo_reg, right? And is it a switch or an LDO?

Thanks,
N?colas

> 
> > +               enable-active-high;
> > +               regulator-boot-on;
> > +               gpio = <&pio 126 GPIO_ACTIVE_HIGH>;
> > +       };
> > +
[..]