linux-kernel - Re: [PATCH] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the default supply for pp3300

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 5 Nov 2020 07:57:42 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Matthias Kaehlcke <mka@...omium.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arm64: dts: qcom: sc7180-trogdor: Make pp3300_a the
 default supply for pp3300_hub

Hi,

On Wed, Nov 4, 2020 at 5:55 PM Matthias Kaehlcke <mka@...omium.org> wrote:
>
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > index bf875589d364..2d64e75a2d6d 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > @@ -174,6 +174,21 @@ pp3300_fp_tp: pp3300-fp-tp-regulator {
> > >                 vin-supply = <&pp3300_a>;
> > >         };
> > >
> > > +       pp3300_hub: pp3300-hub {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "pp3300_hub";
> > > +
> > > +               regulator-min-microvolt = <3300000>;
> > > +               regulator-max-microvolt = <3300000>;
> > > +
> > > +               gpio = <&tlmm 84 GPIO_ACTIVE_HIGH>;
> > > +               enable-active-high;
> > > +               pinctrl-names = "default";
> > > +               pinctrl-0 = <&en_pp3300_hub>;
> > > +
> > > +               vin-supply = <&pp3300_a>;
> >
> > You're leaving things in a bit of an inconsistent state here.  The
> > "pp3300_hub_7c" is always_on / boot_on.  This new one isn't.
>
> Actually the new "pp3300_hub" it is also on at boot, the Chrome OS bootloader
> asserts the GPIO.
>
> > I know this is slightly more complicated due to the fact that downstream we
> > have a way to control the hub power but didn't quite get that resolved
> > upstream, but the way you have it now, on new hardware upstream will
> > power off the hub but also keep "pp3300_hub_7c" powered on for no
> > reason.  Seems like that should be fixed?
>
> Our EEs told me that it would be ok in terms of power to keep "pp3300_hub_7c"
> powered, since there would be no significant power consumption without load.
>
> In any case unused RPMH regulators are switched off by the kernel ~30s after
> boot, so I think we are ok:
>
> [   31.202219] ldo7: disabling
>
> The above is from the l7c regulator on a Lazor rev2.

I assume this is with the downstream codebase, though?  With what you
have posted upstream I don't think ldo7 will ever get disabled because
it's marked "always-on"?

Similarly, with what you've posted upstream I think your new
"pp3300_hub" _will_ get disabled ~30 seconds after boot because it's
not marked "always-on" and it has no clients.

-Doug