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

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Dragan,

On 10/9/24 9:16 AM, Dragan Simic wrote:
> 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. :)
> 

Wasn't this feature added this summer by Guenter?

c.f. 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/hwmon/amc6821.c?id=becbd16ed2f8f427239ffda66b5d894008bc56af

Mode 4 is 
https://elixir.bootlin.com/linux/v6.11.3/source/drivers/hwmon/amc6821.c#L367 
([FDRC1:FDRC0] = [01] -> Software-RPM Control Mode (Fan Speed Regulator) 
according to the datasheet).

In any case, we cannot compare those for our products as we do not have 
a genuine AMC6821 but a handmade simulation of the IP we run in an MCU.

But that'd be an interesting data point indeed :)

Cheers,
Quentin