All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Add Mediatek thermal dirver and dtsi
@ 2019-05-02 10:43 ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao-NuS5LvNUpcJWk0Htik3J/w,
	fan.chen-NuS5LvNUpcJWk0Htik3J/w,
	jamesjj.liao-NuS5LvNUpcJWk0Htik3J/w,
	dawei.chien-NuS5LvNUpcJWk0Htik3J/w,
	louis.yu-NuS5LvNUpcJWk0Htik3J/w, roger.lu-NuS5LvNUpcJWk0Htik3J/w,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, Matthias Brugger
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-pm-u79uwXL29TY76Z2rM5mHXA

This patchset supports for MT8183 chip to mtk_thermal.c.
Add thermal zone of all the thermal sensor in SoC for
another get temperatrue. And we add thermal zone nodes
and bind coolers.

This patch series base on these patches [1][2][3][4].

[1]support for reading chip ID and efuse (https://patchwork.kernel.org/patch/10902131/)
[2]arm64: dts: mt8183: Add reset-cells in infracfg (https://patchwork.kernel.org/patch/10908653/)
[3]clk: reset: Modify reset-controller driver (https://patchwork.kernel.org/patch/10908657/)
[4]PM / AVS: SVS: Introduce SVS engine (https://patchwork.kernel.org/patch/10923289/)

Matthias Kaehlcke (2):
	arm64: dts: mt8183: Configure CPU cooling
	arm64: dts: mt8183: Increase polling frequency for CPU thermal zone

Michael Kao (6):
	arm64: dts: mt8183: add thermal zone node
	arm64: dts: mt8183: add/update dynamic power coefficients
	arm64: dts: mt8183: Add #cooling-cells to CPU nodes
	thermal: mediatek: mt8183: fix bank number settings
	thermal: mediatek: add another get_temp ops for thermal sensors
	thermal: mediatek: use spinlock to protect PTPCORESEL

 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 137 +++++++++++++++++++++++++++++++
 drivers/thermal/mtk_thermal.c            |  82 ++++++++++++++----
 2 files changed, 205 insertions(+), 14 deletions(-)

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 0/8] Add Mediatek thermal dirver and dtsi
@ 2019-05-02 10:43 ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

This patchset supports for MT8183 chip to mtk_thermal.c.
Add thermal zone of all the thermal sensor in SoC for
another get temperatrue. And we add thermal zone nodes
and bind coolers.

This patch series base on these patches [1][2][3][4].

[1]support for reading chip ID and efuse (https://patchwork.kernel.org/patch/10902131/)
[2]arm64: dts: mt8183: Add reset-cells in infracfg (https://patchwork.kernel.org/patch/10908653/)
[3]clk: reset: Modify reset-controller driver (https://patchwork.kernel.org/patch/10908657/)
[4]PM / AVS: SVS: Introduce SVS engine (https://patchwork.kernel.org/patch/10923289/)

Matthias Kaehlcke (2):
	arm64: dts: mt8183: Configure CPU cooling
	arm64: dts: mt8183: Increase polling frequency for CPU thermal zone

Michael Kao (6):
	arm64: dts: mt8183: add thermal zone node
	arm64: dts: mt8183: add/update dynamic power coefficients
	arm64: dts: mt8183: Add #cooling-cells to CPU nodes
	thermal: mediatek: mt8183: fix bank number settings
	thermal: mediatek: add another get_temp ops for thermal sensors
	thermal: mediatek: use spinlock to protect PTPCORESEL

 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 137 +++++++++++++++++++++++++++++++
 drivers/thermal/mtk_thermal.c            |  82 ++++++++++++++----
 2 files changed, 205 insertions(+), 14 deletions(-)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
  2019-05-02 10:43 ` michael.kao
@ 2019-05-02 10:43     ` michael.kao
  -1 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao-NuS5LvNUpcJWk0Htik3J/w,
	fan.chen-NuS5LvNUpcJWk0Htik3J/w,
	jamesjj.liao-NuS5LvNUpcJWk0Htik3J/w,
	dawei.chien-NuS5LvNUpcJWk0Htik3J/w,
	louis.yu-NuS5LvNUpcJWk0Htik3J/w, roger.lu-NuS5LvNUpcJWk0Htik3J/w,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, Matthias Brugger
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-pm-u79uwXL29TY76Z2rM5mHXA

Add thermal zone node to Mediatek MT8183 dts file.

Signed-off-by: Michael Kao <michael.kao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 926df75..b92116f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -334,6 +334,67 @@
 			status = "disabled";
 		};
 
+		thermal: thermal@1100b000 {
+			#thermal-sensor-cells = <1>;
+			compatible = "mediatek,mt8183-thermal";
+			reg = <0 0x1100b000 0 0x1000>;
+			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&infracfg CLK_INFRA_THERM>,
+				 <&infracfg CLK_INFRA_AUXADC>;
+			clock-names = "therm", "auxadc";
+			resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
+			mediatek,auxadc = <&auxadc>;
+			mediatek,apmixedsys = <&apmixedsys>;
+			mediatek,hw-reset-temp = <117000>;
+			nvmem-cells = <&thermal_calibration>;
+			nvmem-cell-names = "calibration-data";
+		};
+
+		thermal-zones {
+			cpu_thermal: cpu_thermal {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+
+				thermal-sensors = <&thermal 0>;
+				sustainable-power = <1500>;
+			};
+
+			tzts1: tzts1 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 1>;
+			};
+
+			tzts2: tzts2 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 2>;
+			};
+
+			tzts3: tzts3 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 3>;
+			};
+
+			tzts4: tzts4 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 4>;
+			};
+
+			tzts5: tzts5 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 5>;
+			};
+
+			tztsABB: tztsABB {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 6>;
+			};
+		};
 		audiosys: syscon@11220000 {
 			compatible = "mediatek,mt8183-audiosys", "syscon";
 			reg = <0 0x11220000 0 0x1000>;
@@ -368,6 +429,9 @@
 			compatible = "mediatek,mt8183-efuse",
 				     "mediatek,efuse";
 			reg = <0 0x11f10000 0 0x1000>;
+			thermal_calibration: calib@180 {
+				reg = <0x180 0xc>;
+			};
 		};
 
 		mfgcfg: syscon@13000000 {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-02 10:43     ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

Add thermal zone node to Mediatek MT8183 dts file.

Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 926df75..b92116f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -334,6 +334,67 @@
 			status = "disabled";
 		};
 
+		thermal: thermal@1100b000 {
+			#thermal-sensor-cells = <1>;
+			compatible = "mediatek,mt8183-thermal";
+			reg = <0 0x1100b000 0 0x1000>;
+			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&infracfg CLK_INFRA_THERM>,
+				 <&infracfg CLK_INFRA_AUXADC>;
+			clock-names = "therm", "auxadc";
+			resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
+			mediatek,auxadc = <&auxadc>;
+			mediatek,apmixedsys = <&apmixedsys>;
+			mediatek,hw-reset-temp = <117000>;
+			nvmem-cells = <&thermal_calibration>;
+			nvmem-cell-names = "calibration-data";
+		};
+
+		thermal-zones {
+			cpu_thermal: cpu_thermal {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+
+				thermal-sensors = <&thermal 0>;
+				sustainable-power = <1500>;
+			};
+
+			tzts1: tzts1 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 1>;
+			};
+
+			tzts2: tzts2 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 2>;
+			};
+
+			tzts3: tzts3 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 3>;
+			};
+
+			tzts4: tzts4 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 4>;
+			};
+
+			tzts5: tzts5 {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 5>;
+			};
+
+			tztsABB: tztsABB {
+				polling-delay-passive = <1000>;
+				polling-delay = <1000>;
+				thermal-sensors = <&thermal 6>;
+			};
+		};
 		audiosys: syscon@11220000 {
 			compatible = "mediatek,mt8183-audiosys", "syscon";
 			reg = <0 0x11220000 0 0x1000>;
@@ -368,6 +429,9 @@
 			compatible = "mediatek,mt8183-efuse",
 				     "mediatek,efuse";
 			reg = <0 0x11f10000 0 0x1000>;
+			thermal_calibration: calib@180 {
+				reg = <0x180 0xc>;
+			};
 		};
 
 		mfgcfg: syscon@13000000 {
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 2/8] arm64: dts: mt8183: add/update dynamic power coefficients
  2019-05-02 10:43 ` michael.kao
@ 2019-05-02 10:43   ` michael.kao
  -1 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

Add dynamic power coefficients for all cores and update those of
CPU0 and CPU4.

Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index b92116f..5668fb8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -58,6 +58,8 @@
 			compatible = "arm,cortex-a53";
 			reg = <0x000>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu1: cpu@1 {
@@ -65,6 +67,8 @@
 			compatible = "arm,cortex-a53";
 			reg = <0x001>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu2: cpu@2 {
@@ -72,6 +76,8 @@
 			compatible = "arm,cortex-a53";
 			reg = <0x002>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu3: cpu@3 {
@@ -79,6 +85,8 @@
 			compatible = "arm,cortex-a53";
 			reg = <0x003>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu4: cpu@100 {
@@ -86,6 +94,8 @@
 			compatible = "arm,cortex-a73";
 			reg = <0x100>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu5: cpu@101 {
@@ -93,6 +103,8 @@
 			compatible = "arm,cortex-a73";
 			reg = <0x101>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu6: cpu@102 {
@@ -100,6 +112,8 @@
 			compatible = "arm,cortex-a73";
 			reg = <0x102>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu7: cpu@103 {
@@ -107,6 +121,8 @@
 			compatible = "arm,cortex-a73";
 			reg = <0x103>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 	};
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 2/8] arm64: dts: mt8183: add/update dynamic power coefficients
@ 2019-05-02 10:43   ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

Add dynamic power coefficients for all cores and update those of
CPU0 and CPU4.

Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index b92116f..5668fb8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -58,6 +58,8 @@
 			compatible = "arm,cortex-a53";
 			reg = <0x000>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu1: cpu@1 {
@@ -65,6 +67,8 @@
 			compatible = "arm,cortex-a53";
 			reg = <0x001>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu2: cpu@2 {
@@ -72,6 +76,8 @@
 			compatible = "arm,cortex-a53";
 			reg = <0x002>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu3: cpu@3 {
@@ -79,6 +85,8 @@
 			compatible = "arm,cortex-a53";
 			reg = <0x003>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu4: cpu@100 {
@@ -86,6 +94,8 @@
 			compatible = "arm,cortex-a73";
 			reg = <0x100>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu5: cpu@101 {
@@ -93,6 +103,8 @@
 			compatible = "arm,cortex-a73";
 			reg = <0x101>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu6: cpu@102 {
@@ -100,6 +112,8 @@
 			compatible = "arm,cortex-a73";
 			reg = <0x102>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu7: cpu@103 {
@@ -107,6 +121,8 @@
 			compatible = "arm,cortex-a73";
 			reg = <0x103>;
 			enable-method = "psci";
+			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 	};
 
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 3/8] arm64: dts: mt8183: Add #cooling-cells to CPU nodes
  2019-05-02 10:43 ` michael.kao
@ 2019-05-02 10:43   ` michael.kao
  -1 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

The #cooling-cells property needs to be specified to allow a CPU
to be used as cooling device.

Signed-off-by: michael.kao <Michael.Kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5668fb8..95f1d7b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -11,6 +11,8 @@
 #include <dt-bindings/power/mt8183-power.h>
 #include <dt-bindings/reset-controller/mt8183-resets.h>
 #include "mt8183-pinfunc.h"
+#include <dt-bindings/thermal/thermal.h>
+
 / {
 	compatible = "mediatek,mt8183";
 	interrupt-parent = <&sysirq>;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 3/8] arm64: dts: mt8183: Add #cooling-cells to CPU nodes
@ 2019-05-02 10:43   ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

The #cooling-cells property needs to be specified to allow a CPU
to be used as cooling device.

Signed-off-by: michael.kao <Michael.Kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5668fb8..95f1d7b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -11,6 +11,8 @@
 #include <dt-bindings/power/mt8183-power.h>
 #include <dt-bindings/reset-controller/mt8183-resets.h>
 #include "mt8183-pinfunc.h"
+#include <dt-bindings/thermal/thermal.h>
+
 / {
 	compatible = "mediatek,mt8183";
 	interrupt-parent = <&sysirq>;
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 4/8] arm64: dts: mt8183: Configure CPU cooling
  2019-05-02 10:43 ` michael.kao
@ 2019-05-02 10:43   ` michael.kao
  -1 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-pm, linux-kernel, Matthias Kaehlcke,
	linux-mediatek, linux-arm-kernel

From: Matthias Kaehlcke <mka@chromium.org>

Add two passive trip points at 68°C and 85°C for the CPU temperature.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 55 ++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 95f1d7b..0b3294b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -375,6 +375,61 @@
 
 				thermal-sensors = <&thermal 0>;
 				sustainable-power = <1500>;
+
+				trips {
+					threshold: trip-point@0 {
+						temperature = <68000>;
+						hysteresis = <2000>;
+						type = "passive";
+					};
+
+					target: trip-point@1 {
+						temperature = <85000>;
+						hysteresis = <2000>;
+						type = "passive";
+					};
+
+					cpu_crit: cpu-crit {
+						temperature = <115000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+
+				cooling-maps {
+					map0 {
+						trip = <&target>;
+						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>;
+						contribution = <3072>;
+					};
+					map1 {
+						trip = <&target>;
+						cooling-device = <&cpu4
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu5
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu6
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu7
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>;
+						contribution = <1024>;
+					};
+				};
 			};
 
 			tzts1: tzts1 {
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 4/8] arm64: dts: mt8183: Configure CPU cooling
@ 2019-05-02 10:43   ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-pm, linux-kernel, Matthias Kaehlcke,
	linux-mediatek, linux-arm-kernel

From: Matthias Kaehlcke <mka@chromium.org>

Add two passive trip points at 68°C and 85°C for the CPU temperature.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 55 ++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 95f1d7b..0b3294b 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -375,6 +375,61 @@
 
 				thermal-sensors = <&thermal 0>;
 				sustainable-power = <1500>;
+
+				trips {
+					threshold: trip-point@0 {
+						temperature = <68000>;
+						hysteresis = <2000>;
+						type = "passive";
+					};
+
+					target: trip-point@1 {
+						temperature = <85000>;
+						hysteresis = <2000>;
+						type = "passive";
+					};
+
+					cpu_crit: cpu-crit {
+						temperature = <115000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+
+				cooling-maps {
+					map0 {
+						trip = <&target>;
+						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>;
+						contribution = <3072>;
+					};
+					map1 {
+						trip = <&target>;
+						cooling-device = <&cpu4
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu5
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu6
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu7
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>;
+						contribution = <1024>;
+					};
+				};
 			};
 
 			tzts1: tzts1 {
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 5/8] arm64: dts: mt8183: Increase polling frequency for CPU thermal zone
  2019-05-02 10:43 ` michael.kao
@ 2019-05-02 10:43   ` michael.kao
  -1 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-pm, linux-kernel, Matthias Kaehlcke,
	linux-mediatek, linux-arm-kernel

From: Matthias Kaehlcke <mka@chromium.org>

Evaluate the thermal zone every 500ms while not cooling and every
100ms when passive cooling is performed.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 0b3294b..be879ac 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -370,8 +370,8 @@
 
 		thermal-zones {
 			cpu_thermal: cpu_thermal {
-				polling-delay-passive = <1000>;
-				polling-delay = <1000>;
+				polling-delay-passive = <100>;
+				polling-delay = <500>;
 
 				thermal-sensors = <&thermal 0>;
 				sustainable-power = <1500>;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 5/8] arm64: dts: mt8183: Increase polling frequency for CPU thermal zone
@ 2019-05-02 10:43   ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-pm, linux-kernel, Matthias Kaehlcke,
	linux-mediatek, linux-arm-kernel

From: Matthias Kaehlcke <mka@chromium.org>

Evaluate the thermal zone every 500ms while not cooling and every
100ms when passive cooling is performed.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 0b3294b..be879ac 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -370,8 +370,8 @@
 
 		thermal-zones {
 			cpu_thermal: cpu_thermal {
-				polling-delay-passive = <1000>;
-				polling-delay = <1000>;
+				polling-delay-passive = <100>;
+				polling-delay = <500>;
 
 				thermal-sensors = <&thermal 0>;
 				sustainable-power = <1500>;
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 6/8] thermal: mediatek: mt8183: fix bank number settings
  2019-05-02 10:43 ` michael.kao
@ 2019-05-02 10:43     ` michael.kao
  -1 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao-NuS5LvNUpcJWk0Htik3J/w,
	fan.chen-NuS5LvNUpcJWk0Htik3J/w,
	jamesjj.liao-NuS5LvNUpcJWk0Htik3J/w,
	dawei.chien-NuS5LvNUpcJWk0Htik3J/w,
	louis.yu-NuS5LvNUpcJWk0Htik3J/w, roger.lu-NuS5LvNUpcJWk0Htik3J/w,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, Matthias Brugger
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-pm-u79uwXL29TY76Z2rM5mHXA

From: Michael Kao <michael.kao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

MT8183_NUM_ZONES should be set to 1
because MT8183 doesn't have multiple banks.

Fixes: a4ffe6b52d27 ("thermal: mediatek: add support for MT8183")
Signed-off-by: Michael Kao <Michael.Kao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/thermal/mtk_thermal.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 5c07a61..cb41e46 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -216,6 +216,9 @@ enum {
 /* The total number of temperature sensors in the MT8183 */
 #define MT8183_NUM_SENSORS	6
 
+/* The number of banks in the MT8183 */
+#define MT8183_NUM_ZONES               1
+
 /* The number of sensing points per bank */
 #define MT8183_NUM_SENSORS_PER_ZONE	 6
 
@@ -503,7 +506,7 @@ struct mtk_thermal {
 
 static const struct mtk_thermal_data mt8183_thermal_data = {
 	.auxadc_channel = MT8183_TEMP_AUXADC_CHANNEL,
-	.num_banks = MT8183_NUM_SENSORS_PER_ZONE,
+	.num_banks = MT8183_NUM_ZONES,
 	.num_sensors = MT8183_NUM_SENSORS,
 	.vts_index = mt8183_vts_index,
 	.cali_val = MT8183_CALIBRATION,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 6/8] thermal: mediatek: mt8183: fix bank number settings
@ 2019-05-02 10:43     ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

From: Michael Kao <michael.kao@mediatek.com>

MT8183_NUM_ZONES should be set to 1
because MT8183 doesn't have multiple banks.

Fixes: a4ffe6b52d27 ("thermal: mediatek: add support for MT8183")
Signed-off-by: Michael Kao <Michael.Kao@mediatek.com>
---
 drivers/thermal/mtk_thermal.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 5c07a61..cb41e46 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -216,6 +216,9 @@ enum {
 /* The total number of temperature sensors in the MT8183 */
 #define MT8183_NUM_SENSORS	6
 
+/* The number of banks in the MT8183 */
+#define MT8183_NUM_ZONES               1
+
 /* The number of sensing points per bank */
 #define MT8183_NUM_SENSORS_PER_ZONE	 6
 
@@ -503,7 +506,7 @@ struct mtk_thermal {
 
 static const struct mtk_thermal_data mt8183_thermal_data = {
 	.auxadc_channel = MT8183_TEMP_AUXADC_CHANNEL,
-	.num_banks = MT8183_NUM_SENSORS_PER_ZONE,
+	.num_banks = MT8183_NUM_ZONES,
 	.num_sensors = MT8183_NUM_SENSORS,
 	.vts_index = mt8183_vts_index,
 	.cali_val = MT8183_CALIBRATION,
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 7/8] thermal: mediatek: add another get_temp ops for thermal sensors
  2019-05-02 10:43 ` michael.kao
@ 2019-05-02 10:43   ` michael.kao
  -1 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

From: Michael Kao <michael.kao@mediatek.com>

Provide thermal zone to read thermal sensor
in the SoC. We can read all the thermal sensors
value in the SoC by the node /sys/class/thermal/

Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 drivers/thermal/mtk_thermal.c | 68 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index cb41e46..d5c78b0 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -230,6 +230,11 @@ enum {
 
 struct mtk_thermal;
 
+struct mtk_thermal_zone {
+	struct mtk_thermal *mt;
+	int id;
+};
+
 struct thermal_bank_cfg {
 	unsigned int num_sensors;
 	const int *sensors;
@@ -612,7 +617,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
 		 * not immediately shut down.
 		 */
 		if (temp > 200000)
-			temp = 0;
+			temp = -EACCES;
 
 		if (temp > max)
 			max = temp;
@@ -623,7 +628,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
 
 static int mtk_read_temp(void *data, int *temperature)
 {
-	struct mtk_thermal *mt = data;
+	struct mtk_thermal_zone *tz = data;
+	struct mtk_thermal *mt = tz->mt;
 	int i;
 	int tempmax = INT_MIN;
 
@@ -636,16 +642,48 @@ static int mtk_read_temp(void *data, int *temperature)
 
 		mtk_thermal_put_bank(bank);
 	}
-
 	*temperature = tempmax;
 
 	return 0;
 }
 
+static int mtk_read_sensor_temp(void *data, int *temperature)
+{
+	struct mtk_thermal_zone *tz = data;
+	struct mtk_thermal *mt = tz->mt;
+	const struct mtk_thermal_data *conf = mt->conf;
+	int id = tz->id - 1;
+	int temp = INT_MIN;
+	u32 raw;
+
+	if (id < 0)
+		return  -EACCES;
+
+	raw = readl(mt->thermal_base + conf->msr[id]);
+
+	temp = raw_to_mcelsius(mt, id, raw);
+
+	/*
+	 * The first read of a sensor often contains very high bogus
+	 * temperature value. Filter these out so that the system does
+	 * not immediately shut down.
+	 */
+
+	if (temp > 200000)
+		return  -EACCES;
+
+	*temperature = temp;
+	return 0;
+}
+
 static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
 	.get_temp = mtk_read_temp,
 };
 
+static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
+	.get_temp = mtk_read_sensor_temp,
+};
+
 static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
 				  u32 apmixed_phys_base, u32 auxadc_phys_base,
 				  int ctrl_id)
@@ -878,6 +916,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 	struct resource *res;
 	u64 auxadc_phys_base, apmixed_phys_base;
 	struct thermal_zone_device *tzdev;
+	struct mtk_thermal_zone *tz;
 
 	mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
 	if (!mt)
@@ -959,11 +998,24 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mt);
 
-	tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
-						     &mtk_thermal_ops);
-	if (IS_ERR(tzdev)) {
-		ret = PTR_ERR(tzdev);
-		goto err_disable_clk_peri_therm;
+	for (i = 0; i < mt->conf->num_sensors + 1; i++) {
+		tz = kmalloc(sizeof(*tz), GFP_KERNEL);
+		if (!tz)
+			return -ENOMEM;
+
+		tz->mt = mt;
+		tz->id = i;
+
+		tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
+				tz, (i == 0) ?
+				&mtk_thermal_ops : &mtk_thermal_sensor_ops);
+
+		if (IS_ERR(tzdev)) {
+			if (IS_ERR(tzdev) != -EACCES) {
+				ret = PTR_ERR(tzdev);
+				goto err_disable_clk_peri_therm;
+			}
+		}
 	}
 
 	return 0;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 7/8] thermal: mediatek: add another get_temp ops for thermal sensors
@ 2019-05-02 10:43   ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

From: Michael Kao <michael.kao@mediatek.com>

Provide thermal zone to read thermal sensor
in the SoC. We can read all the thermal sensors
value in the SoC by the node /sys/class/thermal/

Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 drivers/thermal/mtk_thermal.c | 68 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 60 insertions(+), 8 deletions(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index cb41e46..d5c78b0 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -230,6 +230,11 @@ enum {
 
 struct mtk_thermal;
 
+struct mtk_thermal_zone {
+	struct mtk_thermal *mt;
+	int id;
+};
+
 struct thermal_bank_cfg {
 	unsigned int num_sensors;
 	const int *sensors;
@@ -612,7 +617,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
 		 * not immediately shut down.
 		 */
 		if (temp > 200000)
-			temp = 0;
+			temp = -EACCES;
 
 		if (temp > max)
 			max = temp;
@@ -623,7 +628,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
 
 static int mtk_read_temp(void *data, int *temperature)
 {
-	struct mtk_thermal *mt = data;
+	struct mtk_thermal_zone *tz = data;
+	struct mtk_thermal *mt = tz->mt;
 	int i;
 	int tempmax = INT_MIN;
 
@@ -636,16 +642,48 @@ static int mtk_read_temp(void *data, int *temperature)
 
 		mtk_thermal_put_bank(bank);
 	}
-
 	*temperature = tempmax;
 
 	return 0;
 }
 
+static int mtk_read_sensor_temp(void *data, int *temperature)
+{
+	struct mtk_thermal_zone *tz = data;
+	struct mtk_thermal *mt = tz->mt;
+	const struct mtk_thermal_data *conf = mt->conf;
+	int id = tz->id - 1;
+	int temp = INT_MIN;
+	u32 raw;
+
+	if (id < 0)
+		return  -EACCES;
+
+	raw = readl(mt->thermal_base + conf->msr[id]);
+
+	temp = raw_to_mcelsius(mt, id, raw);
+
+	/*
+	 * The first read of a sensor often contains very high bogus
+	 * temperature value. Filter these out so that the system does
+	 * not immediately shut down.
+	 */
+
+	if (temp > 200000)
+		return  -EACCES;
+
+	*temperature = temp;
+	return 0;
+}
+
 static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
 	.get_temp = mtk_read_temp,
 };
 
+static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
+	.get_temp = mtk_read_sensor_temp,
+};
+
 static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
 				  u32 apmixed_phys_base, u32 auxadc_phys_base,
 				  int ctrl_id)
@@ -878,6 +916,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 	struct resource *res;
 	u64 auxadc_phys_base, apmixed_phys_base;
 	struct thermal_zone_device *tzdev;
+	struct mtk_thermal_zone *tz;
 
 	mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
 	if (!mt)
@@ -959,11 +998,24 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mt);
 
-	tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
-						     &mtk_thermal_ops);
-	if (IS_ERR(tzdev)) {
-		ret = PTR_ERR(tzdev);
-		goto err_disable_clk_peri_therm;
+	for (i = 0; i < mt->conf->num_sensors + 1; i++) {
+		tz = kmalloc(sizeof(*tz), GFP_KERNEL);
+		if (!tz)
+			return -ENOMEM;
+
+		tz->mt = mt;
+		tz->id = i;
+
+		tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
+				tz, (i == 0) ?
+				&mtk_thermal_ops : &mtk_thermal_sensor_ops);
+
+		if (IS_ERR(tzdev)) {
+			if (IS_ERR(tzdev) != -EACCES) {
+				ret = PTR_ERR(tzdev);
+				goto err_disable_clk_peri_therm;
+			}
+		}
 	}
 
 	return 0;
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 8/8] thermal: mediatek: use spinlock to protect PTPCORESEL
  2019-05-02 10:43 ` michael.kao
@ 2019-05-02 10:43   ` michael.kao
  -1 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

The driver of thermal and svs will use the
same register for the project which should select
bank before reading sensor value.

Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
---
 drivers/thermal/mtk_thermal.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index d5c78b0..6d7e839 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -30,6 +30,7 @@
 #include <linux/thermal.h>
 #include <linux/reset.h>
 #include <linux/types.h>
+#include <linux/power/mtk_svs.h>
 
 /* AUXADC Registers */
 #define AUXADC_CON1_SET_V	0x008
@@ -267,7 +268,7 @@ struct mtk_thermal {
 	struct clk *clk_peri_therm;
 	struct clk *clk_auxadc;
 	/* lock: for getting and putting banks */
-	struct mutex lock;
+	unsigned long flags;
 
 	/* Calibration values */
 	s32 adc_ge;
@@ -566,7 +567,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
 	u32 val;
 
 	if (mt->conf->need_switch_bank) {
-		mutex_lock(&mt->lock);
+		mt->flags = claim_mtk_svs_lock();
 
 		val = readl(mt->thermal_base + PTPCORESEL);
 		val &= ~0xf;
@@ -586,7 +587,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
 	struct mtk_thermal *mt = bank->mt;
 
 	if (mt->conf->need_switch_bank)
-		mutex_unlock(&mt->lock);
+		release_mtk_svs_lock(mt->flags);
 }
 
 /**
@@ -941,8 +942,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	mutex_init(&mt->lock);
-
 	mt->dev = &pdev->dev;
 
 	auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 8/8] thermal: mediatek: use spinlock to protect PTPCORESEL
@ 2019-05-02 10:43   ` michael.kao
  0 siblings, 0 replies; 52+ messages in thread
From: michael.kao @ 2019-05-02 10:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

The driver of thermal and svs will use the
same register for the project which should select
bank before reading sensor value.

Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
---
 drivers/thermal/mtk_thermal.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index d5c78b0..6d7e839 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -30,6 +30,7 @@
 #include <linux/thermal.h>
 #include <linux/reset.h>
 #include <linux/types.h>
+#include <linux/power/mtk_svs.h>
 
 /* AUXADC Registers */
 #define AUXADC_CON1_SET_V	0x008
@@ -267,7 +268,7 @@ struct mtk_thermal {
 	struct clk *clk_peri_therm;
 	struct clk *clk_auxadc;
 	/* lock: for getting and putting banks */
-	struct mutex lock;
+	unsigned long flags;
 
 	/* Calibration values */
 	s32 adc_ge;
@@ -566,7 +567,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
 	u32 val;
 
 	if (mt->conf->need_switch_bank) {
-		mutex_lock(&mt->lock);
+		mt->flags = claim_mtk_svs_lock();
 
 		val = readl(mt->thermal_base + PTPCORESEL);
 		val &= ~0xf;
@@ -586,7 +587,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
 	struct mtk_thermal *mt = bank->mt;
 
 	if (mt->conf->need_switch_bank)
-		mutex_unlock(&mt->lock);
+		release_mtk_svs_lock(mt->flags);
 }
 
 /**
@@ -941,8 +942,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	mutex_init(&mt->lock);
-
 	mt->dev = &pdev->dev;
 
 	auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/8] arm64: dts: mt8183: add/update dynamic power coefficients
  2019-05-02 10:43   ` michael.kao
@ 2019-05-03  7:16     ` Hsin-Yi Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-03  7:16 UTC (permalink / raw)
  To: michael.kao
  Cc: fan.chen, jamesjj.liao, dawei.chien, louis.yu, roger.lu,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, Matthias Brugger, devicetree, linux-mediatek,
	linux-kernel, linux-arm-kernel, linux-pm

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> Add dynamic power coefficients for all cores and update those of
> CPU0 and CPU4.
>
> Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index b92116f..5668fb8 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -58,6 +58,8 @@
>                         compatible = "arm,cortex-a53";
>                         reg = <0x000>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <84>;
> +                       #cooling-cells = <2>;
Should this line be in [3/8] arm64: dts: mt8183: Add #cooling-cells to
CPU nodes?

>                 };
>
>                 cpu1: cpu@1 {
> @@ -65,6 +67,8 @@
>                         compatible = "arm,cortex-a53";
>                         reg = <0x001>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <84>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu2: cpu@2 {
> @@ -72,6 +76,8 @@
>                         compatible = "arm,cortex-a53";
>                         reg = <0x002>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <84>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu3: cpu@3 {
> @@ -79,6 +85,8 @@
>                         compatible = "arm,cortex-a53";
>                         reg = <0x003>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <84>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu4: cpu@100 {
> @@ -86,6 +94,8 @@
>                         compatible = "arm,cortex-a73";
>                         reg = <0x100>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <211>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu5: cpu@101 {
> @@ -93,6 +103,8 @@
>                         compatible = "arm,cortex-a73";
>                         reg = <0x101>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <211>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu6: cpu@102 {
> @@ -100,6 +112,8 @@
>                         compatible = "arm,cortex-a73";
>                         reg = <0x102>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <211>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu7: cpu@103 {
> @@ -107,6 +121,8 @@
>                         compatible = "arm,cortex-a73";
>                         reg = <0x103>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <211>;
> +                       #cooling-cells = <2>;
>                 };
>         };
>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/8] arm64: dts: mt8183: add/update dynamic power coefficients
@ 2019-05-03  7:16     ` Hsin-Yi Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-03  7:16 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, linux-kernel,
	Eduardo Valentin, fan.chen, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, linux-arm-kernel

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> Add dynamic power coefficients for all cores and update those of
> CPU0 and CPU4.
>
> Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index b92116f..5668fb8 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -58,6 +58,8 @@
>                         compatible = "arm,cortex-a53";
>                         reg = <0x000>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <84>;
> +                       #cooling-cells = <2>;
Should this line be in [3/8] arm64: dts: mt8183: Add #cooling-cells to
CPU nodes?

>                 };
>
>                 cpu1: cpu@1 {
> @@ -65,6 +67,8 @@
>                         compatible = "arm,cortex-a53";
>                         reg = <0x001>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <84>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu2: cpu@2 {
> @@ -72,6 +76,8 @@
>                         compatible = "arm,cortex-a53";
>                         reg = <0x002>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <84>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu3: cpu@3 {
> @@ -79,6 +85,8 @@
>                         compatible = "arm,cortex-a53";
>                         reg = <0x003>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <84>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu4: cpu@100 {
> @@ -86,6 +94,8 @@
>                         compatible = "arm,cortex-a73";
>                         reg = <0x100>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <211>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu5: cpu@101 {
> @@ -93,6 +103,8 @@
>                         compatible = "arm,cortex-a73";
>                         reg = <0x101>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <211>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu6: cpu@102 {
> @@ -100,6 +112,8 @@
>                         compatible = "arm,cortex-a73";
>                         reg = <0x102>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <211>;
> +                       #cooling-cells = <2>;
>                 };
>
>                 cpu7: cpu@103 {
> @@ -107,6 +121,8 @@
>                         compatible = "arm,cortex-a73";
>                         reg = <0x103>;
>                         enable-method = "psci";
> +                       dynamic-power-coefficient = <211>;
> +                       #cooling-cells = <2>;
>                 };
>         };
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
  2019-05-02 10:43     ` michael.kao
@ 2019-05-03  8:03       ` Hsin-Yi Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-03  8:03 UTC (permalink / raw)
  To: michael.kao
  Cc: fan.chen, jamesjj.liao, dawei.chien, louis.yu, roger.lu,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, Matthias Brugger, devicetree, linux-mediatek,
	linux-kernel, linux-arm-kernel, linux-pm

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> Add thermal zone node to Mediatek MT8183 dts file.
>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 926df75..b92116f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -334,6 +334,67 @@
>                         status = "disabled";
>                 };
>
> +               thermal: thermal@1100b000 {
> +                       #thermal-sensor-cells = <1>;
> +                       compatible = "mediatek,mt8183-thermal";
> +                       reg = <0 0x1100b000 0 0x1000>;
> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> +                       clocks = <&infracfg CLK_INFRA_THERM>,
> +                                <&infracfg CLK_INFRA_AUXADC>;
> +                       clock-names = "therm", "auxadc";
> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> +                       mediatek,auxadc = <&auxadc>;
> +                       mediatek,apmixedsys = <&apmixedsys>;
> +                       mediatek,hw-reset-temp = <117000>;
> +                       nvmem-cells = <&thermal_calibration>;
> +                       nvmem-cell-names = "calibration-data";
> +               };
> +
> +               thermal-zones {
> +                       cpu_thermal: cpu_thermal {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +
> +                               thermal-sensors = <&thermal 0>;
> +                               sustainable-power = <1500>;
> +                       };
> +
> +                       tzts1: tzts1 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 1>;
Is sustainable-power required for tzts? Though it's an optional
property, kernel would have warning:
[    0.631556] thermal thermal_zone1: power_allocator:
sustainable_power will be estimated
[    0.639586] thermal thermal_zone2: power_allocator:
sustainable_power will be estimated
[    0.647611] thermal thermal_zone3: power_allocator:
sustainable_power will be estimated
[    0.655635] thermal thermal_zone4: power_allocator:
sustainable_power will be estimated
[    0.663658] thermal thermal_zone5: power_allocator:
sustainable_power will be estimated
if no sustainable-power assigned.

> +                       };
> +
> +                       tzts2: tzts2 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 2>;
> +                       };
> +
> +                       tzts3: tzts3 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 3>;
> +                       };
> +
> +                       tzts4: tzts4 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 4>;
> +                       };
> +
> +                       tzts5: tzts5 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 5>;
> +                       };
> +
> +                       tztsABB: tztsABB {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 6>;
> +                       };
> +               };
>                 audiosys: syscon@11220000 {
>                         compatible = "mediatek,mt8183-audiosys", "syscon";
>                         reg = <0 0x11220000 0 0x1000>;
> @@ -368,6 +429,9 @@
>                         compatible = "mediatek,mt8183-efuse",
>                                      "mediatek,efuse";
>                         reg = <0 0x11f10000 0 0x1000>;
> +                       thermal_calibration: calib@180 {
> +                               reg = <0x180 0xc>;
> +                       };
>                 };
>
>                 mfgcfg: syscon@13000000 {

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-03  8:03       ` Hsin-Yi Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-03  8:03 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, linux-kernel,
	Eduardo Valentin, fan.chen, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, linux-arm-kernel

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> Add thermal zone node to Mediatek MT8183 dts file.
>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 926df75..b92116f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -334,6 +334,67 @@
>                         status = "disabled";
>                 };
>
> +               thermal: thermal@1100b000 {
> +                       #thermal-sensor-cells = <1>;
> +                       compatible = "mediatek,mt8183-thermal";
> +                       reg = <0 0x1100b000 0 0x1000>;
> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> +                       clocks = <&infracfg CLK_INFRA_THERM>,
> +                                <&infracfg CLK_INFRA_AUXADC>;
> +                       clock-names = "therm", "auxadc";
> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> +                       mediatek,auxadc = <&auxadc>;
> +                       mediatek,apmixedsys = <&apmixedsys>;
> +                       mediatek,hw-reset-temp = <117000>;
> +                       nvmem-cells = <&thermal_calibration>;
> +                       nvmem-cell-names = "calibration-data";
> +               };
> +
> +               thermal-zones {
> +                       cpu_thermal: cpu_thermal {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +
> +                               thermal-sensors = <&thermal 0>;
> +                               sustainable-power = <1500>;
> +                       };
> +
> +                       tzts1: tzts1 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 1>;
Is sustainable-power required for tzts? Though it's an optional
property, kernel would have warning:
[    0.631556] thermal thermal_zone1: power_allocator:
sustainable_power will be estimated
[    0.639586] thermal thermal_zone2: power_allocator:
sustainable_power will be estimated
[    0.647611] thermal thermal_zone3: power_allocator:
sustainable_power will be estimated
[    0.655635] thermal thermal_zone4: power_allocator:
sustainable_power will be estimated
[    0.663658] thermal thermal_zone5: power_allocator:
sustainable_power will be estimated
if no sustainable-power assigned.

> +                       };
> +
> +                       tzts2: tzts2 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 2>;
> +                       };
> +
> +                       tzts3: tzts3 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 3>;
> +                       };
> +
> +                       tzts4: tzts4 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 4>;
> +                       };
> +
> +                       tzts5: tzts5 {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 5>;
> +                       };
> +
> +                       tztsABB: tztsABB {
> +                               polling-delay-passive = <1000>;
> +                               polling-delay = <1000>;
> +                               thermal-sensors = <&thermal 6>;
> +                       };
> +               };
>                 audiosys: syscon@11220000 {
>                         compatible = "mediatek,mt8183-audiosys", "syscon";
>                         reg = <0 0x11220000 0 0x1000>;
> @@ -368,6 +429,9 @@
>                         compatible = "mediatek,mt8183-efuse",
>                                      "mediatek,efuse";
>                         reg = <0 0x11f10000 0 0x1000>;
> +                       thermal_calibration: calib@180 {
> +                               reg = <0x180 0xc>;
> +                       };
>                 };
>
>                 mfgcfg: syscon@13000000 {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
  2019-05-03  8:03       ` Hsin-Yi Wang
@ 2019-05-03 16:46         ` Matthias Kaehlcke
  -1 siblings, 0 replies; 52+ messages in thread
From: Matthias Kaehlcke @ 2019-05-03 16:46 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, Matthias Brugger, devicetree,
	linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

Hi,

On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> >
> > Add thermal zone node to Mediatek MT8183 dts file.
> >
> > Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 926df75..b92116f 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -334,6 +334,67 @@
> >                         status = "disabled";
> >                 };
> >
> > +               thermal: thermal@1100b000 {
> > +                       #thermal-sensor-cells = <1>;
> > +                       compatible = "mediatek,mt8183-thermal";
> > +                       reg = <0 0x1100b000 0 0x1000>;
> > +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> > +                       clocks = <&infracfg CLK_INFRA_THERM>,
> > +                                <&infracfg CLK_INFRA_AUXADC>;
> > +                       clock-names = "therm", "auxadc";
> > +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> > +                       mediatek,auxadc = <&auxadc>;
> > +                       mediatek,apmixedsys = <&apmixedsys>;
> > +                       mediatek,hw-reset-temp = <117000>;
> > +                       nvmem-cells = <&thermal_calibration>;
> > +                       nvmem-cell-names = "calibration-data";
> > +               };
> > +
> > +               thermal-zones {
> > +                       cpu_thermal: cpu_thermal {
> > +                               polling-delay-passive = <1000>;
> > +                               polling-delay = <1000>;
> > +
> > +                               thermal-sensors = <&thermal 0>;
> > +                               sustainable-power = <1500>;
> > +                       };
> > +
> > +                       tzts1: tzts1 {
> > +                               polling-delay-passive = <1000>;
> > +                               polling-delay = <1000>;
> > +                               thermal-sensors = <&thermal 1>;
> Is sustainable-power required for tzts? Though it's an optional
> property, kernel would have warning:
> [    0.631556] thermal thermal_zone1: power_allocator:
> sustainable_power will be estimated
> [    0.639586] thermal thermal_zone2: power_allocator:
> sustainable_power will be estimated
> [    0.647611] thermal thermal_zone3: power_allocator:
> sustainable_power will be estimated
> [    0.655635] thermal thermal_zone4: power_allocator:
> sustainable_power will be estimated
> [    0.663658] thermal thermal_zone5: power_allocator:
> sustainable_power will be estimated
> if no sustainable-power assigned.

The property is indeed optional, if it isn't specified IPA will use
the sum of the minimum power of all 'power actors' of the zone as
estimate (see estimate_sustainable_power()). This may lead to overly
agressive throttling, since the nominal sustainable power will always
be <= the requested power.

In my understanding the sustainable power may varies between devices,
even for the same SoC. One could have all the hardware crammed into a
tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
laptop form factor and a metal enclosure (e.g. ASUS C201). Both
examples are based on an Rockchip rk3288, but they have completely
different thermal behavior, and would likely have different values for
'sustainable-power'.

In this sense I tend to consider 'sustainable-power' more a device,
than a SoC property. You could specify a 'reasonable' value as a
starting point, but it will likely not be optimal for all or even most
devices. The warning might even be useful for device makers by
indicating them that there is room for tweaking.

I'm not an expert in the matter though, just happend to look into this
recently :)

Cheers

Matthias

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-03 16:46         ` Matthias Kaehlcke
  0 siblings, 0 replies; 52+ messages in thread
From: Matthias Kaehlcke @ 2019-05-03 16:46 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, linux-kernel, michael.kao,
	Eduardo Valentin, fan.chen, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, linux-arm-kernel

Hi,

On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> >
> > Add thermal zone node to Mediatek MT8183 dts file.
> >
> > Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 926df75..b92116f 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -334,6 +334,67 @@
> >                         status = "disabled";
> >                 };
> >
> > +               thermal: thermal@1100b000 {
> > +                       #thermal-sensor-cells = <1>;
> > +                       compatible = "mediatek,mt8183-thermal";
> > +                       reg = <0 0x1100b000 0 0x1000>;
> > +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> > +                       clocks = <&infracfg CLK_INFRA_THERM>,
> > +                                <&infracfg CLK_INFRA_AUXADC>;
> > +                       clock-names = "therm", "auxadc";
> > +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> > +                       mediatek,auxadc = <&auxadc>;
> > +                       mediatek,apmixedsys = <&apmixedsys>;
> > +                       mediatek,hw-reset-temp = <117000>;
> > +                       nvmem-cells = <&thermal_calibration>;
> > +                       nvmem-cell-names = "calibration-data";
> > +               };
> > +
> > +               thermal-zones {
> > +                       cpu_thermal: cpu_thermal {
> > +                               polling-delay-passive = <1000>;
> > +                               polling-delay = <1000>;
> > +
> > +                               thermal-sensors = <&thermal 0>;
> > +                               sustainable-power = <1500>;
> > +                       };
> > +
> > +                       tzts1: tzts1 {
> > +                               polling-delay-passive = <1000>;
> > +                               polling-delay = <1000>;
> > +                               thermal-sensors = <&thermal 1>;
> Is sustainable-power required for tzts? Though it's an optional
> property, kernel would have warning:
> [    0.631556] thermal thermal_zone1: power_allocator:
> sustainable_power will be estimated
> [    0.639586] thermal thermal_zone2: power_allocator:
> sustainable_power will be estimated
> [    0.647611] thermal thermal_zone3: power_allocator:
> sustainable_power will be estimated
> [    0.655635] thermal thermal_zone4: power_allocator:
> sustainable_power will be estimated
> [    0.663658] thermal thermal_zone5: power_allocator:
> sustainable_power will be estimated
> if no sustainable-power assigned.

The property is indeed optional, if it isn't specified IPA will use
the sum of the minimum power of all 'power actors' of the zone as
estimate (see estimate_sustainable_power()). This may lead to overly
agressive throttling, since the nominal sustainable power will always
be <= the requested power.

In my understanding the sustainable power may varies between devices,
even for the same SoC. One could have all the hardware crammed into a
tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
laptop form factor and a metal enclosure (e.g. ASUS C201). Both
examples are based on an Rockchip rk3288, but they have completely
different thermal behavior, and would likely have different values for
'sustainable-power'.

In this sense I tend to consider 'sustainable-power' more a device,
than a SoC property. You could specify a 'reasonable' value as a
starting point, but it will likely not be optimal for all or even most
devices. The warning might even be useful for device makers by
indicating them that there is room for tweaking.

I'm not an expert in the matter though, just happend to look into this
recently :)

Cheers

Matthias

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 3/8] arm64: dts: mt8183: Add #cooling-cells to CPU nodes
  2019-05-02 10:43   ` michael.kao
@ 2019-05-06  5:25     ` Eddie Huang
  -1 siblings, 0 replies; 52+ messages in thread
From: Eddie Huang @ 2019-05-06  5:25 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, JamesJJ Liao (廖建智),
	devicetree, Louis Yu (游政錕),
	Dawei Chien (錢大衛),
	linux-pm, Daniel Lezcano, Roger Lu (陸瑞傑),
	linux-kernel, Eduardo Valentin, Fan Chen (陳凡),
	Rob Herring, linux-mediatek, Matthias Brugger, Zhang Rui

Hi Michael,

On Thu, 2019-05-02 at 18:43 +0800, michael.kao wrote:
> The #cooling-cells property needs to be specified to allow a CPU
> to be used as cooling device.
> 
> Signed-off-by: michael.kao <Michael.Kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 5668fb8..95f1d7b 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -11,6 +11,8 @@
>  #include <dt-bindings/power/mt8183-power.h>
>  #include <dt-bindings/reset-controller/mt8183-resets.h>
>  #include "mt8183-pinfunc.h"
> +#include <dt-bindings/thermal/thermal.h>
> +
>  / {
>  	compatible = "mediatek,mt8183";
>  	interrupt-parent = <&sysirq>;

It seems this patch don't have #cooling-cells but in your 2/8 patch in
this series

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 3/8] arm64: dts: mt8183: Add #cooling-cells to CPU nodes
@ 2019-05-06  5:25     ` Eddie Huang
  0 siblings, 0 replies; 52+ messages in thread
From: Eddie Huang @ 2019-05-06  5:25 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, JamesJJ Liao (廖建智),
	devicetree, Louis Yu (游政錕),
	Dawei Chien (錢大衛),
	linux-pm, Daniel Lezcano, Roger Lu (陸瑞傑),
	linux-kernel, Eduardo Valentin, Fan Chen (陳凡),
	Rob Herring, linux-mediatek, Matthias Brugger, Zhang Rui,
	linux-arm-kernel

Hi Michael,

On Thu, 2019-05-02 at 18:43 +0800, michael.kao wrote:
> The #cooling-cells property needs to be specified to allow a CPU
> to be used as cooling device.
> 
> Signed-off-by: michael.kao <Michael.Kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 5668fb8..95f1d7b 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -11,6 +11,8 @@
>  #include <dt-bindings/power/mt8183-power.h>
>  #include <dt-bindings/reset-controller/mt8183-resets.h>
>  #include "mt8183-pinfunc.h"
> +#include <dt-bindings/thermal/thermal.h>
> +
>  / {
>  	compatible = "mediatek,mt8183";
>  	interrupt-parent = <&sysirq>;

It seems this patch don't have #cooling-cells but in your 2/8 patch in
this series



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
  2019-05-02 10:43     ` michael.kao
@ 2019-05-06  9:43       ` Daniel Lezcano
  -1 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2019-05-06  9:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland,
	Matthias Brugger
  Cc: linux-pm, devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

On 02/05/2019 12:43, michael.kao wrote:
> Add thermal zone node to Mediatek MT8183 dts file.
> 
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---

Hi Michael,

the device tree binding for thermal specifies the thermal zone must
define a cooling-maps (it is a required field).

All the thermal zones below tzts1, tzts2, etc ... do not have it.


>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 926df75..b92116f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -334,6 +334,67 @@
>  			status = "disabled";
>  		};
>  
> +		thermal: thermal@1100b000 {
> +			#thermal-sensor-cells = <1>;
> +			compatible = "mediatek,mt8183-thermal";
> +			reg = <0 0x1100b000 0 0x1000>;
> +			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> +			clocks = <&infracfg CLK_INFRA_THERM>,
> +				 <&infracfg CLK_INFRA_AUXADC>;
> +			clock-names = "therm", "auxadc";
> +			resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> +			mediatek,auxadc = <&auxadc>;
> +			mediatek,apmixedsys = <&apmixedsys>;
> +			mediatek,hw-reset-temp = <117000>;
> +			nvmem-cells = <&thermal_calibration>;
> +			nvmem-cell-names = "calibration-data";
> +		};
> +
> +		thermal-zones {
> +			cpu_thermal: cpu_thermal {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +
> +				thermal-sensors = <&thermal 0>;
> +				sustainable-power = <1500>;
> +			};
> +
> +			tzts1: tzts1 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 1>;
> +			};
> +
> +			tzts2: tzts2 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 2>;
> +			};
> +
> +			tzts3: tzts3 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 3>;
> +			};
> +
> +			tzts4: tzts4 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 4>;
> +			};
> +
> +			tzts5: tzts5 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 5>;
> +			};
> +
> +			tztsABB: tztsABB {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 6>;
> +			};
> +		};
>  		audiosys: syscon@11220000 {
>  			compatible = "mediatek,mt8183-audiosys", "syscon";
>  			reg = <0 0x11220000 0 0x1000>;
> @@ -368,6 +429,9 @@
>  			compatible = "mediatek,mt8183-efuse",
>  				     "mediatek,efuse";
>  			reg = <0 0x11f10000 0 0x1000>;
> +			thermal_calibration: calib@180 {
> +				reg = <0x180 0xc>;
> +			};
>  		};
>  
>  		mfgcfg: syscon@13000000 {
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-06  9:43       ` Daniel Lezcano
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2019-05-06  9:43 UTC (permalink / raw)
  To: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland,
	Matthias Brugger
  Cc: devicetree, linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

On 02/05/2019 12:43, michael.kao wrote:
> Add thermal zone node to Mediatek MT8183 dts file.
> 
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---

Hi Michael,

the device tree binding for thermal specifies the thermal zone must
define a cooling-maps (it is a required field).

All the thermal zones below tzts1, tzts2, etc ... do not have it.


>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 926df75..b92116f 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -334,6 +334,67 @@
>  			status = "disabled";
>  		};
>  
> +		thermal: thermal@1100b000 {
> +			#thermal-sensor-cells = <1>;
> +			compatible = "mediatek,mt8183-thermal";
> +			reg = <0 0x1100b000 0 0x1000>;
> +			interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> +			clocks = <&infracfg CLK_INFRA_THERM>,
> +				 <&infracfg CLK_INFRA_AUXADC>;
> +			clock-names = "therm", "auxadc";
> +			resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> +			mediatek,auxadc = <&auxadc>;
> +			mediatek,apmixedsys = <&apmixedsys>;
> +			mediatek,hw-reset-temp = <117000>;
> +			nvmem-cells = <&thermal_calibration>;
> +			nvmem-cell-names = "calibration-data";
> +		};
> +
> +		thermal-zones {
> +			cpu_thermal: cpu_thermal {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +
> +				thermal-sensors = <&thermal 0>;
> +				sustainable-power = <1500>;
> +			};
> +
> +			tzts1: tzts1 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 1>;
> +			};
> +
> +			tzts2: tzts2 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 2>;
> +			};
> +
> +			tzts3: tzts3 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 3>;
> +			};
> +
> +			tzts4: tzts4 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 4>;
> +			};
> +
> +			tzts5: tzts5 {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 5>;
> +			};
> +
> +			tztsABB: tztsABB {
> +				polling-delay-passive = <1000>;
> +				polling-delay = <1000>;
> +				thermal-sensors = <&thermal 6>;
> +			};
> +		};
>  		audiosys: syscon@11220000 {
>  			compatible = "mediatek,mt8183-audiosys", "syscon";
>  			reg = <0 0x11220000 0 0x1000>;
> @@ -368,6 +429,9 @@
>  			compatible = "mediatek,mt8183-efuse",
>  				     "mediatek,efuse";
>  			reg = <0 0x11f10000 0 0x1000>;
> +			thermal_calibration: calib@180 {
> +				reg = <0x180 0xc>;
> +			};
>  		};
>  
>  		mfgcfg: syscon@13000000 {
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
  2019-05-03 16:46         ` Matthias Kaehlcke
@ 2019-05-06 10:43           ` Daniel Lezcano
  -1 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2019-05-06 10:43 UTC (permalink / raw)
  To: Matthias Kaehlcke, Hsin-Yi Wang
  Cc: michael.kao, fan.chen, jamesjj.liao, dawei.chien, louis.yu,
	roger.lu, Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland,
	Matthias Brugger, devicetree, linux-mediatek, linux-kernel,
	linux-arm-kernel, linux-pm

On 03/05/2019 18:46, Matthias Kaehlcke wrote:
> Hi,
> 
> On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
>> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>>>
>>> Add thermal zone node to Mediatek MT8183 dts file.
>>>
>>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
>>> ---
>>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
>>>  1 file changed, 64 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> index 926df75..b92116f 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> @@ -334,6 +334,67 @@
>>>                         status = "disabled";
>>>                 };
>>>
>>> +               thermal: thermal@1100b000 {
>>> +                       #thermal-sensor-cells = <1>;
>>> +                       compatible = "mediatek,mt8183-thermal";
>>> +                       reg = <0 0x1100b000 0 0x1000>;
>>> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
>>> +                       clocks = <&infracfg CLK_INFRA_THERM>,
>>> +                                <&infracfg CLK_INFRA_AUXADC>;
>>> +                       clock-names = "therm", "auxadc";
>>> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
>>> +                       mediatek,auxadc = <&auxadc>;
>>> +                       mediatek,apmixedsys = <&apmixedsys>;
>>> +                       mediatek,hw-reset-temp = <117000>;
>>> +                       nvmem-cells = <&thermal_calibration>;
>>> +                       nvmem-cell-names = "calibration-data";
>>> +               };
>>> +
>>> +               thermal-zones {
>>> +                       cpu_thermal: cpu_thermal {
>>> +                               polling-delay-passive = <1000>;
>>> +                               polling-delay = <1000>;
>>> +
>>> +                               thermal-sensors = <&thermal 0>;
>>> +                               sustainable-power = <1500>;
>>> +                       };
>>> +
>>> +                       tzts1: tzts1 {
>>> +                               polling-delay-passive = <1000>;
>>> +                               polling-delay = <1000>;
>>> +                               thermal-sensors = <&thermal 1>;
>> Is sustainable-power required for tzts? Though it's an optional
>> property, kernel would have warning:
>> [    0.631556] thermal thermal_zone1: power_allocator:
>> sustainable_power will be estimated
>> [    0.639586] thermal thermal_zone2: power_allocator:
>> sustainable_power will be estimated
>> [    0.647611] thermal thermal_zone3: power_allocator:
>> sustainable_power will be estimated
>> [    0.655635] thermal thermal_zone4: power_allocator:
>> sustainable_power will be estimated
>> [    0.663658] thermal thermal_zone5: power_allocator:
>> sustainable_power will be estimated
>> if no sustainable-power assigned.
> 
> The property is indeed optional, if it isn't specified IPA will use
> the sum of the minimum power of all 'power actors' of the zone as
> estimate (see estimate_sustainable_power()). This may lead to overly
> agressive throttling, since the nominal sustainable power will always
> be <= the requested power.
> 
> In my understanding the sustainable power may varies between devices,
> even for the same SoC. One could have all the hardware crammed into a
> tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
> laptop form factor and a metal enclosure (e.g. ASUS C201). Both
> examples are based on an Rockchip rk3288, but they have completely
> different thermal behavior, and would likely have different values for
> 'sustainable-power'.
> 
> In this sense I tend to consider 'sustainable-power' more a device,
> than a SoC property. You could specify a 'reasonable' value as a
> starting point, but it will likely not be optimal for all or even most
> devices. The warning might even be useful for device makers by
> indicating them that there is room for tweaking.


The sustainable power is the power dissipated by the devices belonging
to the thermal zone at the given trip temperature.

With the power numbers and the cooling devices, the IPA will change the
states of the cooling devices to leverage the dissipated power to the
sustainable power.

The contribution is the cooling effect of the cooling device.

However, the IPA is limited to one thermal zone and the cooling device
is the cpu cooling device. There is the devfreq cooling device but as
the graphic driver is not upstream, it is found in the android tree only
for the moment.

As you mentioned the sustainable power can vary depending on the form
factor and the production process for the same SoC (they can go to
higher frequencies thus dissipate more power). That is the reason why we
split the DT per SoC and we override the values on a per SoC version basis.

You can have a look the rk3399.dtsi and their variant for experimental
board (*-rock960.dts) and the chromebook version (*-gru-kevin.dts).

Do you want a empiric procedure to find out the sustainable power ?







-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-06 10:43           ` Daniel Lezcano
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2019-05-06 10:43 UTC (permalink / raw)
  To: Matthias Kaehlcke, Hsin-Yi Wang
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, roger.lu, linux-kernel, michael.kao, Eduardo Valentin,
	fan.chen, Rob Herring, linux-mediatek, Matthias Brugger,
	Zhang Rui, linux-arm-kernel

On 03/05/2019 18:46, Matthias Kaehlcke wrote:
> Hi,
> 
> On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
>> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>>>
>>> Add thermal zone node to Mediatek MT8183 dts file.
>>>
>>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
>>> ---
>>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
>>>  1 file changed, 64 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> index 926df75..b92116f 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
>>> @@ -334,6 +334,67 @@
>>>                         status = "disabled";
>>>                 };
>>>
>>> +               thermal: thermal@1100b000 {
>>> +                       #thermal-sensor-cells = <1>;
>>> +                       compatible = "mediatek,mt8183-thermal";
>>> +                       reg = <0 0x1100b000 0 0x1000>;
>>> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
>>> +                       clocks = <&infracfg CLK_INFRA_THERM>,
>>> +                                <&infracfg CLK_INFRA_AUXADC>;
>>> +                       clock-names = "therm", "auxadc";
>>> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
>>> +                       mediatek,auxadc = <&auxadc>;
>>> +                       mediatek,apmixedsys = <&apmixedsys>;
>>> +                       mediatek,hw-reset-temp = <117000>;
>>> +                       nvmem-cells = <&thermal_calibration>;
>>> +                       nvmem-cell-names = "calibration-data";
>>> +               };
>>> +
>>> +               thermal-zones {
>>> +                       cpu_thermal: cpu_thermal {
>>> +                               polling-delay-passive = <1000>;
>>> +                               polling-delay = <1000>;
>>> +
>>> +                               thermal-sensors = <&thermal 0>;
>>> +                               sustainable-power = <1500>;
>>> +                       };
>>> +
>>> +                       tzts1: tzts1 {
>>> +                               polling-delay-passive = <1000>;
>>> +                               polling-delay = <1000>;
>>> +                               thermal-sensors = <&thermal 1>;
>> Is sustainable-power required for tzts? Though it's an optional
>> property, kernel would have warning:
>> [    0.631556] thermal thermal_zone1: power_allocator:
>> sustainable_power will be estimated
>> [    0.639586] thermal thermal_zone2: power_allocator:
>> sustainable_power will be estimated
>> [    0.647611] thermal thermal_zone3: power_allocator:
>> sustainable_power will be estimated
>> [    0.655635] thermal thermal_zone4: power_allocator:
>> sustainable_power will be estimated
>> [    0.663658] thermal thermal_zone5: power_allocator:
>> sustainable_power will be estimated
>> if no sustainable-power assigned.
> 
> The property is indeed optional, if it isn't specified IPA will use
> the sum of the minimum power of all 'power actors' of the zone as
> estimate (see estimate_sustainable_power()). This may lead to overly
> agressive throttling, since the nominal sustainable power will always
> be <= the requested power.
> 
> In my understanding the sustainable power may varies between devices,
> even for the same SoC. One could have all the hardware crammed into a
> tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
> laptop form factor and a metal enclosure (e.g. ASUS C201). Both
> examples are based on an Rockchip rk3288, but they have completely
> different thermal behavior, and would likely have different values for
> 'sustainable-power'.
> 
> In this sense I tend to consider 'sustainable-power' more a device,
> than a SoC property. You could specify a 'reasonable' value as a
> starting point, but it will likely not be optimal for all or even most
> devices. The warning might even be useful for device makers by
> indicating them that there is room for tweaking.


The sustainable power is the power dissipated by the devices belonging
to the thermal zone at the given trip temperature.

With the power numbers and the cooling devices, the IPA will change the
states of the cooling devices to leverage the dissipated power to the
sustainable power.

The contribution is the cooling effect of the cooling device.

However, the IPA is limited to one thermal zone and the cooling device
is the cpu cooling device. There is the devfreq cooling device but as
the graphic driver is not upstream, it is found in the android tree only
for the moment.

As you mentioned the sustainable power can vary depending on the form
factor and the production process for the same SoC (they can go to
higher frequencies thus dissipate more power). That is the reason why we
split the DT per SoC and we override the values on a per SoC version basis.

You can have a look the rk3399.dtsi and their variant for experimental
board (*-rock960.dts) and the chromebook version (*-gru-kevin.dts).

Do you want a empiric procedure to find out the sustainable power ?







-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 4/8] arm64: dts: mt8183: Configure CPU cooling
  2019-05-02 10:43   ` michael.kao
@ 2019-05-07  8:57     ` Hsin-Yi Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-07  8:57 UTC (permalink / raw)
  To: michael.kao
  Cc: fan.chen, jamesjj.liao, dawei.chien, louis.yu, roger.lu,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, Matthias Brugger, devicetree, linux-pm,
	linux-kernel, Matthias Kaehlcke, linux-mediatek,
	linux-arm-kernel

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Matthias Kaehlcke <mka@chromium.org>
>
> Add two passive trip points at 68°C and 85°C for the CPU temperature.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 55 ++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 95f1d7b..0b3294b 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -375,6 +375,61 @@
>
>                                 thermal-sensors = <&thermal 0>;
>                                 sustainable-power = <1500>;
> +
> +                               trips {
> +                                       threshold: trip-point@0 {
> +                                               temperature = <68000>;
> +                                               hysteresis = <2000>;
> +                                               type = "passive";
> +                                       };
> +
> +                                       target: trip-point@1 {
> +                                               temperature = <85000>;
> +                                               hysteresis = <2000>;
> +                                               type = "passive";
> +                                       };
> +
> +                                       cpu_crit: cpu-crit {
> +                                               temperature = <115000>;
> +                                               hysteresis = <2000>;
> +                                               type = "critical";
> +                                       };
> +                               };
> +
> +                               cooling-maps {
> +                                       map0 {
> +                                               trip = <&target>;
> +                                               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>;
> +                                               contribution = <3072>;
> +                                       };
> +                                       map1 {
> +                                               trip = <&target>;
> +                                               cooling-device = <&cpu4
> +                                                       THERMAL_NO_LIMIT
> +                                                       THERMAL_NO_LIMIT>,
> +                                                                <&cpu5
> +                                                       THERMAL_NO_LIMIT
> +                                                       THERMAL_NO_LIMIT>,
> +                                                                <&cpu6
> +                                                       THERMAL_NO_LIMIT
> +                                                       THERMAL_NO_LIMIT>,
> +                                                                <&cpu7
> +                                                       THERMAL_NO_LIMIT
> +                                                       THERMAL_NO_LIMIT>;
> +                                               contribution = <1024>;
> +                                       };
> +                               };
>                         };
>
>                         tzts1: tzts1 {

Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 4/8] arm64: dts: mt8183: Configure CPU cooling
@ 2019-05-07  8:57     ` Hsin-Yi Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-07  8:57 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, linux-kernel,
	Eduardo Valentin, fan.chen, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, Matthias Kaehlcke, linux-arm-kernel

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Matthias Kaehlcke <mka@chromium.org>
>
> Add two passive trip points at 68°C and 85°C for the CPU temperature.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 55 ++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 95f1d7b..0b3294b 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -375,6 +375,61 @@
>
>                                 thermal-sensors = <&thermal 0>;
>                                 sustainable-power = <1500>;
> +
> +                               trips {
> +                                       threshold: trip-point@0 {
> +                                               temperature = <68000>;
> +                                               hysteresis = <2000>;
> +                                               type = "passive";
> +                                       };
> +
> +                                       target: trip-point@1 {
> +                                               temperature = <85000>;
> +                                               hysteresis = <2000>;
> +                                               type = "passive";
> +                                       };
> +
> +                                       cpu_crit: cpu-crit {
> +                                               temperature = <115000>;
> +                                               hysteresis = <2000>;
> +                                               type = "critical";
> +                                       };
> +                               };
> +
> +                               cooling-maps {
> +                                       map0 {
> +                                               trip = <&target>;
> +                                               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>;
> +                                               contribution = <3072>;
> +                                       };
> +                                       map1 {
> +                                               trip = <&target>;
> +                                               cooling-device = <&cpu4
> +                                                       THERMAL_NO_LIMIT
> +                                                       THERMAL_NO_LIMIT>,
> +                                                                <&cpu5
> +                                                       THERMAL_NO_LIMIT
> +                                                       THERMAL_NO_LIMIT>,
> +                                                                <&cpu6
> +                                                       THERMAL_NO_LIMIT
> +                                                       THERMAL_NO_LIMIT>,
> +                                                                <&cpu7
> +                                                       THERMAL_NO_LIMIT
> +                                                       THERMAL_NO_LIMIT>;
> +                                               contribution = <1024>;
> +                                       };
> +                               };
>                         };
>
>                         tzts1: tzts1 {

Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/8] arm64: dts: mt8183: Increase polling frequency for CPU thermal zone
  2019-05-02 10:43   ` michael.kao
@ 2019-05-07  8:58     ` Hsin-Yi Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-07  8:58 UTC (permalink / raw)
  To: michael.kao
  Cc: fan.chen, jamesjj.liao, dawei.chien, louis.yu, roger.lu,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, Matthias Brugger, devicetree, linux-pm,
	linux-kernel, Matthias Kaehlcke, linux-mediatek,
	linux-arm-kernel

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Matthias Kaehlcke <mka@chromium.org>
>
> Evaluate the thermal zone every 500ms while not cooling and every
> 100ms when passive cooling is performed.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 0b3294b..be879ac 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -370,8 +370,8 @@
>
>                 thermal-zones {
>                         cpu_thermal: cpu_thermal {
> -                               polling-delay-passive = <1000>;
> -                               polling-delay = <1000>;
> +                               polling-delay-passive = <100>;
> +                               polling-delay = <500>;
>
>                                 thermal-sensors = <&thermal 0>;
>                                 sustainable-power = <1500>;

Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 5/8] arm64: dts: mt8183: Increase polling frequency for CPU thermal zone
@ 2019-05-07  8:58     ` Hsin-Yi Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-07  8:58 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, linux-kernel,
	Eduardo Valentin, fan.chen, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, Matthias Kaehlcke, linux-arm-kernel

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Matthias Kaehlcke <mka@chromium.org>
>
> Evaluate the thermal zone every 500ms while not cooling and every
> 100ms when passive cooling is performed.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 0b3294b..be879ac 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -370,8 +370,8 @@
>
>                 thermal-zones {
>                         cpu_thermal: cpu_thermal {
> -                               polling-delay-passive = <1000>;
> -                               polling-delay = <1000>;
> +                               polling-delay-passive = <100>;
> +                               polling-delay = <500>;
>
>                                 thermal-sensors = <&thermal 0>;
>                                 sustainable-power = <1500>;

Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 6/8] thermal: mediatek: mt8183: fix bank number settings
  2019-05-02 10:43     ` michael.kao
@ 2019-05-07  8:58       ` Hsin-Yi Wang
  -1 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-07  8:58 UTC (permalink / raw)
  To: michael.kao
  Cc: fan.chen, jamesjj.liao, dawei.chien, louis.yu, roger.lu,
	Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, Matthias Brugger, devicetree, linux-mediatek,
	linux-kernel, linux-arm-kernel, linux-pm

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Michael Kao <michael.kao@mediatek.com>
>
> MT8183_NUM_ZONES should be set to 1
> because MT8183 doesn't have multiple banks.
>
> Fixes: a4ffe6b52d27 ("thermal: mediatek: add support for MT8183")
> Signed-off-by: Michael Kao <Michael.Kao@mediatek.com>
> ---
>  drivers/thermal/mtk_thermal.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 5c07a61..cb41e46 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -216,6 +216,9 @@ enum {
>  /* The total number of temperature sensors in the MT8183 */
>  #define MT8183_NUM_SENSORS     6
>
> +/* The number of banks in the MT8183 */
> +#define MT8183_NUM_ZONES               1
> +
>  /* The number of sensing points per bank */
>  #define MT8183_NUM_SENSORS_PER_ZONE     6
>
> @@ -503,7 +506,7 @@ struct mtk_thermal {
>
>  static const struct mtk_thermal_data mt8183_thermal_data = {
>         .auxadc_channel = MT8183_TEMP_AUXADC_CHANNEL,
> -       .num_banks = MT8183_NUM_SENSORS_PER_ZONE,
> +       .num_banks = MT8183_NUM_ZONES,
>         .num_sensors = MT8183_NUM_SENSORS,
>         .vts_index = mt8183_vts_index,
>         .cali_val = MT8183_CALIBRATION,

Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 6/8] thermal: mediatek: mt8183: fix bank number settings
@ 2019-05-07  8:58       ` Hsin-Yi Wang
  0 siblings, 0 replies; 52+ messages in thread
From: Hsin-Yi Wang @ 2019-05-07  8:58 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, linux-kernel,
	Eduardo Valentin, fan.chen, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, linux-arm-kernel

On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Michael Kao <michael.kao@mediatek.com>
>
> MT8183_NUM_ZONES should be set to 1
> because MT8183 doesn't have multiple banks.
>
> Fixes: a4ffe6b52d27 ("thermal: mediatek: add support for MT8183")
> Signed-off-by: Michael Kao <Michael.Kao@mediatek.com>
> ---
>  drivers/thermal/mtk_thermal.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 5c07a61..cb41e46 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -216,6 +216,9 @@ enum {
>  /* The total number of temperature sensors in the MT8183 */
>  #define MT8183_NUM_SENSORS     6
>
> +/* The number of banks in the MT8183 */
> +#define MT8183_NUM_ZONES               1
> +
>  /* The number of sensing points per bank */
>  #define MT8183_NUM_SENSORS_PER_ZONE     6
>
> @@ -503,7 +506,7 @@ struct mtk_thermal {
>
>  static const struct mtk_thermal_data mt8183_thermal_data = {
>         .auxadc_channel = MT8183_TEMP_AUXADC_CHANNEL,
> -       .num_banks = MT8183_NUM_SENSORS_PER_ZONE,
> +       .num_banks = MT8183_NUM_ZONES,
>         .num_sensors = MT8183_NUM_SENSORS,
>         .vts_index = mt8183_vts_index,
>         .cali_val = MT8183_CALIBRATION,

Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
  2019-05-06 10:43           ` Daniel Lezcano
  (?)
@ 2019-05-08 12:23             ` Michael Kao
  -1 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-08 12:23 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Matthias Kaehlcke, Hsin-Yi Wang, fan.chen, jamesjj.liao,
	dawei.chien, louis.yu, roger.lu, Zhang Rui, Eduardo Valentin,
	Rob Herring, Mark Rutland, Matthias Brugger, devicetree,
	linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

On Mon, 2019-05-06 at 12:43 +0200, Daniel Lezcano wrote:
> On 03/05/2019 18:46, Matthias Kaehlcke wrote:
> > Hi,
> > 
> > On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
> >> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> >>>
> >>> Add thermal zone node to Mediatek MT8183 dts file.
> >>>
> >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> >>> ---
> >>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
> >>>  1 file changed, 64 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> index 926df75..b92116f 100644
> >>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> @@ -334,6 +334,67 @@
> >>>                         status = "disabled";
> >>>                 };
> >>>
> >>> +               thermal: thermal@1100b000 {
> >>> +                       #thermal-sensor-cells = <1>;
> >>> +                       compatible = "mediatek,mt8183-thermal";
> >>> +                       reg = <0 0x1100b000 0 0x1000>;
> >>> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> >>> +                       clocks = <&infracfg CLK_INFRA_THERM>,
> >>> +                                <&infracfg CLK_INFRA_AUXADC>;
> >>> +                       clock-names = "therm", "auxadc";
> >>> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> >>> +                       mediatek,auxadc = <&auxadc>;
> >>> +                       mediatek,apmixedsys = <&apmixedsys>;
> >>> +                       mediatek,hw-reset-temp = <117000>;
> >>> +                       nvmem-cells = <&thermal_calibration>;
> >>> +                       nvmem-cell-names = "calibration-data";
> >>> +               };
> >>> +
> >>> +               thermal-zones {
> >>> +                       cpu_thermal: cpu_thermal {
> >>> +                               polling-delay-passive = <1000>;
> >>> +                               polling-delay = <1000>;
> >>> +
> >>> +                               thermal-sensors = <&thermal 0>;
> >>> +                               sustainable-power = <1500>;
> >>> +                       };
> >>> +
> >>> +                       tzts1: tzts1 {
> >>> +                               polling-delay-passive = <1000>;
> >>> +                               polling-delay = <1000>;
> >>> +                               thermal-sensors = <&thermal 1>;
> >> Is sustainable-power required for tzts? Though it's an optional
> >> property, kernel would have warning:
> >> [    0.631556] thermal thermal_zone1: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.639586] thermal thermal_zone2: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.647611] thermal thermal_zone3: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.655635] thermal thermal_zone4: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.663658] thermal thermal_zone5: power_allocator:
> >> sustainable_power will be estimated
> >> if no sustainable-power assigned.
> > 
> > The property is indeed optional, if it isn't specified IPA will use
> > the sum of the minimum power of all 'power actors' of the zone as
> > estimate (see estimate_sustainable_power()). This may lead to overly
> > agressive throttling, since the nominal sustainable power will always
> > be <= the requested power.
> > 
> > In my understanding the sustainable power may varies between devices,
> > even for the same SoC. One could have all the hardware crammed into a
> > tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
> > laptop form factor and a metal enclosure (e.g. ASUS C201). Both
> > examples are based on an Rockchip rk3288, but they have completely
> > different thermal behavior, and would likely have different values for
> > 'sustainable-power'.
> > 
> > In this sense I tend to consider 'sustainable-power' more a device,
> > than a SoC property. You could specify a 'reasonable' value as a
> > starting point, but it will likely not be optimal for all or even most
> > devices. The warning might even be useful for device makers by
> > indicating them that there is room for tweaking.
> 
> 
> The sustainable power is the power dissipated by the devices belonging
> to the thermal zone at the given trip temperature.
> 
> With the power numbers and the cooling devices, the IPA will change the
> states of the cooling devices to leverage the dissipated power to the
> sustainable power.
> 
> The contribution is the cooling effect of the cooling device.
> 
> However, the IPA is limited to one thermal zone and the cooling device
> is the cpu cooling device. There is the devfreq cooling device but as
> the graphic driver is not upstream, it is found in the android tree only
> for the moment.
> 
> As you mentioned the sustainable power can vary depending on the form
> factor and the production process for the same SoC (they can go to
> higher frequencies thus dissipate more power). That is the reason why we
> split the DT per SoC and we override the values on a per SoC version basis.
> 
> You can have a look the rk3399.dtsi and their variant for experimental
> board (*-rock960.dts) and the chromebook version (*-gru-kevin.dts).
> 
> Do you want a empiric procedure to find out the sustainable power ?
> 
> 
> 
OK, I will add the cooling map. But the tzts1 ~ tzts6 don't need to binding cooler.
The "cpu_thermal" is max value of tzts1 ~tzts6. And cpu_thermal bind
cooler with IPA. tzts1~6 don't need to add cooler. So, do I just add
cooling map without any binding any cooling-cell?

I think thermal framework will add estimated sustainable power. Maybe I
should add by myself. What's procedure do you recommend to find
sustainable power?



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-08 12:23             ` Michael Kao
  0 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-08 12:23 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Matthias Kaehlcke, Hsin-Yi Wang, fan.chen, jamesjj.liao,
	dawei.chien, louis.yu, roger.lu, Zhang Rui, Eduardo Valentin,
	Rob Herring, Mark Rutland, Matthias Brugger, devicetree,
	linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

On Mon, 2019-05-06 at 12:43 +0200, Daniel Lezcano wrote:
> On 03/05/2019 18:46, Matthias Kaehlcke wrote:
> > Hi,
> > 
> > On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
> >> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> >>>
> >>> Add thermal zone node to Mediatek MT8183 dts file.
> >>>
> >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> >>> ---
> >>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
> >>>  1 file changed, 64 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> index 926df75..b92116f 100644
> >>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> @@ -334,6 +334,67 @@
> >>>                         status = "disabled";
> >>>                 };
> >>>
> >>> +               thermal: thermal@1100b000 {
> >>> +                       #thermal-sensor-cells = <1>;
> >>> +                       compatible = "mediatek,mt8183-thermal";
> >>> +                       reg = <0 0x1100b000 0 0x1000>;
> >>> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> >>> +                       clocks = <&infracfg CLK_INFRA_THERM>,
> >>> +                                <&infracfg CLK_INFRA_AUXADC>;
> >>> +                       clock-names = "therm", "auxadc";
> >>> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> >>> +                       mediatek,auxadc = <&auxadc>;
> >>> +                       mediatek,apmixedsys = <&apmixedsys>;
> >>> +                       mediatek,hw-reset-temp = <117000>;
> >>> +                       nvmem-cells = <&thermal_calibration>;
> >>> +                       nvmem-cell-names = "calibration-data";
> >>> +               };
> >>> +
> >>> +               thermal-zones {
> >>> +                       cpu_thermal: cpu_thermal {
> >>> +                               polling-delay-passive = <1000>;
> >>> +                               polling-delay = <1000>;
> >>> +
> >>> +                               thermal-sensors = <&thermal 0>;
> >>> +                               sustainable-power = <1500>;
> >>> +                       };
> >>> +
> >>> +                       tzts1: tzts1 {
> >>> +                               polling-delay-passive = <1000>;
> >>> +                               polling-delay = <1000>;
> >>> +                               thermal-sensors = <&thermal 1>;
> >> Is sustainable-power required for tzts? Though it's an optional
> >> property, kernel would have warning:
> >> [    0.631556] thermal thermal_zone1: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.639586] thermal thermal_zone2: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.647611] thermal thermal_zone3: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.655635] thermal thermal_zone4: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.663658] thermal thermal_zone5: power_allocator:
> >> sustainable_power will be estimated
> >> if no sustainable-power assigned.
> > 
> > The property is indeed optional, if it isn't specified IPA will use
> > the sum of the minimum power of all 'power actors' of the zone as
> > estimate (see estimate_sustainable_power()). This may lead to overly
> > agressive throttling, since the nominal sustainable power will always
> > be <= the requested power.
> > 
> > In my understanding the sustainable power may varies between devices,
> > even for the same SoC. One could have all the hardware crammed into a
> > tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
> > laptop form factor and a metal enclosure (e.g. ASUS C201). Both
> > examples are based on an Rockchip rk3288, but they have completely
> > different thermal behavior, and would likely have different values for
> > 'sustainable-power'.
> > 
> > In this sense I tend to consider 'sustainable-power' more a device,
> > than a SoC property. You could specify a 'reasonable' value as a
> > starting point, but it will likely not be optimal for all or even most
> > devices. The warning might even be useful for device makers by
> > indicating them that there is room for tweaking.
> 
> 
> The sustainable power is the power dissipated by the devices belonging
> to the thermal zone at the given trip temperature.
> 
> With the power numbers and the cooling devices, the IPA will change the
> states of the cooling devices to leverage the dissipated power to the
> sustainable power.
> 
> The contribution is the cooling effect of the cooling device.
> 
> However, the IPA is limited to one thermal zone and the cooling device
> is the cpu cooling device. There is the devfreq cooling device but as
> the graphic driver is not upstream, it is found in the android tree only
> for the moment.
> 
> As you mentioned the sustainable power can vary depending on the form
> factor and the production process for the same SoC (they can go to
> higher frequencies thus dissipate more power). That is the reason why we
> split the DT per SoC and we override the values on a per SoC version basis.
> 
> You can have a look the rk3399.dtsi and their variant for experimental
> board (*-rock960.dts) and the chromebook version (*-gru-kevin.dts).
> 
> Do you want a empiric procedure to find out the sustainable power ?
> 
> 
> 
OK, I will add the cooling map. But the tzts1 ~ tzts6 don't need to binding cooler.
The "cpu_thermal" is max value of tzts1 ~tzts6. And cpu_thermal bind
cooler with IPA. tzts1~6 don't need to add cooler. So, do I just add
cooling map without any binding any cooling-cell?

I think thermal framework will add estimated sustainable power. Maybe I
should add by myself. What's procedure do you recommend to find
sustainable power?

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-08 12:23             ` Michael Kao
  0 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-08 12:23 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, roger.lu,
	linux-pm, dawei.chien, linux-kernel, Rob Herring,
	Eduardo Valentin, fan.chen, Matthias Kaehlcke, linux-mediatek,
	Hsin-Yi Wang, Matthias Brugger, Zhang Rui, linux-arm-kernel

On Mon, 2019-05-06 at 12:43 +0200, Daniel Lezcano wrote:
> On 03/05/2019 18:46, Matthias Kaehlcke wrote:
> > Hi,
> > 
> > On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
> >> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> >>>
> >>> Add thermal zone node to Mediatek MT8183 dts file.
> >>>
> >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> >>> ---
> >>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
> >>>  1 file changed, 64 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> index 926df75..b92116f 100644
> >>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> >>> @@ -334,6 +334,67 @@
> >>>                         status = "disabled";
> >>>                 };
> >>>
> >>> +               thermal: thermal@1100b000 {
> >>> +                       #thermal-sensor-cells = <1>;
> >>> +                       compatible = "mediatek,mt8183-thermal";
> >>> +                       reg = <0 0x1100b000 0 0x1000>;
> >>> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> >>> +                       clocks = <&infracfg CLK_INFRA_THERM>,
> >>> +                                <&infracfg CLK_INFRA_AUXADC>;
> >>> +                       clock-names = "therm", "auxadc";
> >>> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> >>> +                       mediatek,auxadc = <&auxadc>;
> >>> +                       mediatek,apmixedsys = <&apmixedsys>;
> >>> +                       mediatek,hw-reset-temp = <117000>;
> >>> +                       nvmem-cells = <&thermal_calibration>;
> >>> +                       nvmem-cell-names = "calibration-data";
> >>> +               };
> >>> +
> >>> +               thermal-zones {
> >>> +                       cpu_thermal: cpu_thermal {
> >>> +                               polling-delay-passive = <1000>;
> >>> +                               polling-delay = <1000>;
> >>> +
> >>> +                               thermal-sensors = <&thermal 0>;
> >>> +                               sustainable-power = <1500>;
> >>> +                       };
> >>> +
> >>> +                       tzts1: tzts1 {
> >>> +                               polling-delay-passive = <1000>;
> >>> +                               polling-delay = <1000>;
> >>> +                               thermal-sensors = <&thermal 1>;
> >> Is sustainable-power required for tzts? Though it's an optional
> >> property, kernel would have warning:
> >> [    0.631556] thermal thermal_zone1: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.639586] thermal thermal_zone2: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.647611] thermal thermal_zone3: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.655635] thermal thermal_zone4: power_allocator:
> >> sustainable_power will be estimated
> >> [    0.663658] thermal thermal_zone5: power_allocator:
> >> sustainable_power will be estimated
> >> if no sustainable-power assigned.
> > 
> > The property is indeed optional, if it isn't specified IPA will use
> > the sum of the minimum power of all 'power actors' of the zone as
> > estimate (see estimate_sustainable_power()). This may lead to overly
> > agressive throttling, since the nominal sustainable power will always
> > be <= the requested power.
> > 
> > In my understanding the sustainable power may varies between devices,
> > even for the same SoC. One could have all the hardware crammed into a
> > tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
> > laptop form factor and a metal enclosure (e.g. ASUS C201). Both
> > examples are based on an Rockchip rk3288, but they have completely
> > different thermal behavior, and would likely have different values for
> > 'sustainable-power'.
> > 
> > In this sense I tend to consider 'sustainable-power' more a device,
> > than a SoC property. You could specify a 'reasonable' value as a
> > starting point, but it will likely not be optimal for all or even most
> > devices. The warning might even be useful for device makers by
> > indicating them that there is room for tweaking.
> 
> 
> The sustainable power is the power dissipated by the devices belonging
> to the thermal zone at the given trip temperature.
> 
> With the power numbers and the cooling devices, the IPA will change the
> states of the cooling devices to leverage the dissipated power to the
> sustainable power.
> 
> The contribution is the cooling effect of the cooling device.
> 
> However, the IPA is limited to one thermal zone and the cooling device
> is the cpu cooling device. There is the devfreq cooling device but as
> the graphic driver is not upstream, it is found in the android tree only
> for the moment.
> 
> As you mentioned the sustainable power can vary depending on the form
> factor and the production process for the same SoC (they can go to
> higher frequencies thus dissipate more power). That is the reason why we
> split the DT per SoC and we override the values on a per SoC version basis.
> 
> You can have a look the rk3399.dtsi and their variant for experimental
> board (*-rock960.dts) and the chromebook version (*-gru-kevin.dts).
> 
> Do you want a empiric procedure to find out the sustainable power ?
> 
> 
> 
OK, I will add the cooling map. But the tzts1 ~ tzts6 don't need to binding cooler.
The "cpu_thermal" is max value of tzts1 ~tzts6. And cpu_thermal bind
cooler with IPA. tzts1~6 don't need to add cooler. So, do I just add
cooling map without any binding any cooling-cell?

I think thermal framework will add estimated sustainable power. Maybe I
should add by myself. What's procedure do you recommend to find
sustainable power?



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/8] arm64: dts: mt8183: add/update dynamic power coefficients
  2019-05-03  7:16     ` Hsin-Yi Wang
  (?)
@ 2019-05-08 12:27       ` Michael Kao
  -1 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-08 12:27 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, linux-kernel,
	Eduardo Valentin, fan.chen, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, linux-arm-kernel

On Fri, 2019-05-03 at 15:16 +0800, Hsin-Yi Wang wrote:
> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> >
> > Add dynamic power coefficients for all cores and update those of
> > CPU0 and CPU4.
> >
> > Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index b92116f..5668fb8 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -58,6 +58,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x000>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> Should this line be in [3/8] arm64: dts: mt8183: Add #cooling-cells to
> CPU nodes?
> 
I will fix the mistake at v2 patch list.
> >                 };
> >
> >                 cpu1: cpu@1 {
> > @@ -65,6 +67,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x001>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu2: cpu@2 {
> > @@ -72,6 +76,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x002>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu3: cpu@3 {
> > @@ -79,6 +85,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x003>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu4: cpu@100 {
> > @@ -86,6 +94,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x100>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu5: cpu@101 {
> > @@ -93,6 +103,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x101>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu6: cpu@102 {
> > @@ -100,6 +112,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x102>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu7: cpu@103 {
> > @@ -107,6 +121,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x103>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >         };
> >
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/8] arm64: dts: mt8183: add/update dynamic power coefficients
@ 2019-05-08 12:27       ` Michael Kao
  0 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-08 12:27 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, linux-kernel,
	Eduardo Valentin, fan.chen, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, linux-arm-kernel

On Fri, 2019-05-03 at 15:16 +0800, Hsin-Yi Wang wrote:
> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> >
> > Add dynamic power coefficients for all cores and update those of
> > CPU0 and CPU4.
> >
> > Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index b92116f..5668fb8 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -58,6 +58,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x000>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> Should this line be in [3/8] arm64: dts: mt8183: Add #cooling-cells to
> CPU nodes?
> 
I will fix the mistake at v2 patch list.
> >                 };
> >
> >                 cpu1: cpu@1 {
> > @@ -65,6 +67,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x001>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu2: cpu@2 {
> > @@ -72,6 +76,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x002>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu3: cpu@3 {
> > @@ -79,6 +85,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x003>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu4: cpu@100 {
> > @@ -86,6 +94,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x100>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu5: cpu@101 {
> > @@ -93,6 +103,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x101>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu6: cpu@102 {
> > @@ -100,6 +112,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x102>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu7: cpu@103 {
> > @@ -107,6 +121,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x103>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >         };
> >
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 2/8] arm64: dts: mt8183: add/update dynamic power coefficients
@ 2019-05-08 12:27       ` Michael Kao
  0 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-08 12:27 UTC (permalink / raw)
  To: Hsin-Yi Wang
  Cc: Mark Rutland, jamesjj.liao, linux-pm, louis.yu, roger.lu,
	Daniel Lezcano, dawei.chien, linux-kernel, Eduardo Valentin,
	fan.chen, devicetree, Rob Herring, linux-mediatek,
	Matthias Brugger, Zhang Rui, linux-arm-kernel

On Fri, 2019-05-03 at 15:16 +0800, Hsin-Yi Wang wrote:
> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> >
> > Add dynamic power coefficients for all cores and update those of
> > CPU0 and CPU4.
> >
> > Signed-off-by: Michael.Kao <michael.kao@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index b92116f..5668fb8 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -58,6 +58,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x000>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> Should this line be in [3/8] arm64: dts: mt8183: Add #cooling-cells to
> CPU nodes?
> 
I will fix the mistake at v2 patch list.
> >                 };
> >
> >                 cpu1: cpu@1 {
> > @@ -65,6 +67,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x001>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu2: cpu@2 {
> > @@ -72,6 +76,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x002>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu3: cpu@3 {
> > @@ -79,6 +85,8 @@
> >                         compatible = "arm,cortex-a53";
> >                         reg = <0x003>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <84>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu4: cpu@100 {
> > @@ -86,6 +94,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x100>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu5: cpu@101 {
> > @@ -93,6 +103,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x101>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu6: cpu@102 {
> > @@ -100,6 +112,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x102>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >
> >                 cpu7: cpu@103 {
> > @@ -107,6 +121,8 @@
> >                         compatible = "arm,cortex-a73";
> >                         reg = <0x103>;
> >                         enable-method = "psci";
> > +                       dynamic-power-coefficient = <211>;
> > +                       #cooling-cells = <2>;
> >                 };
> >         };
> >
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 3/8] arm64: dts: mt8183: Add #cooling-cells to CPU nodes
  2019-05-06  5:25     ` Eddie Huang
@ 2019-05-08 12:29       ` Michael Kao
  -1 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-08 12:29 UTC (permalink / raw)
  To: Eddie Huang
  Cc: Mark Rutland, JamesJJ Liao (廖建智),
	devicetree, Louis Yu (游政錕),
	Dawei Chien (錢大衛),
	linux-pm, Daniel Lezcano, Roger Lu (陸瑞傑),
	linux-kernel, Eduardo Valentin, Fan Chen (陳凡),
	Rob Herring, linux-mediatek, Matthias Brugger, Zhang Rui

On Mon, 2019-05-06 at 13:25 +0800, Eddie Huang wrote:
> Hi Michael,
> 
> On Thu, 2019-05-02 at 18:43 +0800, michael.kao wrote:
> > The #cooling-cells property needs to be specified to allow a CPU
> > to be used as cooling device.
> > 
> > Signed-off-by: michael.kao <Michael.Kao@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 5668fb8..95f1d7b 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -11,6 +11,8 @@
> >  #include <dt-bindings/power/mt8183-power.h>
> >  #include <dt-bindings/reset-controller/mt8183-resets.h>
> >  #include "mt8183-pinfunc.h"
> > +#include <dt-bindings/thermal/thermal.h>
> > +
> >  / {
> >  	compatible = "mediatek,mt8183";
> >  	interrupt-parent = <&sysirq>;
> 
> It seems this patch don't have #cooling-cells but in your 2/8 patch in
> this series
> 
> 
I will fix the mistake at v2 patch list. 

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 3/8] arm64: dts: mt8183: Add #cooling-cells to CPU nodes
@ 2019-05-08 12:29       ` Michael Kao
  0 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-08 12:29 UTC (permalink / raw)
  To: Eddie Huang
  Cc: Mark Rutland, JamesJJ Liao (廖建智),
	devicetree, Louis Yu (游政錕),
	Dawei Chien (錢大衛),
	linux-pm, Daniel Lezcano, Roger Lu (陸瑞傑),
	linux-kernel, Eduardo Valentin, Fan Chen (陳凡),
	Rob Herring, linux-mediatek, Matthias Brugger, Zhang Rui,
	linux-arm-kernel

On Mon, 2019-05-06 at 13:25 +0800, Eddie Huang wrote:
> Hi Michael,
> 
> On Thu, 2019-05-02 at 18:43 +0800, michael.kao wrote:
> > The #cooling-cells property needs to be specified to allow a CPU
> > to be used as cooling device.
> > 
> > Signed-off-by: michael.kao <Michael.Kao@mediatek.com>
> > ---
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 5668fb8..95f1d7b 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -11,6 +11,8 @@
> >  #include <dt-bindings/power/mt8183-power.h>
> >  #include <dt-bindings/reset-controller/mt8183-resets.h>
> >  #include "mt8183-pinfunc.h"
> > +#include <dt-bindings/thermal/thermal.h>
> > +
> >  / {
> >  	compatible = "mediatek,mt8183";
> >  	interrupt-parent = <&sysirq>;
> 
> It seems this patch don't have #cooling-cells but in your 2/8 patch in
> this series
> 
> 
I will fix the mistake at v2 patch list. 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
  2019-05-08 12:23             ` Michael Kao
@ 2019-05-10  8:14               ` Daniel Lezcano
  -1 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2019-05-10  8:14 UTC (permalink / raw)
  To: Michael Kao
  Cc: Matthias Kaehlcke, Hsin-Yi Wang, fan.chen, jamesjj.liao,
	dawei.chien, louis.yu, roger.lu, Zhang Rui, Eduardo Valentin,
	Rob Herring, Mark Rutland, Matthias Brugger, devicetree,
	linux-mediatek, linux-kernel, linux-arm-kernel, linux-pm

On 08/05/2019 14:23, Michael Kao wrote:
> On Mon, 2019-05-06 at 12:43 +0200, Daniel Lezcano wrote:
>> On 03/05/2019 18:46, Matthias Kaehlcke wrote:
>>> Hi,
>>> 
>>> On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
>>>> On Thu, May 2, 2019 at 10:43 AM michael.kao 
>>>> <michael.kao@mediatek.com> wrote:
>>>>> 
>>>>> Add thermal zone node to Mediatek MT8183 dts file.
>>>>> 
>>>>> Signed-off-by: Michael Kao <michael.kao@mediatek.com> --- 
>>>>> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 
>>>>> ++++++++++++++++++++++++++++++++ 1 file changed, 64 
>>>>> insertions(+)
>>>>> 
>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi 
>>>>> b/arch/arm64/boot/dts/mediatek/mt8183.dtsi index 
>>>>> 926df75..b92116f 100644 --- 
>>>>> a/arch/arm64/boot/dts/mediatek/mt8183.dtsi +++ 
>>>>> b/arch/arm64/boot/dts/mediatek/mt8183.dtsi @@ -334,6 +334,67 
>>>>> @@ status = "disabled"; };
>>>>> 
>>>>> +               thermal: thermal@1100b000 { + 
>>>>> #thermal-sensor-cells = <1>; + compatible =
>>>>> "mediatek,mt8183-thermal"; + reg = <0 0x1100b000 0 0x1000>;
>>>>> + interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>; + clocks =
>>>>> <&infracfg CLK_INFRA_THERM>, + <&infracfg CLK_INFRA_AUXADC>;
>>>>> + clock-names = "therm", "auxadc"; + resets = <&infracfg
>>>>> MT8183_INFRACFG_AO_THERM_SW_RST>; + mediatek,auxadc =
>>>>> <&auxadc>; + mediatek,apmixedsys = <&apmixedsys>; + 
>>>>> mediatek,hw-reset-temp = <117000>; + nvmem-cells =
>>>>> <&thermal_calibration>; + nvmem-cell-names =
>>>>> "calibration-data"; +               }; + + thermal-zones { +
>>>>> cpu_thermal: cpu_thermal { + polling-delay-passive = <1000>;
>>>>> + polling-delay = <1000>; + + thermal-sensors = <&thermal 0>;
>>>>> + sustainable-power = <1500>; +                       }; + + 
>>>>> tzts1: tzts1 { + polling-delay-passive = <1000>; + 
>>>>> polling-delay = <1000>; + thermal-sensors = <&thermal 1>;
>>>> Is sustainable-power required for tzts? Though it's an optional
>>>> property, kernel would have warning: [    0.631556] thermal
>>>> thermal_zone1: power_allocator: sustainable_power will be
>>>> estimated [    0.639586] thermal thermal_zone2: 
>>>> power_allocator: sustainable_power will be estimated [ 
>>>> 0.647611] thermal thermal_zone3: power_allocator: 
>>>> sustainable_power will be estimated [    0.655635] thermal 
>>>> thermal_zone4: power_allocator: sustainable_power will be 
>>>> estimated [    0.663658] thermal thermal_zone5: 
>>>> power_allocator: sustainable_power will be estimated if no 
>>>> sustainable-power assigned.
>>> 
>>> The property is indeed optional, if it isn't specified IPA will 
>>> use the sum of the minimum power of all 'power actors' of the 
>>> zone as estimate (see estimate_sustainable_power()). This may 
>>> lead to overly agressive throttling, since the nominal 
>>> sustainable power will always be <= the requested power.
>>> 
>>> In my understanding the sustainable power may varies between 
>>> devices, even for the same SoC. One could have all the hardware 
>>> crammed into a tiny plastic enclosure (e.g. ASUS Chromebit), 
>>> another might have a laptop form factor and a metal enclosure 
>>> (e.g. ASUS C201). Both examples are based on an Rockchip rk3288, 
>>> but they have completely different thermal behavior, and would 
>>> likely have different values for 'sustainable-power'.
>>> 
>>> In this sense I tend to consider 'sustainable-power' more a 
>>> device, than a SoC property. You could specify a 'reasonable' 
>>> value as a starting point, but it will likely not be optimal for 
>>> all or even most devices. The warning might even be useful for 
>>> device makers by indicating them that there is room for 
>>> tweaking.
>> 
>> 
>> The sustainable power is the power dissipated by the devices 
>> belonging to the thermal zone at the given trip temperature.
>> 
>> With the power numbers and the cooling devices, the IPA will
>> change the states of the cooling devices to leverage the dissipated
>> power to the sustainable power.
>> 
>> The contribution is the cooling effect of the cooling device.
>> 
>> However, the IPA is limited to one thermal zone and the cooling 
>> device is the cpu cooling device. There is the devfreq cooling 
>> device but as the graphic driver is not upstream, it is found in 
>> the android tree only for the moment.
>> 
>> As you mentioned the sustainable power can vary depending on the 
>> form factor and the production process for the same SoC (they can 
>> go to higher frequencies thus dissipate more power). That is the 
>> reason why we split the DT per SoC and we override the values on a 
>> per SoC version basis.
>> 
>> You can have a look the rk3399.dtsi and their variant for 
>> experimental board (*-rock960.dts) and the chromebook version 
>> (*-gru-kevin.dts).
>> 
>> Do you want a empiric procedure to find out the sustainable power 
>> ?
>> 
>> 
>> 
> OK, I will add the cooling map. But the tzts1 ~ tzts6 don't need to 
> binding cooler. The "cpu_thermal" is max value of tzts1 ~tzts6. And 
> cpu_thermal bind cooler with IPA. tzts1~6 don't need to add cooler. 
> So, do I just add cooling map without any binding any cooling-cell?

For the moment, I suggest to drop the tzts1..tzts6 thermal zones
definition, so you save the discussion with required-optional field for
the cooling maps and you save multiple wake up on the system to poll
thermal zones to do nothing on them.

When multiple thermal zones or multiple sensors will be supported then
you can re-add them if that makes sense.

A side note, the 'max' approach will drop the performances of the board.
If there is one core overheating because it is 100% busy, the frequency
will be capped impacting the performances on all other cores.

> I think thermal framework will add estimated sustainable power.
> Maybe I should add by myself. What's procedure do you recommend to
> find sustainable power?

So the following assumes the values for the dynamic power coefficient
are correct as well as the resulting computed power per OPP.

Let's take the example there is one thermal zone with a cluster of 4
cores and the cpufreq driver acting on this cluster as a cooling device.

First step:

 - Use the stepwise governor

 - Run dhrystone on all cores for a long time

 - When the mitigation begins and the temperature stabilizes on the
desired trip point, capture the cpufreq transitions for, let's say a
period of 30 seconds.

 - Compute the total duration for each cpu freq state during this period

 - Compute the total energy for each cpu freq state

 - Compute the average energy for this period

 - Divide the average energy by the period, you have the sustainable power

Second step:

Nothing is perfect, so the value found above may need to be tweaked.

 - Add the sustainable power in the DT

 - Switch to the IPA

 - Run dhrystone on all cores for a long time

 - Read the temperature and verify it stabilizes at the desired trip
point and readjust the sustainable power if needed.

Please note, as the IPA is based on a open loop regulation P-I-D, it
should begin to acquire temperature before reaching the mitigation trip
point, so it is a good idea to add a trip point 5 or 10 degrees under
the mitigation trip point. The purpose of this 'pre-mitigate' trip point
is to force the IPA to read temperature in advance.

Hope that helps.

  -- Daniel







-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-10  8:14               ` Daniel Lezcano
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2019-05-10  8:14 UTC (permalink / raw)
  To: Michael Kao
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, roger.lu,
	linux-pm, dawei.chien, linux-kernel, Rob Herring,
	Eduardo Valentin, fan.chen, Matthias Kaehlcke, linux-mediatek,
	Hsin-Yi Wang, Matthias Brugger, Zhang Rui, linux-arm-kernel

On 08/05/2019 14:23, Michael Kao wrote:
> On Mon, 2019-05-06 at 12:43 +0200, Daniel Lezcano wrote:
>> On 03/05/2019 18:46, Matthias Kaehlcke wrote:
>>> Hi,
>>> 
>>> On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
>>>> On Thu, May 2, 2019 at 10:43 AM michael.kao 
>>>> <michael.kao@mediatek.com> wrote:
>>>>> 
>>>>> Add thermal zone node to Mediatek MT8183 dts file.
>>>>> 
>>>>> Signed-off-by: Michael Kao <michael.kao@mediatek.com> --- 
>>>>> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 
>>>>> ++++++++++++++++++++++++++++++++ 1 file changed, 64 
>>>>> insertions(+)
>>>>> 
>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi 
>>>>> b/arch/arm64/boot/dts/mediatek/mt8183.dtsi index 
>>>>> 926df75..b92116f 100644 --- 
>>>>> a/arch/arm64/boot/dts/mediatek/mt8183.dtsi +++ 
>>>>> b/arch/arm64/boot/dts/mediatek/mt8183.dtsi @@ -334,6 +334,67 
>>>>> @@ status = "disabled"; };
>>>>> 
>>>>> +               thermal: thermal@1100b000 { + 
>>>>> #thermal-sensor-cells = <1>; + compatible =
>>>>> "mediatek,mt8183-thermal"; + reg = <0 0x1100b000 0 0x1000>;
>>>>> + interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>; + clocks =
>>>>> <&infracfg CLK_INFRA_THERM>, + <&infracfg CLK_INFRA_AUXADC>;
>>>>> + clock-names = "therm", "auxadc"; + resets = <&infracfg
>>>>> MT8183_INFRACFG_AO_THERM_SW_RST>; + mediatek,auxadc =
>>>>> <&auxadc>; + mediatek,apmixedsys = <&apmixedsys>; + 
>>>>> mediatek,hw-reset-temp = <117000>; + nvmem-cells =
>>>>> <&thermal_calibration>; + nvmem-cell-names =
>>>>> "calibration-data"; +               }; + + thermal-zones { +
>>>>> cpu_thermal: cpu_thermal { + polling-delay-passive = <1000>;
>>>>> + polling-delay = <1000>; + + thermal-sensors = <&thermal 0>;
>>>>> + sustainable-power = <1500>; +                       }; + + 
>>>>> tzts1: tzts1 { + polling-delay-passive = <1000>; + 
>>>>> polling-delay = <1000>; + thermal-sensors = <&thermal 1>;
>>>> Is sustainable-power required for tzts? Though it's an optional
>>>> property, kernel would have warning: [    0.631556] thermal
>>>> thermal_zone1: power_allocator: sustainable_power will be
>>>> estimated [    0.639586] thermal thermal_zone2: 
>>>> power_allocator: sustainable_power will be estimated [ 
>>>> 0.647611] thermal thermal_zone3: power_allocator: 
>>>> sustainable_power will be estimated [    0.655635] thermal 
>>>> thermal_zone4: power_allocator: sustainable_power will be 
>>>> estimated [    0.663658] thermal thermal_zone5: 
>>>> power_allocator: sustainable_power will be estimated if no 
>>>> sustainable-power assigned.
>>> 
>>> The property is indeed optional, if it isn't specified IPA will 
>>> use the sum of the minimum power of all 'power actors' of the 
>>> zone as estimate (see estimate_sustainable_power()). This may 
>>> lead to overly agressive throttling, since the nominal 
>>> sustainable power will always be <= the requested power.
>>> 
>>> In my understanding the sustainable power may varies between 
>>> devices, even for the same SoC. One could have all the hardware 
>>> crammed into a tiny plastic enclosure (e.g. ASUS Chromebit), 
>>> another might have a laptop form factor and a metal enclosure 
>>> (e.g. ASUS C201). Both examples are based on an Rockchip rk3288, 
>>> but they have completely different thermal behavior, and would 
>>> likely have different values for 'sustainable-power'.
>>> 
>>> In this sense I tend to consider 'sustainable-power' more a 
>>> device, than a SoC property. You could specify a 'reasonable' 
>>> value as a starting point, but it will likely not be optimal for 
>>> all or even most devices. The warning might even be useful for 
>>> device makers by indicating them that there is room for 
>>> tweaking.
>> 
>> 
>> The sustainable power is the power dissipated by the devices 
>> belonging to the thermal zone at the given trip temperature.
>> 
>> With the power numbers and the cooling devices, the IPA will
>> change the states of the cooling devices to leverage the dissipated
>> power to the sustainable power.
>> 
>> The contribution is the cooling effect of the cooling device.
>> 
>> However, the IPA is limited to one thermal zone and the cooling 
>> device is the cpu cooling device. There is the devfreq cooling 
>> device but as the graphic driver is not upstream, it is found in 
>> the android tree only for the moment.
>> 
>> As you mentioned the sustainable power can vary depending on the 
>> form factor and the production process for the same SoC (they can 
>> go to higher frequencies thus dissipate more power). That is the 
>> reason why we split the DT per SoC and we override the values on a 
>> per SoC version basis.
>> 
>> You can have a look the rk3399.dtsi and their variant for 
>> experimental board (*-rock960.dts) and the chromebook version 
>> (*-gru-kevin.dts).
>> 
>> Do you want a empiric procedure to find out the sustainable power 
>> ?
>> 
>> 
>> 
> OK, I will add the cooling map. But the tzts1 ~ tzts6 don't need to 
> binding cooler. The "cpu_thermal" is max value of tzts1 ~tzts6. And 
> cpu_thermal bind cooler with IPA. tzts1~6 don't need to add cooler. 
> So, do I just add cooling map without any binding any cooling-cell?

For the moment, I suggest to drop the tzts1..tzts6 thermal zones
definition, so you save the discussion with required-optional field for
the cooling maps and you save multiple wake up on the system to poll
thermal zones to do nothing on them.

When multiple thermal zones or multiple sensors will be supported then
you can re-add them if that makes sense.

A side note, the 'max' approach will drop the performances of the board.
If there is one core overheating because it is 100% busy, the frequency
will be capped impacting the performances on all other cores.

> I think thermal framework will add estimated sustainable power.
> Maybe I should add by myself. What's procedure do you recommend to
> find sustainable power?

So the following assumes the values for the dynamic power coefficient
are correct as well as the resulting computed power per OPP.

Let's take the example there is one thermal zone with a cluster of 4
cores and the cpufreq driver acting on this cluster as a cooling device.

First step:

 - Use the stepwise governor

 - Run dhrystone on all cores for a long time

 - When the mitigation begins and the temperature stabilizes on the
desired trip point, capture the cpufreq transitions for, let's say a
period of 30 seconds.

 - Compute the total duration for each cpu freq state during this period

 - Compute the total energy for each cpu freq state

 - Compute the average energy for this period

 - Divide the average energy by the period, you have the sustainable power

Second step:

Nothing is perfect, so the value found above may need to be tweaked.

 - Add the sustainable power in the DT

 - Switch to the IPA

 - Run dhrystone on all cores for a long time

 - Read the temperature and verify it stabilizes at the desired trip
point and readjust the sustainable power if needed.

Please note, as the IPA is based on a open loop regulation P-I-D, it
should begin to acquire temperature before reaching the mitigation trip
point, so it is a good idea to add a trip point 5 or 10 degrees under
the mitigation trip point. The purpose of this 'pre-mitigate' trip point
is to force the IPA to read temperature in advance.

Hope that helps.

  -- Daniel







-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
  2019-05-08 12:23             ` Michael Kao
  (?)
@ 2019-05-10 10:52               ` Michael Kao
  -1 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-10 10:52 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Matthias Kaehlcke, Hsin-Yi Wang, fan.chen, jamesjj.liao,
	dawei.chien, louis.yu, roger.lu, Zhang Rui, hsinyi,
	Eduardo Valentin, Rob Herring, Mark Rutland, Matthias Brugger,
	devicetree, linux-mediatek, linux-kernel, linux-arm-kernel,
	linux-pm

On Wed, 2019-05-08 at 20:23 +0800, Michael Kao wrote:
> On Mon, 2019-05-06 at 12:43 +0200, Daniel Lezcano wrote:
> > On 03/05/2019 18:46, Matthias Kaehlcke wrote:
> > > Hi,
> > > 
> > > On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
> > >> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> > >>>
> > >>> Add thermal zone node to Mediatek MT8183 dts file.
> > >>>
> > >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > >>> ---
> > >>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
> > >>>  1 file changed, 64 insertions(+)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> index 926df75..b92116f 100644
> > >>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> @@ -334,6 +334,67 @@
> > >>>                         status = "disabled";
> > >>>                 };
> > >>>
> > >>> +               thermal: thermal@1100b000 {
> > >>> +                       #thermal-sensor-cells = <1>;
> > >>> +                       compatible = "mediatek,mt8183-thermal";
> > >>> +                       reg = <0 0x1100b000 0 0x1000>;
> > >>> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> > >>> +                       clocks = <&infracfg CLK_INFRA_THERM>,
> > >>> +                                <&infracfg CLK_INFRA_AUXADC>;
> > >>> +                       clock-names = "therm", "auxadc";
> > >>> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> > >>> +                       mediatek,auxadc = <&auxadc>;
> > >>> +                       mediatek,apmixedsys = <&apmixedsys>;
> > >>> +                       mediatek,hw-reset-temp = <117000>;
> > >>> +                       nvmem-cells = <&thermal_calibration>;
> > >>> +                       nvmem-cell-names = "calibration-data";
> > >>> +               };
> > >>> +
> > >>> +               thermal-zones {
> > >>> +                       cpu_thermal: cpu_thermal {
> > >>> +                               polling-delay-passive = <1000>;
> > >>> +                               polling-delay = <1000>;
> > >>> +
> > >>> +                               thermal-sensors = <&thermal 0>;
> > >>> +                               sustainable-power = <1500>;
> > >>> +                       };
> > >>> +
> > >>> +                       tzts1: tzts1 {
> > >>> +                               polling-delay-passive = <1000>;
> > >>> +                               polling-delay = <1000>;
> > >>> +                               thermal-sensors = <&thermal 1>;
> > >> Is sustainable-power required for tzts? Though it's an optional
> > >> property, kernel would have warning:
> > >> [    0.631556] thermal thermal_zone1: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.639586] thermal thermal_zone2: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.647611] thermal thermal_zone3: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.655635] thermal thermal_zone4: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.663658] thermal thermal_zone5: power_allocator:
> > >> sustainable_power will be estimated
> > >> if no sustainable-power assigned.
> > > 
> > > The property is indeed optional, if it isn't specified IPA will use
> > > the sum of the minimum power of all 'power actors' of the zone as
> > > estimate (see estimate_sustainable_power()). This may lead to overly
> > > agressive throttling, since the nominal sustainable power will always
> > > be <= the requested power.
> > > 
> > > In my understanding the sustainable power may varies between devices,
> > > even for the same SoC. One could have all the hardware crammed into a
> > > tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
> > > laptop form factor and a metal enclosure (e.g. ASUS C201). Both
> > > examples are based on an Rockchip rk3288, but they have completely
> > > different thermal behavior, and would likely have different values for
> > > 'sustainable-power'.
> > > 
> > > In this sense I tend to consider 'sustainable-power' more a device,
> > > than a SoC property. You could specify a 'reasonable' value as a
> > > starting point, but it will likely not be optimal for all or even most
> > > devices. The warning might even be useful for device makers by
> > > indicating them that there is room for tweaking.
> > 
> > 
> > The sustainable power is the power dissipated by the devices belonging
> > to the thermal zone at the given trip temperature.
> > 
> > With the power numbers and the cooling devices, the IPA will change the
> > states of the cooling devices to leverage the dissipated power to the
> > sustainable power.
> > 
> > The contribution is the cooling effect of the cooling device.
> > 
> > However, the IPA is limited to one thermal zone and the cooling device
> > is the cpu cooling device. There is the devfreq cooling device but as
> > the graphic driver is not upstream, it is found in the android tree only
> > for the moment.
> > 
> > As you mentioned the sustainable power can vary depending on the form
> > factor and the production process for the same SoC (they can go to
> > higher frequencies thus dissipate more power). That is the reason why we
> > split the DT per SoC and we override the values on a per SoC version basis.
> > 
> > You can have a look the rk3399.dtsi and their variant for experimental
> > board (*-rock960.dts) and the chromebook version (*-gru-kevin.dts).
> > 
> > Do you want a empiric procedure to find out the sustainable power ?
> > 
> > 
> > 
> OK, I will add the cooling map. But the tzts1 ~ tzts6 don't need to binding cooler.
> The "cpu_thermal" is max value of tzts1 ~tzts6. And cpu_thermal bind
> cooler with IPA. tzts1~6 don't need to add cooler. So, do I just add
> cooling map without any binding any cooling-cell?
> 
> I think thermal framework will add estimated sustainable power. Maybe I
> should add by myself. What's procedure do you recommend to find
> sustainable power?
> 
The tzts1~6 are just thermal sensor in the 8183 SoC,
The purpose of adding the six thermal is to support svs driver to read
thermal sensor in the SoC.
https://patchwork.kernel.org/patch/10923289/

The IPA cooling SoC will be applied by cpu_thermal which is the max
sensor value of tzts1~6.
In Document/devicetree/binding/thermal/thermal.txt, I find the statement
that the trip and cooling-map is required for thermal zone.
If we don't set trip and cooling map, it will set disable mode
"tz->mode = THERMAL_DEVICE_DISABLED"
in the drivers/thermal/of-thermal.c
But it still work to read the temperature of thermal zone.
And we don't need to let these thermal zone to bind cooler.
So, we use these way to provide temperature node for svs to read
temperature by thermal_zone_get_zone_by_name only.


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-10 10:52               ` Michael Kao
  0 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-10 10:52 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Matthias Kaehlcke, Hsin-Yi Wang, fan.chen, jamesjj.liao,
	dawei.chien, louis.yu, roger.lu, Zhang Rui

On Wed, 2019-05-08 at 20:23 +0800, Michael Kao wrote:
> On Mon, 2019-05-06 at 12:43 +0200, Daniel Lezcano wrote:
> > On 03/05/2019 18:46, Matthias Kaehlcke wrote:
> > > Hi,
> > > 
> > > On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
> > >> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> > >>>
> > >>> Add thermal zone node to Mediatek MT8183 dts file.
> > >>>
> > >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > >>> ---
> > >>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
> > >>>  1 file changed, 64 insertions(+)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> index 926df75..b92116f 100644
> > >>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> @@ -334,6 +334,67 @@
> > >>>                         status = "disabled";
> > >>>                 };
> > >>>
> > >>> +               thermal: thermal@1100b000 {
> > >>> +                       #thermal-sensor-cells = <1>;
> > >>> +                       compatible = "mediatek,mt8183-thermal";
> > >>> +                       reg = <0 0x1100b000 0 0x1000>;
> > >>> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> > >>> +                       clocks = <&infracfg CLK_INFRA_THERM>,
> > >>> +                                <&infracfg CLK_INFRA_AUXADC>;
> > >>> +                       clock-names = "therm", "auxadc";
> > >>> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> > >>> +                       mediatek,auxadc = <&auxadc>;
> > >>> +                       mediatek,apmixedsys = <&apmixedsys>;
> > >>> +                       mediatek,hw-reset-temp = <117000>;
> > >>> +                       nvmem-cells = <&thermal_calibration>;
> > >>> +                       nvmem-cell-names = "calibration-data";
> > >>> +               };
> > >>> +
> > >>> +               thermal-zones {
> > >>> +                       cpu_thermal: cpu_thermal {
> > >>> +                               polling-delay-passive = <1000>;
> > >>> +                               polling-delay = <1000>;
> > >>> +
> > >>> +                               thermal-sensors = <&thermal 0>;
> > >>> +                               sustainable-power = <1500>;
> > >>> +                       };
> > >>> +
> > >>> +                       tzts1: tzts1 {
> > >>> +                               polling-delay-passive = <1000>;
> > >>> +                               polling-delay = <1000>;
> > >>> +                               thermal-sensors = <&thermal 1>;
> > >> Is sustainable-power required for tzts? Though it's an optional
> > >> property, kernel would have warning:
> > >> [    0.631556] thermal thermal_zone1: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.639586] thermal thermal_zone2: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.647611] thermal thermal_zone3: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.655635] thermal thermal_zone4: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.663658] thermal thermal_zone5: power_allocator:
> > >> sustainable_power will be estimated
> > >> if no sustainable-power assigned.
> > > 
> > > The property is indeed optional, if it isn't specified IPA will use
> > > the sum of the minimum power of all 'power actors' of the zone as
> > > estimate (see estimate_sustainable_power()). This may lead to overly
> > > agressive throttling, since the nominal sustainable power will always
> > > be <= the requested power.
> > > 
> > > In my understanding the sustainable power may varies between devices,
> > > even for the same SoC. One could have all the hardware crammed into a
> > > tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
> > > laptop form factor and a metal enclosure (e.g. ASUS C201). Both
> > > examples are based on an Rockchip rk3288, but they have completely
> > > different thermal behavior, and would likely have different values for
> > > 'sustainable-power'.
> > > 
> > > In this sense I tend to consider 'sustainable-power' more a device,
> > > than a SoC property. You could specify a 'reasonable' value as a
> > > starting point, but it will likely not be optimal for all or even most
> > > devices. The warning might even be useful for device makers by
> > > indicating them that there is room for tweaking.
> > 
> > 
> > The sustainable power is the power dissipated by the devices belonging
> > to the thermal zone at the given trip temperature.
> > 
> > With the power numbers and the cooling devices, the IPA will change the
> > states of the cooling devices to leverage the dissipated power to the
> > sustainable power.
> > 
> > The contribution is the cooling effect of the cooling device.
> > 
> > However, the IPA is limited to one thermal zone and the cooling device
> > is the cpu cooling device. There is the devfreq cooling device but as
> > the graphic driver is not upstream, it is found in the android tree only
> > for the moment.
> > 
> > As you mentioned the sustainable power can vary depending on the form
> > factor and the production process for the same SoC (they can go to
> > higher frequencies thus dissipate more power). That is the reason why we
> > split the DT per SoC and we override the values on a per SoC version basis.
> > 
> > You can have a look the rk3399.dtsi and their variant for experimental
> > board (*-rock960.dts) and the chromebook version (*-gru-kevin.dts).
> > 
> > Do you want a empiric procedure to find out the sustainable power ?
> > 
> > 
> > 
> OK, I will add the cooling map. But the tzts1 ~ tzts6 don't need to binding cooler.
> The "cpu_thermal" is max value of tzts1 ~tzts6. And cpu_thermal bind
> cooler with IPA. tzts1~6 don't need to add cooler. So, do I just add
> cooling map without any binding any cooling-cell?
> 
> I think thermal framework will add estimated sustainable power. Maybe I
> should add by myself. What's procedure do you recommend to find
> sustainable power?
> 
The tzts1~6 are just thermal sensor in the 8183 SoC,
The purpose of adding the six thermal is to support svs driver to read
thermal sensor in the SoC.
https://patchwork.kernel.org/patch/10923289/

The IPA cooling SoC will be applied by cpu_thermal which is the max
sensor value of tzts1~6.
In Document/devicetree/binding/thermal/thermal.txt, I find the statement
that the trip and cooling-map is required for thermal zone.
If we don't set trip and cooling map, it will set disable mode
"tz->mode = THERMAL_DEVICE_DISABLED"
in the drivers/thermal/of-thermal.c
But it still work to read the temperature of thermal zone.
And we don't need to let these thermal zone to bind cooler.
So, we use these way to provide temperature node for svs to read
temperature by thermal_zone_get_zone_by_name only.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 1/8] arm64: dts: mt8183: add thermal zone node
@ 2019-05-10 10:52               ` Michael Kao
  0 siblings, 0 replies; 52+ messages in thread
From: Michael Kao @ 2019-05-10 10:52 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Mark Rutland, jamesjj.liao, devicetree, louis.yu, roger.lu,
	linux-pm, dawei.chien, linux-kernel, Rob Herring,
	Eduardo Valentin, fan.chen, Matthias Kaehlcke, linux-mediatek,
	hsinyi, Matthias Brugger, Zhang Rui, linux-arm-kernel

On Wed, 2019-05-08 at 20:23 +0800, Michael Kao wrote:
> On Mon, 2019-05-06 at 12:43 +0200, Daniel Lezcano wrote:
> > On 03/05/2019 18:46, Matthias Kaehlcke wrote:
> > > Hi,
> > > 
> > > On Fri, May 03, 2019 at 04:03:58PM +0800, Hsin-Yi Wang wrote:
> > >> On Thu, May 2, 2019 at 10:43 AM michael.kao <michael.kao@mediatek.com> wrote:
> > >>>
> > >>> Add thermal zone node to Mediatek MT8183 dts file.
> > >>>
> > >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > >>> ---
> > >>>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 64 ++++++++++++++++++++++++++++++++
> > >>>  1 file changed, 64 insertions(+)
> > >>>
> > >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> index 926df75..b92116f 100644
> > >>> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > >>> @@ -334,6 +334,67 @@
> > >>>                         status = "disabled";
> > >>>                 };
> > >>>
> > >>> +               thermal: thermal@1100b000 {
> > >>> +                       #thermal-sensor-cells = <1>;
> > >>> +                       compatible = "mediatek,mt8183-thermal";
> > >>> +                       reg = <0 0x1100b000 0 0x1000>;
> > >>> +                       interrupts = <0 76 IRQ_TYPE_LEVEL_LOW>;
> > >>> +                       clocks = <&infracfg CLK_INFRA_THERM>,
> > >>> +                                <&infracfg CLK_INFRA_AUXADC>;
> > >>> +                       clock-names = "therm", "auxadc";
> > >>> +                       resets = <&infracfg  MT8183_INFRACFG_AO_THERM_SW_RST>;
> > >>> +                       mediatek,auxadc = <&auxadc>;
> > >>> +                       mediatek,apmixedsys = <&apmixedsys>;
> > >>> +                       mediatek,hw-reset-temp = <117000>;
> > >>> +                       nvmem-cells = <&thermal_calibration>;
> > >>> +                       nvmem-cell-names = "calibration-data";
> > >>> +               };
> > >>> +
> > >>> +               thermal-zones {
> > >>> +                       cpu_thermal: cpu_thermal {
> > >>> +                               polling-delay-passive = <1000>;
> > >>> +                               polling-delay = <1000>;
> > >>> +
> > >>> +                               thermal-sensors = <&thermal 0>;
> > >>> +                               sustainable-power = <1500>;
> > >>> +                       };
> > >>> +
> > >>> +                       tzts1: tzts1 {
> > >>> +                               polling-delay-passive = <1000>;
> > >>> +                               polling-delay = <1000>;
> > >>> +                               thermal-sensors = <&thermal 1>;
> > >> Is sustainable-power required for tzts? Though it's an optional
> > >> property, kernel would have warning:
> > >> [    0.631556] thermal thermal_zone1: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.639586] thermal thermal_zone2: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.647611] thermal thermal_zone3: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.655635] thermal thermal_zone4: power_allocator:
> > >> sustainable_power will be estimated
> > >> [    0.663658] thermal thermal_zone5: power_allocator:
> > >> sustainable_power will be estimated
> > >> if no sustainable-power assigned.
> > > 
> > > The property is indeed optional, if it isn't specified IPA will use
> > > the sum of the minimum power of all 'power actors' of the zone as
> > > estimate (see estimate_sustainable_power()). This may lead to overly
> > > agressive throttling, since the nominal sustainable power will always
> > > be <= the requested power.
> > > 
> > > In my understanding the sustainable power may varies between devices,
> > > even for the same SoC. One could have all the hardware crammed into a
> > > tiny plastic enclosure (e.g. ASUS Chromebit), another might have a
> > > laptop form factor and a metal enclosure (e.g. ASUS C201). Both
> > > examples are based on an Rockchip rk3288, but they have completely
> > > different thermal behavior, and would likely have different values for
> > > 'sustainable-power'.
> > > 
> > > In this sense I tend to consider 'sustainable-power' more a device,
> > > than a SoC property. You could specify a 'reasonable' value as a
> > > starting point, but it will likely not be optimal for all or even most
> > > devices. The warning might even be useful for device makers by
> > > indicating them that there is room for tweaking.
> > 
> > 
> > The sustainable power is the power dissipated by the devices belonging
> > to the thermal zone at the given trip temperature.
> > 
> > With the power numbers and the cooling devices, the IPA will change the
> > states of the cooling devices to leverage the dissipated power to the
> > sustainable power.
> > 
> > The contribution is the cooling effect of the cooling device.
> > 
> > However, the IPA is limited to one thermal zone and the cooling device
> > is the cpu cooling device. There is the devfreq cooling device but as
> > the graphic driver is not upstream, it is found in the android tree only
> > for the moment.
> > 
> > As you mentioned the sustainable power can vary depending on the form
> > factor and the production process for the same SoC (they can go to
> > higher frequencies thus dissipate more power). That is the reason why we
> > split the DT per SoC and we override the values on a per SoC version basis.
> > 
> > You can have a look the rk3399.dtsi and their variant for experimental
> > board (*-rock960.dts) and the chromebook version (*-gru-kevin.dts).
> > 
> > Do you want a empiric procedure to find out the sustainable power ?
> > 
> > 
> > 
> OK, I will add the cooling map. But the tzts1 ~ tzts6 don't need to binding cooler.
> The "cpu_thermal" is max value of tzts1 ~tzts6. And cpu_thermal bind
> cooler with IPA. tzts1~6 don't need to add cooler. So, do I just add
> cooling map without any binding any cooling-cell?
> 
> I think thermal framework will add estimated sustainable power. Maybe I
> should add by myself. What's procedure do you recommend to find
> sustainable power?
> 
The tzts1~6 are just thermal sensor in the 8183 SoC,
The purpose of adding the six thermal is to support svs driver to read
thermal sensor in the SoC.
https://patchwork.kernel.org/patch/10923289/

The IPA cooling SoC will be applied by cpu_thermal which is the max
sensor value of tzts1~6.
In Document/devicetree/binding/thermal/thermal.txt, I find the statement
that the trip and cooling-map is required for thermal zone.
If we don't set trip and cooling map, it will set disable mode
"tz->mode = THERMAL_DEVICE_DISABLED"
in the drivers/thermal/of-thermal.c
But it still work to read the temperature of thermal zone.
And we don't need to let these thermal zone to bind cooler.
So, we use these way to provide temperature node for svs to read
temperature by thermal_zone_get_zone_by_name only.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/8] thermal: mediatek: add another get_temp ops for thermal sensors
  2019-05-02 10:43   ` michael.kao
  (?)
@ 2019-05-11  2:31     ` Nicolas Boichat
  -1 siblings, 0 replies; 52+ messages in thread
From: Nicolas Boichat @ 2019-05-11  2:31 UTC (permalink / raw)
  To: michael.kao
  Cc: Fan Chen, James Liao, dawei.chien, louis.yu, roger.lu, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Rob Herring, Mark Rutland,
	Matthias Brugger, devicetree,
	moderated list:ARM/Mediatek SoC support, lkml,
	linux-arm Mailing List, linux-pm, Hsin-Yi Wang

On Thu, May 2, 2019 at 7:45 PM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Michael Kao <michael.kao@mediatek.com>
>
> Provide thermal zone to read thermal sensor
> in the SoC. We can read all the thermal sensors
> value in the SoC by the node /sys/class/thermal/
>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  drivers/thermal/mtk_thermal.c | 68 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index cb41e46..d5c78b0 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -230,6 +230,11 @@ enum {
>
>  struct mtk_thermal;
>
> +struct mtk_thermal_zone {
> +       struct mtk_thermal *mt;
> +       int id;
> +};
> +
>  struct thermal_bank_cfg {
>         unsigned int num_sensors;
>         const int *sensors;
> @@ -612,7 +617,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>                  * not immediately shut down.
>                  */
>                 if (temp > 200000)
> -                       temp = 0;
> +                       temp = -EACCES;

EACCES/permission denied doesn't really seem to be the right error
code here. Maybe EAGAIN?

>
>                 if (temp > max)
>                         max = temp;
> @@ -623,7 +628,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>
>  static int mtk_read_temp(void *data, int *temperature)
>  {
> -       struct mtk_thermal *mt = data;
> +       struct mtk_thermal_zone *tz = data;
> +       struct mtk_thermal *mt = tz->mt;
>         int i;
>         int tempmax = INT_MIN;
>
> @@ -636,16 +642,48 @@ static int mtk_read_temp(void *data, int *temperature)
>
>                 mtk_thermal_put_bank(bank);
>         }
> -

I'd drop that change.

>         *temperature = tempmax;
>
>         return 0;
>  }
>
> +static int mtk_read_sensor_temp(void *data, int *temperature)
> +{
> +       struct mtk_thermal_zone *tz = data;
> +       struct mtk_thermal *mt = tz->mt;
> +       const struct mtk_thermal_data *conf = mt->conf;
> +       int id = tz->id - 1;
> +       int temp = INT_MIN;

No need to initialize temp.

> +       u32 raw;
> +
> +       if (id < 0)
> +               return  -EACCES;

EINVAL?

> +
> +       raw = readl(mt->thermal_base + conf->msr[id]);
> +
> +       temp = raw_to_mcelsius(mt, id, raw);
> +
> +       /*
> +        * The first read of a sensor often contains very high bogus
> +        * temperature value. Filter these out so that the system does
> +        * not immediately shut down.
> +        */
> +

nit: Drop this blank line

> +       if (temp > 200000)
> +               return  -EACCES;

Again, EAGAIN, maybe?

> +
> +       *temperature = temp;
> +       return 0;
> +}
> +
>  static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
>         .get_temp = mtk_read_temp,
>  };
>
> +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> +       .get_temp = mtk_read_sensor_temp,
> +};
> +
>  static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
>                                   u32 apmixed_phys_base, u32 auxadc_phys_base,
>                                   int ctrl_id)
> @@ -878,6 +916,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>         struct resource *res;
>         u64 auxadc_phys_base, apmixed_phys_base;
>         struct thermal_zone_device *tzdev;
> +       struct mtk_thermal_zone *tz;
>
>         mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
>         if (!mt)
> @@ -959,11 +998,24 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, mt);
>
> -       tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
> -                                                    &mtk_thermal_ops);
> -       if (IS_ERR(tzdev)) {
> -               ret = PTR_ERR(tzdev);
> -               goto err_disable_clk_peri_therm;
> +       for (i = 0; i < mt->conf->num_sensors + 1; i++) {
> +               tz = kmalloc(sizeof(*tz), GFP_KERNEL);

Are we leaking this pointer? Should this be devm_kmalloc?

> +               if (!tz)
> +                       return -ENOMEM;
> +
> +               tz->mt = mt;
> +               tz->id = i;
> +
> +               tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
> +                               tz, (i == 0) ?
> +                               &mtk_thermal_ops : &mtk_thermal_sensor_ops);
> +
> +               if (IS_ERR(tzdev)) {
> +                       if (IS_ERR(tzdev) != -EACCES) {

Why would EACCES ever happen? AFAICT
devm_thermal_zone_of_sensor_register does not actually try to read the
temperature value? Or does the error come from somewhere else?

> +                               ret = PTR_ERR(tzdev);
> +                               goto err_disable_clk_peri_therm;
> +                       }
> +               }
>         }
>
>         return 0;
> --
> 1.9.1
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/8] thermal: mediatek: add another get_temp ops for thermal sensors
@ 2019-05-11  2:31     ` Nicolas Boichat
  0 siblings, 0 replies; 52+ messages in thread
From: Nicolas Boichat @ 2019-05-11  2:31 UTC (permalink / raw)
  To: michael.kao
  Cc: Fan Chen, James Liao, dawei.chien, louis.yu, roger.lu, Zhang Rui,
	Eduardo Valentin, Daniel Lezcano, Rob Herring, Mark Rutland,
	Matthias Brugger, devicetree,
	moderated list:ARM/Mediatek SoC support, lkml,
	linux-arm Mailing List, linux-pm, Hsin-Yi Wang

On Thu, May 2, 2019 at 7:45 PM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Michael Kao <michael.kao@mediatek.com>
>
> Provide thermal zone to read thermal sensor
> in the SoC. We can read all the thermal sensors
> value in the SoC by the node /sys/class/thermal/
>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  drivers/thermal/mtk_thermal.c | 68 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index cb41e46..d5c78b0 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -230,6 +230,11 @@ enum {
>
>  struct mtk_thermal;
>
> +struct mtk_thermal_zone {
> +       struct mtk_thermal *mt;
> +       int id;
> +};
> +
>  struct thermal_bank_cfg {
>         unsigned int num_sensors;
>         const int *sensors;
> @@ -612,7 +617,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>                  * not immediately shut down.
>                  */
>                 if (temp > 200000)
> -                       temp = 0;
> +                       temp = -EACCES;

EACCES/permission denied doesn't really seem to be the right error
code here. Maybe EAGAIN?

>
>                 if (temp > max)
>                         max = temp;
> @@ -623,7 +628,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>
>  static int mtk_read_temp(void *data, int *temperature)
>  {
> -       struct mtk_thermal *mt = data;
> +       struct mtk_thermal_zone *tz = data;
> +       struct mtk_thermal *mt = tz->mt;
>         int i;
>         int tempmax = INT_MIN;
>
> @@ -636,16 +642,48 @@ static int mtk_read_temp(void *data, int *temperature)
>
>                 mtk_thermal_put_bank(bank);
>         }
> -

I'd drop that change.

>         *temperature = tempmax;
>
>         return 0;
>  }
>
> +static int mtk_read_sensor_temp(void *data, int *temperature)
> +{
> +       struct mtk_thermal_zone *tz = data;
> +       struct mtk_thermal *mt = tz->mt;
> +       const struct mtk_thermal_data *conf = mt->conf;
> +       int id = tz->id - 1;
> +       int temp = INT_MIN;

No need to initialize temp.

> +       u32 raw;
> +
> +       if (id < 0)
> +               return  -EACCES;

EINVAL?

> +
> +       raw = readl(mt->thermal_base + conf->msr[id]);
> +
> +       temp = raw_to_mcelsius(mt, id, raw);
> +
> +       /*
> +        * The first read of a sensor often contains very high bogus
> +        * temperature value. Filter these out so that the system does
> +        * not immediately shut down.
> +        */
> +

nit: Drop this blank line

> +       if (temp > 200000)
> +               return  -EACCES;

Again, EAGAIN, maybe?

> +
> +       *temperature = temp;
> +       return 0;
> +}
> +
>  static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
>         .get_temp = mtk_read_temp,
>  };
>
> +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> +       .get_temp = mtk_read_sensor_temp,
> +};
> +
>  static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
>                                   u32 apmixed_phys_base, u32 auxadc_phys_base,
>                                   int ctrl_id)
> @@ -878,6 +916,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>         struct resource *res;
>         u64 auxadc_phys_base, apmixed_phys_base;
>         struct thermal_zone_device *tzdev;
> +       struct mtk_thermal_zone *tz;
>
>         mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
>         if (!mt)
> @@ -959,11 +998,24 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, mt);
>
> -       tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
> -                                                    &mtk_thermal_ops);
> -       if (IS_ERR(tzdev)) {
> -               ret = PTR_ERR(tzdev);
> -               goto err_disable_clk_peri_therm;
> +       for (i = 0; i < mt->conf->num_sensors + 1; i++) {
> +               tz = kmalloc(sizeof(*tz), GFP_KERNEL);

Are we leaking this pointer? Should this be devm_kmalloc?

> +               if (!tz)
> +                       return -ENOMEM;
> +
> +               tz->mt = mt;
> +               tz->id = i;
> +
> +               tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
> +                               tz, (i == 0) ?
> +                               &mtk_thermal_ops : &mtk_thermal_sensor_ops);
> +
> +               if (IS_ERR(tzdev)) {
> +                       if (IS_ERR(tzdev) != -EACCES) {

Why would EACCES ever happen? AFAICT
devm_thermal_zone_of_sensor_register does not actually try to read the
temperature value? Or does the error come from somewhere else?

> +                               ret = PTR_ERR(tzdev);
> +                               goto err_disable_clk_peri_therm;
> +                       }
> +               }
>         }
>
>         return 0;
> --
> 1.9.1
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 7/8] thermal: mediatek: add another get_temp ops for thermal sensors
@ 2019-05-11  2:31     ` Nicolas Boichat
  0 siblings, 0 replies; 52+ messages in thread
From: Nicolas Boichat @ 2019-05-11  2:31 UTC (permalink / raw)
  To: michael.kao
  Cc: Mark Rutland, James Liao, devicetree, louis.yu, dawei.chien,
	linux-pm, Daniel Lezcano, roger.lu, lkml, Eduardo Valentin,
	Fan Chen, Rob Herring, moderated list:ARM/Mediatek SoC support,
	Hsin-Yi Wang, Matthias Brugger, Zhang Rui,
	linux-arm Mailing List

On Thu, May 2, 2019 at 7:45 PM michael.kao <michael.kao@mediatek.com> wrote:
>
> From: Michael Kao <michael.kao@mediatek.com>
>
> Provide thermal zone to read thermal sensor
> in the SoC. We can read all the thermal sensors
> value in the SoC by the node /sys/class/thermal/
>
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  drivers/thermal/mtk_thermal.c | 68 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index cb41e46..d5c78b0 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -230,6 +230,11 @@ enum {
>
>  struct mtk_thermal;
>
> +struct mtk_thermal_zone {
> +       struct mtk_thermal *mt;
> +       int id;
> +};
> +
>  struct thermal_bank_cfg {
>         unsigned int num_sensors;
>         const int *sensors;
> @@ -612,7 +617,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>                  * not immediately shut down.
>                  */
>                 if (temp > 200000)
> -                       temp = 0;
> +                       temp = -EACCES;

EACCES/permission denied doesn't really seem to be the right error
code here. Maybe EAGAIN?

>
>                 if (temp > max)
>                         max = temp;
> @@ -623,7 +628,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>
>  static int mtk_read_temp(void *data, int *temperature)
>  {
> -       struct mtk_thermal *mt = data;
> +       struct mtk_thermal_zone *tz = data;
> +       struct mtk_thermal *mt = tz->mt;
>         int i;
>         int tempmax = INT_MIN;
>
> @@ -636,16 +642,48 @@ static int mtk_read_temp(void *data, int *temperature)
>
>                 mtk_thermal_put_bank(bank);
>         }
> -

I'd drop that change.

>         *temperature = tempmax;
>
>         return 0;
>  }
>
> +static int mtk_read_sensor_temp(void *data, int *temperature)
> +{
> +       struct mtk_thermal_zone *tz = data;
> +       struct mtk_thermal *mt = tz->mt;
> +       const struct mtk_thermal_data *conf = mt->conf;
> +       int id = tz->id - 1;
> +       int temp = INT_MIN;

No need to initialize temp.

> +       u32 raw;
> +
> +       if (id < 0)
> +               return  -EACCES;

EINVAL?

> +
> +       raw = readl(mt->thermal_base + conf->msr[id]);
> +
> +       temp = raw_to_mcelsius(mt, id, raw);
> +
> +       /*
> +        * The first read of a sensor often contains very high bogus
> +        * temperature value. Filter these out so that the system does
> +        * not immediately shut down.
> +        */
> +

nit: Drop this blank line

> +       if (temp > 200000)
> +               return  -EACCES;

Again, EAGAIN, maybe?

> +
> +       *temperature = temp;
> +       return 0;
> +}
> +
>  static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
>         .get_temp = mtk_read_temp,
>  };
>
> +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> +       .get_temp = mtk_read_sensor_temp,
> +};
> +
>  static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
>                                   u32 apmixed_phys_base, u32 auxadc_phys_base,
>                                   int ctrl_id)
> @@ -878,6 +916,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>         struct resource *res;
>         u64 auxadc_phys_base, apmixed_phys_base;
>         struct thermal_zone_device *tzdev;
> +       struct mtk_thermal_zone *tz;
>
>         mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
>         if (!mt)
> @@ -959,11 +998,24 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, mt);
>
> -       tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
> -                                                    &mtk_thermal_ops);
> -       if (IS_ERR(tzdev)) {
> -               ret = PTR_ERR(tzdev);
> -               goto err_disable_clk_peri_therm;
> +       for (i = 0; i < mt->conf->num_sensors + 1; i++) {
> +               tz = kmalloc(sizeof(*tz), GFP_KERNEL);

Are we leaking this pointer? Should this be devm_kmalloc?

> +               if (!tz)
> +                       return -ENOMEM;
> +
> +               tz->mt = mt;
> +               tz->id = i;
> +
> +               tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
> +                               tz, (i == 0) ?
> +                               &mtk_thermal_ops : &mtk_thermal_sensor_ops);
> +
> +               if (IS_ERR(tzdev)) {
> +                       if (IS_ERR(tzdev) != -EACCES) {

Why would EACCES ever happen? AFAICT
devm_thermal_zone_of_sensor_register does not actually try to read the
temperature value? Or does the error come from somewhere else?

> +                               ret = PTR_ERR(tzdev);
> +                               goto err_disable_clk_peri_therm;
> +                       }
> +               }
>         }
>
>         return 0;
> --
> 1.9.1
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2019-05-11  2:31 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-02 10:43 [PATCH 0/8] Add Mediatek thermal dirver and dtsi michael.kao
2019-05-02 10:43 ` michael.kao
2019-05-02 10:43 ` [PATCH 2/8] arm64: dts: mt8183: add/update dynamic power coefficients michael.kao
2019-05-02 10:43   ` michael.kao
2019-05-03  7:16   ` Hsin-Yi Wang
2019-05-03  7:16     ` Hsin-Yi Wang
2019-05-08 12:27     ` Michael Kao
2019-05-08 12:27       ` Michael Kao
2019-05-08 12:27       ` Michael Kao
2019-05-02 10:43 ` [PATCH 3/8] arm64: dts: mt8183: Add #cooling-cells to CPU nodes michael.kao
2019-05-02 10:43   ` michael.kao
2019-05-06  5:25   ` Eddie Huang
2019-05-06  5:25     ` Eddie Huang
2019-05-08 12:29     ` Michael Kao
2019-05-08 12:29       ` Michael Kao
2019-05-02 10:43 ` [PATCH 4/8] arm64: dts: mt8183: Configure CPU cooling michael.kao
2019-05-02 10:43   ` michael.kao
2019-05-07  8:57   ` Hsin-Yi Wang
2019-05-07  8:57     ` Hsin-Yi Wang
2019-05-02 10:43 ` [PATCH 5/8] arm64: dts: mt8183: Increase polling frequency for CPU thermal zone michael.kao
2019-05-02 10:43   ` michael.kao
2019-05-07  8:58   ` Hsin-Yi Wang
2019-05-07  8:58     ` Hsin-Yi Wang
     [not found] ` <1556793795-25204-1-git-send-email-michael.kao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
2019-05-02 10:43   ` [PATCH 1/8] arm64: dts: mt8183: add thermal zone node michael.kao
2019-05-02 10:43     ` michael.kao
2019-05-03  8:03     ` Hsin-Yi Wang
2019-05-03  8:03       ` Hsin-Yi Wang
2019-05-03 16:46       ` Matthias Kaehlcke
2019-05-03 16:46         ` Matthias Kaehlcke
2019-05-06 10:43         ` Daniel Lezcano
2019-05-06 10:43           ` Daniel Lezcano
2019-05-08 12:23           ` Michael Kao
2019-05-08 12:23             ` Michael Kao
2019-05-08 12:23             ` Michael Kao
2019-05-10  8:14             ` Daniel Lezcano
2019-05-10  8:14               ` Daniel Lezcano
2019-05-10 10:52             ` Michael Kao
2019-05-10 10:52               ` Michael Kao
2019-05-10 10:52               ` Michael Kao
2019-05-06  9:43     ` Daniel Lezcano
2019-05-06  9:43       ` Daniel Lezcano
2019-05-02 10:43   ` [PATCH 6/8] thermal: mediatek: mt8183: fix bank number settings michael.kao
2019-05-02 10:43     ` michael.kao
2019-05-07  8:58     ` Hsin-Yi Wang
2019-05-07  8:58       ` Hsin-Yi Wang
2019-05-02 10:43 ` [PATCH 7/8] thermal: mediatek: add another get_temp ops for thermal sensors michael.kao
2019-05-02 10:43   ` michael.kao
2019-05-11  2:31   ` Nicolas Boichat
2019-05-11  2:31     ` Nicolas Boichat
2019-05-11  2:31     ` Nicolas Boichat
2019-05-02 10:43 ` [PATCH 8/8] thermal: mediatek: use spinlock to protect PTPCORESEL michael.kao
2019-05-02 10:43   ` michael.kao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.