Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1643075547.git.benwolsieffer@gmail.com> <9f19df2a0017b71547445ac34df221e827c45bd0.1643075547.git.benwolsieffer@gmail.com>
 <YfDKTGQDh3tDMECz@builder.lan>
In-Reply-To: <YfDKTGQDh3tDMECz@builder.lan>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Sat, 29 Jan 2022 02:53:30 +0100
Message-ID: <CACRpkday7mRWPfirjY+VcS-tB8CwqWQCyunkTUh0om9DXF4F3g@mail.gmail.com>
Subject: Re: [PATCH 3/3] ARM: dts: qcom: basic HP TouchPad support
To:     Bjorn Andersson <bjorn.andersson@linaro.org>
Cc:     Ben Wolsieffer <benwolsieffer@gmail.com>,
        Andy Gross <agross@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>, Olof Johansson <olof@lixom.net>,
        soc@kernel.org, Stephen Boyd <sboyd@codeaurora.org>,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jan 26, 2022 at 5:13 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:

> @Linus, please take a look at the regulator question below.

OK!

> >  arch/arm/boot/dts/qcom-apq8060-tenderloin.dts | 549 ++----------------

Nice to see some use of all the work put into the APQ8060 support!

FYI: if you want graphics, this work may need to get finished:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/log/?h=apq8060-dragonboard-graphics
(mainly the MMCC)

> >               pinctrl@800000 {
> > -                     /* eMMMC pins, all 8 data lines connected */
>
> It would be nice if you could throw a separate patch on the list that
> fixes this spelling mistake in the original as well.

Yes pls.

> >                               l21 {
> > -                                     // 1.1 V according to schematic
> >                                       regulator-min-microvolt = <1200000>;
> >                                       regulator-max-microvolt = <1200000>;
> >                                       bias-pull-down;
> > -                                     regulator-always-on;
> > +                                     /*
> > +                                      * RPM driver can't handle always-on regulators that are
> > +                                      * supplied by regulators initialized after them.
> > +                                      */
>
> That looks like an oversight that should be corrected, perhaps it needs
> similar attention that was given to the smd-rpm driver recently?

Indeed

> But this makes me wonder, how can this work on the other board? Linus?

I suppose these don't supply anything vital?

I know I have seen some regulators switch off and on but I
may have been confused.


> >                               s0 {
> > -                                     // regulator-min-microvolt = <500000>;
> > +                                     // regulator-min-microvolt = <800000>;
> >                                       // regulator-max-microvolt = <1325000>;
>
> This looks like the full range the regulator could do, do you see a
> reason for documenting that here? Unless there's a good reason I think
> you should leave the commented min/max out.

Yours,
Linus Walleij