Re: [PATCH 4/4] arm64: dts: qcom: ipq9574: add thermal zone nodes

From: Dmitry Baryshkov
Date: Tue May 02 2023 - 04:40:02 EST


On Tue, 2 May 2023 at 11:04, Varadarajan Narayanan
<quic_varada@xxxxxxxxxxx> 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@xxxxxxxxxxx> wrote:
> > >
> > > This patch adds thermal zone nodes for the various
> > > sensors present in IPQ9574
> > >
> > > Signed-off-by: Praveenkumar I <quic_ipkumar@xxxxxxxxxxx>
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> > > ---
> > > 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 = <0>;
> > > + polling-delay = <0>;
> > > + thermal-sensors = <&tsens 3>;
> > > +
> > > + trips {
> > > + cpu-critical {
> > > + temperature = <125000>;
> >
> > Can it really go up to 125 °C?
>
> The SoC product requirement is 120°C. It is capable of 125°C.
> This was tested inside a thermal chamber and ensured that it
> hits 125°C and system reboots at that temperature.

ack

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



--
With best wishes
Dmitry