Re: [PATCH] arm64: dts: rockchip: Allow Turing RK1 cooling fan to spin down

[Dragan Simic]

Hello Sam,

On 2025-03-15 21:48, Sam Edwards wrote:
> The RK3588 thermal sensor driver only receives interrupts when a
> higher-temperature threshold is crossed; it cannot notify when the
> sensor cools back off. As a result, the driver must poll for 
> temperature
> changes to detect when the conditions for a thermal trip are no longer
> met. However, it only does so if the DT enables polling.
>
> Before this patch, the RK1 DT did not enable polling, causing the fan 
> to
> continue running at the speed corresponding to the highest temperature
> reached.
>
> Follow suit with similar RK3588 boards by setting a polling-delay of
> 1000ms, enabling the driver to detect when the sensor cools back off,
> allowing the fan speed to decrease as appropriate.
>
> Fixes: 7c8ec5e6b9d6 ("arm64: dts: rockchip: Enable automatic fan
> control on Turing RK1")
> Signed-off-by: Sam Edwards <CFSworks@xxxxxxxxx>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
> index 6bc46734cc14..0270bffce195 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-turing-rk1.dtsi
> @@ -214,6 +214,8 @@ rgmii_phy: ethernet-phy@1 {
>  };
>
>  &package_thermal {
> +	polling-delay = <1000>;
> +
>  	trips {
>  		package_active1: trip-active1 {
>  			temperature = <45000>;

Thanks for the patch, it's looking good to me, with some related
thoughts below.  Please, feel free to include:

Reviewed-by: Dragan Simic <dsimic@xxxxxxxxxxx>

After a quick look at the RK3588 TRM Part 1, it seems possible
to actually generate additional interrupts when the TSADC channel
temperature readouts reach predefined low thresholds.  Moreover,
avoiding the polling would actually help the SoC cool down a tiny
bit faster, which makes it worth detailed investigation in my book,
despite not being used by the downstream kernel code.