linux-kernel - Re: [PATCH] Update H700 opp values

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: <5b07cd47-868d-46a3-b365-ccac2a68b792@arm.com>
Date: Thu, 28 Nov 2024 23:23:31 +0000
From: Andre Przywara <andre.przywara@....com>
To: Philippe Simons <simons.philippe@...il.com>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
 Jernej Skrabec <jernej.skrabec@...il.com>,
 Samuel Holland <samuel@...lland.org>,
 "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
 <devicetree@...r.kernel.org>,
 "moderated list:ARM/Allwinner sunXi SoC support"
 <linux-arm-kernel@...ts.infradead.org>,
 "open list:ARM/Allwinner sunXi SoC support" <linux-sunxi@...ts.linux.dev>,
 open list <linux-kernel@...r.kernel.org>
Cc: Chris Morgan <macromorgan@...mail.com>, Ryan Walklin <ryan@...ttoast.com>
Subject: Re: [PATCH] Update H700 opp values

Hi Philippe,

thanks for taking care of sending a patch!
You would need prefixes on the subject line to point to the right 
subsystem. A "git log path/to/changed_file" would give you the prefix of 
previous patches, for you to copy from. In this case it would be:
arm64: dts: allwinner: h616: update H700 OPP values

On 28/11/2024 15:45, Philippe Simons wrote:
> My H700 (RG35XX-H, RG40XX-V and RG CubeXX) devices are very unstable,
> especially with some OPPs.
> Crashes were fairly easy to reproduce with any dynamic cpufreq governor
> and some load on CPU, usually in matter of minutes.
> Crashes manifested randomly as simply hanging or various kernel oops
> 
> Manufacturer (Anbernic) is using more conservative mircrovolt values,
> so let's use these.
> While using performance gov seems stables at 1.5Ghz, it still crashes
> using a dynamic gov (even with Andre reparenting patch), so let's drop
> it for now, like manufacturer does.
> 
> Signed-off-by: Philippe Simons <simons.philippe@...il.com

The change itself looks alright, bumping the OPP voltages by 50mV looks 
reasonable, and is a no-brainer if it stops the issues you have seen.

The loss of the 1.5GHz top OPP is a bummer, but stability comes 
definitely first, otherwise we will just fail faster ;-)

I would like to hear opinions from Ryan and Chris (both CC:ed). Did you 
see similar issues with say the ondemand governor?

The patch itself looks correct, so for that:

Reviewed-by: Andre Przywara <andre.przywara@....com>

Cheers,
Andre

> ---
>   .../dts/allwinner/sun50i-h616-cpu-opp.dtsi    | 19 +++++++++----------
>   1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi
> index dd10aaf47..ac13fe169 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616-cpu-opp.dtsi
> @@ -50,24 +50,21 @@ opp-1008000000 {
>   			opp-microvolt-speed2 = <950000>;
>   			opp-microvolt-speed3 = <950000>;
>   			opp-microvolt-speed4 = <1020000>;
> -			opp-microvolt-speed5 = <900000>;
> +			opp-microvolt-speed5 = <950000>;
>   			clock-latency-ns = <244144>; /* 8 32k periods */
>   			opp-supported-hw = <0x3f>;
>   		};
>   
>   		opp-1032000000 {
>   			opp-hz = /bits/ 64 <1032000000>;
> -			opp-microvolt = <900000>;
> +			opp-microvolt = <950000>;
>   			clock-latency-ns = <244144>; /* 8 32k periods */
>   			opp-supported-hw = <0x20>;
>   		};
>   
>   		opp-1104000000 {
>   			opp-hz = /bits/ 64 <1104000000>;
> -			opp-microvolt-speed0 = <1000000>;
> -			opp-microvolt-speed2 = <1000000>;
> -			opp-microvolt-speed3 = <1000000>;
> -			opp-microvolt-speed5 = <950000>;
> +			opp-microvolt = <1000000>;			
>   			clock-latency-ns = <244144>; /* 8 32k periods */
>   			opp-supported-hw = <0x2d>;
>   		};
> @@ -79,7 +76,7 @@ opp-1200000000 {
>   			opp-microvolt-speed2 = <1050000>;
>   			opp-microvolt-speed3 = <1050000>;
>   			opp-microvolt-speed4 = <1100000>;
> -			opp-microvolt-speed5 = <1020000>;
> +			opp-microvolt-speed5 = <1050000>;
>   			clock-latency-ns = <244144>; /* 8 32k periods */
>   			opp-supported-hw = <0x3f>;
>   		};
> @@ -93,7 +90,10 @@ opp-1320000000 {
>   
>   		opp-1416000000 {
>   			opp-hz = /bits/ 64 <1416000000>;
> -			opp-microvolt = <1100000>;
> +			opp-microvolt-speed0 = <1100000>;
> +			opp-microvolt-speed2 = <1100000>;
> +			opp-microvolt-speed3 = <1100000>;
> +			opp-microvolt-speed5 = <1160000>;
>   			clock-latency-ns = <244144>; /* 8 32k periods */
>   			opp-supported-hw = <0x2d>;
>   		};
> @@ -102,9 +102,8 @@ opp-1512000000 {
>   			opp-hz = /bits/ 64 <1512000000>;
>   			opp-microvolt-speed1 = <1100000>;
>   			opp-microvolt-speed3 = <1100000>;
> -			opp-microvolt-speed5 = <1160000>;
>   			clock-latency-ns = <244144>; /* 8 32k periods */
> -			opp-supported-hw = <0x2a>;
> +			opp-supported-hw = <0x0a>;
>   		};
>   	};
>   };