On 4/4/23 00:59, Heimpold, Michael wrote:
Hi all, our product “Tarragon” is an embedded Linux system, a charging station controller with the possibility to attach a fan. This fan can be controlled via PWM and the Linux driver we use for it is "pwmfan". On top in userspace, we use fancontrol and pwmconfig from lm-sensors. We recently switched over to a newer Linux kernel version and noticed that commit b99152d4f04b (“hwmon: (pwm-fan) Switch regulator dynamically”) introduced an “issue”: now the pwm1_enable file is present in the sysfs API but the way the driver implements this is not compatible with the expectations by fancontrol/pwmconfig: According to https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface: -snip- pwm[1-*]_enable Fan speed control method: 0: no fan speed control (i.e. fan at full speed) 1: manual fan speed control enabled (using pwm[1-*]) 2+: automatic fan speed control enabled Check individual chip documentation files for automatic mode details. -snap- This is also what pwmconfig and fancontrol expects. But the driver implementation does this: Quote from https://www.kernel.org/doc/Documentation/hwmon/pwm-fan.rst -snip- pwm1_enable rw keep enable mode, defines behaviour when pwm1=0 0 -> disable pwm and regulator 1 -> enable pwm; if pwm==0, disable pwm, keep regulator enabled 2 -> enable pwm; if pwm==0, keep pwm and regulator enabled 3 -> enable pwm; if pwm==0, disable pwm and regulator -snap-
Unfortunately that is a common problem with many drivers. The problem is that especially "no fan speed control (i.e. fan at full speed)" is really a bad API to start with, because it doesn't support a modern system where one may have something else in mind when saying "disable pwm". Maybe we need to revert the patch, or introduce mode 4 for "disable pwm and regulator" and use 0 for "set fan to full speed and keep regulator enabled". Uwe, any thoughts ? Guenter
As workaround, we have adapted the two mentioned scripts. The used approach is to check whether this special hwmon driver is used and ignore the pwm1_enable file in this moment. I posted this topic (combined with our patch) also to lm-sensors list yesterday, for reference see: https://marc.info/?l=lm-sensors&m=168051630023128&w=2 I’m/we are not sure, what the "correct" resolution here would be, so looking forward to your feedback. PS: I'm not subscribed to the list(s), please keep Cc-ing me, thanks. Best regards, Michael
