Re: [RFC] drivers/hwmon: Corsair Commander Pro driver

[Guenter Roeck]

On 6/10/20 5:03 AM, Marius Zachmann wrote:
[ ... ]
>>> +Kernel driver corsair-cpro
>>> +==========================
>>> +
>>> +Supported devices:
>>> +
>>> +  * Corsair Commander Pro
>>> +  * Corsair Commander Pro (1000D)
>>> +
>>> +Author: Marius Zachmann
>>> +
>>> +Description
>>> +-----------
>>> +
>>> +This driver implements the sysfs interface for the Corsair Commander Pro.
>>> +The Corsair Commander Pro is a USB device with 6 fan connectors,
>>> +4 temperature sensor connectors and 2 Corsair LED connectors.
>>> +It can read the voltage levels on the SATA power connector.
>>> +
>>> +Usage Notes
>>> +-----------
>>> +
>>> +Since it is a USB device, hotswapping is possible. The device is autodetected.
>>> +
>>> +Sysfs entries
>>> +-------------
>>> +
>>> +in0_input		Voltage on SATA 12v
>>> +in1_input		Voltage on SATA 5v
>>> +in2_input		Voltage on SATA 3.3v
>>> +
>>> +temp[0-3]_input		Connected temperature sensors
>>> +
>> Index starts with 1 for everything except inX.
>>
>>> +fan[0-5]_input		Connected fan rpm.
>>> +fan[0-5]_label		Shows connection status of the fan as detected by the
>>> +			device.
>>> +			"fanX nc"   no connection
>>> +			"fanX 3pin" 3-pin fan detected
>>> +			"fanX 4pin" 4-pin fan detected
>>> +fan[0-5]_enable		the driver only reports fan speeds when 1
>>> +pwm[0-5]		Sets the fan speed. Values from 0-255.
>>> +			When reading, it reports the last value, which
>>> +			was set by the driver.

Change to:
			When reading, it reports the last value if it was set by the driver.
			Otherwise returns 0.

[ ... ]

>>> +	case hwmon_pwm:
>>> +		switch (attr) {
>>> +		case hwmon_pwm_input:
>>> +			*val = ccp->pwm[channel];
>>
>> This returns 0 if pwm wasn't set. Is this indeed not readable from the
>> device ?
> 
> I could not find any possibility, because the official Corsair software does not use it.
> I am not sure, whether it would be better to return an error, if it was not set.
> 
Bummer. Then change the documentation as suggested above, and add
a comment here explaining that it is unknown how to read pwm values
from the device.

Thanks,
Guenter