Messages in this thread Patch in this message | | | Date | Thu, 6 Oct 2022 13:29:28 +0200 | Subject | Re: [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones without trips. | From | Daniel Lezcano <> |
| |
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@1100b000 { #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@baylibre.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
| |