On středa 26. května 2021 17:40:19 CEST, Guenter Roeck wrote:
Since pwmX_enable is now fixed and only handles pwm support instead
of also enabling/disabling fan tachometers, we need an explicit means
to do that.
For fan channels 7..12, display the enable status if the channel
is configured for fan speed reporting. The displayed status matches
the value of the companion channel but is read-only.
This phrasing is confusing to me. That's once again that "configured to"
which in this context doesn't refer to the kernel, but to an initial config
of the chip. I suggest the following:
Fan channels 7..12 are only available when the chip has been configured
with PWM output N-6 disabled. This configuration is outside of scope of the
kernel. The displayed status matches the value of the companion channel but
is read-only.
+fan[1-12]_enable RW 0=disable fan speed monitoring,
1=enable fan speed monitoring
+ The value is RO for companion channels
(7-12). For those
+ channels, the value matches the value
of the primary channel.
I realize that it probably doesn't belong to this patch because it affects
the other fan_* files, but the docs would be improved by something like:
Tachometer inputs monitor fan tachometer logic outputs for precise (+/-1%)
monitoring and control of fan RPM as well as detection of fan failure.
Six pins are dedicated tachometer inputs. Any of the six PWM outputs can
-also be configured to serve as tachometer inputs.
+also be reconfigured to serve as tachometer inputs by the firmware. The
+kernel will respect the initial configuration of the chip.
Want an extra patch on top of this series?
+ case hwmon_fan_enable:
+ config = data->fan_config[channel];
+ if (val == 0) {
+ /* Disabling TACH_INPUT_EN has no effect in RPM_MODE */
+ if (!(config & MAX31790_FAN_CFG_RPM_MODE))
+ config &= ~MAX31790_FAN_CFG_TACH_INPUT_EN;
This means that a "nonsensical" write from userspace will be silently
ignored, doesn't it? I think it should return an error instead.
Cheers,
Jan
