linux-kernel - Re: [PATCH] arm64: dts: sc7180: Add board regulators for MIPI camera trogdor boards

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YbtX7/fd61OCnv8r@google.com>
Date:   Thu, 16 Dec 2021 07:14:55 -0800
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH] arm64: dts: sc7180: Add board regulators for MIPI camera
 trogdor boards

On Wed, Dec 15, 2021 at 08:43:21PM -0800, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2021-12-15 18:31:41)
> > On Tue, Dec 14, 2021 at 04:36:38PM -0800, Stephen Boyd wrote:
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > index d4f4441179fc..1dd8e35093a8 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > @@ -113,6 +113,40 @@ src_vph_pwr: src-vph-pwr-regulator {
> > >               vin-supply = <&ppvar_sys>;
> > >       };
> > >
> > > +     pp1800_uf_cam: pp1800-uf-cam-regulator {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "pp1800_uf_cam";
> > > +             status = "disabled";
> > > +
> > > +             regulator-min-microvolt = <1800000>;
> > > +             regulator-max-microvolt = <1800000>;
> > > +
> > > +             gpio = <&tlmm 6 GPIO_ACTIVE_HIGH>;
> > > +             enable-active-high;
> > > +             pinctrl-names = "default";
> > > +             pinctrl-0 = <&uf_cam_en>;
> > > +
> > > +             vin-supply = <&pp1800_ldo>;
> > > +             regulator-enable-ramp-delay = <1000>;
> > > +     };
> > > +
> > > +     pp1800_wf_cam: pp1800-wf-cam-regulator {
> > > +             compatible = "regulator-fixed";
> > > +             regulator-name = "pp1800_wf_cam";
> > > +             status = "disabled";
> > > +
> > > +             regulator-min-microvolt = <1800000>;
> > > +             regulator-max-microvolt = <1800000>;
> > > +
> > > +             gpio = <&tlmm 7 GPIO_ACTIVE_HIGH>;
> > > +             enable-active-high;
> > > +             pinctrl-names = "default";
> > > +             pinctrl-0 = <&wf_cam_en>;
> > > +
> > > +             vin-supply = <&pp1800_ldo>;
> > > +             regulator-enable-ramp-delay = <1000>;
> > > +     };
> > > +
> >
> > Shouldn't 'pp1800_ldo' be defined before these ("FIXED REGULATORS
> > - parents above children")?
> 
> Good catch! Fixed it.
> 
> >
> > I suggest to move these two below the top level regulators, i.e.
> > somwhere after pp3300_a (probably pp3300_a and pp5000_a should be
> > swapped, but that's beyond the scope of this patch).
> >
> > >       pp5000_a: pp5000-a-regulator {
> > >               compatible = "regulator-fixed";
> > >               regulator-name = "pp5000_a";
> > > @@ -1517,4 +1611,32 @@ pinconf-sd-cd {
> > >                       drive-strength = <2>;
> > >               };
> > >       };
> > > +
> > > +     uf_cam_en: uf-cam-en {
> > > +             pinmux {
> > > +                     pins = "gpio6";
> > > +                     function = "gpio";
> > > +             };
> > > +
> > > +             pinconf {
> > > +                     pins = "gpio6";
> > > +                     drive-strength = <2>;
> > > +                     /* External pull down */
> >
> > Is there actually an external pull down?
> 
> My understanding is that there's an internal pull down in the LDO so
> while it isn't exactly "external" in the sense there's a pull down on
> the net via a resistor, there's still a pull down that you can't see in
> the schematic in the IC.

Thanks for the clarification!