linux-kernel - Re: [PATCH v3 2/2] ARM: dts: sun8i-r40: Add "cpu-supply" node for sun8i-r40 based board

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]

Message-ID: <20220513081556.knd3tf6pangfblh3@houat>
Date:   Fri, 13 May 2022 10:15:56 +0200
From:   Maxime Ripard <maxime@...no.tech>
To:     qianfan <qianfanguijin@....com>
Cc:     linux-sunxi@...ts.linux.dev, Rob Herring <robh+dt@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v3 2/2] ARM: dts: sun8i-r40: Add "cpu-supply" node for
 sun8i-r40 based board

On Fri, May 13, 2022 at 03:48:50PM +0800, qianfan wrote:
> 
> 
> 在 2022/5/13 15:38, Maxime Ripard 写道:
> > Hi,
> > 
> > On Thu, May 12, 2022 at 03:18:58PM +0800, qianfanguijin@....com wrote:
> > > From: qianfan Zhao <qianfanguijin@....com>
> > > 
> > > sun8i-r40 actived cpufreq feature now, let's add "cpu-supply" node on
> > > board.
> > > 
> > > Signed-off-by: qianfan Zhao <qianfanguijin@....com>
> > > ---
> > >   arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 4 ++++
> > >   arch/arm/boot/dts/sun8i-r40-feta40i.dtsi          | 4 ++++
> > >   arch/arm/boot/dts/sun8i-t3-cqa3t-bv3.dts          | 4 ++++
> > >   arch/arm/boot/dts/sun8i-v40-bananapi-m2-berry.dts | 4 ++++
> > >   4 files changed, 16 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> > > index a6a1087a0c9b..4f30018ec4a2 100644
> > > --- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> > > +++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> > > @@ -113,6 +113,10 @@ &ahci {
> > >   	status = "okay";
> > >   };
> > > +&cpu0 {
> > > +	cpu-supply = <&reg_dcdc2>;
> > > +};
> > > +
> > This will break bisection on those boards. Indeed, you added the OPPs on
> > the first patch, and if you only apply that patch, the boards in the
> > second patch will be missing their CPU regulator. The kernel will then
> > ramp up the frequency to the highest OPP, but will not change the
> > voltage, resulting in a crash.
>
> This is a good point and I will merge those two patch.

That's not what I meant to say. Those two patches are great separated.
You can invert them though.

> > There's a similar issue for all the boards that don't have a regulator
> > in the first place.
> > 
> > The way we worked around this for the other SoCs is to have a DTSI with
> > the OPPs with a frequency higher than what U-Boot boots with (1008MHz?),
> > and only include that DTSI on boards that have a CPU regulator hooked in.
>
> Is this really necessary? It seems like every board based on sun8i-r40
> have a cpu regulator.

This probably won't be the case whenever someone starts a new design,
and then they'll face random crashes for no apparent reason, and waste a
lot of time in the process.

Whereas the alternative is that you would be missing some OPPs,
something that is fairly easy to figure out.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)