Re: [PATCH] arm64: dts: mt8192: add thermal zones, cooling map and trips

From: Matthias Brugger
Date: Fri Sep 03 2021 - 06:40:47 EST



On 23/12/2020 08:49, Michael Kao wrote:
> Add thermal zone node to support mt8192 read temperature.
> Thermal throttle will start at 68C and the
> target temperature is 85C.
>
> This patch depends on [1].

Please provide this kind of information below the three dashes '---'. Otherwise
this will end up in the commit message.

>
> [1]https://patchwork.kernel.org/project/linux-mediatek/patch/20201221061018.18503-3-Yz.Wu@xxxxxxxxxxxx/
>
> Signed-off-by: Michael Kao <michael.kao@xxxxxxxxxxxx>
> ---
> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 169 +++++++++++++++++++++++
> 1 file changed, 169 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> index 4a0d941aec30..4020e40a092a 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> @@ -9,6 +9,7 @@
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
> +#include <dt-bindings/thermal/thermal.h>
>
> / {
> compatible = "mediatek,mt8192";
> @@ -42,6 +43,7 @@
> clock-frequency = <1701000000>;
> next-level-cache = <&l2_0>;
> capacity-dmips-mhz = <530>;
> + #cooling-cells = <2>;
> };
>
> cpu1: cpu@100 {
> @@ -52,6 +54,7 @@
> clock-frequency = <1701000000>;
> next-level-cache = <&l2_0>;
> capacity-dmips-mhz = <530>;
> + #cooling-cells = <2>;
> };
>
> cpu2: cpu@200 {
> @@ -62,6 +65,7 @@
> clock-frequency = <1701000000>;
> next-level-cache = <&l2_0>;
> capacity-dmips-mhz = <530>;
> + #cooling-cells = <2>;
> };
>
> cpu3: cpu@300 {
> @@ -72,6 +76,7 @@
> clock-frequency = <1701000000>;
> next-level-cache = <&l2_0>;
> capacity-dmips-mhz = <530>;
> + #cooling-cells = <2>;
> };
>
> cpu4: cpu@400 {
> @@ -82,6 +87,7 @@
> clock-frequency = <2171000000>;
> next-level-cache = <&l2_1>;
> capacity-dmips-mhz = <1024>;
> + #cooling-cells = <2>;
> };
>
> cpu5: cpu@500 {
> @@ -92,6 +98,7 @@
> clock-frequency = <2171000000>;
> next-level-cache = <&l2_1>;
> capacity-dmips-mhz = <1024>;
> + #cooling-cells = <2>;
> };
>
> cpu6: cpu@600 {
> @@ -102,6 +109,7 @@
> clock-frequency = <2171000000>;
> next-level-cache = <&l2_1>;
> capacity-dmips-mhz = <1024>;
> + #cooling-cells = <2>;
> };
>
> cpu7: cpu@700 {
> @@ -112,6 +120,7 @@
> clock-frequency = <2171000000>;
> next-level-cache = <&l2_1>;
> capacity-dmips-mhz = <1024>;
> + #cooling-cells = <2>;
> };
>
> cpu-map {
> @@ -178,6 +187,140 @@
> method = "smc";
> };
>
> + thermal-zones {
> + soc_max {
> + polling-delay = <1000>; /* milliseconds */
> + polling-delay-passive = <1000>; /* milliseconds */
> + thermal-sensors = <&lvts 0>;
> + sustainable-power = <1500>;
> +
> + trips {
> + threshold: trip-point@0 {
> + temperature = <68000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> +
> + target: target@1 {

Please review the node names.

> + temperature = <85000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> +
> + soc_max_crit: soc_max_crit@0 {
> + 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>;

By binding description value is in per-cent, 3072 does not make sense.

> + };
> + 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>;

Same here.

> + };
> + };
> + };

New line here.

> + cpu_big1 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 1>;
> + };
> + cpu_big2 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 2>;
> + };
> + cpu_big3 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 3>;
> + };
> + cpu_big4 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 4>;
> + };
> + cci1 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 5>;
> + };
> + cci2 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 6>;
> + };
> + cpu_little1 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 7>;
> + };
> + cpu_little2 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 8>;
> + };
> + apu {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 9>;
> + };
> + mlda {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 10>;
> + };
> + gpu1 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 11>;
> + };
> + gpu2 {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 12>;
> + };
> + infra {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 13>;
> + };
> + camsys {
> + polling-delay = <0>; /* milliseconds */
> + polling-delay-passive = <0>; /* milliseconds */
> + thermal-sensors = <&lvts 14>;
> + };

I think I'm missing something. We are creating a whole bunch of thermal zones,
but no trip points neither cooling maps for them. What are the needed for?

> + };
> +
> timer: timer {
> compatible = "arm,armv8-timer";
> interrupt-parent = <&gic>;
> @@ -224,6 +367,10 @@
> compatible = "mediatek,mt8192-infracfg", "syscon";
> reg = <0 0x10001000 0 0x1000>;
> #clock-cells = <1>;
> + ti,reset-bits = <
> + 0x120 0 0x124 0 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE)
> + 0x730 12 0x734 12 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE)
> + >;

How is that related to the commit message? Looks to me like a separate patch.

> };
>
> pericfg: syscon@10003000 {
> @@ -318,6 +465,24 @@
> status = "disabled";
> };
>
> + lvts: lvts@1100b000 {
> + compatible = "mediatek,mt6873-lvts";

This driver is not upstream. Please provide a link to the submission of the
latest version so that I can track progress.

Regards,
Matthias