Re: [PATCH v2 06/14] arm64: dts: rockchip: Remove #cooling-cells from fan on Theobroma boards

[Dragan Simic <dsimic@manjaro.org>]

Hello Heiko,

On 2024-10-08 22:39, Heiko Stuebner wrote:
> All Theobroma boards use a ti,amc6821 as fan controller.
> It normally runs in an automatically controlled way and while it may be
> possible to use it as part of a dt-based thermal management, this is
> not yet specified in the binding, nor implemented in any kernel.
> 
> Newer boards already don't contain that #cooling-cells property, but
> older ones do. So remove them for now, they can be re-added if thermal
> integration gets implemented in the future.
> 
> Fixes: c484cf93f61b ("arm64: dts: rockchip: add PX30-µQ7 (Ringneck)
> SoM with Haikou baseboard")
> Fixes: d99a02bcfa81 ("arm64: dts: rockchip: add RK3368-uQ7 (Lion) SoM")
> Fixes: 2c66fc34e945 ("arm64: dts: rockchip: add RK3399-Q7 (Puma) SoM")
> Cc: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> Cc: Klaus Goger <klaus.goger@theobroma-systems.com>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>

Looking good to me, thanks for the patch.  In addition to the amc6821
driver currently not supporting full integration into the thermal
framework, the "fan" DT node also isn't referenced in any cooling map,
so having it define the "cooling-cells" property is of no use.

By the way, it would be nice to see the amc6821 driver supporting fan
speed regulation, and test it to check who does a better job when it
comes to cooling and fan speed regulation, the thermal framework or
the chip's built-in logic. :)

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
>  arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi | 1 -
>  arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi   | 1 -
>  arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi   | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> index b7163ed74232..a6ea9d8c785c 100644
> --- a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> @@ -293,7 +293,6 @@ &i2c1 {
>  	fan: fan@18 {
>  		compatible = "ti,amc6821";
>  		reg = <0x18>;
> -		#cooling-cells = <2>;
>  	};
> 
>  	rtc_twi: rtc@6f {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
> index 8ac8acf4082d..ab3fda69a1fb 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3368-lion.dtsi
> @@ -61,7 +61,6 @@ i2c_lvds_blc: i2c@0 {
>  			fan: fan@18 {
>  				compatible = "ti,amc6821";
>  				reg = <0x18>;
> -				#cooling-cells = <2>;
>  			};
> 
>  			rtc_twi: rtc@6f {
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
> index 650b1ba9c192..6de3128556b7 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi
> @@ -395,7 +395,6 @@ &i2c7 {
>  	fan: fan@18 {
>  		compatible = "ti,amc6821";
>  		reg = <0x18>;
> -		#cooling-cells = <2>;
>  	};
> 
>  	rtc_twi: rtc@6f {