thinkfan doesn't work on devices with no pwmN_enable

[Aietes]

I was trying to use thinkfan on a Mint Linux machine using a Corsair Commander Pro for fan control. I was running into the issue that the service would not start up:

ERROR: ~HwmonFanDriver: Resetting fan control in /sys/class/hwmon/hwmon8/pwm3: Permission denied

At first I thought it's a permission problem as stated, but it's due to thinkfan trying to enable the PWM channel through pwmN_enable, which does not exist in all configurations, e.g. the USB fan controller under Mint Linux I'm using.

I was now working around it by changing the HwmonFanDriver::init() function to now throw an error if pwmN_enable doesn't exist, and compiled and installed manually, which works fine.

Maybe there is a possibility to account for PWM controllers / systems that do not require pwm_enable?

[koutheir]

If the pwm_enable file does not exist, then why is the reported error Permission denied? Isn't it supposed to be Not Found instead?

[eldorel]

I'm seeing this exact same error on Arch with a corsair commander pro.
Thinking it was an issue with the name: setting in my config, I went ahead and used the path to driver folder method.

ERROR: ~HwmonFanDriver: Resetting fan control in /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm1: Permission denied
ERROR: ~HwmonFanDriver: Resetting fan control in /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm2: Permission denied
ERROR: ~HwmonFanDriver: Resetting fan control in /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm3: Permission denied
ERROR: init: Initializing fan control in /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm1: No such file or directory

[Desktop]$ ls /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7
device fan1_label fan2_input fan2_target fan3_label fan4_input fan4_target fan5_label fan6_input fan6_target in1_input name pwm1 pwm3 pwm5 subsystem
fan1_input fan1_target fan2_label fan3_input fan3_target fan4_label fan5_input fan5_target fan6_label in0_input in2_input power pwm2 pwm4 pwm6 uevent

[Desktop]$ ls /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm*
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm1
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm2
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm3
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm4
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm5
/sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8.2/1-8.2:1.0/0003:1B1C:0C10.0005/hwmon/hwmon7/pwm6

[koutheir]

Does it make sense for you to avoid the hwmon driver altogether and build the lm-sensors branch of thinkfan? which should allow you to use the new format to specify your sensors?

[vmatare]

The error message you're seeing (Resetting fan control...) occurs when thinkfan tries to reset pwm*_enable to the value it had before thinkfan was started. That means that thinkfan must have written (or tried to write) to that file before. @eldorel @Aietes Are you both sure that this is the only error message you're getting? Because there should be some error about Initializing fan control..., as well...

[eldorel]

Since I removed thinkfan and am using a different script, I can't confirm this right away, but I believe you are correct.

However, that file 100% is not created when setup by udev/lm_sensors on any of the three machines I have with this type of USB fan controller on it.

[Aietes]

@vmatare There were mutliple Permission denied errors, I'll have to install the original version of the script again to get the original error message.

@koutheir I didn't realize there was a lm-sensors branch. I'll try that to see if it works out of the box. If it does, it might be good to add a hint to the README or install instructions for other users like me. Thinkfan is great, but I'm not sure people realize how versatile it is.

[Aietes]

I set up another machine with thinkfan, so I have the full error message:

ERROR: ~HwmonFanDriver: Resetting fan control in /sys/class/hwmon/hwmon7/pwm1: Permission denied
ERROR: ~HwmonFanDriver: Resetting fan control in /sys/class/hwmon/hwmon7/pwm2: Permission denied
ERROR: ~HwmonFanDriver: Resetting fan control in /sys/class/hwmon/hwmon7/pwm3: Permission denied
ERROR: ~HwmonFanDriver: Resetting fan control in /sys/class/hwmon/hwmon7/pwm1: Permission denied
ERROR: ~HwmonFanDriver: Resetting fan control in /sys/class/hwmon/hwmon7/pwm2: Permission denied
ERROR: ~HwmonFanDriver: Resetting fan control in /sys/class/hwmon/hwmon7/pwm3: Permission denied
ERROR: init: Initializing fan control in /sys/class/hwmon/hwmon7/pwm1: No such file or directory

This is triggered by the HwmonFanDriver in fans.cpp, which tries to set _enable to on the Hwmon path, which does not exist for all devices.

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

The destructor is trying to reset the value, therefore also causing the error.

The error message is a bit misleading, but I removed the _enable code in the HwmonFanDriver, and now thinkfan works fine for my use case. For increased compatibility it might be good to only show a warning, but not throwing an exception if _enable doesn't exist.

[vmatare]

Thanks a lot for the investigation. I was under the impression that the pwm*_enable are part of the hwmon quasi-standard and are required on any hwmon driver that supports fan control. Now the only remaining question is whether lm-sensors handles the situation correctly...

[Aietes]

The lm-sensors branch handles setting PWM the same way, and throws the same error. Unless I'm missing a way to set fans via a different driver than Hwmon?

[vmatare]

Unless I'm missing a way to set fans via a different driver than Hwmon?

Oh right, sorry, you probably do. The config syntax for lm-sensors isn't properly documented, yet. Take a look at this comment:
#156 (comment)

So basically you want to run sensors and use the names you see there. So let's say we have this output:

# sensors
drivetemp-scsi-3-0
Adapter: SCSI adapter
temp1:        +26.0°C  (low  = +14.0°C, high = +55.0°C)
                       (crit low = +10.0°C, crit = +60.0°C)
                       (lowest = +22.0°C, highest = +26.0°C)

Then drivetemp-scsi-3-0 would be the chip, and temp1 would be an id, i.e.:

sensors:
  - chip: drivetemp-scsi-3-0
    ids: [ temp1 ]

[Aietes]

I did try this with sensors, but I didn't realize it works under "fans:" as well. I looked at the code for fans, and only saw an Hwmon and Tp driver.

[vmatare]

I did try this with sensors, but I didn't realize it works under "fans:" as well. I looked at the code for fans, and only saw an Hwmon and Tp driver.

Yes, lm-sensors support is currently implemented only for temperature inputs. We're considering extending it to fans, as well, but currently I'm concentrating on implementing a clean and efficient retry logic for sensor read errors.

[Aietes]

Keep up the great work, I really like thinkfan and its capabilities!

Would you be open to accept a change to the Hwmon driver, so that it doesn't throw an exception, but rather posts a warning? If I find the time I can see if I can make a pull request...

[vmatare]

Sorry about the mixup with libsensors & fans, apparently paying half-attention is sometimes not good enough^^

Would you be open to accept a change to the Hwmon driver, so that it doesn't throw an exception, but rather posts a warning? If I find the time I can see if I can make a pull request...

For the case of the missing pwm*_enable... Well... I'm not so happy about ignoring things that could indicate an error, but in the official hwmon docs I read:

All entries (except name) are optional, and should only be created in a given driver if the chip has the feature.

So yes, that might actually be the best way to address this issue (Although the documentation is quite ambiguous as to what "the feature" actually refers to in the case of pwm*_enable).

[eldorel]

Does it make sense for you to avoid the hwmon driver altogether and build the lm-sensors branch of thinkfan? which should allow you to use the new format to specify your sensors?

lm-sensors seems to only support sensor inputs and has it's own issues.

Unless I'm missing a way to set fans via a different driver than Hwmon?

Oh right, sorry, you probably do. The config syntax for lm-sensors isn't properly documented, yet. Take a look at this comment: #156 (comment)

So basically you want to run sensors and use the names you see there. So let's say we have this output:

# sensors
drivetemp-scsi-3-0
Adapter: SCSI adapter
temp1:        +26.0°C  (low  = +14.0°C, high = +55.0°C)
                       (crit low = +10.0°C, crit = +60.0°C)
                       (lowest = +22.0°C, highest = +26.0°C)

Then drivetemp-scsi-3-0 would be the chip, and temp1 would be an id, i.e.:

sensors:
  - chip: drivetemp-scsi-3-0
    ids: [ temp1 ]

Just to add a quick note:
The 'chip:' information shown in the sensors output includes port information that can change for some devices depending on module order or currently connected hardware.

For example: My Corsair comander pro alternates between being identified as "corsaircpro-hid-3-2" and "corsaircpro-hid-3-13" in lm_sensors.

Since that device has 6 fan headers missing the "pwmN_enable" function and 4 temperature probes, I would need the _enable bit to be optional and the 'name' functionality of the hwmon device declarations.

So, using LM_sensors throws the same error for the missing file and fails if I reboot three times.