[PATCH 0/6] hwmon: (lm92) Cleanup and conversion to with_info API

[PATCH 0/6] hwmon: (lm92) Cleanup and conversion to with_info API

[Guenter Roeck]

Clean up lm92 driver and convert it to use regmap and the with_info hwmon
API.

----------------------------------------------------------------
Guenter Roeck (6):
      hwmon: (lm92) Improve auto-detection accuracy
      hwmon: (lm92) Reorder include files to alphabetic order
      hwmon: (lm92) Replace chip IDs with limit register resolution
      hwmon: (lm92) Convert to use regmap
      hwmon: (lm92) Convert to with_info hwmon API
      hwmon: (lm92) Update documentation

 Documentation/hwmon/lm92.rst |  26 +--
 drivers/hwmon/Kconfig        |   3 +-
 drivers/hwmon/lm92.c         | 457 ++++++++++++++++++++++++++-----------------
 3 files changed, 291 insertions(+), 195 deletions(-)

[PATCH 1/6] hwmon: (lm92) Improve auto-detection accuracy

[Guenter Roeck]

Checking three configuration register bits and the manufacturer ID
register to auto-detect LM92 is a bit vague. Repeat twice on replicated
register addresses to improve detection accuracy.  Check the manufacturer
ID first and bail out immediately without reading the other register if
there is a mismatch to reduce the number of i2c transfers needed in that
case. Also explicitly test for an error from reading the configuration
register to avoid potential situations where the returned error masked
against 0xe0 is 0.

While at it, drop "lm92: Found National Semiconductor LM92 chip" detection
noise.

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

diff --git a/drivers/hwmon/lm92.c b/drivers/hwmon/lm92.c
index 46579a3e1715..e2d8c8afcbfa 100644
--- a/drivers/hwmon/lm92.c
+++ b/drivers/hwmon/lm92.c
@@ -272,20 +272,28 @@ static int lm92_detect(struct i2c_client *new_client,
 		       struct i2c_board_info *info)
 {
 	struct i2c_adapter *adapter = new_client->adapter;
-	u8 config;
-	u16 man_id;
+	u8 config_addr = LM92_REG_CONFIG;
+	u8 man_id_addr = LM92_REG_MAN_ID;
+	int i, regval;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA
 					    | I2C_FUNC_SMBUS_WORD_DATA))
 		return -ENODEV;
 
-	config = i2c_smbus_read_byte_data(new_client, LM92_REG_CONFIG);
-	man_id = i2c_smbus_read_word_data(new_client, LM92_REG_MAN_ID);
-
-	if ((config & 0xe0) == 0x00 && man_id == 0x0180)
-		pr_info("lm92: Found National Semiconductor LM92 chip\n");
-	else
-		return -ENODEV;
+	/*
+	 * Register values repeat with multiples of 8.
+	 * Read twice to improve detection accuracy.
+	 */
+	for (i = 0; i < 2; i++) {
+		regval = i2c_smbus_read_word_data(new_client, man_id_addr);
+		if (regval != 0x0180)
+			return -ENODEV;
+		regval = i2c_smbus_read_byte_data(new_client, config_addr);
+		if (regval < 0 || (regval & 0xe0))
+			return -ENODEV;
+		config_addr += 8;
+		man_id_addr += 8;
+	}
 
 	strscpy(info->type, "lm92", I2C_NAME_SIZE);
 
-- 
2.39.2

[PATCH 2/6] hwmon: (lm92) Reorder include files to alphabetic order

[Guenter Roeck]

Simplify driver maintenance by reordering files to alphabetic order.

No functional change.

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

diff --git a/drivers/hwmon/lm92.c b/drivers/hwmon/lm92.c
index e2d8c8afcbfa..b0179d72b779 100644
--- a/drivers/hwmon/lm92.c
+++ b/drivers/hwmon/lm92.c
@@ -27,15 +27,15 @@
  * with the LM92.
  */
 
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/slab.h>
-#include <linux/i2c.h>
+#include <linux/err.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
-#include <linux/err.h>
-#include <linux/mutex.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
 #include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
 
 /*
  * The LM92 and MAX6635 have 2 two-state pins for address selection,
-- 
2.39.2

[PATCH 3/6] hwmon: (lm92) Replace chip IDs with limit register resolution

[Guenter Roeck]

The chip IDs are not used by the driver. Drop them. Use driver data to
store the limit register resolution instead, and use this information
when writing temperature limits to improve chip specific rounding and
to avoid writing into unused register bits.

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

diff --git a/drivers/hwmon/lm92.c b/drivers/hwmon/lm92.c
index b0179d72b779..5ff45a0033f1 100644
--- a/drivers/hwmon/lm92.c
+++ b/drivers/hwmon/lm92.c
@@ -43,8 +43,6 @@
  */
 static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b,
 						I2C_CLIENT_END };
-enum chips { lm92, max6635 };
-
 /* The LM92 registers */
 #define LM92_REG_CONFIG			0x01 /* 8-bit, RW */
 #define LM92_REG_TEMP			0x00 /* 16-bit, RO */
@@ -66,10 +64,10 @@ static inline int TEMP_FROM_REG(s16 reg)
 	return reg / 8 * 625 / 10;
 }
 
-static inline s16 TEMP_TO_REG(long val)
+static inline s16 TEMP_TO_REG(long val, int resolution)
 {
 	val = clamp_val(val, -60000, 160000);
-	return val * 10 / 625 * 8;
+	return DIV_ROUND_CLOSEST(val << (resolution - 9), 1000) << (16 - resolution);
 }
 
 /* Alarm flags are stored in the 3 LSB of the temperature register */
@@ -99,6 +97,7 @@ static const u8 regs[t_num_regs] = {
 struct lm92_data {
 	struct i2c_client *client;
 	struct mutex update_lock;
+	int resolution;
 	bool valid; /* false until following fields are valid */
 	unsigned long last_updated; /* in jiffies */
 
@@ -159,7 +158,7 @@ static ssize_t temp_store(struct device *dev,
 		return err;
 
 	mutex_lock(&data->update_lock);
-	data->temp[nr] = TEMP_TO_REG(val);
+	data->temp[nr] = TEMP_TO_REG(val, data->resolution);
 	i2c_smbus_write_word_swapped(client, regs[nr], data->temp[nr]);
 	mutex_unlock(&data->update_lock);
 	return count;
@@ -201,7 +200,8 @@ static ssize_t temp_hyst_store(struct device *dev,
 	val = clamp_val(val, -120000, 220000);
 	mutex_lock(&data->update_lock);
 	data->temp[t_hyst] =
-		TEMP_TO_REG(TEMP_FROM_REG(data->temp[attr->index]) - val);
+		TEMP_TO_REG(TEMP_FROM_REG(data->temp[attr->index]) - val,
+			    data->resolution);
 	i2c_smbus_write_word_swapped(client, LM92_REG_TEMP_HYST,
 				     data->temp[t_hyst]);
 	mutex_unlock(&data->update_lock);
@@ -311,6 +311,7 @@ static int lm92_probe(struct i2c_client *new_client)
 		return -ENOMEM;
 
 	data->client = new_client;
+	data->resolution = (unsigned long)i2c_get_match_data(new_client);
 	mutex_init(&data->update_lock);
 
 	/* Initialize the chipset */
@@ -326,9 +327,10 @@ static int lm92_probe(struct i2c_client *new_client)
  * Module and driver stuff
  */
 
+/* .driver_data is limit register resolution */ 
 static const struct i2c_device_id lm92_id[] = {
-	{ "lm92", lm92 },
-	{ "max6635", max6635 },
+	{ "lm92", 13 },
+	{ "max6635", 9 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, lm92_id);
-- 
2.39.2

[PATCH 4/6] hwmon: (lm92) Convert to use regmap

[Guenter Roeck]

Use regmap for local caching and for multi-byte operations to be able
to use regmap API functions and to reduce the need for locking in the
driver.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/Kconfig |   1 +
 drivers/hwmon/lm92.c  | 204 +++++++++++++++++++++++++++---------------
 2 files changed, 131 insertions(+), 74 deletions(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b60fe2e58ad6..b2ace4857130 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1511,6 +1511,7 @@ config SENSORS_LM90
 config SENSORS_LM92
 	tristate "National Semiconductor LM92 and compatibles"
 	depends on I2C
+	select REGMAP_I2C
 	help
 	  If you say yes here you get support for National Semiconductor LM92
 	  and Maxim MAX6635 sensor chips.
diff --git a/drivers/hwmon/lm92.c b/drivers/hwmon/lm92.c
index 5ff45a0033f1..89befc7f59a5 100644
--- a/drivers/hwmon/lm92.c
+++ b/drivers/hwmon/lm92.c
@@ -32,9 +32,9 @@
 #include <linux/hwmon-sysfs.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
-#include <linux/jiffies.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 /*
@@ -81,65 +81,40 @@ enum temp_index {
 	t_crit,
 	t_min,
 	t_max,
-	t_hyst,
 	t_num_regs
 };
 
-static const u8 regs[t_num_regs] = {
+static const u8 lm92_regs[t_num_regs] = {
 	[t_input] = LM92_REG_TEMP,
 	[t_crit] = LM92_REG_TEMP_CRIT,
 	[t_min] = LM92_REG_TEMP_LOW,
 	[t_max] = LM92_REG_TEMP_HIGH,
-	[t_hyst] = LM92_REG_TEMP_HYST,
 };
 
 /* Client data (each client gets its own) */
 struct lm92_data {
-	struct i2c_client *client;
+	struct regmap *regmap;
 	struct mutex update_lock;
 	int resolution;
-	bool valid; /* false until following fields are valid */
-	unsigned long last_updated; /* in jiffies */
-
-	/* registers values */
-	s16 temp[t_num_regs];	/* index with enum temp_index */
 };
 
 /*
  * Sysfs attributes and callback functions
  */
 
-static struct lm92_data *lm92_update_device(struct device *dev)
-{
-	struct lm92_data *data = dev_get_drvdata(dev);
-	struct i2c_client *client = data->client;
-	int i;
-
-	mutex_lock(&data->update_lock);
-
-	if (time_after(jiffies, data->last_updated + HZ) ||
-	    !data->valid) {
-		dev_dbg(&client->dev, "Updating lm92 data\n");
-		for (i = 0; i < t_num_regs; i++) {
-			data->temp[i] =
-				i2c_smbus_read_word_swapped(client, regs[i]);
-		}
-		data->last_updated = jiffies;
-		data->valid = true;
-	}
-
-	mutex_unlock(&data->update_lock);
-
-	return data;
-}
-
 static ssize_t temp_show(struct device *dev, struct device_attribute *devattr,
 			 char *buf)
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
-	struct lm92_data *data = lm92_update_device(dev);
+	struct lm92_data *data = dev_get_drvdata(dev);
+	u32 temp;
+	int err;
 
-	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[attr->index]));
+	err = regmap_read(data->regmap, lm92_regs[attr->index], &temp);
+	if (err)
+		return err;
+
+	return sprintf(buf, "%d\n", TEMP_FROM_REG(temp));
 }
 
 static ssize_t temp_store(struct device *dev,
@@ -148,7 +123,7 @@ static ssize_t temp_store(struct device *dev,
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
 	struct lm92_data *data = dev_get_drvdata(dev);
-	struct i2c_client *client = data->client;
+	struct regmap *regmap = data->regmap;
 	int nr = attr->index;
 	long val;
 	int err;
@@ -157,10 +132,9 @@ static ssize_t temp_store(struct device *dev,
 	if (err)
 		return err;
 
-	mutex_lock(&data->update_lock);
-	data->temp[nr] = TEMP_TO_REG(val, data->resolution);
-	i2c_smbus_write_word_swapped(client, regs[nr], data->temp[nr]);
-	mutex_unlock(&data->update_lock);
+	err = regmap_write(regmap, lm92_regs[nr], TEMP_TO_REG(val, data->resolution));
+	if (err)
+		return err;
 	return count;
 }
 
@@ -168,28 +142,42 @@ static ssize_t temp_hyst_show(struct device *dev,
 			      struct device_attribute *devattr, char *buf)
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
-	struct lm92_data *data = lm92_update_device(dev);
+	u32 regs[2] = { lm92_regs[attr->index], LM92_REG_TEMP_HYST };
+	struct lm92_data *data = dev_get_drvdata(dev);
+	u16 regvals[2];
+	int err;
 
-	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[attr->index])
-		       - TEMP_FROM_REG(data->temp[t_hyst]));
+	err = regmap_multi_reg_read(data->regmap, regs, regvals, 2);
+	if (err)
+		return err;
+
+	return sprintf(buf, "%d\n",
+		       TEMP_FROM_REG(regvals[0]) - TEMP_FROM_REG(regvals[1]));
 }
 
 static ssize_t temp1_min_hyst_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
-	struct lm92_data *data = lm92_update_device(dev);
+	static u32 regs[2] = { LM92_REG_TEMP_LOW, LM92_REG_TEMP_HYST };
+	struct lm92_data *data = dev_get_drvdata(dev);
+	u16 regvals[2];
+	int err;
 
-	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[t_min])
-		       + TEMP_FROM_REG(data->temp[t_hyst]));
+	err = regmap_multi_reg_read(data->regmap, regs, regvals, 2);
+	if (err)
+		return err;
+
+	return sprintf(buf, "%d\n",
+		       TEMP_FROM_REG(regvals[0]) + TEMP_FROM_REG(regvals[1]));
 }
 
 static ssize_t temp_hyst_store(struct device *dev,
 			       struct device_attribute *devattr,
 			       const char *buf, size_t count)
 {
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
 	struct lm92_data *data = dev_get_drvdata(dev);
-	struct i2c_client *client = data->client;
+	struct regmap *regmap = data->regmap;
+	u32 temp;
 	long val;
 	int err;
 
@@ -199,29 +187,45 @@ static ssize_t temp_hyst_store(struct device *dev,
 
 	val = clamp_val(val, -120000, 220000);
 	mutex_lock(&data->update_lock);
-	data->temp[t_hyst] =
-		TEMP_TO_REG(TEMP_FROM_REG(data->temp[attr->index]) - val,
-			    data->resolution);
-	i2c_smbus_write_word_swapped(client, LM92_REG_TEMP_HYST,
-				     data->temp[t_hyst]);
+	err = regmap_read(regmap, LM92_REG_TEMP_CRIT, &temp);
+	if (err)
+		goto unlock;
+	val = TEMP_TO_REG(TEMP_FROM_REG(temp) - val, data->resolution);
+	err = regmap_write(regmap, LM92_REG_TEMP_HYST, val);
+unlock:
 	mutex_unlock(&data->update_lock);
+	if (err)
+		return err;
 	return count;
 }
 
 static ssize_t alarms_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
-	struct lm92_data *data = lm92_update_device(dev);
+	struct lm92_data *data = dev_get_drvdata(dev);
+	u32 temp;
+	int err;
 
-	return sprintf(buf, "%d\n", ALARMS_FROM_REG(data->temp[t_input]));
+	err = regmap_read(data->regmap, LM92_REG_TEMP, &temp);
+	if (err)
+		return err;
+
+	return sprintf(buf, "%d\n", ALARMS_FROM_REG(temp));
 }
 
 static ssize_t alarm_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
+	struct lm92_data *data = dev_get_drvdata(dev);
 	int bitnr = to_sensor_dev_attr(attr)->index;
-	struct lm92_data *data = lm92_update_device(dev);
-	return sprintf(buf, "%d\n", (data->temp[t_input] >> bitnr) & 1);
+	u32 temp;
+	int err;
+
+	err = regmap_read(data->regmap, LM92_REG_TEMP, &temp);
+	if (err)
+		return err;
+
+	return sprintf(buf, "%d\n", (temp >> bitnr) & 1);
 }
 
 static SENSOR_DEVICE_ATTR_RO(temp1_input, temp, t_input);
@@ -240,15 +244,9 @@ static SENSOR_DEVICE_ATTR_RO(temp1_max_alarm, alarm, 1);
  * Detection and registration
  */
 
-static void lm92_init_client(struct i2c_client *client)
+static int lm92_init_client(struct regmap *regmap)
 {
-	u8 config;
-
-	/* Start the conversions if needed */
-	config = i2c_smbus_read_byte_data(client, LM92_REG_CONFIG);
-	if (config & 0x01)
-		i2c_smbus_write_byte_data(client, LM92_REG_CONFIG,
-					  config & 0xFE);
+	return regmap_clear_bits(regmap, LM92_REG_CONFIG, 0x01);
 }
 
 static struct attribute *lm92_attrs[] = {
@@ -300,25 +298,83 @@ static int lm92_detect(struct i2c_client *new_client,
 	return 0;
 }
 
-static int lm92_probe(struct i2c_client *new_client)
+/* regmap */
+
+static int lm92_reg_read(void *context, unsigned int reg, unsigned int *val)
 {
+	int ret;
+
+	if (reg == LM92_REG_CONFIG)
+		ret = i2c_smbus_read_byte_data(context, reg);
+	else
+		ret = i2c_smbus_read_word_swapped(context, reg);
+	if (ret < 0)
+		return ret;
+
+	*val = ret;
+	return 0;
+}
+
+static int lm92_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	if (reg == LM92_REG_CONFIG)
+		return i2c_smbus_write_byte_data(context, LM92_REG_CONFIG, val);
+
+	return i2c_smbus_write_word_swapped(context, reg, val);
+}
+
+static bool lm92_regmap_is_volatile(struct device *dev, unsigned int reg)
+{
+	return reg == LM92_REG_TEMP;
+}
+
+static bool lm92_regmap_is_writeable(struct device *dev, unsigned int reg)
+{
+	return reg >= LM92_REG_CONFIG;
+}
+
+static const struct regmap_config lm92_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.max_register = LM92_REG_TEMP_HIGH,
+	.cache_type = REGCACHE_MAPLE,
+	.volatile_reg = lm92_regmap_is_volatile,
+	.writeable_reg = lm92_regmap_is_writeable,
+};
+
+static const struct regmap_bus lm92_regmap_bus = {
+	.reg_write = lm92_reg_write,
+	.reg_read = lm92_reg_read,
+};
+
+static int lm92_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
 	struct device *hwmon_dev;
 	struct lm92_data *data;
+	struct regmap *regmap;
+	int err;
 
-	data = devm_kzalloc(&new_client->dev, sizeof(struct lm92_data),
-			    GFP_KERNEL);
+	regmap = devm_regmap_init(dev, &lm92_regmap_bus, client,
+				  &lm92_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	data = devm_kzalloc(dev, sizeof(struct lm92_data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
-	data->client = new_client;
-	data->resolution = (unsigned long)i2c_get_match_data(new_client);
+	data->regmap = regmap;
+	data->resolution = (unsigned long)i2c_get_match_data(client);
 	mutex_init(&data->update_lock);
 
 	/* Initialize the chipset */
-	lm92_init_client(new_client);
+	err = lm92_init_client(regmap);
+	if (err)
+		return err;
 
-	hwmon_dev = devm_hwmon_device_register_with_groups(&new_client->dev,
-							   new_client->name,
+	hwmon_dev = devm_hwmon_device_register_with_groups(dev,
+							   client->name,
 							   data, lm92_groups);
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
-- 
2.39.2

[PATCH 5/6] hwmon: (lm92) Convert to with_info hwmon API

[Guenter Roeck]

Convert driver to with_info hwmon API to simplify the code and
to reduce its size.

No functional change.

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

diff --git a/drivers/hwmon/lm92.c b/drivers/hwmon/lm92.c
index 89befc7f59a5..0be439b38ee1 100644
--- a/drivers/hwmon/lm92.c
+++ b/drivers/hwmon/lm92.c
@@ -29,7 +29,6 @@
 
 #include <linux/err.h>
 #include <linux/hwmon.h>
-#include <linux/hwmon-sysfs.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -76,21 +75,6 @@ static inline u8 ALARMS_FROM_REG(s16 reg)
 	return reg & 0x0007;
 }
 
-enum temp_index {
-	t_input,
-	t_crit,
-	t_min,
-	t_max,
-	t_num_regs
-};
-
-static const u8 lm92_regs[t_num_regs] = {
-	[t_input] = LM92_REG_TEMP,
-	[t_crit] = LM92_REG_TEMP_CRIT,
-	[t_min] = LM92_REG_TEMP_LOW,
-	[t_max] = LM92_REG_TEMP_HIGH,
-};
-
 /* Client data (each client gets its own) */
 struct lm92_data {
 	struct regmap *regmap;
@@ -98,147 +82,209 @@ struct lm92_data {
 	int resolution;
 };
 
-/*
- * Sysfs attributes and callback functions
- */
-
-static ssize_t temp_show(struct device *dev, struct device_attribute *devattr,
-			 char *buf)
+static int lm92_temp_read(struct lm92_data *data, u32 attr, int channel, long *val)
 {
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
-	struct lm92_data *data = dev_get_drvdata(dev);
-	u32 temp;
-	int err;
-
-	err = regmap_read(data->regmap, lm92_regs[attr->index], &temp);
-	if (err)
-		return err;
-
-	return sprintf(buf, "%d\n", TEMP_FROM_REG(temp));
-}
-
-static ssize_t temp_store(struct device *dev,
-			  struct device_attribute *devattr, const char *buf,
-			  size_t count)
-{
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
-	struct lm92_data *data = dev_get_drvdata(dev);
-	struct regmap *regmap = data->regmap;
-	int nr = attr->index;
-	long val;
-	int err;
-
-	err = kstrtol(buf, 10, &val);
-	if (err)
-		return err;
-
-	err = regmap_write(regmap, lm92_regs[nr], TEMP_TO_REG(val, data->resolution));
-	if (err)
-		return err;
-	return count;
-}
-
-static ssize_t temp_hyst_show(struct device *dev,
-			      struct device_attribute *devattr, char *buf)
-{
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
-	u32 regs[2] = { lm92_regs[attr->index], LM92_REG_TEMP_HYST };
-	struct lm92_data *data = dev_get_drvdata(dev);
-	u16 regvals[2];
-	int err;
-
-	err = regmap_multi_reg_read(data->regmap, regs, regvals, 2);
-	if (err)
-		return err;
-
-	return sprintf(buf, "%d\n",
-		       TEMP_FROM_REG(regvals[0]) - TEMP_FROM_REG(regvals[1]));
-}
-
-static ssize_t temp1_min_hyst_show(struct device *dev,
-				   struct device_attribute *attr, char *buf)
-{
-	static u32 regs[2] = { LM92_REG_TEMP_LOW, LM92_REG_TEMP_HYST };
-	struct lm92_data *data = dev_get_drvdata(dev);
-	u16 regvals[2];
-	int err;
-
-	err = regmap_multi_reg_read(data->regmap, regs, regvals, 2);
-	if (err)
-		return err;
-
-	return sprintf(buf, "%d\n",
-		       TEMP_FROM_REG(regvals[0]) + TEMP_FROM_REG(regvals[1]));
-}
-
-static ssize_t temp_hyst_store(struct device *dev,
-			       struct device_attribute *devattr,
-			       const char *buf, size_t count)
-{
-	struct lm92_data *data = dev_get_drvdata(dev);
+	int reg = -1, hyst_reg = -1, alarm_bit = 0;
 	struct regmap *regmap = data->regmap;
 	u32 temp;
-	long val;
-	int err;
+	int ret;
 
-	err = kstrtol(buf, 10, &val);
-	if (err)
-		return err;
+	switch (attr) {
+	case hwmon_temp_input:
+		reg = LM92_REG_TEMP;
+		break;
+	case hwmon_temp_min:
+		reg = LM92_REG_TEMP_LOW;
+		break;
+	case hwmon_temp_max:
+		reg = LM92_REG_TEMP_HIGH;
+		break;
+	case hwmon_temp_crit:
+		reg = LM92_REG_TEMP_CRIT;
+		break;
+	case hwmon_temp_min_hyst:
+		hyst_reg = LM92_REG_TEMP_LOW;
+		break;
+	case hwmon_temp_max_hyst:
+		hyst_reg = LM92_REG_TEMP_HIGH;
+		break;
+	case hwmon_temp_crit_hyst:
+		hyst_reg = LM92_REG_TEMP_CRIT;
+		break;
+	case hwmon_temp_min_alarm:
+		alarm_bit = 0;
+		break;
+	case hwmon_temp_max_alarm:
+		alarm_bit = 1;
+		break;
+	case hwmon_temp_crit_alarm:
+		alarm_bit = 2;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	if (reg >= 0) {
+		ret = regmap_read(regmap, reg, &temp);
+		if (ret < 0)
+			return ret;
+		*val = TEMP_FROM_REG(temp);
+	} else if (hyst_reg >= 0) {
+		u32 regs[2] = { hyst_reg, LM92_REG_TEMP_HYST };
+		u16 regvals[2];
 
-	val = clamp_val(val, -120000, 220000);
-	mutex_lock(&data->update_lock);
-	err = regmap_read(regmap, LM92_REG_TEMP_CRIT, &temp);
-	if (err)
-		goto unlock;
-	val = TEMP_TO_REG(TEMP_FROM_REG(temp) - val, data->resolution);
-	err = regmap_write(regmap, LM92_REG_TEMP_HYST, val);
+		ret = regmap_multi_reg_read(regmap, regs, regvals, 2);
+		if (ret)
+			return ret;
+		if (attr == hwmon_temp_min_hyst)
+			*val = TEMP_FROM_REG(regvals[0]) + TEMP_FROM_REG(regvals[1]);
+		else
+			*val = TEMP_FROM_REG(regvals[0]) - TEMP_FROM_REG(regvals[1]);
+	} else {
+		ret = regmap_read(regmap, LM92_REG_TEMP, &temp);
+		if (ret)
+			return ret;
+		*val = !!(temp & BIT(alarm_bit));
+	}
+	return 0;
+}
+
+static int lm92_chip_read(struct lm92_data *data, u32 attr, long *val)
+{
+	u32 temp;
+	int ret;
+
+	switch (attr) {
+	case hwmon_chip_alarms:
+		ret = regmap_read(data->regmap, LM92_REG_TEMP, &temp);
+		if (ret)
+			return ret;
+		*val = ALARMS_FROM_REG(temp);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
+static int lm92_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
+		     int channel, long *val)
+{
+	struct lm92_data *data = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_chip:
+		return lm92_chip_read(data, attr, val);
+	case hwmon_temp:
+		return lm92_temp_read(data, attr, channel, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int lm92_temp_write(struct lm92_data *data, u32 attr, long val)
+{
+	struct regmap *regmap = data->regmap;
+	int reg, err;
+	u32 temp;
+
+	switch (attr) {
+	case hwmon_temp_min:
+		reg = LM92_REG_TEMP_LOW;
+		break;
+	case hwmon_temp_max:
+		reg = LM92_REG_TEMP_HIGH;
+		break;
+	case hwmon_temp_crit:
+		reg = LM92_REG_TEMP_CRIT;
+		break;
+	case hwmon_temp_crit_hyst:
+		val = clamp_val(val, -120000, 220000);
+		mutex_lock(&data->update_lock);
+		err = regmap_read(regmap, LM92_REG_TEMP_CRIT, &temp);
+		if (err)
+			goto unlock;
+		val = TEMP_TO_REG(TEMP_FROM_REG(temp) - val, data->resolution);
+		err = regmap_write(regmap, LM92_REG_TEMP_HYST, val);
 unlock:
-	mutex_unlock(&data->update_lock);
-	if (err)
+		mutex_unlock(&data->update_lock);
 		return err;
-	return count;
+	default:
+		return -EOPNOTSUPP;
+	}
+	return regmap_write(regmap, reg, TEMP_TO_REG(val, data->resolution));
 }
 
-static ssize_t alarms_show(struct device *dev, struct device_attribute *attr,
-			   char *buf)
+static int lm92_write(struct device *dev, enum hwmon_sensor_types type,
+		      u32 attr, int channel, long val)
 {
 	struct lm92_data *data = dev_get_drvdata(dev);
-	u32 temp;
-	int err;
 
-	err = regmap_read(data->regmap, LM92_REG_TEMP, &temp);
-	if (err)
-		return err;
-
-	return sprintf(buf, "%d\n", ALARMS_FROM_REG(temp));
+	switch (type) {
+	case hwmon_temp:
+		return lm92_temp_write(data, attr, val);
+	default:
+		return -EOPNOTSUPP;
+	}
 }
 
-static ssize_t alarm_show(struct device *dev, struct device_attribute *attr,
-			  char *buf)
+static umode_t lm92_is_visible(const void *_data, enum hwmon_sensor_types type,
+			       u32 attr, int channel)
 {
-	struct lm92_data *data = dev_get_drvdata(dev);
-	int bitnr = to_sensor_dev_attr(attr)->index;
-	u32 temp;
-	int err;
-
-	err = regmap_read(data->regmap, LM92_REG_TEMP, &temp);
-	if (err)
-		return err;
-
-	return sprintf(buf, "%d\n", (temp >> bitnr) & 1);
+	switch (type) {
+	case hwmon_chip:
+		switch (attr) {
+		case hwmon_chip_alarms:
+			return 0444;
+		default:
+			break;
+		}
+		break;
+	case hwmon_temp:
+		switch (attr) {
+		case hwmon_temp_min:
+		case hwmon_temp_max:
+		case hwmon_temp_crit:
+		case hwmon_temp_crit_hyst:
+			return 0644;
+		case hwmon_temp_input:
+		case hwmon_temp_min_hyst:
+		case hwmon_temp_max_hyst:
+		case hwmon_temp_min_alarm:
+		case hwmon_temp_max_alarm:
+		case hwmon_temp_crit_alarm:
+			return 0444;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+	return 0;
 }
 
-static SENSOR_DEVICE_ATTR_RO(temp1_input, temp, t_input);
-static SENSOR_DEVICE_ATTR_RW(temp1_crit, temp, t_crit);
-static SENSOR_DEVICE_ATTR_RW(temp1_crit_hyst, temp_hyst, t_crit);
-static SENSOR_DEVICE_ATTR_RW(temp1_min, temp, t_min);
-static DEVICE_ATTR_RO(temp1_min_hyst);
-static SENSOR_DEVICE_ATTR_RW(temp1_max, temp, t_max);
-static SENSOR_DEVICE_ATTR_RO(temp1_max_hyst, temp_hyst, t_max);
-static DEVICE_ATTR_RO(alarms);
-static SENSOR_DEVICE_ATTR_RO(temp1_crit_alarm, alarm, 2);
-static SENSOR_DEVICE_ATTR_RO(temp1_min_alarm, alarm, 0);
-static SENSOR_DEVICE_ATTR_RO(temp1_max_alarm, alarm, 1);
+static const struct hwmon_channel_info * const lm92_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_ALARMS),
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT |
+			   HWMON_T_MIN | HWMON_T_MIN_HYST |
+			   HWMON_T_MAX | HWMON_T_MAX_HYST |
+			   HWMON_T_CRIT | HWMON_T_CRIT_HYST |
+			   HWMON_T_MIN_ALARM | HWMON_T_MAX_ALARM |
+			   HWMON_T_CRIT_ALARM),
+	NULL
+};
+
+static const struct hwmon_ops lm92_hwmon_ops = {
+	.is_visible = lm92_is_visible,
+	.read = lm92_read,
+	.write = lm92_write,
+};
+
+static const struct hwmon_chip_info lm92_chip_info = {
+	.ops = &lm92_hwmon_ops,
+	.info = lm92_info,
+};
 
 /*
  * Detection and registration
@@ -249,22 +295,6 @@ static int lm92_init_client(struct regmap *regmap)
 	return regmap_clear_bits(regmap, LM92_REG_CONFIG, 0x01);
 }
 
-static struct attribute *lm92_attrs[] = {
-	&sensor_dev_attr_temp1_input.dev_attr.attr,
-	&sensor_dev_attr_temp1_crit.dev_attr.attr,
-	&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp1_min.dev_attr.attr,
-	&dev_attr_temp1_min_hyst.attr,
-	&sensor_dev_attr_temp1_max.dev_attr.attr,
-	&sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
-	&dev_attr_alarms.attr,
-	&sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
-	NULL
-};
-ATTRIBUTE_GROUPS(lm92);
-
 /* Return 0 if detection is successful, -ENODEV otherwise */
 static int lm92_detect(struct i2c_client *new_client,
 		       struct i2c_board_info *info)
@@ -373,9 +403,8 @@ static int lm92_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
-	hwmon_dev = devm_hwmon_device_register_with_groups(dev,
-							   client->name,
-							   data, lm92_groups);
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
+							 &lm92_chip_info, NULL);
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
-- 
2.39.2

[PATCH 6/6] hwmon: (lm92) Update documentation

[Guenter Roeck]

Update datasheet references. Replace misleading 'force parameter needed'
with 'must be instantiated explicitly'. Explain the reason for the missing
auto-detection. Mention all supported chips in Kconfig.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/hwmon/lm92.rst | 26 +++++++++++++-------------
 drivers/hwmon/Kconfig        |  2 +-
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/Documentation/hwmon/lm92.rst b/Documentation/hwmon/lm92.rst
index c131b923ed36..d71cdb2af339 100644
--- a/Documentation/hwmon/lm92.rst
+++ b/Documentation/hwmon/lm92.rst
@@ -3,29 +3,29 @@ Kernel driver lm92
 
 Supported chips:
 
-  * National Semiconductor LM92
+  * National Semiconductor / Texas Instruments LM92
 
     Prefix: 'lm92'
 
     Addresses scanned: I2C 0x48 - 0x4b
 
-    Datasheet: http://www.national.com/pf/LM/LM92.html
+    Datasheet: https://www.ti.com/lit/gpn/LM92
 
-  * National Semiconductor LM76
+  * National Semiconductor / Texas Instruments LM76
 
     Prefix: 'lm92'
 
-    Addresses scanned: none, force parameter needed
+    Addresses scanned: none, must be instantiated explicitly
 
-    Datasheet: http://www.national.com/pf/LM/LM76.html
+    Datasheet: https://www.ti.com/lit/gpn/LM76
 
-  * Maxim MAX6633/MAX6634/MAX6635
+  * Maxim /Analog Devices MAX6633/MAX6634/MAX6635
 
     Prefix: 'max6635'
 
-    Addresses scanned: none, force parameter needed
+    Addresses scanned: none, must be instantiated explicitly
 
-    Datasheet: http://www.maxim-ic.com/quick_view2.cfm/qv_pk/3074
+    Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/max6633-max6635.pdf
 
 
 Authors:
@@ -36,13 +36,13 @@ Authors:
 Description
 -----------
 
-This driver implements support for the National Semiconductor LM92
-temperature sensor.
+This driver implements support for the National Semiconductor / Texas
+Instruments LM92 temperature sensor.
 
 Each LM92 temperature sensor supports a single temperature sensor. There are
 alarms for high, low, and critical thresholds. There's also an hysteresis to
 control the thresholds for resetting alarms.
 
-Support was added later for the LM76 and Maxim MAX6633/MAX6634/MAX6635,
-which are mostly compatible. They have not all been tested, so you
-may need to use the force parameter.
+The driver also supports LM76 and Maxim MAX6633/MAX6634/MAX6635, which are
+mostly compatible but do not have a vendor ID register and therefore must be
+instantiated explicitly.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index b2ace4857130..702ed8502aa2 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1514,7 +1514,7 @@ config SENSORS_LM92
 	select REGMAP_I2C
 	help
 	  If you say yes here you get support for National Semiconductor LM92
-	  and Maxim MAX6635 sensor chips.
+	  and LM76 as well as Maxim MAX6633/6634/6635 sensor chips.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called lm92.
-- 
2.39.2

Re: [PATCH 1/6] hwmon: (lm92) Improve auto-detection accuracy

[Tzung-Bi Shih]

On Thu, Aug 01, 2024 at 07:49:13AM -0700, Guenter Roeck wrote:
> Checking three configuration register bits and the manufacturer ID
> register to auto-detect LM92 is a bit vague. Repeat twice on replicated
> register addresses to improve detection accuracy.  Check the manufacturer
> ID first and bail out immediately without reading the other register if
> there is a mismatch to reduce the number of i2c transfers needed in that
> case. Also explicitly test for an error from reading the configuration
> register to avoid potential situations where the returned error masked
> against 0xe0 is 0.
> 
> While at it, drop "lm92: Found National Semiconductor LM92 chip" detection
> noise.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 2/6] hwmon: (lm92) Reorder include files to alphabetic order

[Tzung-Bi Shih]

On Thu, Aug 01, 2024 at 07:49:14AM -0700, Guenter Roeck wrote:
> Simplify driver maintenance by reordering files to alphabetic order.
> 
> No functional change.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 3/6] hwmon: (lm92) Replace chip IDs with limit register resolution

[Tzung-Bi Shih]

On Thu, Aug 01, 2024 at 07:49:15AM -0700, Guenter Roeck wrote:
> The chip IDs are not used by the driver. Drop them. Use driver data to
> store the limit register resolution instead, and use this information
> when writing temperature limits to improve chip specific rounding and
> to avoid writing into unused register bits.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 4/6] hwmon: (lm92) Convert to use regmap

[Tzung-Bi Shih]

On Thu, Aug 01, 2024 at 07:49:16AM -0700, Guenter Roeck wrote:
> Use regmap for local caching and for multi-byte operations to be able
> to use regmap API functions and to reduce the need for locking in the
> driver.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 5/6] hwmon: (lm92) Convert to with_info hwmon API

[Tzung-Bi Shih]

On Thu, Aug 01, 2024 at 07:49:17AM -0700, Guenter Roeck wrote:
> Convert driver to with_info hwmon API to simplify the code and
> to reduce its size.
> 
> No functional change.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 6/6] hwmon: (lm92) Update documentation

[Tzung-Bi Shih]

On Thu, Aug 01, 2024 at 07:49:18AM -0700, Guenter Roeck wrote:
> Update datasheet references. Replace misleading 'force parameter needed'
> with 'must be instantiated explicitly'. Explain the reason for the missing
> auto-detection. Mention all supported chips in Kconfig.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

Re: [PATCH 6/6] hwmon: (lm92) Update documentation

[Guenter Roeck]

On 8/1/24 22:12, Tzung-Bi Shih wrote:
> On Thu, Aug 01, 2024 at 07:49:18AM -0700, Guenter Roeck wrote:
>> Update datasheet references. Replace misleading 'force parameter needed'
>> with 'must be instantiated explicitly'. Explain the reason for the missing
>> auto-detection. Mention all supported chips in Kconfig.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
> 

Thanks a lot for the reviews!

Guenter