linux-kernel - Re: [PATCH] Fix mt7622.dtsi thermal cpu

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a4e41929-6ab4-fabb-741e-f25a5fd14e3b@linaro.org>
Date:   Wed, 23 Jun 2021 17:58:29 +0200
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Eric Woudstra <ericwouds@...il.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix mt7622.dtsi thermal cpu

On 23/06/2021 17:35, Eric Woudstra wrote:
> It is only useful to set 1 map with the regulated temperature for cpu
> frequency throttling. Same as in the kernel document example.
> 
> 
> It has no use to set frequency scaling on 2 different temperature
> trip points, as the lowest one makes sure the higher one(s) are never
> reached.

I looked more closely the DT and there is a misunderstanding of the
thermal framework in the definition.

There is one trip point with the passive type and the cpu cooling
device, followed by a second trip point with the active type *but* the
same cpu cooling device. That is wrong.

And finally, there is the hot trip point as a third mapping and the same
cooling device.

The hot trip point is only there to notify userspace and let it take an
immediate action to prevent an emergency shutdown when reaching the
critical temperature.

> It can be applied only at 1 trip point. Multiple trip points
> is only usefully for fan control to make sure the fan is not too
> noisy when it is not necessary to be noisy.
> 
> 
> The CPU will almost come to a dead stop when it starts to pass the
> lowest thermal map with frequency throttling.
> 
> This is why it is a bug and needs a fix, not only adjustment.

Yes, you are right. It should be something like (verbatim copy):

diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
index 890a942ec608..88c81d24f4ff 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -136,24 +136,18 @@ secmon_reserved: secmon@...00000 {

 	thermal-zones {
 		cpu_thermal: cpu-thermal {
-			polling-delay-passive = <1000>;
+			polling-delay-passive = <250>;
 			polling-delay = <1000>;

 			thermal-sensors = <&thermal 0>;

 			trips {
 				cpu_passive: cpu-passive {
-					temperature = <47000>;
+					temperature = <77000>;
 					hysteresis = <2000>;
 					type = "passive";
 				};

-				cpu_active: cpu-active {
-					temperature = <67000>;
-					hysteresis = <2000>;
-					type = "active";
-				};
-
 				cpu_hot: cpu-hot {
 					temperature = <87000>;
 					hysteresis = <2000>;
@@ -173,18 +167,6 @@ map0 {
 					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
 							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
 				};
-
-				map1 {
-					trip = <&cpu_active>;
-					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
-
-				map2 {
-					trip = <&cpu_hot>;
-					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
-							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
-				};
 			};
 		};
 	};


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog