All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver
@ 2022-05-25 13:56 Guenter Roeck
  2022-05-25 13:56 ` [PATCH 01/40] hwmon: (lm90) Generate sysfs and udev events for all alarms Guenter Roeck
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Guenter Roeck @ 2022-05-25 13:56 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, Jean Delvare, Slawomir Stepien, Guenter Roeck

This patch series implements various improvements to the lm90 driver.

- Generate sysfs and udev events for all alarms
- Add support for LM84, ADM1020, ADM1021, ADT7421, ADT7481, ADT7482,
  ADT7483A, NCT210, NCT214, NCT218, NCT72, MAX1617, MAX1617A, MAX6642,
  NE1617, NE1617A, NE1618, GL523SM, THMC10, and MC1066
- Explicit support for MAX6648
- Handle differences between ADT7461 and ADT7461A
- Individual drivers for ADM1021 and MAX6642 are now obsolete.
- Improved PEC support. Some chips support PEC only in read operations,
  others support PEC for both read and write operations.
- Improved alarm handling
- Streamlined temperature conversion functions 
- Reorganized and improved chip detection code

----------------------------------------------------------------
Guenter Roeck (40):
      hwmon: (lm90) Generate sysfs and udev events for all alarms
      hwmon: (lm90) Rework alarm/status handling
      hwmon: (lm90) Reorder include files in alphabetical order
      hwmon: (lm90) Reorder chip enumeration to be in alphabetical order
      hwmon: (lm90) Use BIT macro
      hwmon: (lm90) Move status register bit shifts to compile time
      hwmon: (lm90) Stop using R_/W_ register prefix
      hwmon: (lm90) Improve PEC support
      hwmon: (lm90) Add partial PEC support for ADT7461
      hwmon: (lm90) Enable full PEC support for ADT7461A
      hwmon: (lm90) Add support for unsigned and signed temperatures
      hwmon: (lm90) Only re-read registers if volatile
      hwmon: (lm90) Support multiple temperature resolutions
      hwmon: (lm90) Use single flag to indicate extended temperature support
      hwmon: (lm90) Rework detect function
      hwmon: (lm90) Add support for additional chip revision of NCT1008
      hwmon: (lm90) Fix/Add detection of G781-1
      hwmon: (lm90) Add flag to indicate 'alarms' attribute support
      hwmon: (lm90) Add explicit support for MAX6648/MAX6692
      hwmon: (lm90) Add support for ADT7481, ADT7482, and ADT7483
      hwmon: (lm90) Strengthen chip detection for ADM1032, ADT7461(A), and NCT1008
      hwmon: (lm90) Add support for MAX6690
      hwmon: (lm90) Add flag to indicate support for minimum temperature limits
      hwmon: (lm90) Add flag to indicate conversion rate support
      hwmon: (lm90) Add support for MAX6642
      hwmon: (lm90) Let lm90_read16() handle 8-bit read operations
      hwmon: (lm90) Introduce 16-bit register write function
      hwmon: (lm90) Support MAX1617 and LM84
      hwmon: (lm90) Add support for ADM1021, ADM1021A, and ADM1023
      hwmon: (lm90) Add remaining chips supported by adm1021 driver
      hwmon: (lm90) Combine lm86 and lm90 configuration
      hwmon: (lm90) Add explicit support for NCT210
      hwmon: (lm90) Add support for ON Semiconductor NCT214 and NCT72
      hwmon: (lm90) Add support for ON Semiconductor NCT218
      hwmon: (lm90) Add support for ADT7421
      hwmon: (lm90) Only disable alerts if not already disabled
      hwmon: (lm90) Add explicit support for ADM1020
      hwmon: (lm90) Add support and detection of Philips/NXP NE1618
      hwmon: (lm90) Add table with supported Analog/ONSEMI devices
      hwmon: (lm90) Support temp_samples attribute

 Documentation/hwmon/lm90.rst |  233 +++-
 drivers/hwmon/Kconfig        |   17 +-
 drivers/hwmon/lm90.c         | 2415 ++++++++++++++++++++++++++++--------------
 3 files changed, 1832 insertions(+), 833 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 01/40] hwmon: (lm90) Generate sysfs and udev events for all alarms
  2022-05-25 13:56 [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver Guenter Roeck
@ 2022-05-25 13:56 ` Guenter Roeck
  2022-05-25 13:56 ` [PATCH 02/40] hwmon: (lm90) Rework alarm/status handling Guenter Roeck
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2022-05-25 13:56 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, Jean Delvare, Slawomir Stepien, Guenter Roeck

So far the driver only generated sysfs and udev events for minimum and
maximum alarms. Also generate events for critical and emergency alarms.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 3820f0e61510..83d027c134be 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -1829,6 +1829,26 @@ static bool lm90_is_tripped(struct i2c_client *client, u16 *status)
 		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
 				   hwmon_temp_max_alarm, 2);
 
+	if (st & LM90_STATUS_LTHRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp,
+				   hwmon_temp_crit_alarm, 0);
+	if (st & LM90_STATUS_RTHRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp,
+				   hwmon_temp_crit_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2THRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp,
+				   hwmon_temp_crit_alarm, 2);
+
+	if (st2 & MAX6696_STATUS2_LOT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp,
+				   hwmon_temp_emergency_alarm, 0);
+	if (st2 & MAX6696_STATUS2_ROT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp,
+				   hwmon_temp_emergency_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2OT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp,
+				   hwmon_temp_emergency_alarm, 2);
+
 	return true;
 }
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 02/40] hwmon: (lm90) Rework alarm/status handling
  2022-05-25 13:56 [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver Guenter Roeck
  2022-05-25 13:56 ` [PATCH 01/40] hwmon: (lm90) Generate sysfs and udev events for all alarms Guenter Roeck
@ 2022-05-25 13:56 ` Guenter Roeck
  2022-05-25 13:56 ` [PATCH 03/40] hwmon: (lm90) Reorder include files in alphabetical order Guenter Roeck
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2022-05-25 13:56 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, Jean Delvare, Slawomir Stepien, Guenter Roeck

Many chips supported by this driver clear status registers after it
is read and update it in the next measurement cycle. Normally this falls
under the radar because all registers are only read once per measurement
cycle. However, there is an exception: Status registers are always read
during interrupt and laert handling. This can result in invalid status
reports if userspace reads an alarm attribute immediately afterwards.

Rework alarm/status handling by keeping a shadow register with 'current'
alarms, and by ensuring that the register is either only updated once per
measurement cycle or not cleared.

A second problem is related to alert handling: Alert handling is disabled
for chips with broken alert after an alert was reported, but only
re-enabled if attributes are read by the user. This means that alert
conditions may appear and disappear unnoticed. Remedy the situation
by introducing a worker to periodically read the status register(s) while
alert handling is disabled, and re-enable alerts after the alert condition
clears.

Yet another problem is that sysfs and udev events are currently only
reported to userspace if an alarm is raised, but not if an alarm condition
clears. Use the new worker to detect that situation and also generate
sysfs and udev events in that case.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c | 314 +++++++++++++++++++++++++++----------------
 1 file changed, 199 insertions(+), 115 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 83d027c134be..63ada2d0d839 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -92,6 +92,7 @@
 #include <linux/sysfs.h>
 #include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
+#include <linux/workqueue.h>
 
 /*
  * Addresses to scan
@@ -499,8 +500,11 @@ struct lm90_data {
 	const struct hwmon_channel_info *info[3];
 	struct hwmon_chip_info chip;
 	struct mutex update_lock;
+	struct delayed_work alert_work;
 	bool valid;		/* true if register values are valid */
+	bool alarms_valid;	/* true if status register values are valid */
 	unsigned long last_updated; /* in jiffies */
+	unsigned long alarms_updated; /* in jiffies */
 	int kind;
 	u32 flags;
 
@@ -518,7 +522,9 @@ struct lm90_data {
 	s8 temp8[TEMP8_REG_NUM];
 	s16 temp11[TEMP11_REG_NUM];
 	u8 temp_hyst;
-	u16 alarms; /* bitvector (upper 8 bits for max6695/96) */
+	u16 reported_alarms;	/* alarms reported as sysfs/udev events */
+	u16 current_alarms;	/* current alarms, reported by chip */
+	u16 alarms;		/* alarms not yet reported to user */
 };
 
 /*
@@ -771,6 +777,158 @@ static int lm90_update_limits(struct device *dev)
 	return 0;
 }
 
+static void lm90_report_alarms(struct device *dev, struct lm90_data *data)
+{
+	u16 cleared_alarms = data->reported_alarms & ~data->current_alarms;
+	u16 new_alarms = data->current_alarms & ~data->reported_alarms;
+	struct device *hwmon_dev = data->hwmon_dev;
+	int st, st2;
+
+	if (!cleared_alarms && !new_alarms)
+		return;
+
+	st = new_alarms & 0xff;
+	st2 = new_alarms >> 8;
+
+	if ((st & (LM90_STATUS_LLOW | LM90_STATUS_LHIGH | LM90_STATUS_LTHRM)) ||
+	    (st2 & MAX6696_STATUS2_LOT2))
+		dev_dbg(dev, "temp%d out of range, please check!\n", 1);
+	if ((st & (LM90_STATUS_RLOW | LM90_STATUS_RHIGH | LM90_STATUS_RTHRM)) ||
+	    (st2 & MAX6696_STATUS2_ROT2))
+		dev_dbg(dev, "temp%d out of range, please check!\n", 2);
+	if (st & LM90_STATUS_ROPEN)
+		dev_dbg(dev, "temp%d diode open, please check!\n", 2);
+	if (st2 & (MAX6696_STATUS2_R2LOW | MAX6696_STATUS2_R2HIGH |
+		   MAX6696_STATUS2_R2THRM | MAX6696_STATUS2_R2OT2))
+		dev_dbg(dev, "temp%d out of range, please check!\n", 3);
+	if (st2 & MAX6696_STATUS2_R2OPEN)
+		dev_dbg(dev, "temp%d diode open, please check!\n", 3);
+
+	st |= cleared_alarms & 0xff;
+	st2 |= cleared_alarms >> 8;
+
+	if (st & LM90_STATUS_LLOW)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_min_alarm, 0);
+	if (st & LM90_STATUS_RLOW)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_min_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2LOW)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_min_alarm, 2);
+
+	if (st & LM90_STATUS_LHIGH)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_max_alarm, 0);
+	if (st & LM90_STATUS_RHIGH)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_max_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2HIGH)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_max_alarm, 2);
+
+	if (st & LM90_STATUS_LTHRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_crit_alarm, 0);
+	if (st & LM90_STATUS_RTHRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_crit_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2THRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_crit_alarm, 2);
+
+	if (st2 & MAX6696_STATUS2_LOT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_emergency_alarm, 0);
+	if (st2 & MAX6696_STATUS2_ROT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_emergency_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2OT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_emergency_alarm, 2);
+
+	data->reported_alarms = data->current_alarms;
+}
+
+static int lm90_update_alarms_locked(struct lm90_data *data, bool force)
+{
+	if (force || !data->alarms_valid ||
+	    time_after(jiffies, data->alarms_updated + msecs_to_jiffies(data->update_interval))) {
+		struct i2c_client *client = data->client;
+		bool check_enable;
+		u16 alarms;
+		int val;
+
+		data->alarms_valid = false;
+
+		val = lm90_read_reg(client, LM90_REG_R_STATUS);
+		if (val < 0)
+			return val;
+		alarms = val & ~LM90_STATUS_BUSY;
+
+		if (data->kind == max6696) {
+			val = lm90_read_reg(client, MAX6696_REG_R_STATUS2);
+			if (val < 0)
+				return val;
+			alarms |= val << 8;
+		}
+		/*
+		 * If the update is forced (called from interrupt or alert
+		 * handler) and alarm data is valid, the alarms may have been
+		 * updated after the last update interval, and the status
+		 * register may still be cleared. Only add additional alarms
+		 * in this case. Alarms will be cleared later if appropriate.
+		 */
+		if (force && data->alarms_valid)
+			data->current_alarms |= alarms;
+		else
+			data->current_alarms = alarms;
+		data->alarms |= alarms;
+
+		check_enable = (client->irq || !(data->config_orig & 0x80)) &&
+			(data->config & 0x80);
+
+		if (force || check_enable)
+			lm90_report_alarms(&client->dev, data);
+
+		/*
+		 * Re-enable ALERT# output if it was originally enabled, relevant
+		 * alarms are all clear, and alerts are currently disabled.
+		 * Otherwise (re)schedule worker if needed.
+		 */
+		if (check_enable) {
+			if (!(data->current_alarms & data->alert_alarms)) {
+				dev_dbg(&client->dev, "Re-enabling ALERT#\n");
+				lm90_update_confreg(data, data->config & ~0x80);
+				/*
+				 * We may have been called from the update handler.
+				 * If so, the worker, if scheduled, is no longer
+				 * needed. Cancel it. Don't synchronize because
+				 * it may already be running.
+				 */
+				cancel_delayed_work(&data->alert_work);
+			} else {
+				schedule_delayed_work(&data->alert_work,
+					max_t(int, HZ, msecs_to_jiffies(data->update_interval)));
+			}
+		}
+		data->alarms_updated = jiffies;
+		data->alarms_valid = true;
+	}
+	return 0;
+}
+
+static int lm90_update_alarms(struct lm90_data *data, bool force)
+{
+	int err;
+
+	mutex_lock(&data->update_lock);
+	err = lm90_update_alarms_locked(data, force);
+	mutex_unlock(&data->update_lock);
+
+	return err;
+}
+
+static void lm90_alert_work(struct work_struct *__work)
+{
+	struct delayed_work *delayed_work = container_of(__work, struct delayed_work, work);
+	struct lm90_data *data = container_of(delayed_work, struct lm90_data, alert_work);
+
+	/* Nothing to do if alerts are enabled */
+	if (!(data->config & 0x80))
+		return;
+
+	lm90_update_alarms(data, true);
+}
+
 static int lm90_update_device(struct device *dev)
 {
 	struct lm90_data *data = dev_get_drvdata(dev);
@@ -819,11 +977,6 @@ static int lm90_update_device(struct device *dev)
 			return val;
 		data->temp11[REMOTE_TEMP] = val;
 
-		val = lm90_read_reg(client, LM90_REG_R_STATUS);
-		if (val < 0)
-			return val;
-		data->alarms = val & ~LM90_STATUS_BUSY;
-
 		if (data->kind == max6696) {
 			val = lm90_select_remote_channel(data, 1);
 			if (val < 0)
@@ -838,24 +991,11 @@ static int lm90_update_device(struct device *dev)
 			data->temp11[REMOTE2_TEMP] = val;
 
 			lm90_select_remote_channel(data, 0);
-
-			val = lm90_read_reg(client, MAX6696_REG_R_STATUS2);
-			if (val < 0)
-				return val;
-			data->alarms |= val << 8;
 		}
 
-		/*
-		 * Re-enable ALERT# output if it was originally enabled and
-		 * relevant alarms are all clear
-		 */
-		if ((client->irq || !(data->config_orig & 0x80)) &&
-		    !(data->alarms & data->alert_alarms)) {
-			if (data->config & 0x80) {
-				dev_dbg(&client->dev, "Re-enabling ALERT#\n");
-				lm90_update_confreg(data, data->config & ~0x80);
-			}
-		}
+		val = lm90_update_alarms_locked(data, false);
+		if (val < 0)
+			return val;
 
 		data->last_updated = jiffies;
 		data->valid = true;
@@ -1212,7 +1352,7 @@ static const u8 lm90_fault_bits[3] = { 0, 2, 10 };
 static int lm90_temp_read(struct device *dev, u32 attr, int channel, long *val)
 {
 	struct lm90_data *data = dev_get_drvdata(dev);
-	int err;
+	int err, bit;
 
 	mutex_lock(&data->update_lock);
 	err = lm90_update_device(dev);
@@ -1225,22 +1365,33 @@ static int lm90_temp_read(struct device *dev, u32 attr, int channel, long *val)
 		*val = lm90_get_temp11(data, lm90_temp_index[channel]);
 		break;
 	case hwmon_temp_min_alarm:
-		*val = (data->alarms >> lm90_min_alarm_bits[channel]) & 1;
-		break;
 	case hwmon_temp_max_alarm:
-		*val = (data->alarms >> lm90_max_alarm_bits[channel]) & 1;
-		break;
 	case hwmon_temp_crit_alarm:
-		if (data->flags & LM90_HAVE_CRIT_ALRM_SWP)
-			*val = (data->alarms >> lm90_crit_alarm_bits_swapped[channel]) & 1;
-		else
-			*val = (data->alarms >> lm90_crit_alarm_bits[channel]) & 1;
-		break;
 	case hwmon_temp_emergency_alarm:
-		*val = (data->alarms >> lm90_emergency_alarm_bits[channel]) & 1;
-		break;
 	case hwmon_temp_fault:
-		*val = (data->alarms >> lm90_fault_bits[channel]) & 1;
+		switch (attr) {
+		case hwmon_temp_min_alarm:
+			bit = BIT(lm90_min_alarm_bits[channel]);
+			break;
+		case hwmon_temp_max_alarm:
+			bit = BIT(lm90_max_alarm_bits[channel]);
+			break;
+		case hwmon_temp_crit_alarm:
+			if (data->flags & LM90_HAVE_CRIT_ALRM_SWP)
+				bit = BIT(lm90_crit_alarm_bits_swapped[channel]);
+			else
+				bit = BIT(lm90_crit_alarm_bits[channel]);
+			break;
+		case hwmon_temp_emergency_alarm:
+			bit = BIT(lm90_emergency_alarm_bits[channel]);
+			break;
+		case hwmon_temp_fault:
+			bit = BIT(lm90_fault_bits[channel]);
+			break;
+		}
+		*val = !!(data->alarms & bit);
+		data->alarms &= ~bit;
+		data->alarms |= data->current_alarms;
 		break;
 	case hwmon_temp_min:
 		if (channel == 0)
@@ -1699,6 +1850,8 @@ static void lm90_restore_conf(void *_data)
 	struct lm90_data *data = _data;
 	struct i2c_client *client = data->client;
 
+	cancel_delayed_work_sync(&data->alert_work);
+
 	/* Restore initial configuration */
 	lm90_write_convrate(data, data->convrate_orig);
 	i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
@@ -1771,93 +1924,23 @@ static int lm90_init_client(struct i2c_client *client, struct lm90_data *data)
 	return devm_add_action_or_reset(&client->dev, lm90_restore_conf, data);
 }
 
-static bool lm90_is_tripped(struct i2c_client *client, u16 *status)
+static bool lm90_is_tripped(struct i2c_client *client)
 {
 	struct lm90_data *data = i2c_get_clientdata(client);
-	int st, st2 = 0;
-
-	st = lm90_read_reg(client, LM90_REG_R_STATUS);
-	if (st < 0)
-		return false;
-
-	if (data->kind == max6696) {
-		st2 = lm90_read_reg(client, MAX6696_REG_R_STATUS2);
-		if (st2 < 0)
-			return false;
-	}
-
-	*status = st | (st2 << 8);
+	int ret;
 
-	if ((st & 0x7f) == 0 && (st2 & 0xfe) == 0)
+	ret = lm90_update_alarms(data, true);
+	if (ret < 0)
 		return false;
 
-	if ((st & (LM90_STATUS_LLOW | LM90_STATUS_LHIGH | LM90_STATUS_LTHRM)) ||
-	    (st2 & MAX6696_STATUS2_LOT2))
-		dev_dbg(&client->dev,
-			"temp%d out of range, please check!\n", 1);
-	if ((st & (LM90_STATUS_RLOW | LM90_STATUS_RHIGH | LM90_STATUS_RTHRM)) ||
-	    (st2 & MAX6696_STATUS2_ROT2))
-		dev_dbg(&client->dev,
-			"temp%d out of range, please check!\n", 2);
-	if (st & LM90_STATUS_ROPEN)
-		dev_dbg(&client->dev,
-			"temp%d diode open, please check!\n", 2);
-	if (st2 & (MAX6696_STATUS2_R2LOW | MAX6696_STATUS2_R2HIGH |
-		   MAX6696_STATUS2_R2THRM | MAX6696_STATUS2_R2OT2))
-		dev_dbg(&client->dev,
-			"temp%d out of range, please check!\n", 3);
-	if (st2 & MAX6696_STATUS2_R2OPEN)
-		dev_dbg(&client->dev,
-			"temp%d diode open, please check!\n", 3);
-
-	if (st & LM90_STATUS_LLOW)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_min_alarm, 0);
-	if (st & LM90_STATUS_RLOW)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_min_alarm, 1);
-	if (st2 & MAX6696_STATUS2_R2LOW)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_min_alarm, 2);
-	if (st & LM90_STATUS_LHIGH)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_max_alarm, 0);
-	if (st & LM90_STATUS_RHIGH)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_max_alarm, 1);
-	if (st2 & MAX6696_STATUS2_R2HIGH)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_max_alarm, 2);
-
-	if (st & LM90_STATUS_LTHRM)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_crit_alarm, 0);
-	if (st & LM90_STATUS_RTHRM)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_crit_alarm, 1);
-	if (st2 & MAX6696_STATUS2_R2THRM)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_crit_alarm, 2);
-
-	if (st2 & MAX6696_STATUS2_LOT2)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_emergency_alarm, 0);
-	if (st2 & MAX6696_STATUS2_ROT2)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_emergency_alarm, 1);
-	if (st2 & MAX6696_STATUS2_R2OT2)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_emergency_alarm, 2);
-
-	return true;
+	return !!data->current_alarms;
 }
 
 static irqreturn_t lm90_irq_thread(int irq, void *dev_id)
 {
 	struct i2c_client *client = dev_id;
-	u16 status;
 
-	if (lm90_is_tripped(client, &status))
+	if (lm90_is_tripped(client))
 		return IRQ_HANDLED;
 	else
 		return IRQ_NONE;
@@ -1911,6 +1994,7 @@ static int lm90_probe(struct i2c_client *client)
 	data->client = client;
 	i2c_set_clientdata(client, data);
 	mutex_init(&data->update_lock);
+	INIT_DELAYED_WORK(&data->alert_work, lm90_alert_work);
 
 	/* Set the device type */
 	if (client->dev.of_node)
@@ -2027,12 +2111,10 @@ static int lm90_probe(struct i2c_client *client)
 static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type,
 		       unsigned int flag)
 {
-	u16 alarms;
-
 	if (type != I2C_PROTOCOL_SMBUS_ALERT)
 		return;
 
-	if (lm90_is_tripped(client, &alarms)) {
+	if (lm90_is_tripped(client)) {
 		/*
 		 * Disable ALERT# output, because these chips don't implement
 		 * SMBus alert correctly; they should only hold the alert line
@@ -2041,9 +2123,11 @@ static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type,
 		struct lm90_data *data = i2c_get_clientdata(client);
 
 		if ((data->flags & LM90_HAVE_BROKEN_ALERT) &&
-		    (alarms & data->alert_alarms)) {
+		    (data->current_alarms & data->alert_alarms)) {
 			dev_dbg(&client->dev, "Disabling ALERT#\n");
 			lm90_update_confreg(data, data->config | 0x80);
+			schedule_delayed_work(&data->alert_work,
+				max_t(int, HZ, msecs_to_jiffies(data->update_interval)));
 		}
 	} else {
 		dev_dbg(&client->dev, "Everything OK\n");
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 03/40] hwmon: (lm90) Reorder include files in alphabetical order
  2022-05-25 13:56 [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver Guenter Roeck
  2022-05-25 13:56 ` [PATCH 01/40] hwmon: (lm90) Generate sysfs and udev events for all alarms Guenter Roeck
  2022-05-25 13:56 ` [PATCH 02/40] hwmon: (lm90) Rework alarm/status handling Guenter Roeck
@ 2022-05-25 13:56 ` Guenter Roeck
  2022-05-25 13:56 ` [PATCH 04/40] hwmon: (lm90) Reorder chip enumeration to be " Guenter Roeck
  2022-05-25 13:56 ` [PATCH 05/40] hwmon: (lm90) Use BIT macro Guenter Roeck
  4 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2022-05-25 13:56 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, Jean Delvare, Slawomir Stepien, Guenter Roeck

Reorder include files in alphabetical order to reduce the chance of
duplicates and to make it clear where new include files should be
added. Drop the unnecessary include of linux/sysfs.h. Include
linux/device.h instead because that is what is actually used.

No functional change.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 63ada2d0d839..b7f5b743c9f5 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -80,18 +80,18 @@
  * concern all supported chipsets, unless mentioned otherwise.
  */
 
-#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
 #include <linux/init.h>
-#include <linux/slab.h>
+#include <linux/interrupt.h>
 #include <linux/jiffies.h>
-#include <linux/i2c.h>
 #include <linux/hwmon.h>
-#include <linux/err.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
-#include <linux/sysfs.h>
-#include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
+#include <linux/slab.h>
 #include <linux/workqueue.h>
 
 /*
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 04/40] hwmon: (lm90) Reorder chip enumeration to be in alphabetical order
  2022-05-25 13:56 [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver Guenter Roeck
                   ` (2 preceding siblings ...)
  2022-05-25 13:56 ` [PATCH 03/40] hwmon: (lm90) Reorder include files in alphabetical order Guenter Roeck
@ 2022-05-25 13:56 ` Guenter Roeck
  2022-05-25 13:56 ` [PATCH 05/40] hwmon: (lm90) Use BIT macro Guenter Roeck
  4 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2022-05-25 13:56 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, Jean Delvare, Slawomir Stepien, Guenter Roeck

Reorder chip enumeration in alphabetical order to make it easier to
see which chips are supported, and to clarify where to add support
new chip types. No functional change.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index b7f5b743c9f5..6728520a21ca 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -113,8 +113,10 @@ static const unsigned short normal_i2c[] = {
 	0x18, 0x19, 0x1a, 0x29, 0x2a, 0x2b, 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 	0x4d, 0x4e, 0x4f, I2C_CLIENT_END };
 
-enum chips { lm90, adm1032, lm99, lm86, max6657, max6659, adt7461, max6680,
-	max6646, w83l771, max6696, sa56004, g781, tmp451, tmp461, max6654 };
+enum chips { adm1032, adt7461, g781, lm86, lm90, lm99,
+	max6646, max6654, max6657, max6659, max6680, max6696,
+	sa56004, tmp451, tmp461, w83l771,
+};
 
 /*
  * The LM90 registers
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 05/40] hwmon: (lm90) Use BIT macro
  2022-05-25 13:56 [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver Guenter Roeck
                   ` (3 preceding siblings ...)
  2022-05-25 13:56 ` [PATCH 04/40] hwmon: (lm90) Reorder chip enumeration to be " Guenter Roeck
@ 2022-05-25 13:56 ` Guenter Roeck
  4 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2022-05-25 13:56 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, Jean Delvare, Slawomir Stepien, Guenter Roeck

Use BIT macro instead of shift operation to improve readability.
No functional change.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c | 55 ++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 6728520a21ca..0f3fadc1631c 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -80,6 +80,7 @@
  * concern all supported chipsets, unless mentioned otherwise.
  */
 
+#include <linux/bits.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
@@ -182,36 +183,36 @@ enum chips { adm1032, adt7461, g781, lm86, lm90, lm99,
 /*
  * Device flags
  */
-#define LM90_FLAG_ADT7461_EXT	(1 << 0) /* ADT7461 extended mode	*/
+#define LM90_FLAG_ADT7461_EXT	BIT(0)	/* ADT7461 extended mode	*/
 /* Device features */
-#define LM90_HAVE_OFFSET	(1 << 1) /* temperature offset register	*/
-#define LM90_HAVE_REM_LIMIT_EXT	(1 << 3) /* extended remote limit	*/
-#define LM90_HAVE_EMERGENCY	(1 << 4) /* 3rd upper (emergency) limit	*/
-#define LM90_HAVE_EMERGENCY_ALARM (1 << 5)/* emergency alarm		*/
-#define LM90_HAVE_TEMP3		(1 << 6) /* 3rd temperature sensor	*/
-#define LM90_HAVE_BROKEN_ALERT	(1 << 7) /* Broken alert		*/
-#define LM90_HAVE_EXTENDED_TEMP	(1 << 8) /* extended temperature support*/
-#define LM90_PAUSE_FOR_CONFIG	(1 << 9) /* Pause conversion for config	*/
-#define LM90_HAVE_CRIT		(1 << 10)/* Chip supports CRIT/OVERT register	*/
-#define LM90_HAVE_CRIT_ALRM_SWP	(1 << 11)/* critical alarm bits swapped	*/
+#define LM90_HAVE_OFFSET	BIT(1)	/* temperature offset register	*/
+#define LM90_HAVE_REM_LIMIT_EXT	BIT(3)	/* extended remote limit	*/
+#define LM90_HAVE_EMERGENCY	BIT(4)	/* 3rd upper (emergency) limit	*/
+#define LM90_HAVE_EMERGENCY_ALARM BIT(5)/* emergency alarm		*/
+#define LM90_HAVE_TEMP3		BIT(6)	/* 3rd temperature sensor	*/
+#define LM90_HAVE_BROKEN_ALERT	BIT(7)	/* Broken alert			*/
+#define LM90_HAVE_EXTENDED_TEMP	BIT(8)	/* extended temperature support	*/
+#define LM90_PAUSE_FOR_CONFIG	BIT(9)	/* Pause conversion for config	*/
+#define LM90_HAVE_CRIT		BIT(10)	/* Chip supports CRIT/OVERT register	*/
+#define LM90_HAVE_CRIT_ALRM_SWP	BIT(11)	/* critical alarm bits swapped	*/
 
 /* LM90 status */
-#define LM90_STATUS_LTHRM	(1 << 0) /* local THERM limit tripped */
-#define LM90_STATUS_RTHRM	(1 << 1) /* remote THERM limit tripped */
-#define LM90_STATUS_ROPEN	(1 << 2) /* remote is an open circuit */
-#define LM90_STATUS_RLOW	(1 << 3) /* remote low temp limit tripped */
-#define LM90_STATUS_RHIGH	(1 << 4) /* remote high temp limit tripped */
-#define LM90_STATUS_LLOW	(1 << 5) /* local low temp limit tripped */
-#define LM90_STATUS_LHIGH	(1 << 6) /* local high temp limit tripped */
-#define LM90_STATUS_BUSY	(1 << 7) /* conversion is ongoing */
-
-#define MAX6696_STATUS2_R2THRM	(1 << 1) /* remote2 THERM limit tripped */
-#define MAX6696_STATUS2_R2OPEN	(1 << 2) /* remote2 is an open circuit */
-#define MAX6696_STATUS2_R2LOW	(1 << 3) /* remote2 low temp limit tripped */
-#define MAX6696_STATUS2_R2HIGH	(1 << 4) /* remote2 high temp limit tripped */
-#define MAX6696_STATUS2_ROT2	(1 << 5) /* remote emergency limit tripped */
-#define MAX6696_STATUS2_R2OT2	(1 << 6) /* remote2 emergency limit tripped */
-#define MAX6696_STATUS2_LOT2	(1 << 7) /* local emergency limit tripped */
+#define LM90_STATUS_LTHRM	BIT(0)	/* local THERM limit tripped */
+#define LM90_STATUS_RTHRM	BIT(1)	/* remote THERM limit tripped */
+#define LM90_STATUS_ROPEN	BIT(2)	/* remote is an open circuit */
+#define LM90_STATUS_RLOW	BIT(3)	/* remote low temp limit tripped */
+#define LM90_STATUS_RHIGH	BIT(4)	/* remote high temp limit tripped */
+#define LM90_STATUS_LLOW	BIT(5)	/* local low temp limit tripped */
+#define LM90_STATUS_LHIGH	BIT(6)	/* local high temp limit tripped */
+#define LM90_STATUS_BUSY	BIT(7)	/* conversion is ongoing */
+
+#define MAX6696_STATUS2_R2THRM	BIT(1)	/* remote2 THERM limit tripped */
+#define MAX6696_STATUS2_R2OPEN	BIT(2)	/* remote2 is an open circuit */
+#define MAX6696_STATUS2_R2LOW	BIT(3)	/* remote2 low temp limit tripped */
+#define MAX6696_STATUS2_R2HIGH	BIT(4)	/* remote2 high temp limit tripped */
+#define MAX6696_STATUS2_ROT2	BIT(5)	/* remote emergency limit tripped */
+#define MAX6696_STATUS2_R2OT2	BIT(6)	/* remote2 emergency limit tripped */
+#define MAX6696_STATUS2_LOT2	BIT(7)	/* local emergency limit tripped */
 
 /*
  * Driver data (common to all clients)
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 02/40] hwmon: (lm90) Rework alarm/status handling
  2022-05-25 13:57 [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver Guenter Roeck
@ 2022-05-25 13:57 ` Guenter Roeck
  0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2022-05-25 13:57 UTC (permalink / raw)
  To: linux-hwmon; +Cc: linux-kernel, Jean Delvare, Slawomir Stepien, Guenter Roeck

Many chips supported by this driver clear status registers after it
is read and update it in the next measurement cycle. Normally this falls
under the radar because all registers are only read once per measurement
cycle. However, there is an exception: Status registers are always read
during interrupt and laert handling. This can result in invalid status
reports if userspace reads an alarm attribute immediately afterwards.

Rework alarm/status handling by keeping a shadow register with 'current'
alarms, and by ensuring that the register is either only updated once per
measurement cycle or not cleared.

A second problem is related to alert handling: Alert handling is disabled
for chips with broken alert after an alert was reported, but only
re-enabled if attributes are read by the user. This means that alert
conditions may appear and disappear unnoticed. Remedy the situation
by introducing a worker to periodically read the status register(s) while
alert handling is disabled, and re-enable alerts after the alert condition
clears.

Yet another problem is that sysfs and udev events are currently only
reported to userspace if an alarm is raised, but not if an alarm condition
clears. Use the new worker to detect that situation and also generate
sysfs and udev events in that case.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c | 314 +++++++++++++++++++++++++++----------------
 1 file changed, 199 insertions(+), 115 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 83d027c134be..63ada2d0d839 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -92,6 +92,7 @@
 #include <linux/sysfs.h>
 #include <linux/interrupt.h>
 #include <linux/regulator/consumer.h>
+#include <linux/workqueue.h>
 
 /*
  * Addresses to scan
@@ -499,8 +500,11 @@ struct lm90_data {
 	const struct hwmon_channel_info *info[3];
 	struct hwmon_chip_info chip;
 	struct mutex update_lock;
+	struct delayed_work alert_work;
 	bool valid;		/* true if register values are valid */
+	bool alarms_valid;	/* true if status register values are valid */
 	unsigned long last_updated; /* in jiffies */
+	unsigned long alarms_updated; /* in jiffies */
 	int kind;
 	u32 flags;
 
@@ -518,7 +522,9 @@ struct lm90_data {
 	s8 temp8[TEMP8_REG_NUM];
 	s16 temp11[TEMP11_REG_NUM];
 	u8 temp_hyst;
-	u16 alarms; /* bitvector (upper 8 bits for max6695/96) */
+	u16 reported_alarms;	/* alarms reported as sysfs/udev events */
+	u16 current_alarms;	/* current alarms, reported by chip */
+	u16 alarms;		/* alarms not yet reported to user */
 };
 
 /*
@@ -771,6 +777,158 @@ static int lm90_update_limits(struct device *dev)
 	return 0;
 }
 
+static void lm90_report_alarms(struct device *dev, struct lm90_data *data)
+{
+	u16 cleared_alarms = data->reported_alarms & ~data->current_alarms;
+	u16 new_alarms = data->current_alarms & ~data->reported_alarms;
+	struct device *hwmon_dev = data->hwmon_dev;
+	int st, st2;
+
+	if (!cleared_alarms && !new_alarms)
+		return;
+
+	st = new_alarms & 0xff;
+	st2 = new_alarms >> 8;
+
+	if ((st & (LM90_STATUS_LLOW | LM90_STATUS_LHIGH | LM90_STATUS_LTHRM)) ||
+	    (st2 & MAX6696_STATUS2_LOT2))
+		dev_dbg(dev, "temp%d out of range, please check!\n", 1);
+	if ((st & (LM90_STATUS_RLOW | LM90_STATUS_RHIGH | LM90_STATUS_RTHRM)) ||
+	    (st2 & MAX6696_STATUS2_ROT2))
+		dev_dbg(dev, "temp%d out of range, please check!\n", 2);
+	if (st & LM90_STATUS_ROPEN)
+		dev_dbg(dev, "temp%d diode open, please check!\n", 2);
+	if (st2 & (MAX6696_STATUS2_R2LOW | MAX6696_STATUS2_R2HIGH |
+		   MAX6696_STATUS2_R2THRM | MAX6696_STATUS2_R2OT2))
+		dev_dbg(dev, "temp%d out of range, please check!\n", 3);
+	if (st2 & MAX6696_STATUS2_R2OPEN)
+		dev_dbg(dev, "temp%d diode open, please check!\n", 3);
+
+	st |= cleared_alarms & 0xff;
+	st2 |= cleared_alarms >> 8;
+
+	if (st & LM90_STATUS_LLOW)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_min_alarm, 0);
+	if (st & LM90_STATUS_RLOW)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_min_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2LOW)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_min_alarm, 2);
+
+	if (st & LM90_STATUS_LHIGH)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_max_alarm, 0);
+	if (st & LM90_STATUS_RHIGH)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_max_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2HIGH)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_max_alarm, 2);
+
+	if (st & LM90_STATUS_LTHRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_crit_alarm, 0);
+	if (st & LM90_STATUS_RTHRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_crit_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2THRM)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_crit_alarm, 2);
+
+	if (st2 & MAX6696_STATUS2_LOT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_emergency_alarm, 0);
+	if (st2 & MAX6696_STATUS2_ROT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_emergency_alarm, 1);
+	if (st2 & MAX6696_STATUS2_R2OT2)
+		hwmon_notify_event(hwmon_dev, hwmon_temp, hwmon_temp_emergency_alarm, 2);
+
+	data->reported_alarms = data->current_alarms;
+}
+
+static int lm90_update_alarms_locked(struct lm90_data *data, bool force)
+{
+	if (force || !data->alarms_valid ||
+	    time_after(jiffies, data->alarms_updated + msecs_to_jiffies(data->update_interval))) {
+		struct i2c_client *client = data->client;
+		bool check_enable;
+		u16 alarms;
+		int val;
+
+		data->alarms_valid = false;
+
+		val = lm90_read_reg(client, LM90_REG_R_STATUS);
+		if (val < 0)
+			return val;
+		alarms = val & ~LM90_STATUS_BUSY;
+
+		if (data->kind == max6696) {
+			val = lm90_read_reg(client, MAX6696_REG_R_STATUS2);
+			if (val < 0)
+				return val;
+			alarms |= val << 8;
+		}
+		/*
+		 * If the update is forced (called from interrupt or alert
+		 * handler) and alarm data is valid, the alarms may have been
+		 * updated after the last update interval, and the status
+		 * register may still be cleared. Only add additional alarms
+		 * in this case. Alarms will be cleared later if appropriate.
+		 */
+		if (force && data->alarms_valid)
+			data->current_alarms |= alarms;
+		else
+			data->current_alarms = alarms;
+		data->alarms |= alarms;
+
+		check_enable = (client->irq || !(data->config_orig & 0x80)) &&
+			(data->config & 0x80);
+
+		if (force || check_enable)
+			lm90_report_alarms(&client->dev, data);
+
+		/*
+		 * Re-enable ALERT# output if it was originally enabled, relevant
+		 * alarms are all clear, and alerts are currently disabled.
+		 * Otherwise (re)schedule worker if needed.
+		 */
+		if (check_enable) {
+			if (!(data->current_alarms & data->alert_alarms)) {
+				dev_dbg(&client->dev, "Re-enabling ALERT#\n");
+				lm90_update_confreg(data, data->config & ~0x80);
+				/*
+				 * We may have been called from the update handler.
+				 * If so, the worker, if scheduled, is no longer
+				 * needed. Cancel it. Don't synchronize because
+				 * it may already be running.
+				 */
+				cancel_delayed_work(&data->alert_work);
+			} else {
+				schedule_delayed_work(&data->alert_work,
+					max_t(int, HZ, msecs_to_jiffies(data->update_interval)));
+			}
+		}
+		data->alarms_updated = jiffies;
+		data->alarms_valid = true;
+	}
+	return 0;
+}
+
+static int lm90_update_alarms(struct lm90_data *data, bool force)
+{
+	int err;
+
+	mutex_lock(&data->update_lock);
+	err = lm90_update_alarms_locked(data, force);
+	mutex_unlock(&data->update_lock);
+
+	return err;
+}
+
+static void lm90_alert_work(struct work_struct *__work)
+{
+	struct delayed_work *delayed_work = container_of(__work, struct delayed_work, work);
+	struct lm90_data *data = container_of(delayed_work, struct lm90_data, alert_work);
+
+	/* Nothing to do if alerts are enabled */
+	if (!(data->config & 0x80))
+		return;
+
+	lm90_update_alarms(data, true);
+}
+
 static int lm90_update_device(struct device *dev)
 {
 	struct lm90_data *data = dev_get_drvdata(dev);
@@ -819,11 +977,6 @@ static int lm90_update_device(struct device *dev)
 			return val;
 		data->temp11[REMOTE_TEMP] = val;
 
-		val = lm90_read_reg(client, LM90_REG_R_STATUS);
-		if (val < 0)
-			return val;
-		data->alarms = val & ~LM90_STATUS_BUSY;
-
 		if (data->kind == max6696) {
 			val = lm90_select_remote_channel(data, 1);
 			if (val < 0)
@@ -838,24 +991,11 @@ static int lm90_update_device(struct device *dev)
 			data->temp11[REMOTE2_TEMP] = val;
 
 			lm90_select_remote_channel(data, 0);
-
-			val = lm90_read_reg(client, MAX6696_REG_R_STATUS2);
-			if (val < 0)
-				return val;
-			data->alarms |= val << 8;
 		}
 
-		/*
-		 * Re-enable ALERT# output if it was originally enabled and
-		 * relevant alarms are all clear
-		 */
-		if ((client->irq || !(data->config_orig & 0x80)) &&
-		    !(data->alarms & data->alert_alarms)) {
-			if (data->config & 0x80) {
-				dev_dbg(&client->dev, "Re-enabling ALERT#\n");
-				lm90_update_confreg(data, data->config & ~0x80);
-			}
-		}
+		val = lm90_update_alarms_locked(data, false);
+		if (val < 0)
+			return val;
 
 		data->last_updated = jiffies;
 		data->valid = true;
@@ -1212,7 +1352,7 @@ static const u8 lm90_fault_bits[3] = { 0, 2, 10 };
 static int lm90_temp_read(struct device *dev, u32 attr, int channel, long *val)
 {
 	struct lm90_data *data = dev_get_drvdata(dev);
-	int err;
+	int err, bit;
 
 	mutex_lock(&data->update_lock);
 	err = lm90_update_device(dev);
@@ -1225,22 +1365,33 @@ static int lm90_temp_read(struct device *dev, u32 attr, int channel, long *val)
 		*val = lm90_get_temp11(data, lm90_temp_index[channel]);
 		break;
 	case hwmon_temp_min_alarm:
-		*val = (data->alarms >> lm90_min_alarm_bits[channel]) & 1;
-		break;
 	case hwmon_temp_max_alarm:
-		*val = (data->alarms >> lm90_max_alarm_bits[channel]) & 1;
-		break;
 	case hwmon_temp_crit_alarm:
-		if (data->flags & LM90_HAVE_CRIT_ALRM_SWP)
-			*val = (data->alarms >> lm90_crit_alarm_bits_swapped[channel]) & 1;
-		else
-			*val = (data->alarms >> lm90_crit_alarm_bits[channel]) & 1;
-		break;
 	case hwmon_temp_emergency_alarm:
-		*val = (data->alarms >> lm90_emergency_alarm_bits[channel]) & 1;
-		break;
 	case hwmon_temp_fault:
-		*val = (data->alarms >> lm90_fault_bits[channel]) & 1;
+		switch (attr) {
+		case hwmon_temp_min_alarm:
+			bit = BIT(lm90_min_alarm_bits[channel]);
+			break;
+		case hwmon_temp_max_alarm:
+			bit = BIT(lm90_max_alarm_bits[channel]);
+			break;
+		case hwmon_temp_crit_alarm:
+			if (data->flags & LM90_HAVE_CRIT_ALRM_SWP)
+				bit = BIT(lm90_crit_alarm_bits_swapped[channel]);
+			else
+				bit = BIT(lm90_crit_alarm_bits[channel]);
+			break;
+		case hwmon_temp_emergency_alarm:
+			bit = BIT(lm90_emergency_alarm_bits[channel]);
+			break;
+		case hwmon_temp_fault:
+			bit = BIT(lm90_fault_bits[channel]);
+			break;
+		}
+		*val = !!(data->alarms & bit);
+		data->alarms &= ~bit;
+		data->alarms |= data->current_alarms;
 		break;
 	case hwmon_temp_min:
 		if (channel == 0)
@@ -1699,6 +1850,8 @@ static void lm90_restore_conf(void *_data)
 	struct lm90_data *data = _data;
 	struct i2c_client *client = data->client;
 
+	cancel_delayed_work_sync(&data->alert_work);
+
 	/* Restore initial configuration */
 	lm90_write_convrate(data, data->convrate_orig);
 	i2c_smbus_write_byte_data(client, LM90_REG_W_CONFIG1,
@@ -1771,93 +1924,23 @@ static int lm90_init_client(struct i2c_client *client, struct lm90_data *data)
 	return devm_add_action_or_reset(&client->dev, lm90_restore_conf, data);
 }
 
-static bool lm90_is_tripped(struct i2c_client *client, u16 *status)
+static bool lm90_is_tripped(struct i2c_client *client)
 {
 	struct lm90_data *data = i2c_get_clientdata(client);
-	int st, st2 = 0;
-
-	st = lm90_read_reg(client, LM90_REG_R_STATUS);
-	if (st < 0)
-		return false;
-
-	if (data->kind == max6696) {
-		st2 = lm90_read_reg(client, MAX6696_REG_R_STATUS2);
-		if (st2 < 0)
-			return false;
-	}
-
-	*status = st | (st2 << 8);
+	int ret;
 
-	if ((st & 0x7f) == 0 && (st2 & 0xfe) == 0)
+	ret = lm90_update_alarms(data, true);
+	if (ret < 0)
 		return false;
 
-	if ((st & (LM90_STATUS_LLOW | LM90_STATUS_LHIGH | LM90_STATUS_LTHRM)) ||
-	    (st2 & MAX6696_STATUS2_LOT2))
-		dev_dbg(&client->dev,
-			"temp%d out of range, please check!\n", 1);
-	if ((st & (LM90_STATUS_RLOW | LM90_STATUS_RHIGH | LM90_STATUS_RTHRM)) ||
-	    (st2 & MAX6696_STATUS2_ROT2))
-		dev_dbg(&client->dev,
-			"temp%d out of range, please check!\n", 2);
-	if (st & LM90_STATUS_ROPEN)
-		dev_dbg(&client->dev,
-			"temp%d diode open, please check!\n", 2);
-	if (st2 & (MAX6696_STATUS2_R2LOW | MAX6696_STATUS2_R2HIGH |
-		   MAX6696_STATUS2_R2THRM | MAX6696_STATUS2_R2OT2))
-		dev_dbg(&client->dev,
-			"temp%d out of range, please check!\n", 3);
-	if (st2 & MAX6696_STATUS2_R2OPEN)
-		dev_dbg(&client->dev,
-			"temp%d diode open, please check!\n", 3);
-
-	if (st & LM90_STATUS_LLOW)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_min_alarm, 0);
-	if (st & LM90_STATUS_RLOW)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_min_alarm, 1);
-	if (st2 & MAX6696_STATUS2_R2LOW)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_min_alarm, 2);
-	if (st & LM90_STATUS_LHIGH)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_max_alarm, 0);
-	if (st & LM90_STATUS_RHIGH)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_max_alarm, 1);
-	if (st2 & MAX6696_STATUS2_R2HIGH)
-		hwmon_notify_event(data->hwmon_dev, hwmon_temp,
-				   hwmon_temp_max_alarm, 2);
-
-	if (st & LM90_STATUS_LTHRM)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_crit_alarm, 0);
-	if (st & LM90_STATUS_RTHRM)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_crit_alarm, 1);
-	if (st2 & MAX6696_STATUS2_R2THRM)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_crit_alarm, 2);
-
-	if (st2 & MAX6696_STATUS2_LOT2)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_emergency_alarm, 0);
-	if (st2 & MAX6696_STATUS2_ROT2)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_emergency_alarm, 1);
-	if (st2 & MAX6696_STATUS2_R2OT2)
-		hwmon_notify_event(hwmon_dev, hwmon_temp,
-				   hwmon_temp_emergency_alarm, 2);
-
-	return true;
+	return !!data->current_alarms;
 }
 
 static irqreturn_t lm90_irq_thread(int irq, void *dev_id)
 {
 	struct i2c_client *client = dev_id;
-	u16 status;
 
-	if (lm90_is_tripped(client, &status))
+	if (lm90_is_tripped(client))
 		return IRQ_HANDLED;
 	else
 		return IRQ_NONE;
@@ -1911,6 +1994,7 @@ static int lm90_probe(struct i2c_client *client)
 	data->client = client;
 	i2c_set_clientdata(client, data);
 	mutex_init(&data->update_lock);
+	INIT_DELAYED_WORK(&data->alert_work, lm90_alert_work);
 
 	/* Set the device type */
 	if (client->dev.of_node)
@@ -2027,12 +2111,10 @@ static int lm90_probe(struct i2c_client *client)
 static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type,
 		       unsigned int flag)
 {
-	u16 alarms;
-
 	if (type != I2C_PROTOCOL_SMBUS_ALERT)
 		return;
 
-	if (lm90_is_tripped(client, &alarms)) {
+	if (lm90_is_tripped(client)) {
 		/*
 		 * Disable ALERT# output, because these chips don't implement
 		 * SMBus alert correctly; they should only hold the alert line
@@ -2041,9 +2123,11 @@ static void lm90_alert(struct i2c_client *client, enum i2c_alert_protocol type,
 		struct lm90_data *data = i2c_get_clientdata(client);
 
 		if ((data->flags & LM90_HAVE_BROKEN_ALERT) &&
-		    (alarms & data->alert_alarms)) {
+		    (data->current_alarms & data->alert_alarms)) {
 			dev_dbg(&client->dev, "Disabling ALERT#\n");
 			lm90_update_confreg(data, data->config | 0x80);
+			schedule_delayed_work(&data->alert_work,
+				max_t(int, HZ, msecs_to_jiffies(data->update_interval)));
 		}
 	} else {
 		dev_dbg(&client->dev, "Everything OK\n");
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-05-25 14:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-25 13:56 [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver Guenter Roeck
2022-05-25 13:56 ` [PATCH 01/40] hwmon: (lm90) Generate sysfs and udev events for all alarms Guenter Roeck
2022-05-25 13:56 ` [PATCH 02/40] hwmon: (lm90) Rework alarm/status handling Guenter Roeck
2022-05-25 13:56 ` [PATCH 03/40] hwmon: (lm90) Reorder include files in alphabetical order Guenter Roeck
2022-05-25 13:56 ` [PATCH 04/40] hwmon: (lm90) Reorder chip enumeration to be " Guenter Roeck
2022-05-25 13:56 ` [PATCH 05/40] hwmon: (lm90) Use BIT macro Guenter Roeck
2022-05-25 13:57 [PATCH 00/40] hwmon: (lm90) Various improvements to lm90 driver Guenter Roeck
2022-05-25 13:57 ` [PATCH 02/40] hwmon: (lm90) Rework alarm/status handling Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.