Re: [PATCH] arm64: dts: qcom: x1e80100: Enable tsens and thermal nodes

[Konrad Dybcio]

On 27.05.2024 8:51 AM, Abel Vesa wrote:
> From: Rajendra Nayak <quic_rjendra@xxxxxxxxxxx>
> 
> Add tsens and thermal nodes for x1e80100 SoC.
> 
> Signed-off-by: Rajendra Nayak <quic_rjendra@xxxxxxxxxxx>
> Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 1356 ++++++++++++++++++++++++++++++++
>  1 file changed, 1356 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 5f90a0b3c016..2e34086b0ddd 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -2505,6 +2505,66 @@ &config_noc SLAVE_QUP_0 QCOM_ICC_TAG_ALWAYS>,
>  			};
>  		};
>  
> +		tsens0: thermal-sensor@c271000 {
> +			compatible = "qcom,x1e80100-tsens", "qcom,tsens-v2";
> +			reg = <0 0x0c271000 0 0x1000>, /* TM */
> +			      <0 0x0c222000 0 0x1000>; /* SROT */

Please drop the comments

[...]

> +
> +	thermal-zones {
> +		aoss0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;

Remove polling-delay (irq driven), put something in polling-delay-passive
(250 sounds decent)

> +			thermal-sensors = <&tsens0 0>;
> +
> +			trips {
> +				thermal-engine-config {
> +					temperature = <125000>;
> +					hysteresis = <1000>;
> +					type = "passive";
> +				};
> +
> +				reset-mon-config {

Drop all reset-mon-* stuff

> +					temperature = <115000>;
> +					hysteresis = <5000>;
> +					type = "passive";
> +				};
> +			};
> +		};
> +
> +		cpu0-0-0-thermal {

This naming is a bit too much.. is that cluster:core:sensor?

If so, do the sensors have some sensible differentiator, like physical
placement that could be used instead of 0/1?

[...]

> +		cpuss0-0-thermal {
> +			polling-delay-passive = <0>;
> +			polling-delay = <0>;
> +			thermal-sensors = <&tsens0 9>;
> +
> +			trips {
> +				thermal-engine-config {

Themal engine is also a downstream invention, replace this with a

trip-point0 {
	temperature = <110000>;
        hysteresis = <1000>;
        type = "critical";
}

110 is in line with all the other cpu nodes

> +					temperature = <125000>;
> +					hysteresis = <1000>;
> +					type = "passive";
> +				};
> +
> +				reset-mon-config {
> +					temperature = <115000>;
> +					hysteresis = <5000>;
> +					type = "passive";
> +				};
> +			};
> +		};
> +
> +		cpuss0-1-thermal {

cpuss-x.y also isn't very meaningful.. I'm assuming x is cluster and y
is some sensor within?

[...]

> +		gpuss-1-thermal {
> +			polling-delay-passive = <10>;
> +			polling-delay = <0>;
> +			thermal-sensors = <&tsens3 6>;
> +
> +			trips {
> +				thermal-engine-config {
> +					temperature = <125000>;
> +					hysteresis = <1000>;
> +					type = "passive";
> +				};
> +
> +				thermal-hal-config {
> +					temperature = <125000>;
> +					hysteresis = <1000>;
> +					type = "passive";
> +				};
> +
> +				reset-mon-config {
> +					temperature = <115000>;
> +					hysteresis = <5000>;
> +					type = "passive";
> +				};
> +
> +				gpu1_junction_config: junction-config {
> +					temperature = <95000>;
> +					hysteresis = <5000>;
> +					type = "passive";
> +				};
> +			};

Please replace with something like:

https://lore.kernel.org/linux-arm-msm/20240510-topic-gpus_are_cool_now-v1-12-ababc269a438@xxxxxxxxxx/

Konrad