Merge pull request #87 from xaizek/add-pwm-enable

Add `pwm[1-8]_enable`

README.md

@@ -135,6 +135,7 @@ This module was tested on Ubuntu 20.04 with all kernel available on motherboard
 
 - Add support for MSI B460M Bazooka having NCT6687 with another device ID
 - Add support to use generic voltage input without multiplier, allows sensors custom conf
+- Support giving fan control back to the firmware
 <br>
 
 ## VOLTAGE MANUAL CONFIGURATION
@@ -185,10 +186,93 @@ chip "nct6687-*"
     compute in4       (@ * 2), (@ / 2)
 ```
 
+## MODULE PARAMETERS
+
+- **force** (bool) (default: false)
+  Set to enable support for unknown vendors.
+
+- **manual** (bool) (default: false)
+  Set voltage input and voltage label configured with external sensors file.
+  You can use custom labels and ignore inputs without setting this option if
+  you can figure out their names (see which `*_label` contains builtin label).
+
+## CONFIGURATION VIA SYSFS
+
+In order to be able to use this interface you need to know the path as which
+it's published. The path isn't fixed and depends on the order in which chips are
+registered by the kernel. One way to find it is by device class (`hwmon`) via a
+simple command like this:
+```
+for d in /sys/class/hwmon/*; do echo "$d: $(cat "$d/name")"; done | grep nct6687
+```
+
+Possible output:
+```
+/sys/class/hwmon/hwmon5: nct6687
+```
+
+This means that your base path for examples below is `/sys/class/hwmon/hwmon5`
+(note that adding/removing hardware can change the path, drop `grep` from the
+command above to see all sensors and their relative ordering).
+
+Another way to look it up is by a device (class path actually just points to
+device path) like in:
+
+`cd /sys/devices/platform/nct6687.*/hwmon/hwmon*`
+
+The first asterisk will be expanded to an address (`2592` which is `0xa20` that
+you can see in `sensors` output) and the second one to a number like `5` from
+above.
+
+### `pwm[1-8]`
+
+Gets/sets PWM duty cycle or DC value that defines fan speed.  Which unit is used
+depends on what was configured by firmware.
+
+Accepted values: `0`-`255` (slowest to full speed).
+
+Writing to this file changes fan control to manual mode.
+
+Example:
+
+```
+# slow down a fan as much as possible (will stop it if the fan supports zero RPM mode)
+echo 0 > pwm6
+# fix a fan at around half its speed (actual behaviour depends on the fan)
+echo 128 > pwm6
+# full speed
+echo 255 > pwm6
+```
+
+### `pwm[1-8]_enable`
+
+Gets/sets controls mode of fan/temperature control.
+
+Accepted values:
+ * `1` - manual speed management through `pwm[1-8]`
+ * `99` - whatever automatic mode was configured by firmware
+          (this is a deliberately weird value to be dropped after adding more
+           modes)
+
+Example:
+
+```
+# fix a fan at current speed (`echo pwm6` will be constant from now on)
+echo 1 > pwm6_enable
+# switch back to automatic control set up by firmware (`echo pwm6` is again dynamic after this)
+echo 99 > pwm6_enable
+# switch to ~25% of max speed
+echo 64 > pwm6
+# automatic
+echo 99 > pwm6_enable
+# back to ~25% (it seems to be remembered)
+echo 1 > pwm6_enable
+```
+
 ## VERIFIED
 **1. Fan speed control**
 
-- Changing fan speed was tested succesfuly by users, see reported issue.
+- Changing fan speed was tested successfully by users, see in [reported issues](https://github.com/Fred78290/nct6687d/issues).
 
 ## Issues
 ### ACPI

nct6687.c

@@ -47,6 +47,13 @@ enum kinds
 	nct6687
 };
 
+enum pwm_mode
+{
+	manual_mode = 1,
+	// There are multiple automatic modes, none of which is configurable by this module yet.
+	firmware_mode = 99,
+};
+
 static bool force;
 static bool manual;
 
@@ -337,6 +344,7 @@ struct nct6687_data
 	bool _restoreDefaultFanControlRequired[NCT6687_NUM_REG_FAN];
 
 	u8 pwm[NCT6687_NUM_REG_PWM];
+	enum pwm_mode pwm_mode[NCT6687_NUM_REG_PWM];
 
 	/* Remember extra register values over suspend/resume */
 	u8 hwm_cfg;
@@ -573,6 +581,16 @@ static void nct6687_update_voltage(struct nct6687_data *data)
 	pr_debug("nct6687_update_voltage\n");
 }
 
+static enum pwm_mode nct6687_get_pwm_mode(struct nct6687_data *data, int index)
+{
+	u16 bitMask = 0x01 << index;
+	if (nct6687_read(data, NCT6687_REG_FAN_CTRL_MODE(index)) & bitMask)
+	{
+		return manual_mode;
+	}
+	return firmware_mode;
+}
+
 static void nct6687_update_fans(struct nct6687_data *data)
 {
 	int i;
@@ -591,6 +609,7 @@ static void nct6687_update_fans(struct nct6687_data *data)
 	for (i = 0; i < NCT6687_NUM_REG_PWM; i++)
 	{
 		data->pwm[i] = nct6687_read(data, NCT6687_REG_PWM(i));
+		data->pwm_mode[i] = nct6687_get_pwm_mode(data, i);
 
 		pr_debug("nct6687_update_fans[%d], pwm=%d", i, data->pwm[i]);
 	}
@@ -797,7 +816,53 @@ static ssize_t store_pwm(struct device *dev, struct device_attribute *attr, cons
 	return count;
 }
 
+static ssize_t show_pwm_mode(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct nct6687_data *data = nct6687_update_device(dev);
+	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+
+	return sprintf(buf, "%d\n", data->pwm_mode[sattr->nr]);
+}
+
+static ssize_t store_pwm_mode(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+	struct nct6687_data *data = dev_get_drvdata(dev);
+	int index = sattr->nr;
+	unsigned long val;
+	u16 mode;
+	u8 bitMask;
+
+	if (index >= NCT6687_NUM_REG_FAN || kstrtoul(buf, 10, &val))
+		return -EINVAL;
+	if (val != manual_mode && val != firmware_mode)
+		return -EINVAL;
+
+	mutex_lock(&data->update_lock);
+
+	nct6687_save_fan_control(data, index);
+
+	mode = nct6687_read(data, NCT6687_REG_FAN_CTRL_MODE(index));
+
+	bitMask = (u8)(0x01 << index);
+	if (val == manual_mode)
+	{
+		mode = (u8)(mode | bitMask);
+	}
+	else if (val == firmware_mode)
+	{
+		mode = (u8)(mode & ~bitMask);
+	}
+
+	nct6687_write(data, NCT6687_REG_FAN_CTRL_MODE(index), mode);
+
+	mutex_unlock(&data->update_lock);
+
+	return count;
+}
+
 SENSOR_TEMPLATE(pwm, "pwm%d", S_IRUGO, show_pwm, store_pwm, 0);
+SENSOR_TEMPLATE_2(pwm_mode, "pwm%d_enable", S_IRUGO, show_pwm_mode, store_pwm_mode, 0, 0);
 
 static void nct6687_save_fan_control(struct nct6687_data *data, int index)
 {
@@ -819,7 +884,8 @@ static void nct6687_restore_fan_control(struct nct6687_data *data, int index)
 	if (data->_restoreDefaultFanControlRequired[index])
 	{
 		u8 mode = nct6687_read(data, NCT6687_REG_FAN_CTRL_MODE(index));
-		mode = (u8)(mode & ~data->_initialFanControlMode[index]);
+		u8 bitMask = 0x01 << index;
+		mode = (u8)((mode & ~bitMask) | data->_initialFanControlMode[index]);
 
 		nct6687_write(data, NCT6687_REG_FAN_CTRL_MODE(index), mode);
 
@@ -842,6 +908,7 @@ static umode_t nct6687_pwm_is_visible(struct kobject *kobj, struct attribute *at
 
 static struct sensor_device_template *nct6687_attributes_pwm_template[] = {
 	&sensor_dev_template_pwm,
+	&sensor_dev_template_pwm_mode,
 	NULL,
 };
 
@@ -945,8 +1012,14 @@ static void nct6687_setup_pwm(struct nct6687_data *data)
 	{
 		data->_initialFanPwmCommand[i] = nct6687_read(data, NCT6687_REG_FAN_PWM_COMMAND(i));
 		data->pwm[i] = nct6687_read(data, NCT6687_REG_PWM(i));
+		data->pwm_mode[i] = nct6687_get_pwm_mode(data, i);
 
-		pr_debug("nct6687_setup_pwm[%d], addr=%04X, pwm=%d, _initialFanPwmCommand=%d\n", i, NCT6687_REG_FAN_PWM_COMMAND(i), data->pwm[i], data->_initialFanPwmCommand[i]);
+		pr_debug("nct6687_setup_pwm[%d], addr=%04X, pwm=%d, pwm_mode=%d, _initialFanPwmCommand=%d\n",
+		         i,
+		         NCT6687_REG_FAN_PWM_COMMAND(i),
+		         data->pwm[i],
+		         data->pwm_mode[i],
+		         data->_initialFanPwmCommand[i]);
 	}
 }