Hello Quentin,
On 2024-10-14 17:39, Quentin Schulz wrote:
On 10/9/24 9:16 AM, Dragan Simic wrote:
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).
Ah, SENSOR_DEVICE_ATTR_RW(fan1_target, fan, IDX_FAN1_TARGET)...
How did I miss that? Hmm... Maybe I was looking at some older
local branch, which happened not to include that commit.
Anywyay, good to know, thanks.
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.
I seem to remember your MCU that performs a few tasks, back from
some related discussions. I wonder what was the reason to implement
it in software, instead of using actual fan controller chip?
But that'd be an interesting data point indeed :)
I'm glad that you agree. :)
