Re: [PATCH] dt-bindings: arm: sunxi: update H2+/H3 cpu clocks

[Maxime Ripard]

Hi!

Thanks for contributing

The prefix isn't right though.

dt-bindings is used when you're modifying the binding itself, ie the
description of what the node is supposed to look like, not when you
actually use that node in a DT.

In that case, that would be ARM: dts: sunxi:

(we're on the ARM architecture, modifying dts's, for the sunxi platform)

On Thu, Sep 03, 2020 at 12:07:08PM +0200, Wilken Gottwalt wrote:
> Change H2+/H3 clocks to 8 steps from 528 MHz up to 1200 MHz to support a
> more fine-grained powersave setup. The SoCs are made for 1296 MHz, so
> these clocks are still in a safe range. Tested on a NanoPi Duo and
> OrangePi Zero.

How was this tested?

cpufreq OPP misconfiguration on Allwinner SoCs has been known to create
some errors that are fairly hard to spot and be quite easy to go
unnoticed (like caches corruptions).

The only reliable test we have is:
https://github.com/ssvb/cpuburn-arm/blob/master/cpufreq-ljt-stress-test

> Signed-off-by: Wilken Gottwalt <wilken.gottwalt@xxxxxxxxxxx>
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 34 +++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 4e89701df91f..5517fcc02b7d 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -48,23 +48,53 @@ cpu0_opp_table: opp_table0 {
>  		compatible = "operating-points-v2";
>  		opp-shared;
>  
> -		opp-648000000 {
> -			opp-hz = /bits/ 64 <648000000>;
> +		opp-528000000 {
> +			opp-hz = /bits/ 64 <528000000>;
> +			opp-microvolt = <1020000 1020000 1300000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp-624000000 {
> +			opp-hz = /bits/ 64 <624000000>;
>  			opp-microvolt = <1040000 1040000 1300000>;
>  			clock-latency-ns = <244144>; /* 8 32k periods */
>  		};
>  
> +		opp-720000000 {
> +			opp-hz = /bits/ 64 <720000000>;
> +			opp-microvolt = <1060000 1060000 1300000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
>  		opp-816000000 {
>  			opp-hz = /bits/ 64 <816000000>;
>  			opp-microvolt = <1100000 1100000 1300000>;
>  			clock-latency-ns = <244144>; /* 8 32k periods */
>  		};
>  
> +		opp-912000000 {
> +			opp-hz = /bits/ 64 <912000000>;
> +			opp-microvolt = <1140000 1140000 1300000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
>  		opp-1008000000 {
>  			opp-hz = /bits/ 64 <1008000000>;
>  			opp-microvolt = <1200000 1200000 1300000>;
>  			clock-latency-ns = <244144>; /* 8 32k periods */
>  		};
> +
> +		opp-1104000000 {
> +			opp-hz = /bits/ 64 <1104000000>;
> +			opp-microvolt = <1240000 1240000 1300000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
> +
> +		opp-1200000000 {
> +			opp-hz = /bits/ 64 <1200000000>;
> +			opp-microvolt = <1300000 1300000 1300000>;
> +			clock-latency-ns = <244144>; /* 8 32k periods */
> +		};
>  	};

IIRC U-Boot will start the CPU at 1008 MHz on the H3, so in the
situation where a board doesn't list the regulators attached to the CPU,
the kernel will happily use the 1104 and 1200 MHz frequencies, even
though it won't adjust the voltage accordingly, causing errors.

The way we worked around that is to provide the extra OOPs in a separate
DTSI so that the boards with regulator support can opt-in.

Maxime

>  
>  	cpus {
> -- 
> 2.28.0
>

Attachment: signature.asc
Description: PGP signature