Re: [PATCH v2 10/10] arm64: dts: Create SoC thermal zone for Juno

Hi Liviu,

Thanks for having a look.

Liviu Dudau [off-list ref] writes:

On Mon, Aug 03, 2015 at 04:22:58PM +0100, Punit Agrawal wrote:

quoted

Setup a thermal zone driven by the SoC temperature sensor on Juno
r0. Create passive trip points and bind them to cpufreq cooling
devices that support the power extensions.

Hi Punit,

quoted

Signed-off-by: Punit Agrawal <redacted>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Sudeep Holla <redacted>
---
 arch/arm64/boot/dts/arm/juno.dts | 50 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index d2e67f3..664961c 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts

@@ -9,6 +9,7 @@
 /dts-v1/;
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	model = "ARM Juno development board (r0)";

@@ -90,6 +91,8 @@
 			next-level-cache = <&A57_L2>;
 			clocks = <&scpi_dvfs 0>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <530>;

The Documentation/devicetree/bindings/thermal/thermal.txt document describes two additional
required properties, cooling-min-state and cooling-max-state which are missing here.

The bindings do suggest that the cooling-*-state are required but I
couldn't find any code making use of this property.

I'll send a patch with the next version making those properties
optional.

quoted

 		};
 
 		A57_1: cpu@1 {

@@ -100,6 +103,8 @@
 			next-level-cache = <&A57_L2>;
 			clocks = <&scpi_dvfs 0>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <530>;
 		};
 
 		A53_0: cpu@100 {

@@ -110,6 +115,8 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <140>;
 		};
 
 		A53_1: cpu@101 {

@@ -120,6 +127,8 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <140>;
 		};
 
 		A53_2: cpu@102 {

@@ -130,6 +139,8 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <140>;
 		};
 
 		A53_3: cpu@103 {

@@ -140,6 +151,8 @@
 			next-level-cache = <&A53_L2>;
 			clocks = <&scpi_dvfs 1>;
 			cpu-idle-states = <&CPU_SLEEP_0 &CLUSTER_SLEEP_0>;
+			#cooling-cells = <2>;
+			dynamic-power-coefficient = <140>;
 		};
 
 		A57_L2: l2-cache0 {

@@ -167,5 +180,42 @@
 				     <&A53_3>;
 	};
 
+	thermal-zones {
+		soc_thermal {
+			polling-delay = <1000>;
+			polling-delay-passive = <100>;
+			sustainable-power = <2500>;
+
+			thermal-sensors = <&scpi_sensors0 3>;
+
+			trips {
+				threshold: trip-point@0 {

You also need a reg= property if you have the @index thing.

Having looked at the examples, I don't need to have the @index for the
trip points. I'll update in the next version.

Thanks,
Punit

Best regards,
Liviu

quoted

+					temperature = <55000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+				target: trip-point@1 {
+					temperature = <65000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+			};
+
+			cooling-maps {
+				map0 {
+					trip = <&target>;
+					cooling-device = <&A53_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+					contribution = <2048>;
+				};
+				map1 {
+					trip = <&target>;
+					cooling-device = <&A57_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+					contribution = <1024>;
+				};
+
+			};
+		};
+	};
+
 	#include "juno-base.dtsi"
 };
-- 
2.1.4