Incompatible userspace API of pwm-fan driver

Incompatible userspace API of pwm-fan driver

[Heimpold, Michael]

[-- Attachment #1: Type: text/plain, Size: 2289 bytes --]

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-

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

-- 
chargebyte GmbH • Bitterfelder Str. 1-5 • D-04129 Leipzig
 
Michael Heimpold
Software Developer
 
www.chargebyte.com
 
Geschäftsführer: Thomas Wagner
Registergericht: Amtsgericht Leipzig, HRB 237 84
Umsatzsteuer-Identnummer: DE 811 528 334


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7829 bytes --]

Re: Incompatible userspace API of pwm-fan driver

[Guenter Roeck]

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
>