Re: [PATCH v2 8/8] arm64: dts: qcom: Add device tree for SA8797P Ride board

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Flawed thermal zone configurations with incorrect types, descending order, and identical duplicate trip points.
--

commit feec939ce542144572175512accde079d1c6829f
Author: Deepti Jaggi <deepti.jaggi@oss.qualcomm.com>

arm64: dts: qcom: Add device tree for SA8797P Ride board

This commit adds the initial device tree for the Qualcomm SA8797P Ride
reference board. It configures UART ports, enables UFS storage, and
defines various thermal zones using the SCMI sensor protocol.

> diff --git a/arch/arm64/boot/dts/qcom/sa8797p-ride.dts b/arch/arm64/boot/dts/qcom/sa8797p-ride.dts
> new file mode 100644
> index 0000000000000..2eab2c6fc173d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sa8797p-ride.dts

[ ... ]

> +	pm_kobra_thermal: pm-a-die-thermal {
> +		polling-delay-passive = <100>;
> +		polling-delay = <0>;
> +		thermal-sensors = <&scmi23_sensor 3>;
> +
> +		trips {
> +			trip0 {
> +				temperature = <135000>;
> +				hysteresis = <5000>;
> +				type = "passive";
> +			};
> +
> +			pm_kobra_trip1: trip1 {
> +				temperature = <115000>;
> +				hysteresis = <5000>;
> +				type = "passive";
> +			};
> +		};
> +	};

Should trip0 be configured as an emergency hardware shutdown trip point
instead of passive, considering the high 135C temperature limit?

Also, does the thermal framework expect these trip points to be defined in
ascending order of temperature rather than descending?

[ ... ]

> +	pm_kai_ufs_thermal: ufs-thermal {
> +		polling-delay-passive = <0>;
> +		polling-delay = <0>;
> +		thermal-sensors = <&scmi23_sensor 0>;
> +
> +		trips {
> +			trip0 {
> +				temperature = <125000>;
> +				hysteresis = <5000>;
> +				type = "passive";
> +			};
> +
> +			trip1 {
> +				temperature = <125000>;
> +				hysteresis = <5000>;
> +				type = "passive";
> +			};
> +		};
> +	};

Are these duplicate trip points intentional? Both trip0 and trip1 are defined
with the exact same temperature, hysteresis, and passive type.

Is it possible that trip1 was intended to have a higher temperature and
serve as a different type of trip point?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519063505.883379-1-shengchao.guo@oss.qualcomm.com?part=8