linux-kernel - Re: [PATCH v4 5/6] arm64: dts: allwinner: a100: Add Add CPU Operating Performance Points table

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]

Message-ID: <20201208095123.guu7zqskxckd5vsc@gilmour>
Date:   Tue, 8 Dec 2020 10:51:23 +0100
From:   Maxime Ripard <maxime@...no.tech>
To:     Shuosheng Huang <huangshuosheng@...winnertech.com>
Cc:     robh+dt@...nel.org, wens@...e.org, jernej.skrabec@...l.net,
        tiny.windzz@...il.com, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 5/6] arm64: dts: allwinner: a100: Add Add CPU
 Operating Performance Points table

On Tue, Dec 08, 2020 at 03:20:46PM +0800, Shuosheng Huang wrote:
> Add an Operating Performance Points table for the CPU cores to
> enable Dynamic Voltage & Frequency Scaling on the A100.
> 
> Signed-off-by: Shuosheng Huang <huangshuosheng@...winnertech.com>
> ---
>  .../allwinner/sun50i-a100-allwinner-perf1.dts |  1 +
>  .../dts/allwinner/sun50i-a100-cpu-opp.dtsi    | 90 +++++++++++++++++++
>  2 files changed, 91 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a100-cpu-opp.dtsi
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100-allwinner-perf1.dts b/arch/arm64/boot/dts/allwinner/sun50i-a100-allwinner-perf1.dts
> index d34c2bb1079f..301793c72cb7 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a100-allwinner-perf1.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a100-allwinner-perf1.dts
> @@ -6,6 +6,7 @@
>  /dts-v1/;
>  
>  #include "sun50i-a100.dtsi"
> +#include "sun50i-a100-cpu-opp.dtsi"

This should be on the next patch

>  /{
>  	model = "Allwinner A100 Perf1";
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100-cpu-opp.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a100-cpu-opp.dtsi
> new file mode 100644
> index 000000000000..e245823d70e8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a100-cpu-opp.dtsi
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +// Copyright (c) 2020 Yangtao Li <frank@...winnertech.com>
> +// Copyright (c) 2020 ShuoSheng Huang <huangshuosheng@...winnertech.com>
> +
> +/ {
> +	cpu_opp_table: cpu-opp-table {
> +		compatible = "allwinner,sun50i-h6-operating-points";
> +		nvmem-cells = <&cpu_speed_grade>;
> +		opp-shared;
> +
> +		opp@...000000 {

This node name will create a DTC warning if compiled with W=1, since the
unit-address doesn't match the reg property (there's none), and it's
supposed to.

> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <408000000>;
> +
> +			opp-microvolt-speed0 = <900000 900000 1200000>;
> +			opp-microvolt-speed1 = <900000 900000 1200000>;
> +			opp-microvolt-speed2 = <900000 900000 1200000>;
> +		};
> +
> +		opp@...000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <600000000>;
> +
> +			opp-microvolt-speed0 = <900000 900000 1200000>;
> +			opp-microvolt-speed1 = <900000 900000 1200000>;
> +			opp-microvolt-speed2 = <900000 900000 1200000>;
> +		};
> +
> +		opp@...000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <816000000>;
> +
> +			opp-microvolt-speed0 = <940000 940000 1200000>;
> +			opp-microvolt-speed1 = <900000 900000 1200000>;
> +			opp-microvolt-speed2 = <900000 900000 1200000>;
> +		};
> +
> +		opp@...0000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1080000000>;
> +
> +			opp-microvolt-speed0 = <1020000 1020000 1200000>;
> +			opp-microvolt-speed1 = <980000 980000 1200000>;
> +			opp-microvolt-speed2 = <950000 950000 1200000>;
> +		};
> +
> +		opp@...0000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1200000000>;
> +
> +			opp-microvolt-speed0 = <1100000 1100000 1200000>;
> +			opp-microvolt-speed1 = <1020000 1020000 1200000>;
> +			opp-microvolt-speed2 = <1000000 1000000 1200000>;
> +		};
> +
> +		opp@...0000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1320000000>;
> +
> +			opp-microvolt-speed0 = <1160000 1160000 1200000>;
> +			opp-microvolt-speed1 = <1060000 1060000 1200000>;
> +			opp-microvolt-speed2 = <1030000 1030000 1200000>;
> +		};
> +
> +		opp@...4000000 {
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +			opp-hz = /bits/ 64 <1464000000>;
> +
> +			opp-microvolt-speed0 = <1180000 1180000 1200000>;
> +			opp-microvolt-speed1 = <1180000 1180000 1200000>;
> +			opp-microvolt-speed2 = <1130000 1130000 1200000>;
> +		};
> +	};
> +};

Can you run cpufreq-ljt-stress-test from
https://github.com/ssvb/cpuburn-arm and paste the result in the cover
letter to make sure all the OPPs are working fine?

Also, at what frequency is the bootloader expected to set the CPU when
booting? If it's anything lower than 1464MHz, we should separate the
OPPs higher than the initial frequency.

A board without the regulator set would try to use those OPPs without
raising the voltage of the CPU, resulting in fairly hard to debug
crashes.

Maxime

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