linux-mediatek.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones without trips.
@ 2022-10-04 10:11 Amjad Ouled-Ameur
  2022-10-04 10:47 ` Daniel Lezcano
  0 siblings, 1 reply; 4+ messages in thread
From: Amjad Ouled-Ameur @ 2022-10-04 10:11 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Matthias Brugger
  Cc: Amjad Ouled-Ameur, linux-mediatek, devicetree, linux-arm-kernel,
	linux-kernel

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.

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 {};
 			};
-- 
2.37.3



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones without trips.
  2022-10-04 10:11 [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones without trips Amjad Ouled-Ameur
@ 2022-10-04 10:47 ` Daniel Lezcano
  2022-10-06 11:08   ` Amjad Ouled-Ameur
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Lezcano @ 2022-10-04 10:47 UTC (permalink / raw)
  To: Amjad Ouled-Ameur, Rob Herring, Krzysztof Kozlowski, Matthias Brugger
  Cc: linux-mediatek, devicetree, linux-arm-kernel, linux-kernel


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.

Alternatively to removal, you can:

  - remove 'sustainable-power'
  - add a passive trip point, optionally a hot trip point and a critical 
trip point

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones without trips.
  2022-10-04 10:47 ` Daniel Lezcano
@ 2022-10-06 11:08   ` Amjad Ouled-Ameur
  2022-10-06 11:29     ` Daniel Lezcano
  0 siblings, 1 reply; 4+ messages in thread
From: Amjad Ouled-Ameur @ 2022-10-06 11:08 UTC (permalink / raw)
  To: Daniel Lezcano, Rob Herring, Krzysztof Kozlowski, Matthias Brugger
  Cc: linux-mediatek, devicetree, linux-arm-kernel, linux-kernel

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.
>
> 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 ? Especially that, if a user needs to 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.


Regards,

Amjad

>
> 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 {};
>>               };
>
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones without trips.
  2022-10-06 11:08   ` Amjad Ouled-Ameur
@ 2022-10-06 11:29     ` Daniel Lezcano
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Lezcano @ 2022-10-06 11:29 UTC (permalink / raw)
  To: Amjad Ouled-Ameur, Rob Herring, Krzysztof Kozlowski, Matthias Brugger
  Cc: linux-mediatek, devicetree, linux-arm-kernel, linux-kernel

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-10-06 11:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-04 10:11 [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones without trips Amjad Ouled-Ameur
2022-10-04 10:47 ` Daniel Lezcano
2022-10-06 11:08   ` Amjad Ouled-Ameur
2022-10-06 11:29     ` Daniel Lezcano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).