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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f2bd5bd9-d664-61af-86c9-8d3926c53b55@roeck-us.net>
Date:   Wed, 10 Jun 2020 06:13:21 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Marius Zachmann <mail@...iuszachmann.de>
Cc:     jdelvare@...e.com, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [RFC] drivers/hwmon: Corsair Commander Pro driver

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