[PATCH 1/5] hwmon: (nct6683) Retry pwm writes until they take effect

[Johan Dahlin]

store_pwm() waits a fixed 1-2 ms for the EC to hand over the fan
configuration registers before writing. That delay dates back to
commit 91918d13eb17 ("hwmon: (nct6683) Add basic support for NCT6683 on
Mitac boards") and, pwm being read-only elsewhere, has never been
exercised against other firmware.

It is too short for at least one of them: on an ASRock B850 Steel Legend
WiFi (NCT6686D), two of six consecutive pwm writes were silently discarded,
leaving the fan on its previous setting.

Read the value back and repeat the sequence when it did not stick, waiting
longer each time and reporting -EIO if it never does. Raising the delay for
everyone would penalise boards that work today, while a write that succeeds
first time issues the same register sequence as before.

Signed-off-by: Johan Dahlin <jdahlin@xxxxxxxxx>
---
 drivers/hwmon/nct6683.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/nct6683.c b/drivers/hwmon/nct6683.c
index e1c36c95affb..b524a45b1471 100644
--- a/drivers/hwmon/nct6683.c
+++ b/drivers/hwmon/nct6683.c
@@ -934,18 +934,29 @@ store_pwm(struct device *dev, struct device_attribute *attr, const char *buf,
 	struct nct6683_data *data = dev_get_drvdata(dev);
 	int index = sattr->index;
 	unsigned long val;
+	int tries;
 
 	if (kstrtoul(buf, 10, &val) || val > 255)
 		return -EINVAL;
 
+	/*
+	 * The EC does not always release the fan configuration registers
+	 * within the settling delay, and a write issued before it does is
+	 * silently discarded. Repeat the sequence until the value sticks.
+	 */
 	mutex_lock(&data->update_lock);
-	nct6683_write(data, NCT6683_REG_FAN_CFG_CTRL, NCT6683_FAN_CFG_REQ);
-	usleep_range(1000, 2000);
-	nct6683_write(data, NCT6683_REG_PWM_WRITE(index), val);
-	nct6683_write(data, NCT6683_REG_FAN_CFG_CTRL, NCT6683_FAN_CFG_DONE);
+	for (tries = 0; tries < 3; tries++) {
+		nct6683_write(data, NCT6683_REG_FAN_CFG_CTRL, NCT6683_FAN_CFG_REQ);
+		/* A failed attempt means the EC needed longer than we waited */
+		usleep_range(1000 * (tries + 1), 2000 * (tries + 1));
+		nct6683_write(data, NCT6683_REG_PWM_WRITE(index), val);
+		nct6683_write(data, NCT6683_REG_FAN_CFG_CTRL, NCT6683_FAN_CFG_DONE);
+		if (nct6683_read(data, NCT6683_REG_PWM_WRITE(index)) == val)
+			break;
+	}
 	mutex_unlock(&data->update_lock);
 
-	return count;
+	return tries == 3 ? -EIO : count;
 }
 
 SENSOR_TEMPLATE(pwm, "pwm%d", S_IRUGO, show_pwm, store_pwm, 0);
-- 
2.53.0