Re: [PATCH v2 3/3] ARM: dts: sun9i-a80: Add thermal sensor and thermal zones

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Thermal zones for CPU clusters are misnamed and cross-mapped relative to the actual CPU cores.
--

--- Patch [3]: [PATCH v2 3/3] ARM: dts: sun9i-a80: Add thermal sensor and thermal zones ---
commit 073b6503b11cee70b348c673193855cbda7082e3
Author: Philipp Rossak <[email protected]>

ARM: dts: sun9i-a80: Add thermal sensor and thermal zones

This commit adds the device node for the A80 thermal sensor and defines
the thermal zones in the device tree. It maps the sensors to the big CPU
cluster, DRAM, GPU, and little CPU cluster.

> diff --git a/arch/arm/boot/dts/allwinner/sun9i-a80.dtsi b/arch/arm/boot/dts/allwinner/sun9i-a80.dtsi
> index a1ae0929cec9d..8e7f8cd943d2e 100644
> --- a/arch/arm/boot/dts/allwinner/sun9i-a80.dtsi
> +++ b/arch/arm/boot/dts/allwinner/sun9i-a80.dtsi
[ ... ]
> @@ -1250,4 +1260,62 @@ r_rsb: rsb@8003400 {
>  			#size-cells = <0>;
>  		};
>  	};
> +
> +	thermal-zones {
> +		cpu0_thermal: cpu0-thermal {
> +			polling-delay-passive = <1000>;
> +			polling-delay = <5000>;
> +			thermal-sensors = <&ths 0>;

[Severity: Medium]
Since cpu0 on the sun9i-a80 is part of the little CPU cluster, does this
map the cpu0 thermal zone to the wrong sensor? The commit message notes
that sensor 0 is beside the big CPU cluster. This could lead to mislabeled
temperatures in userspace and potentially throttle the wrong cluster if
cooling maps are added later.

> +
> +			trips {
> +				cpu0_crit: cpu0-crit {
> +					temperature = <100000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +		};
[ ... ]
> +		cpu2_thermal: cpu2-thermal {
> +			polling-delay-passive = <1000>;
> +			polling-delay = <5000>;
> +			thermal-sensors = <&ths 3>;

[Severity: Medium]
Are these names cross-mapped? This maps cpu2-thermal to sensor 3, which the
commit message states is beside the little CPU cluster. Since cpu2 is also
typically a little core, should the node names better reflect the actual
big/little clusters to avoid confusion?

> +
> +			trips {
> +				cpu2_crit: cpu2-crit {
> +					temperature = <100000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +	};
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3