Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <cdebdf53-d691-41b6-bb8e-f66bb6f56e56@baylibre.com>
Date:   Thu, 6 Oct 2022 13:08:19 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH v3] arm64: dts: mediatek: mt8183: disable thermal zones
 without trips.
Content-Language: en-US
To:     Daniel Lezcano <daniel.lezcano@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Matthias Brugger <matthias.bgg@gmail.com>
Cc:     linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
References: <20221004101130.17256-1-aouledameur@baylibre.com>
 <17f3fd40-70c1-2e8d-8002-dfe9690aed88@linaro.org>
From:   Amjad Ouled-Ameur <aouledameur@baylibre.com>
In-Reply-To: <17f3fd40-70c1-2e8d-8002-dfe9690aed88@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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