Guenter (and others on this list), I am getting ready to submit a driver for NZXT Grid+ V3 and Smart Device (V1) fan controllers, but I am having trouble deciding how to expose their PWM control due to some device limitations. Before getting into those, let me first give some very basic context... These devices are USB HIDs, and asynchronously send "status" reports every 200 ms to communicate speed, current, voltage and control mode for their channels (one channel per report). Fans can be controlled by sending HID output reports to the device, and both DC and PWM modes are supported. The device features a special initialization routine (that must be requested during probe) which automatically detects the appropriate control mode for each channel. Back to the device limitations... The first is that PWM values can be set, but not read back. And neither hwmon[1] nor lm-sensors' pwmconfig/fancontrol expect pmw* attributes to be WO. One solution is to have the driver track the PWM values that are set through it and return those, but is this acceptable? The other starts with PWM control being disabled for channels that the device identifies as unconnected. This is not in itself a problem, but the initialization routine (where the detection happens) is asynchronous, takes somewhere around 5 seconds, and we do not have any way of directly querying its result. We only know the control mode of each channel (be it DC, PWM or disabled) from the regular status reports. These limitations make it complicated to simply use is_visible() to hide pwm attributes of unconnected channels. We would need to register with the hwmon subsystem only after getting enough post-initialization status reports for all channels, and this would essentially mean to sleep for 6+ seconds. We would also need to unregister and re-register when going through a suspend-reset_resume cycle, because the device may have its state wiped, requiring reinitialization.[2] A different approach to handle this, which I have preferred _so far,_ is to use pwm*_enable = 0 to report the unconnected channels to user-space, while keeping the other pwm attributes visible. But this comes with other problems. First, lm-sensors' pwmconfig expects to be able to write to a pwm*_enable attribute if it exists, but the device does not support that operation. The hwmon documentation states that RW values may be RO, but pwmconfig is out there and in use. So far I simply return 0 to attempts at those writes, silently ignoring them; functional, but certainly a hack. Second, if PWM control is disabled for a channel, but its pwm* and pwm*_mode attributes are still visible, what should we return when user-space attempts to write to them? The practical answer may simply be to return -EOPNOTSUPP, but this makes me wonder if the whole approach (of handling these cases with pwm*_enable instead of is_visible()) is not doomed. A final minor problem is that channels detected as unconnected run at 40% PWM, but the documentation for pwm*_enable == 0 is a bit too specific: "no fan speed control (i.e. fan at *full* speed)" (emphasis mine). Do you have any suggestions and/or recommendations? If it helps, a pre-RFC (but functional and mostly clean) version of the driver can be found at: https://github.com/jonasmalacofilho/linux/blob/p-hwmon-add-nzxt-smartdevice-gridplus3/drivers/hwmon/nzxt-smartdevice.c Thanks, Jonas [1] According to Documentation/hwmon/sysfs-interface.rst. [2] The device also does not respond to HID Get_Report, so it is not trivial to check whether it really needs to be reinitialized, since the only symptom of that being necessary is the absence of the asynchronous status reports.
