[2/2] hwmon: (nct7802) Fix non-working alarm on voltages

Commit Message

Gilles Buloz Nov. 29, 2019, 10:39 a.m. UTC

From: Gilles Buloz <[e-mail]>
Date: Fri, 29 Nov 2019 10:56:05 +0100
Subject: [PATCH 2/2] hwmon: (nct7802) Fix non-working alarm on voltages

No alarm is reported by /sys/.../inX_alarm

In detail:

The SMI Voltage status register is the only register giving a status
for voltages, but it does not work like the non-SMI status registers
used for temperatures and fans.
A bit is set for each input crossing a threshold, in both direction,
but the "inside" or "outside" limits info is not available.
Also this register is cleared on read.
Note : this is not explicitly spelled out in the datasheet, but from
experiment.
As a result if an input is crossing a threshold (min or max in any
direction), the alarm is reported only once even if the input is
still outside limits. Also if the alarm for another input is read
before the one of this input, no alarm is reported at all.

Signed-off-by: Gilles Buloz <[e-mail]>
---
 drivers/hwmon/nct7802.c | 71 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 67 insertions(+), 4 deletions(-)

Patch

diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
index 7915c2f2c85d..35fb352d5a52 100644
--- a/drivers/hwmon/nct7802.c
+++ b/drivers/hwmon/nct7802.c
@@ -58,6 +58,8 @@  static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
 struct nct7802_data {
 	struct regmap *regmap;
 	struct mutex access_lock; /* for multi-byte read and write operations */
+	u8 in_status;
+	struct mutex in_alarm_lock;
 };
 
 static ssize_t temp_type_show(struct device *dev,
@@ -368,6 +370,66 @@  static ssize_t in_store(struct device *dev, struct device_attribute *attr,
 	return err ? : count;
 }
 
+static ssize_t in_alarm_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+	struct nct7802_data *data = dev_get_drvdata(dev);
+	int volt, min, max, ret;
+	unsigned int val;
+
+	mutex_lock(&data->in_alarm_lock);
+
+	/*
+	 * The SMI Voltage status register is the only register giving a status
+	 * for voltages. A bit is set for each input crossing a threshold, in
+	 * both direction, but the "inside" or "outside" limits info is not
+	 * available. Also this register is cleared on read.
+	 * Note : this is not explicitly spelled out in the datasheet, but
+	 * from experiment.
+	 * To deal with this we use a status cache with one validity bit and
+	 * one status bit for each input. Validity is cleared at startup and
+	 * each time the register reports a change, and the status is processed
+	 * by software based on current input value and limits.
+	 */
+	ret = regmap_read(data->regmap, 0x1e, &val); /* SMI Voltage status */
+	if (ret < 0)
+		goto abort;
+
+	/* invalidate cached status for all inputs crossing a threshold */
+	data->in_status &= ~((val & 0x0f) << 4);
+
+	/* if cached status for requested input is invalid, update it */
+	if (!(data->in_status & (0x10 << sattr->index))) {
+		ret = nct7802_read_voltage(data, sattr->nr, 0);
+		if (ret < 0)
+			goto abort;
+		volt = ret;
+
+		ret = nct7802_read_voltage(data, sattr->nr, 1);
+		if (ret < 0)
+			goto abort;
+		min = ret;
+
+		ret = nct7802_read_voltage(data, sattr->nr, 2);
+		if (ret < 0)
+			goto abort;
+		max = ret;
+
+		if (volt < min || volt > max)
+			data->in_status |= (1 << sattr->index);
+		else
+			data->in_status &= ~(1 << sattr->index);
+
+		data->in_status |= 0x10 << sattr->index;
+	}
+
+	ret = sprintf(buf, "%u\n", !!(data->in_status & (1 << sattr->index)));
+abort:
+	mutex_unlock(&data->in_alarm_lock);
+	return ret;
+}
+
 static ssize_t temp_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
@@ -660,7 +722,7 @@  static const struct attribute_group nct7802_temp_group = {
 static SENSOR_DEVICE_ATTR_2_RO(in0_input, in, 0, 0);
 static SENSOR_DEVICE_ATTR_2_RW(in0_min, in, 0, 1);
 static SENSOR_DEVICE_ATTR_2_RW(in0_max, in, 0, 2);
-static SENSOR_DEVICE_ATTR_2_RO(in0_alarm, alarm, 0x1e, 3);
+static SENSOR_DEVICE_ATTR_2_RO(in0_alarm, in_alarm, 0, 3);
 static SENSOR_DEVICE_ATTR_2_RW(in0_beep, beep, 0x5a, 3);
 
 static SENSOR_DEVICE_ATTR_2_RO(in1_input, in, 1, 0);
@@ -668,19 +730,19 @@  static SENSOR_DEVICE_ATTR_2_RO(in1_input, in, 1, 0);
 static SENSOR_DEVICE_ATTR_2_RO(in2_input, in, 2, 0);
 static SENSOR_DEVICE_ATTR_2_RW(in2_min, in, 2, 1);
 static SENSOR_DEVICE_ATTR_2_RW(in2_max, in, 2, 2);
-static SENSOR_DEVICE_ATTR_2_RO(in2_alarm, alarm, 0x1e, 0);
+static SENSOR_DEVICE_ATTR_2_RO(in2_alarm, in_alarm, 2, 0);
 static SENSOR_DEVICE_ATTR_2_RW(in2_beep, beep, 0x5a, 0);
 
 static SENSOR_DEVICE_ATTR_2_RO(in3_input, in, 3, 0);
 static SENSOR_DEVICE_ATTR_2_RW(in3_min, in, 3, 1);
 static SENSOR_DEVICE_ATTR_2_RW(in3_max, in, 3, 2);
-static SENSOR_DEVICE_ATTR_2_RO(in3_alarm, alarm, 0x1e, 1);
+static SENSOR_DEVICE_ATTR_2_RO(in3_alarm, in_alarm, 3, 1);
 static SENSOR_DEVICE_ATTR_2_RW(in3_beep, beep, 0x5a, 1);
 
 static SENSOR_DEVICE_ATTR_2_RO(in4_input, in, 4, 0);
 static SENSOR_DEVICE_ATTR_2_RW(in4_min, in, 4, 1);
 static SENSOR_DEVICE_ATTR_2_RW(in4_max, in, 4, 2);
-static SENSOR_DEVICE_ATTR_2_RO(in4_alarm, alarm, 0x1e, 2);
+static SENSOR_DEVICE_ATTR_2_RO(in4_alarm, in_alarm, 4, 2);
 static SENSOR_DEVICE_ATTR_2_RW(in4_beep, beep, 0x5a, 2);
 
 static struct attribute *nct7802_in_attrs[] = {
@@ -1011,6 +1073,7 @@  static int nct7802_probe(struct i2c_client *client,
 		return PTR_ERR(data->regmap);
 
 	mutex_init(&data->access_lock);
+	mutex_init(&data->in_alarm_lock);
 
 	ret = nct7802_init_chip(data);
 	if (ret < 0)