[PATCH 1/2] arm64: dts: rockchip: add package_thermal for Radxa ROCK 5A/5C

[PATCH 1/2] arm64: dts: rockchip: add package_thermal for Radxa ROCK 5A/5C

[FUKAUMI Naoki]

add and enable package temperature based active cooling.

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
this patch depends on [1] which depends on [2].

[1] https://patchwork.kernel.org/project/linux-rockchip/cover/20241128121929.62646-1-naoki@radxa.com/
[2] https://patchwork.kernel.org/project/linux-rockchip/patch/20241119095113.78151-1-naoki@radxa.com/
---
 .../boot/dts/rockchip/rk3588s-rock-5.dtsi     | 32 ++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
index a8f40f43c838..a1cac40d439e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
@@ -33,7 +33,7 @@ analog-sound {
 			  "Headphone", "Headphones";
 	};
 
-	fan {
+	fan: fan {
 		compatible = "pwm-fan";
 		#cooling-cells = <2>;
 		cooling-levels = <0 64 128 192 255>;
@@ -379,6 +379,36 @@ rgmii_phy1: ethernet-phy@1 {
 	};
 };
 
+&package_thermal {
+	polling-delay = <1000>;
+
+	trips {
+		package_fan0: package-fan0 {
+			hysteresis = <2000>;
+			temperature = <55000>;
+			type = "active";
+		};
+
+		package_fan1: package-fan1 {
+			hysteresis = <2000>;
+			temperature = <65000>;
+			type = "active";
+		};
+	};
+
+	cooling-maps {
+		map0 {
+			cooling-device = <&fan THERMAL_NO_LIMIT 1>;
+			trip = <&package_fan0>;
+		};
+
+		map1 {
+			cooling-device = <&fan 2 THERMAL_NO_LIMIT>;
+			trip = <&package_fan1>;
+		};
+	};
+};
+
 &pcie2x1l2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pcie20x1_2_perstn_m0>,
-- 
2.43.0

[PATCH 2/2] arm64: dts: rockchip: fix cooling-levels in pwm-fan for Radxa ROCK 5A/5C

[FUKAUMI Naoki]

fan behavior is better than current configuration.

Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
---
this patch depends on [1] which depends on [2].

[1] https://patchwork.kernel.org/project/linux-rockchip/cover/20241128121929.62646-1-naoki@radxa.com/
[2] https://patchwork.kernel.org/project/linux-rockchip/patch/20241119095113.78151-1-naoki@radxa.com/
---
 arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
index a1cac40d439e..472b41f0d47f 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
@@ -36,7 +36,7 @@ analog-sound {
 	fan: fan {
 		compatible = "pwm-fan";
 		#cooling-cells = <2>;
-		cooling-levels = <0 64 128 192 255>;
+		cooling-levels = <0 120 150 180 210 240 255>;
 		fan-supply = <&vcc_5v0>;
 		pwms = <&pwm3 0 60000 0>;
 	};
-- 
2.43.0

Re: [PATCH 1/2] arm64: dts: rockchip: add package_thermal for Radxa ROCK 5A/5C

[Dragan Simic]

Hello Fukaumi,

Please, see a few comments below.

On 2024-12-04 05:54, FUKAUMI Naoki wrote:
> add and enable package temperature based active cooling.

I'd suggest that you rewrite the patch summary to read like this:

   arm64: dts: rockchip: Enable automatic fan control on Rock 5A/5C

and to rewrite the patch description like this, which is a recycled
and a bit improved description of the commit 4a152231b050 ("arm64:
dts: rockchip: enable automatic fan control on Rock 5B"):

   Link the PWM fan on Radxa ROCK 5A/5C as an active cooling device
   managed automatically by the thermal subsystem, with a target SoC
   temperature of 65 oC and a minimum-spin interval from 55 oC to
   65 oC, to ensure airflow when the system gets warm.

> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> this patch depends on [1] which depends on [2].
> 
> [1] 
> https://patchwork.kernel.org/project/linux-rockchip/cover/20241128121929.62646-1-naoki@radxa.com/
> [2] 
> https://patchwork.kernel.org/project/linux-rockchip/patch/20241119095113.78151-1-naoki@radxa.com/
> ---
>  .../boot/dts/rockchip/rk3588s-rock-5.dtsi     | 32 ++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> index a8f40f43c838..a1cac40d439e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> @@ -33,7 +33,7 @@ analog-sound {
>  			  "Headphone", "Headphones";
>  	};
> 
> -	fan {
> +	fan: fan {

It would be better to use "fan: pwm-fan { ... }", to ensure some kind
of consistency with the other similar portions of board dts files.

>  		compatible = "pwm-fan";
>  		#cooling-cells = <2>;
>  		cooling-levels = <0 64 128 192 255>;
> @@ -379,6 +379,36 @@ rgmii_phy1: ethernet-phy@1 {
>  	};
>  };
> 
> +&package_thermal {
> +	polling-delay = <1000>;
> +
> +	trips {
> +		package_fan0: package-fan0 {
> +			hysteresis = <2000>;
> +			temperature = <55000>;
> +			type = "active";
> +		};
> +
> +		package_fan1: package-fan1 {
> +			hysteresis = <2000>;
> +			temperature = <65000>;
> +			type = "active";
> +		};
> +	};
> +
> +	cooling-maps {
> +		map0 {
> +			cooling-device = <&fan THERMAL_NO_LIMIT 1>;
> +			trip = <&package_fan0>;
> +		};
> +
> +		map1 {
> +			cooling-device = <&fan 2 THERMAL_NO_LIMIT>;
> +			trip = <&package_fan1>;
> +		};
> +	};
> +};
> +
>  &pcie2x1l2 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie20x1_2_perstn_m0>,

The rest of the patch is looking good to me.  Thanks for the patch!

Re: [PATCH 2/2] arm64: dts: rockchip: fix cooling-levels in pwm-fan for Radxa ROCK 5A/5C

[Dragan Simic]

Hello Fukaumi,

On 2024-12-04 05:54, FUKAUMI Naoki wrote:
> fan behavior is better than current configuration.

Could you, please, describe better what makes this configuration
better, which fan make and model are you using, etc.?

> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> this patch depends on [1] which depends on [2].
> 
> [1] 
> https://patchwork.kernel.org/project/linux-rockchip/cover/20241128121929.62646-1-naoki@radxa.com/
> [2] 
> https://patchwork.kernel.org/project/linux-rockchip/patch/20241119095113.78151-1-naoki@radxa.com/
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> index a1cac40d439e..472b41f0d47f 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
> @@ -36,7 +36,7 @@ analog-sound {
>  	fan: fan {
>  		compatible = "pwm-fan";
>  		#cooling-cells = <2>;
> -		cooling-levels = <0 64 128 192 255>;
> +		cooling-levels = <0 120 150 180 210 240 255>;
>  		fan-supply = <&vcc_5v0>;
>  		pwms = <&pwm3 0 60000 0>;
>  	};

Re: [PATCH 2/2] arm64: dts: rockchip: fix cooling-levels in pwm-fan for Radxa ROCK 5A/5C

[FUKAUMI Naoki]

Hi,

thank you for your review!

On 12/4/24 14:33, Dragan Simic wrote:
> Hello Fukaumi,
> 
> On 2024-12-04 05:54, FUKAUMI Naoki wrote:
>> fan behavior is better than current configuration.
> 
> Could you, please, describe better what makes this configuration
> better, which fan make and model are you using, etc.?

I have three Heatshink 2513[1] and I noticed they behave very 
differently... it's difficult to say which cooling-levels is better or 
worse with these heatsink.

I'll drop this patch.

Best regards,

[1] https://radxa.com/products/accessories/heatsink-2513

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
>> ---
>> this patch depends on [1] which depends on [2].
>>
>> [1] https://patchwork.kernel.org/project/linux-rockchip/ 
>> cover/20241128121929.62646-1-naoki@radxa.com/
>> [2] https://patchwork.kernel.org/project/linux-rockchip/ 
>> patch/20241119095113.78151-1-naoki@radxa.com/
>> ---
>>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
>> index a1cac40d439e..472b41f0d47f 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
>> @@ -36,7 +36,7 @@ analog-sound {
>>      fan: fan {
>>          compatible = "pwm-fan";
>>          #cooling-cells = <2>;
>> -        cooling-levels = <0 64 128 192 255>;
>> +        cooling-levels = <0 120 150 180 210 240 255>;
>>          fan-supply = <&vcc_5v0>;
>>          pwms = <&pwm3 0 60000 0>;
>>      };
> 

Re: [PATCH 1/2] arm64: dts: rockchip: add package_thermal for Radxa ROCK 5A/5C

[FUKAUMI Naoki]

Hi,

On 12/4/24 14:32, Dragan Simic wrote:
> Hello Fukaumi,
> 
> Please, see a few comments below.
> 
> On 2024-12-04 05:54, FUKAUMI Naoki wrote:
>> add and enable package temperature based active cooling.
> 
> I'd suggest that you rewrite the patch summary to read like this:
> 
>    arm64: dts: rockchip: Enable automatic fan control on Rock 5A/5C
> 
> and to rewrite the patch description like this, which is a recycled
> and a bit improved description of the commit 4a152231b050 ("arm64:
> dts: rockchip: enable automatic fan control on Rock 5B"):
> 
>    Link the PWM fan on Radxa ROCK 5A/5C as an active cooling device
>    managed automatically by the thermal subsystem, with a target SoC
>    temperature of 65 oC and a minimum-spin interval from 55 oC to
>    65 oC, to ensure airflow when the system gets warm.

thanks! I'll use this message :)

>> Signed-off-by: FUKAUMI Naoki <naoki@radxa.com>
>> ---
>> this patch depends on [1] which depends on [2].
>>
>> [1] https://patchwork.kernel.org/project/linux-rockchip/ 
>> cover/20241128121929.62646-1-naoki@radxa.com/
>> [2] https://patchwork.kernel.org/project/linux-rockchip/ 
>> patch/20241119095113.78151-1-naoki@radxa.com/
>> ---
>>  .../boot/dts/rockchip/rk3588s-rock-5.dtsi     | 32 ++++++++++++++++++-
>>  1 file changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
>> b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
>> index a8f40f43c838..a1cac40d439e 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
>> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5.dtsi
>> @@ -33,7 +33,7 @@ analog-sound {
>>                "Headphone", "Headphones";
>>      };
>>
>> -    fan {
>> +    fan: fan {
> 
> It would be better to use "fan: pwm-fan { ... }", to ensure some kind
> of consistency with the other similar portions of board dts files.

sure. I'll change it in previous patch series for Radxa ROCK 4C.

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

>>          compatible = "pwm-fan";
>>          #cooling-cells = <2>;
>>          cooling-levels = <0 64 128 192 255>;
>> @@ -379,6 +379,36 @@ rgmii_phy1: ethernet-phy@1 {
>>      };
>>  };
>>
>> +&package_thermal {
>> +    polling-delay = <1000>;
>> +
>> +    trips {
>> +        package_fan0: package-fan0 {
>> +            hysteresis = <2000>;
>> +            temperature = <55000>;
>> +            type = "active";
>> +        };
>> +
>> +        package_fan1: package-fan1 {
>> +            hysteresis = <2000>;
>> +            temperature = <65000>;
>> +            type = "active";
>> +        };
>> +    };
>> +
>> +    cooling-maps {
>> +        map0 {
>> +            cooling-device = <&fan THERMAL_NO_LIMIT 1>;
>> +            trip = <&package_fan0>;
>> +        };
>> +
>> +        map1 {
>> +            cooling-device = <&fan 2 THERMAL_NO_LIMIT>;
>> +            trip = <&package_fan1>;
>> +        };
>> +    };
>> +};
>> +
>>  &pcie2x1l2 {
>>      pinctrl-names = "default";
>>      pinctrl-0 = <&pcie20x1_2_perstn_m0>,
> 
> The rest of the patch is looking good to me.  Thanks for the patch!
>