linux-kernel - Re: [PATCH v2] arm64: dts: rockchip: add pwm-fan for NanoPC-T6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <da6721a3-8f59-9c0c-762c-bc02b39ac472@manjaro.org>
Date: Mon, 10 Nov 2025 01:23:06 +0100
From: "Dragan Simic" <dsimic@...jaro.org>
To: "Hugh Cole-Baker" <sigmaris@...il.com>
Cc: "Rob Herring" <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>, "Heiko Stuebner" <heiko@...ech.de>, "Alexey Charkov" <alchark@...il.com>, devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] arm64: dts: rockchip: add pwm-fan for NanoPC-T6

Hello Hugh,

Thanks for the v2!  Please, see a couple of nitpicks below.

On Sunday, November 09, 2025 20:20 CET, Hugh Cole-Baker <sigmaris@...il.com> wrote:
> FriendlyELEC offers an optional heatsink and fan addon for the NanoPC-T6
> and T6 LTS, which plugs in to the fan connector on the board driven by
> pwm1. Add the fan as an active cooling device for the SoC package.
> 
> Signed-off-by: Hugh Cole-Baker <sigmaris@...il.com>
> ---
> Changes from v1: https://lore.kernel.org/linux-rockchip/20251026194858.92461-1-sigmaris@gmail.com/
> * add the fan to the base board dtsi instead of overlay (Heiko)
> * just use 2 trip points for warm and hot temperatures (Dragan, Alexey)
> 
> References:
> FriendlyELEC heatsink with fan addon:
> https://www.friendlyelec.com/index.php?route=product/product&product_id=305
> Vendor DT with trip points and PWM duty cycle values:
> https://github.com/friendlyarm/kernel-rockchip/blob/4944602540b62f5aad139fe602a76cf7c3176128/arch/arm64/boot/dts/rockchip/rk3588-nanopi6-rev01.dts#L75-L90

I think it would be better to move these references to the patch
description, so they become directly available in the repository.
It might be the best to use the "... [n]" form for the references,
which puts them as close to the backed contents as possible.

Oh, and I think that "arm64: dts: rockchip: Enable active cooling
on NanoPC-T6" as the patch subject would read nicer. :)

>  .../boot/dts/rockchip/rk3588-nanopc-t6.dtsi   | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
> index fafeabe9adf9e..9164a0ee6228e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-nanopc-t6.dtsi
> @@ -11,6 +11,7 @@
>  #include <dt-bindings/input/input.h>
>  #include <dt-bindings/pinctrl/rockchip.h>
>  #include <dt-bindings/soc/rockchip,vop2.h>
> +#include <dt-bindings/thermal/thermal.h>
>  #include <dt-bindings/usb/pd.h>
>  #include "rk3588.dtsi"
>  
> @@ -89,6 +90,14 @@ usr_led: led-1 {
>  		};
>  	};
>  
> +	fan: pwm-fan {
> +		compatible = "pwm-fan";
> +		cooling-levels = <0 35 64 100 150 255>;
> +		fan-supply = <&vcc5v0_sys>;
> +		pwms = <&pwm1 0 50000 0>;
> +		#cooling-cells = <2>;
> +	};
> +
>  	sound {
>  		compatible = "simple-audio-card";
>  		pinctrl-names = "default";
> @@ -591,6 +600,36 @@ &i2s6_8ch {
>  	status = "okay";
>  };
>  
> +&package_thermal {
> +	polling-delay = <1000>;
> +
> +	trips {
> +		package_warm: package-warm {
> +			temperature = <50000>;
> +			hysteresis = <2000>;
> +			type = "active";
> +		};
> +
> +		package_hot: package-hot {
> +			temperature = <60000>;
> +			hysteresis = <2000>;
> +			type = "active";
> +		};
> +	};

It should be better to use 55 and 65 oC as the trip thresholds,
because people often report around 50 oC as the observed idle-state
temperature of their RK3588 SoCs, so increasing the first threshold
to 55 oC should be beneficial by preventing the fan from spinning
when the SoC is actually idle.  The second threshold is usually set
to be 10 oC higher, so it should end up at 65 oC.

> +	cooling-maps {
> +		map0 {
> +			trip = <&package_warm>;
> +			cooling-device = <&fan THERMAL_NO_LIMIT 1>;
> +		};
> +
> +		map1 {
> +			trip = <&package_hot>;
> +			cooling-device = <&fan 2 THERMAL_NO_LIMIT>;
> +		};
> +	};
> +};
> +
>  &pcie2x1l0 {
>  	reset-gpios = <&gpio4 RK_PB3 GPIO_ACTIVE_HIGH>;
>  	vpcie3v3-supply = <&vcc_3v3_pcie20>;