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@xxxxxxxxxxxxxxxxxxxxx>
Cc: Klaus Goger <klaus.goger@xxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx>
Reviewed-by: Quentin Schulz <quentin.schulz@xxxxxxxxx>
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
