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: <cover.1682682753.git.quic_varada@quicinc.com> <1bda63e18f7257f60cc1082b423aca129abfa3b0.1682682753.git.quic_varada@quicinc.com>
 <CAA8EJpq0RXGf8_oBa_XF0+nOg31ouMUVJ3LhNRh_HtmgJvCJHQ@mail.gmail.com> <20230502080440.GA26126@varda-linux.qualcomm.com>
In-Reply-To: <20230502080440.GA26126@varda-linux.qualcomm.com>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date:   Tue, 2 May 2023 11:39:27 +0300
Message-ID: <CAA8EJpoHMdNFFu4rX2O_N64TV62uAEj4tZqRLpWP8WZrf-PQhQ@mail.gmail.com>
Subject: Re: [PATCH 4/4] arm64: dts: qcom: ipq9574: add thermal zone nodes
To:     Varadarajan Narayanan <quic_varada@quicinc.com>
Cc:     agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org,
        amitk@kernel.org, thara.gopinath@gmail.com, rafael@kernel.org,
        daniel.lezcano@linaro.org, rui.zhang@intel.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, linux-arm-msm@vger.kernel.org,
        linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Praveenkumar I <quic_ipkumar@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, 2 May 2023 at 11:04, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Fri, Apr 28, 2023 at 10:49:34PM +0300, Dmitry Baryshkov wrote:
> > On Fri, 28 Apr 2023 at 17:53, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> > >
> > > This patch adds thermal zone nodes for the various
> > > sensors present in IPQ9574
> > >
> > > Signed-off-by: Praveenkumar I <quic_ipkumar@quicinc.com>
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/ipq9574.dtsi | 208 ++++++++++++++++++++++++=
++++++++++
> > >  1 file changed, 208 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/=
dts/qcom/ipq9574.dtsi
> > > index 7cd5bdb..a7cb2b4c 100644
> > > --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> > > @@ -947,6 +947,214 @@
> > >                 };
> > >         };
> > >
> > > +       thermal_zones: thermal-zones {
> > > +               tsens_tz_sensor3 {
> >
> > Please provide sensible names for all thermal zones. Please follow the
> > examples in other DT files.
>
> Ok.
>
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 3>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> >
> > Can it really go up to 125 =C2=B0C?
>
> The SoC product requirement is 120=C2=B0C. It is capable of 125=C2=B0C.
> This was tested inside a thermal chamber and ensured that it
> hits 125=C2=B0C and system reboots at that temperature.

ack

>
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> >
> > Please provide also a passive trip point, so that the passive cooling
> > can be engaged.
>
> The linux kernel cannot take any steps to initiate cooling for
> components other than the 4 CPU cores. The f/w that runs on these
> IP blocks have to take steps to initiate cooling. Additionally,
> the passive temperature for the non-cpu sensors are not
> characterised yet and I don't have the values now.
>
> We have these nodes to be able to read the temperature of the
> other blocks via the sysfs entry /sys/devices/virtual/thermal/*/temp
> Have given the critical trip point so that the setup is rebooted
> if the critical temperature is reached.
>
> Have given passive trip point for the CPU core related thermal
> nodes (tsens_tz_sensor10, tsens_tz_sensor11, tsens_tz_sensor12
> and tsens_tz_sensor13).

That's the point why we asked for the logical names for the sensors,
like we have on the other platforms. Otherwise it is very hard to
understand what is going on.

>
> If this is not acceptable, please let me know. Will remove the
> non-cpu nodes and post a patch with just the CPU entries.

This is perfectly fine. Please take a look at other DTSI files.

>
> Thanks
> Varada
>
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor4 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 4>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor5 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 5>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor6 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 6>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor7 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 7>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor8 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 8>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor9 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 9>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor10 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 10>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <120000>;
> > > +                                       hysteresis =3D <10000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +
> > > +                               cpu-passive {
> > > +                                       temperature =3D <110000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "passive";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor11 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 11>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <120000>;
> > > +                                       hysteresis =3D <10000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +
> > > +                               cpu-passive {
> > > +                                       temperature =3D <110000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "passive";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor12 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 12>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <120000>;
> > > +                                       hysteresis =3D <10000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +
> > > +                               cpu-passive {
> > > +                                       temperature =3D <110000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "passive";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor13 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 13>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <120000>;
> > > +                                       hysteresis =3D <10000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +
> > > +                               cpu-passive {
> > > +                                       temperature =3D <110000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "passive";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor14 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 14>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               tsens_tz_sensor15 {
> > > +                       polling-delay-passive =3D <0>;
> > > +                       polling-delay =3D <0>;
> > > +                       thermal-sensors =3D <&tsens 15>;
> > > +
> > > +                       trips {
> > > +                               cpu-critical {
> > > +                                       temperature =3D <125000>;
> > > +                                       hysteresis =3D <1000>;
> > > +                                       type =3D "critical";
> > > +                               };
> > > +                       };
> > > +               };
> > > +       };
> > > +
> > >         timer {
> > >                 compatible =3D "arm,armv8-timer";
> > >                 interrupts =3D <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | I=
RQ_TYPE_LEVEL_LOW)>,
> > > --
> > > 2.7.4
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry


--=20
With best wishes
Dmitry