| 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
| ||
|
Message-ID: <f66c795b-03e7-083c-e4ba-5b5c150b88df@linaro.org>
Date: Thu, 6 Oct 2022 13:29:28 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Amjad Ouled-Ameur <aouledameur@...libre.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Matthias Brugger <matthias.bgg@...il.com>
Cc: linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones
without trips.
On 06/10/2022 13:08, Amjad Ouled-Ameur wrote:
> Hi Daniel,
>
> Thank you for your feedback.
>
> On 10/4/22 12:47, Daniel Lezcano wrote:
>>
>> Hi Amjad,
>>
>> On 04/10/2022 12:11, Amjad Ouled-Ameur wrote:
>>> Thermal zones without trip point are not registered by thermal core.
>>>
>>> tzts1 ~ tzts6 zones of mt8183 were intially introduced for test-purpose
>>> only.
>>>
>>> Disable the zones above and keep only cpu_thermal enabled.
>>
>> It does not make sense to disable the thermal zones. Either the
>> thermal zones are needed or they are not. Keeping them for debug
>> purpose is not desired.
> As Matthias Brugger mentioned in previous versions, DTS should describe
> the HW as it is, the sensors are in the HW.
Yes, it is here:
thermal: thermal@...0b000 {
#thermal-sensor-cells = <1>;
compatible = "mediatek,mt8183-thermal";
reg = <0 0x1100b000 0 0x1000>;
clocks = <&infracfg CLK_INFRA_THERM>,
<&infracfg CLK_INFRA_AUXADC>;
clock-names = "therm", "auxadc";
resets = <&infracfg
MT8183_INFRACFG_AO_THERM_SW_RST>;
interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
mediatek,auxadc = <&auxadc>;
mediatek,apmixedsys = <&apmixedsys>;
nvmem-cells = <&thermal_calibration>;
nvmem-cell-names = "calibration-data";
};
>> Alternatively to removal, you can:
>>
>> - remove 'sustainable-power'
>> - add a passive trip point, optionally a hot trip point and a
>> critical trip point
>
> Why removing "sustainable-power" instead of simply disabling the device
> ?
Because sustainable-power is wrong. It is probably coming from a
copy-paste. It does not make sense to have this.
The sustainable-power is for the power allocator governor which is
limited to CPU and GPU. Here the thermal zones are for different devices.
Especially that, if a user needs to use the sensor
If the thermal zone is disabled, how can it use the sensor?
> in the future, they might not be able to find the right
> sustainable-power ; thus I think it should remain the way it is.
>
> As to adding tripping points, MediaTek does not have ones to add for now
> for those sensors.
Please do this:
Add:
trips {
threshold : trip1 {
temperature = <50000>;
type = "passive";
};
};
And remove all the empty cooling maps and the sustainable power properties.
>> The passive trip point will allow the userspace to set a value in
>> order to get notified about the devices temperature (writable trip
>> point). The hot temperature will send a notification to userspace so
>> it can take a last chance decision to drop the temperature before the
>> critical temperature.
>>
>> The passive trip point temperature could be a high temperature.
>>
>> The mitigation is also managed from userspace as a whole.
>>
>>
>>> Signed-off-by: Amjad Ouled-Ameur <aouledameur@...libre.com>
>>> ---
>>> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> index 9d32871973a2..53f7a0fbaa88 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> @@ -1191,6 +1191,7 @@ tzts1: tzts1 {
>>> polling-delay = <0>;
>>> thermal-sensors = <&thermal 1>;
>>> sustainable-power = <5000>;
>>> + status = "disabled";
>>> trips {};
>>> cooling-maps {};
>>> };
>>> @@ -1200,6 +1201,7 @@ tzts2: tzts2 {
>>> polling-delay = <0>;
>>> thermal-sensors = <&thermal 2>;
>>> sustainable-power = <5000>;
>>> + status = "disabled";
>>> trips {};
>>> cooling-maps {};
>>> };
>>> @@ -1209,6 +1211,7 @@ tzts3: tzts3 {
>>> polling-delay = <0>;
>>> thermal-sensors = <&thermal 3>;
>>> sustainable-power = <5000>;
>>> + status = "disabled";
>>> trips {};
>>> cooling-maps {};
>>> };
>>> @@ -1218,6 +1221,7 @@ tzts4: tzts4 {
>>> polling-delay = <0>;
>>> thermal-sensors = <&thermal 4>;
>>> sustainable-power = <5000>;
>>> + status = "disabled";
>>> trips {};
>>> cooling-maps {};
>>> };
>>> @@ -1227,6 +1231,7 @@ tzts5: tzts5 {
>>> polling-delay = <0>;
>>> thermal-sensors = <&thermal 5>;
>>> sustainable-power = <5000>;
>>> + status = "disabled";
>>> trips {};
>>> cooling-maps {};
>>> };
>>> @@ -1236,6 +1241,7 @@ tztsABB: tztsABB {
>>> polling-delay = <0>;
>>> thermal-sensors = <&thermal 6>;
>>> sustainable-power = <5000>;
>>> + status = "disabled";
>>> trips {};
>>> cooling-maps {};
>>> };
>>
>>
--
<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
Powered by blists - more mailing lists