Re: [PATCH V2 3/3] ARM: dts: qcom: Add Thermal zone support in SC7280.

[Matthias Kaehlcke]

On Fri, Apr 23, 2021 at 11:52:57PM +0530, Rajeshwari wrote:

> Subject: ARM: dts: qcom: Add Thermal zone support in SC7280

s/ARM/arm64/

same for patch [2/2]

nit: s/Thermal/thermal/

>
> Added Thermal zone support, cooling maps and added hysteresis as zero as
> it is not needed as system is supposed to shut down when critical trip point
> is reached.

nit: s/Thermal/thermal/

I don't think it's necessary to explain the hysteresis values unless
they are really uncommon.

> Signed-off-by: Rajeshwari <rkambl@xxxxxxxxxxxxxx>
> ---
>  arch/arm64/boot/dts/qcom/sc7280.dtsi | 844 +++++++++++++++++++++++++++++++++++
>  1 file changed, 844 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 275113c..80f4ecb 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -12,6 +12,7 @@
>  #include <dt-bindings/power/qcom-aoss-qmp.h>
>  #include <dt-bindings/power/qcom-rpmpd.h>
>  #include <dt-bindings/soc/qcom,rpmh-rsc.h>
> +#include <dt-bindings/thermal/thermal.h>
>  
>  / {
>  	interrupt-parent = <&intc>;
> @@ -1140,6 +1141,849 @@
>  		};
>  	};
>  
> +	thermal_zones: thermal-zones {
> +		cpu0-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens0 1>;
> +
> +			trips {
> +				cpu0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "passive";
> +				};
> +
> +				cpu0_alert1: trip-point1 {
> +					temperature = <95000>;
> +					hysteresis = <0>;
> +					type = "passive";
> +				};
> +
> +				cpu0_crit: cpu_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};

In the review of v1 (https://patchwork.kernel.org/project/linux-arm-msm/patch/1619005442-16485-2-git-send-email-rkambl@xxxxxxxxxxxxxx/)
I suggested that a hysteresis isn't needed for the critical trip
points, but this patch also also gets rid of the hysteresis values
for the passive and hot trip points, which isn't correct.

> +			};
> +
> +			cooling-maps {
> +				map0 {
> +					trip = <&cpu0_alert0>;
> +					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +				map1 {
> +					trip = <&cpu0_alert1>;
> +					cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +							 <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +				};
> +			};
> +		};
>
> ...
>
> +		aoss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens0 0>;
> +
> +			trips {
> +				aoss0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				aoss0_crit: aoss0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		aoss1-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 0>;
> +
> +			trips {
> +				aoss1_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				aoss1_crit: aoss1_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpuss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens0 5>;
> +
> +			trips {
> +				cpuss0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +				cpuss0_crit: cluster0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpuss1-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens0 6>;
> +
> +			trips {
> +				cpuss1_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +				cpuss1_crit: cluster0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		gpuss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 1>;
> +
> +			trips {
> +				gpuss0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				gpuss0_crit: gpuss0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		gpuss1-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 2>;
> +
> +			trips {
> +				gpuss1_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				gpuss1_crit: gpuss1_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		nspss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 3>;
> +
> +			trips {
> +				nspss0_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				nspss0_crit: nspss0_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		nspss1-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 4>;
> +
> +			trips {
> +				nspss1_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				nspss1_crit: nspss1_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		video-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +
> +			thermal-sensors = <&tsens1 5>;
> +
> +			trips {
> +				video_alert0: trip-point0 {
> +					temperature = <90000>;
> +					hysteresis = <0>;
> +					type = "hot";
> +				};
> +
> +				video_crit: video_crit {
> +					temperature = <110000>;
> +					hysteresis = <0>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		ddr-thermal {

uber-nit: I still think an alphabetical order would be preferable, it
makes it easy to locate the zones, whereas the indices of the TSENS
sensor are mostly irrelevant for the order. Anyway, AFAIK there are
no clear rules or conventions regarding this, so it's ok if yo
prefer to leave it as is.