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;
MIME-Version: 1.0
References: <20230420094433.42794-1-angelogioacchino.delregno@collabora.com>
 <20230420094433.42794-2-angelogioacchino.delregno@collabora.com>
 <CAGXv+5GuEQMxYTKrnia1ipYHLt_B2h6By7EejE7MjCypfavnFg@mail.gmail.com>
 <5ede9421-54d6-4dda-91af-2acbf46c2d3e@notapiano> <097d60ba-5604-a2c9-c6d2-121db2d55e36@collabora.com>
In-Reply-To: <097d60ba-5604-a2c9-c6d2-121db2d55e36@collabora.com>
From:   Chen-Yu Tsai <wenst@chromium.org>
Date:   Mon, 24 Apr 2023 15:38:25 +0800
Message-ID: <CAGXv+5G9Qh2ftB0NG4Km3wHQCgyxHK+Ae17QWJrv8V9VJaBH4A@mail.gmail.com>
Subject: Re: [PATCH 1/5] arm64: dts: mediatek: cherry: Add platform thermal configuration
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>
Cc:     =?UTF-8?B?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= 
        <nfraprado@collabora.com>, matthias.bgg@gmail.com,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, kernel@collabora.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Apr 24, 2023 at 2:31=E2=80=AFPM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 21/04/23 22:53, N=C3=ADcolas 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=E2=80=AFPM AngeloGioacchino Del Regno
> >> <angelogioacchino.delregno@collabora.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@=
collabora.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/a=
rm64/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 =3D "generic-adc-thermal";
> >>> +               #thermal-sensor-cells =3D <0>;
> >>> +               io-channels =3D <&auxadc 0>;
> >>> +               io-channel-names =3D "sensor-channel";
> >>> +               temperature-lookup-table =3D <    (-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 =3D "generic-adc-thermal";
> >>> +               #thermal-sensor-cells =3D <0>;
> >>> +               io-channels =3D <&auxadc 1>;
> >>> +               io-channel-names =3D "sensor-channel";
> >>> +               temperature-lookup-table =3D <    (-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 =3D "regulator-fixed";
> >>>                  regulator-name =3D "usb-vbus";
> >>> @@ -260,6 +331,10 @@ &gpu {
> >>>          mali-supply =3D <&mt6315_7_vbuck1>;
> >>>   };
> >>>
> >>> +&auxadc {
> >>> +       status =3D "okay";
> >>> +};
> >>> +
> >>>   &i2c0 {
> >>>          status =3D "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 und=
er 20
> > characters otherwise it will fail to be registered with -22 like below.
> > (Also, node names shouldn't contain underscore)
> >
> > Thanks,
> > N=C3=ADcolas
> >
> >>> +               polling-delay =3D <1000>;
> >>> +               polling-delay-passive =3D <250>;
> >>> +               thermal-sensors =3D <&tboard_thermistor1>;
> >>> +
> >>> +               trips {
> >>> +                       trip-crit {
> >>> +                               temperature =3D <95000>;
> >>> +                               hysteresis =3D <2000>;
> >>> +                               type =3D "critical";
> >>> +                       };
> >>> +               };
> >>> +       };
> >>> +
> >>> +       pmic_area_ntc {
> >>> +               polling-delay =3D <1000>;
> >>> +               polling-delay-passive =3D <0>;
> >>> +               thermal-sensors =3D <&tboard_thermistor2>;
> >>> +
> >>> +               trips {
> >>> +                       trip-crit {
> >>> +                               temperature =3D <95000>;
> >>> +                               hysteresis =3D <2000>;
> >>> +                               type =3D "critical";
> >>> +                       };
> >>> +               };
> >>> +       };
> >>
> >> I'm still getting:
> >>
> >> thermal_sys: Failed to find 'trips' node
> >> thermal_sys: Failed to find trip points for thermal-sensor-t1 id=3D0
> >> generic-adc-thermal thermal-sensor-t1: Thermal zone sensor register fa=
iled: -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=3D0
> >> generic-adc-thermal thermal-sensor-t2: Thermal zone sensor register fa=
iled: -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=3D0
> >> generic-adc-thermal thermal-sensor-t3: Thermal zone sensor register fa=
iled: -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 i=
n 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.

Either way this is

Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>