linux-kernel - Re: [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 27 Sep 2022 17:13:37 -0400
From:   Nícolas F. R. A. Prado 
        <nfraprado@...labora.com>
To:     Chen-Yu Tsai <wenst@...omium.org>
Cc:     Matthias Brugger <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>, kernel@...labora.com,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators

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@...labora.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@...labora.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@...00000 {
> >                 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>;
> > +       };
> > +
[..]