[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a052b2ba-e211-c997-7508-0d012663dedb@collabora.com>
Date: Mon, 24 Apr 2023 09:40:36 +0200
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Chen-Yu Tsai <wenst@...omium.org>
Cc: Nícolas F. R. A. Prado
<nfraprado@...labora.com>, matthias.bgg@...il.com,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH 1/5] arm64: dts: mediatek: cherry: Add platform thermal
configuration
Il 24/04/23 09:38, Chen-Yu Tsai ha scritto:
> On Mon, Apr 24, 2023 at 2:31 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> wrote:
>>
>> Il 21/04/23 22:53, Nícolas F. R. A. Prado ha scritto:
>>> On Fri, Apr 21, 2023 at 03:37:52PM +0800, Chen-Yu Tsai wrote:
>>>> On Thu, Apr 20, 2023 at 5:45 PM AngeloGioacchino Del Regno
>>>> <angelogioacchino.delregno@...labora.com> wrote:
>>>>>
>>>>> This platform has three auxiliary NTC thermistors, connected to the
>>>>> SoC's ADC pins. Enable the auxadc in order to be able to read the
>>>>> ADC values, add a generic-adc-thermal LUT for each and finally assign
>>>>> them to the SoC's thermal zones.
>>>>>
>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>>>>> ---
>>>>> .../boot/dts/mediatek/mt8195-cherry.dtsi | 105 ++++++++++++++++++
>>>>> 1 file changed, 105 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
>>>>> index 8ac80a136c37..0820e9ba3829 100644
>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195-cherry.dtsi
>>>>> @@ -114,6 +114,77 @@ ppvar_sys: regulator-ppvar-sys {
>>>>> regulator-boot-on;
>>>>> };
>>>>>
>>>>> + /* Murata NCP03WF104F05RL */
>>>>> + tboard_thermistor1: thermal-sensor-t1 {
>>>>> + compatible = "generic-adc-thermal";
>>>>> + #thermal-sensor-cells = <0>;
>>>>> + io-channels = <&auxadc 0>;
>>>>> + io-channel-names = "sensor-channel";
>>>>> + temperature-lookup-table = < (-10000) 1553
>>>>> + (-5000) 1485
>>>>> + 0 1406
>>>>> + 5000 1317
>>>>> + 10000 1219
>>>>> + 15000 1115
>>>>> + 20000 1007
>>>>> + 25000 900
>>>>> + 30000 796
>>>>> + 35000 697
>>>>> + 40000 605
>>>>> + 45000 523
>>>>> + 50000 449
>>>>> + 55000 384
>>>>> + 60000 327
>>>>> + 65000 279
>>>>> + 70000 237
>>>>> + 75000 202
>>>>> + 80000 172
>>>>> + 85000 147
>>>>> + 90000 125
>>>>> + 95000 107
>>>>> + 100000 92
>>>>> + 105000 79
>>>>> + 110000 68
>>>>> + 115000 59
>>>>> + 120000 51
>>>>> + 125000 44>;
>>>>> + };
>>>>> +
>>>>> + tboard_thermistor2: thermal-sensor-t2 {
>>>>> + compatible = "generic-adc-thermal";
>>>>> + #thermal-sensor-cells = <0>;
>>>>> + io-channels = <&auxadc 1>;
>>>>> + io-channel-names = "sensor-channel";
>>>>> + temperature-lookup-table = < (-10000) 1553
>>>>> + (-5000) 1485
>>>>> + 0 1406
>>>>> + 5000 1317
>>>>> + 10000 1219
>>>>> + 15000 1115
>>>>> + 20000 1007
>>>>> + 25000 900
>>>>> + 30000 796
>>>>> + 35000 697
>>>>> + 40000 605
>>>>> + 45000 523
>>>>> + 50000 449
>>>>> + 55000 384
>>>>> + 60000 327
>>>>> + 65000 279
>>>>> + 70000 237
>>>>> + 75000 202
>>>>> + 80000 172
>>>>> + 85000 147
>>>>> + 90000 125
>>>>> + 95000 107
>>>>> + 100000 92
>>>>> + 105000 79
>>>>> + 110000 68
>>>>> + 115000 59
>>>>> + 120000 51
>>>>> + 125000 44>;
>>>>> + };
>>>>> +
>>>>> usb_vbus: regulator-5v0-usb-vbus {
>>>>> compatible = "regulator-fixed";
>>>>> regulator-name = "usb-vbus";
>>>>> @@ -260,6 +331,10 @@ &gpu {
>>>>> mali-supply = <&mt6315_7_vbuck1>;
>>>>> };
>>>>>
>>>>> +&auxadc {
>>>>> + status = "okay";
>>>>> +};
>>>>> +
>>>>> &i2c0 {
>>>>> status = "okay";
>>>>>
>>>>> @@ -1098,6 +1173,36 @@ mt6315_7_vbuck1: vbuck1 {
>>>>> };
>>>>> };
>>>>>
>>>>> +&thermal_zones {
>>>>> + soc_area_ntc {
>>>
>>> Not sure if that's what's causing the issue, but the thermal zone name should
>>> end with -thermal as per the binding. Also note that it needs to be under 20
>>> characters otherwise it will fail to be registered with -22 like below.
>>> (Also, node names shouldn't contain underscore)
>>>
>>> Thanks,
>>> Nícolas
>>>
>>>>> + polling-delay = <1000>;
>>>>> + polling-delay-passive = <250>;
>>>>> + thermal-sensors = <&tboard_thermistor1>;
>>>>> +
>>>>> + trips {
>>>>> + trip-crit {
>>>>> + temperature = <95000>;
>>>>> + hysteresis = <2000>;
>>>>> + type = "critical";
>>>>> + };
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + pmic_area_ntc {
>>>>> + polling-delay = <1000>;
>>>>> + polling-delay-passive = <0>;
>>>>> + thermal-sensors = <&tboard_thermistor2>;
>>>>> +
>>>>> + trips {
>>>>> + trip-crit {
>>>>> + temperature = <95000>;
>>>>> + hysteresis = <2000>;
>>>>> + type = "critical";
>>>>> + };
>>>>> + };
>>>>> + };
>>>>
>>>> I'm still getting:
>>>>
>>>> thermal_sys: Failed to find 'trips' node
>>>> thermal_sys: Failed to find trip points for thermal-sensor-t1 id=0
>>>> generic-adc-thermal thermal-sensor-t1: Thermal zone sensor register failed: -22
>>>> generic-adc-thermal: probe of thermal-sensor-t1 failed with error -22
>>>> thermal_sys: Failed to find 'trips' node
>>>> thermal_sys: Failed to find trip points for thermal-sensor-t2 id=0
>>>> generic-adc-thermal thermal-sensor-t2: Thermal zone sensor register failed: -22
>>>> generic-adc-thermal: probe of thermal-sensor-t2 failed with error -22
>>>> thermal_sys: Failed to find 'trips' node
>>>> thermal_sys: Failed to find trip points for thermal-sensor-t3 id=0
>>>> generic-adc-thermal thermal-sensor-t3: Thermal zone sensor register failed: -22
>>>> generic-adc-thermal: probe of thermal-sensor-t3 failed with error -22
>>>>
>>
>> I think you have something wrong locally - there's no thermal-sensor-t3 in this
>> devicetree...
>
> I seem to have run a stale kernel image. Rebuilt the kernel and everything
> seems to work OK now.
>
> BTW, I think the design went for a lower trip point. At least the hardware
> thermal protection IC on the Acer device trips at 85 degrees C, instead of
> 95 degrees C. Maybe that's accounting for the fact that these are external
> thermal sensors and have some latency and temperature difference. The PMIC
> specifies 85 degrees C maximum ambient air temperature. The SoC doesn't
> specify.
>
Let's play safe then, I'll change that to 85 for the next version.
> Either way this is
>
> Tested-by: Chen-Yu Tsai <wenst@...omium.org>
> Reviewed-by: Chen-Yu Tsai <wenst@...omium.org>
Thanks!
Powered by blists - more mailing lists