Support pwm-fan platform devices

[depau]

In the following lines

thinkfan/src/drivers.cpp

Lines 196 to 198 in 6f29448

	std::fstream f(path_ + "_enable");
	if (!(f.is_open() && f.good()))
	throw IOerror(MSG_FAN_INIT(path_), errno);

Thinkfan tries to open and write to pwmX_enable. However, not all drivers provide this file. For example, pwm-fan which can be used to drive a fan connected to a PWM output on ARM boards, does not provide it since the fan is always controlled by the kernel and always enabled.

# ls -lAh /sys/devices/platform/pwm-fan/hwmon/hwmon2/ 
total 0
lrwxrwxrwx 1 root root    0 Sep 20 06:23 device -> ../../../pwm-fan
-r--r--r-- 1 root root 4.0K Sep 20 06:23 fan1_input
-r--r--r-- 1 root root 4.0K Sep 20 06:23 name
lrwxrwxrwx 1 root root    0 Sep 20 07:06 of_node -> ../../../../../firmware/devicetree/base/pwm-fan
drwxr-xr-x 2 root root    0 Sep 20 06:43 power
-rw-r--r-- 1 root root 4.0K Sep 20 07:06 pwm1
lrwxrwxrwx 1 root root    0 Sep 20 07:06 subsystem -> ../../../../../class/hwmon
-rw-r--r-- 1 root root 4.0K Jan 18  2013 uevent

Relevant config snippet:

fans:
  - hwmon: /sys/devices/platform/pwm-fan/hwmon
    indices: [1]

# /usr/bin/thinkfan -v

ERROR: ~HwmonFanDriver: Resetting fan control in /sys/devices/platform/pwm-fan/hwmon/hwmon2/pwm1: Permission denied
ERROR: init: Initializing fan control in /sys/devices/platform/pwm-fan/hwmon/hwmon2/pwm1: No such file or directory

EDIT: possible equivalent behavior in this situation

Normally, when using pwm-fan, you provide both the fan and some thermal trips + cooling maps to bind thermal events to values to be applied to cooling devices.

If pwm-fan is used and bound to some thermal trips, you will find a bunch of dedicated cooling maps that have the fan set as the cooling device (see pwm-fan dt docs)

An equivalent to the pwmX_enable behavior would be to detach the cooling device from the cooling maps. I'm not sure how to do that, but I can provide info to reproduce my setup. To bypass the problem, since I'm making my own device tree, I simply provided an emergency trip that runs the fan in take-off mode when the CPU temperature reaches 95°C, but I made sure it is otherwise untouched.

To get a similar setup you'd need some ARM SBC (I'm using an Orange Pi 4, dts, adapted from Armbian's), a 5V PWM fan (I'm using a Noctua NF-A6x25 5V PWM), then this dts patch to add the fan:

--- a/arch/arm64/boot/dts/rockchip/rk3399-orangepi-4.dts	2020-09-20 07:32:59.143938544 +0200
+++ b/arch/arm64/boot/dts/rockchip/rk3399-orangepi-4.dts	2020-09-20 07:46:53.090222362 +0200
@@ -335,6 +335,45 @@
 		 */
 		reset-gpios = <&gpio0 10 GPIO_ACTIVE_LOW>; /* GPIO0_B2 */
 	};
+
+   fan0: pwm-fan {
+     cooling-levels = <0 125 255>;
+     interrupts = <RK_PA1 IRQ_TYPE_EDGE_FALLING>;
+     interrupt-parent = <&gpio1>;
+     compatible = "pwm-fan";
+     pulses-per-revolution = <2>;
+     interrupt-names = "tach";
+     pwms = <&pwm1 0 10000 0>;
+     #cooling-cells = <2>;
+  };
+};
+
+&cpu_thermal {
+  trips {
+    cpu_cold: cpu_cold {
+      temperature = <0>;
+      hysteresis = <2000>;
+      type = "passive";
+    };
+
+    cpu_crispy: cpu_crispy {
+      temperature = <90000>;
+      hysteresis = <2000>;
+      type = "passive";
+    };
+  };
+
+  cooling_maps {
+    map2 {
+      trip = <&cpu_cold>;
+      cooling-device = <&fan0 0 1>;
+    };
+    
+    map3 {
+      trip = <&cpu_crispy>;
+      cooling-device = <&fan0 1 2>;
+    };
+  };
 };
 
 &cpu_l0 {

PWM output can be connected to pin 7 (GPIO4_C6/PWM1) and tach to pin 11 (GPIO1_A1) with a 1kΩ to 1.4kΩ pull-up resistor to 5V

[vmatare]

Huh. Interesting. So which solution are you proposing exactly? Sounds to me like this pwm-fan driver isn't directly intended for fan control from userspace. I think of the (non-)presence of pwm*_enable as an indicator for that. So in my mind, the clean solution would be to patch it accordingly, because one thing I wouldn't want to do is to just ignore failures to write to pwm*_enable...

[depau]

Huh. Interesting. So which solution are you proposing exactly? Sounds to me like this pwm-fan driver isn't directly intended for fan control from userspace.

I'm not sure about whether it is intended to be controlled from userspace or not, but it can be controlled and I set up my device tree so that the kernel doesn't keep playing with it.

I built thinkfan with those lines commented and it's been working fine for 24h, it's keeping it quiet.

I'd say thinkfan is pretty much redundant in this case because the kernel itself provides similar options, however thinkfan is still desirable in my case because I'm also monitoring dynamically discovered SATA hard disks. Being them connected to a PCI-e card I can't just add them to a cooling map in the dtb.

My idea was to unbind the hwmon's "sibling" cooling_device from all thermal zones (tit can't really be unbound but you can set its weight to 0, so it should be left untouched by the kernel). However I can't seem to find a reliable way to map some hwmon device to its cooling_device sibling in the thermal subsystem.

because one thing I wouldn't want to do is to just ignore failures to write to pwm*_enable...

I was going to suggest to add a config option to skip the enabling, and maybe document that it shouldn't be needed in most cases on x86/PC platforms.

[vmatare]

I was going to suggest to add a config option to skip the enabling, and maybe document that it shouldn't be needed in most cases on x86/PC platforms.

Right, that pretty much matches the intention of the DANGEROUS option -D. At least it sounds slightly dangerous to me, because what happens when thinkfan dies?

[depau]

I was going to suggest to add a config option to skip the enabling, and maybe document that it shouldn't be needed in most cases on x86/PC platforms.

Right, that pretty much matches the intention of the DANGEROUS option -D. At least it sounds slightly dangerous to me, because what happens when thinkfan dies?

Which is why I added a thermal trip point in the DTB, but that's not a thing you can easily do on platforms without a device-tree, so I agree with you :)