Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
In-Reply-To: <a4e41929-6ab4-fabb-741e-f25a5fd14e3b@linaro.org>
References: <20210619121927.32699-1-ericwouds@gmail.com> <e30a2d01-a200-80cb-88d9-6aea62dd49f1@linaro.org> <56fb5540-fb86-4e6a-a596-1276026b37e5@gmail.com> <a4e41929-6ab4-fabb-741e-f25a5fd14e3b@linaro.org>
Thread-Topic: Re: [PATCH] Fix mt7622.dtsi thermal cpu
User-Agent: Android
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
 charset=UTF-8
Subject: Re: [PATCH] Fix mt7622.dtsi thermal cpu
From:   Eric Woudstra <ericwouds@gmail.com>
Date:   Wed, 23 Jun 2021 20:43:15 +0200
To:     Daniel Lezcano <daniel.lezcano@linaro.org>
CC:     Rob Herring <robh+dt@kernel.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org
Message-ID: <47261865-00e3-41eb-bb36-2b939f81f1e8@gmail.com>
Precedence: bulk


I choose "hot" before, because 87 degrees seems ok to start frequency thro=
ttling=2E But, yes, it should be passive=2E

87 is still quite low if I com=
pare this temperature with the wrt3200acm Marvell dual core arm soc=2E They=
 even went above 100 degrees so I feel for an arm processor inside a router=
 box it is fine to use 87 degrees But maybe someone at Mediatek can give so=
me more details about operating temperatures=2E

It may be possible to leav=
e the active map in the device tree as some users of the bananapi might cho=
ose to install a fan as it is one of the options=2E

=E2=81=A3Get BlueMail =
for Android =E2=80=8B

On Jun 23, 2021, 5:58 PM, at 5:58 PM, Daniel Lezcano=
 <daniel=2Elezcano@linaro=2Eorg> wrote:
>On 23/06/2021 17:35, Eric Woudstra=
 wrote:
>> It is only useful to set 1 map with the regulated temperature fo=
r cpu
>> frequency throttling=2E Same as in the kernel document example=2E
=
>> 
>> 
>> It has no use to set frequency scaling on 2 different temperatur=
e
>> trip points, as the lowest one makes sure the higher one(s) are never
=
>> reached=2E
>
>I looked more closely the DT and there is a misunderstandi=
ng of the
>thermal framework in the definition=2E
>
>There is one trip poin=
t with the passive type and the cpu cooling
>device, followed by a second t=
rip point with the active type *but* the
>same cpu cooling device=2E That i=
s wrong=2E
>
>And finally, there is the hot trip point as a third mapping a=
nd the
>same
>cooling device=2E
>
>The hot trip point is only there to noti=
fy userspace and let it take an
>immediate action to prevent an emergency s=
hutdown when reaching the
>critical temperature=2E
>
>> It can be applied o=
nly at 1 trip point=2E Multiple trip points
>> is only usefully for fan con=
trol to make sure the fan is not too
>> noisy when it is not necessary to b=
e noisy=2E
>> 
>> 
>> The CPU will almost come to a dead stop when it start=
s to pass the
>> lowest thermal map with frequency throttling=2E
>> 
>> Thi=
s is why it is a bug and needs a fix, not only adjustment=2E
>
>Yes, you ar=
e right=2E It should be something like (verbatim copy):
>
>diff --git a/arc=
h/arm64/boot/dts/mediatek/mt7622=2Edtsi
>b/arch/arm64/boot/dts/mediatek/mt7=
622=2Edtsi
>index 890a942ec608=2E=2E88c81d24f4ff 100644
>--- a/arch/arm64/b=
oot/dts/mediatek/mt7622=2Edtsi
>+++ b/arch/arm64/boot/dts/mediatek/mt7622=
=2Edtsi
>@@ -136,24 +136,18 @@ secmon_reserved: secmon@43000000 {
>
> 	ther=
mal-zones {
> 		cpu_thermal: cpu-thermal {
>-			polling-delay-passive =3D <=
1000>;
>+			polling-delay-passive =3D <250>;
> 			polling-delay =3D <1000>;=

>
> 			thermal-sensors =3D <&thermal 0>;
>
> 			trips {
> 				cpu_passive:=
 cpu-passive {
>-					temperature =3D <47000>;
>+					temperature =3D <7700=
0>;
> 					hysteresis =3D <2000>;
> 					type =3D "passive";
> 				};
>
>-	=
			cpu_active: cpu-active {
>-					temperature =3D <67000>;
>-					hysteres=
is =3D <2000>;
>-					type =3D "active";
>-				};
>-
> 				cpu_hot: cpu-hot=
 {
> 					temperature =3D <87000>;
> 					hysteresis =3D <2000>;
>@@ -173,1=
8 +167,6 @@ map0 {
> 					cooling-device =3D <&cpu0 THERMAL_NO_LIMIT THERMA=
L_NO_LIMIT>,
> 							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> 				};
=
>-
>-				map1 {
>-					trip =3D <&cpu_active>;
>-					cooling-device =3D <&=
cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>-							 <&cpu1 THERMAL_NO_LIMIT =
THERMAL_NO_LIMIT>;
>-				};
>-
>-				map2 {
>-					trip =3D <&cpu_hot>;
>-	=
				cooling-device =3D <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>-							=
 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>-				};
> 			};
> 		};
> 	};
>=

>
>-- 
><http://www=2Elinaro=2Eorg/> Linaro=2Eorg =E2=94=82 Open source so=
ftware for ARM SoCs
>
>Follow Linaro:  <http://www=2Efacebook=2Ecom/pages/L=
inaro> Facebook |
><http://twitter=2Ecom/#!/linaroorg> Twitter |
><http://w=
ww=2Elinaro=2Eorg/linaro-blog/> Blog