Re: [PATCH v5 6/7] arm64: dts: rockchip: Add thermal nodes to RK3576

Alexey Charkov
2025-06-05T19:19:53Z

Hi Nicolas,

On Thu, Jun 5, 2025 at 11:07 PM Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
>
> Add the TSADC node to the RK3576. Additionally, add everything the TSADC
> needs to function, i.e. thermal zones, their trip points and maps, as
> well as adjust the CPU cooling-cells property.
>
> The polling-delay properties are set to 0 as we do have interrupts for
> this TSADC on this particular SoC.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
> arch/arm64/boot/dts/rockchip/rk3576.dtsi | 164 ++++++++++++++++++++++++++++++-
> 1 file changed, 162 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> index a6bfef82d50bc9b0203a04324d61e0f232b61a65..1c07ad78c9230f1e46b0ef8817834f58b19eb86b 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> @@ -11,6 +11,7 @@
> #include <dt-bindings/power/rockchip,rk3576-power.h>
> #include <dt-bindings/reset/rockchip,rk3576-cru.h>
> #include <dt-bindings/soc/rockchip,boot-mode.h>
> +#include <dt-bindings/thermal/thermal.h>
>
> / {
> compatible = "rockchip,rk3576";
> @@ -113,9 +114,9 @@ cpu_l0: cpu@0 {
> capacity-dmips-mhz = <485>;
> clocks = <&scmi_clk SCMI_ARMCLK_L>;
> operating-points-v2 = <&cluster0_opp_table>;
> - #cooling-cells = <2>;
> dynamic-power-coefficient = <120>;
> cpu-idle-states = <&CPU_SLEEP>;
> + #cooling-cells = <2>;
> };
>
> cpu_l1: cpu@1 {
> @@ -127,6 +128,7 @@ cpu_l1: cpu@1 {
> clocks = <&scmi_clk SCMI_ARMCLK_L>;
> operating-points-v2 = <&cluster0_opp_table>;
> cpu-idle-states = <&CPU_SLEEP>;
> + #cooling-cells = <2>;
> };
>
> cpu_l2: cpu@2 {
> @@ -138,6 +140,7 @@ cpu_l2: cpu@2 {
> clocks = <&scmi_clk SCMI_ARMCLK_L>;
> operating-points-v2 = <&cluster0_opp_table>;
> cpu-idle-states = <&CPU_SLEEP>;
> + #cooling-cells = <2>;
> };
>
> cpu_l3: cpu@3 {
> @@ -149,6 +152,7 @@ cpu_l3: cpu@3 {
> clocks = <&scmi_clk SCMI_ARMCLK_L>;
> operating-points-v2 = <&cluster0_opp_table>;
> cpu-idle-states = <&CPU_SLEEP>;
> + #cooling-cells = <2>;
> };
>
> cpu_b0: cpu@100 {
> @@ -159,9 +163,9 @@ cpu_b0: cpu@100 {
> capacity-dmips-mhz = <1024>;
> clocks = <&scmi_clk SCMI_ARMCLK_B>;
> operating-points-v2 = <&cluster1_opp_table>;
> - #cooling-cells = <2>;
> dynamic-power-coefficient = <320>;
> cpu-idle-states = <&CPU_SLEEP>;
> + #cooling-cells = <2>;
> };
>
> cpu_b1: cpu@101 {
> @@ -173,6 +177,7 @@ cpu_b1: cpu@101 {
> clocks = <&scmi_clk SCMI_ARMCLK_B>;
> operating-points-v2 = <&cluster1_opp_table>;
> cpu-idle-states = <&CPU_SLEEP>;
> + #cooling-cells = <2>;
> };
>
> cpu_b2: cpu@102 {
> @@ -184,6 +189,7 @@ cpu_b2: cpu@102 {
> clocks = <&scmi_clk SCMI_ARMCLK_B>;
> operating-points-v2 = <&cluster1_opp_table>;
> cpu-idle-states = <&CPU_SLEEP>;
> + #cooling-cells = <2>;
> };
>
> cpu_b3: cpu@103 {
> @@ -195,6 +201,7 @@ cpu_b3: cpu@103 {
> clocks = <&scmi_clk SCMI_ARMCLK_B>;
> operating-points-v2 = <&cluster1_opp_table>;
> cpu-idle-states = <&CPU_SLEEP>;
> + #cooling-cells = <2>;
> };
>
> idle-states {
> @@ -436,6 +443,143 @@ psci {
> method = "smc";
> };
>
> + thermal_zones: thermal-zones {
> + /* sensor near the center of the SoC */
> + package_thermal: package-thermal {
> + polling-delay-passive = <0>;
> + polling-delay = <0>;
> + thermal-sensors = <&tsadc 0>;
> +
> + trips {
> + package_crit: package-crit {
> + temperature = <115000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
> + };
> + };
> +
> + /* sensor for cluster1 (big Cortex-A72 cores) */
> + bigcore_thermal: bigcore-thermal {
> + polling-delay-passive = <0>;

I've tried these on my board, and it seems that with a zero here it
never stops throttling the CPU even after it cools down. I believe you
need something like <100> here, which is what I used on RK3588 for
similar reasons.

I think it's because the TSADC only fires an interrupt when the
temperature crosses the trip point, but the thermal governor also
needs to observe temperature trends and step up / step down the
cooling states depending on whether the system is cooling sufficiently
or not. So it needs to poll the temperature once the cooling device is
activated (passive in this case).

> + polling-delay = <0>;
> + thermal-sensors = <&tsadc 1>;
> +
> + trips {
> + bigcore_alert: bigcore-alert {
> + temperature = <85000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> +
> + bigcore_crit: bigcore-crit {
> + temperature = <115000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&bigcore_alert>;
> + cooling-device =
> + <&cpu_b0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu_b1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu_b2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu_b3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> + };
> + };
> + };
> +
> + /* sensor for cluster0 (little Cortex-A53 cores) */
> + littlecore_thermal: littlecore-thermal {
> + polling-delay-passive = <0>;

polling-delay-passive = <100>;

> + polling-delay = <0>;
> + thermal-sensors = <&tsadc 2>;
> +
> + trips {
> + littlecore_alert: littlecore-alert {
> + temperature = <85000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> +
> + littlecore_crit: littlecore-crit {
> + temperature = <115000>;
> + hysteresis = <0>;
> + type = "critical";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&littlecore_alert>;
> + cooling-device =
> + <&cpu_l0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu_l1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu_l2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu_l3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> + };
> + };
> + };
> +
> + gpu_thermal: gpu-thermal {
> + polling-delay-passive = <0>;

polling-delay-passive = <100>;

Best regards,
Alexey