[PATCH v2 0/3] hwmon: (emc1403) Various improvements

[PATCH v2 0/3] hwmon: (emc1403) Various improvements

[Guenter Roeck]

The first patch of the series converts the driver to register with the
hwmon subsystem using devm_hwmon_device_register_with_info() instead of
devm_hwmon_device_register_with_groups().

The second patch adds support for 11-bit sensor and limit register
accuracy.

The third patch adds support for update_interval attribute.

The changes were module tested with the script at
https://github.com/groeck/module-tests/blob/master/scripts/emc1403.sh

v2: Reworked first patch of the series to prepare for subsequent patches.
    Added patch 2 and 3.

----------------------------------------------------------------
Guenter Roeck (3):
      hwmon: (emc1403) Convert to with_info API
      hwmon: (emc1403) Support 11 bit accuracy
      hwmon: (emc1403) Add support for conversion interval configuration

 drivers/hwmon/emc1403.c | 749 +++++++++++++++++++++++++++++-------------------
 1 file changed, 453 insertions(+), 296 deletions(-)

[PATCH v2 1/3] hwmon: (emc1403) Convert to with_info API

[Guenter Roeck]

Convert driver to register with the hwmon subsystem using
devm_hwmon_device_register_with_info() instead of
devm_hwmon_device_register_with_groups() to simplify the code
and to reduce its size. As side effect, this also fixes a couple
of overflow problems when writing limit and hysteresis registers.

Cc: Lars Petter Mostad <lars.petter.mostad@appear.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Reworked to prepare for subsequent patches.
    No functional changes.

 drivers/hwmon/emc1403.c | 635 +++++++++++++++++++++-------------------
 1 file changed, 338 insertions(+), 297 deletions(-)

diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
index d370efd6f986..e656f0432a57 100644
--- a/drivers/hwmon/emc1403.c
+++ b/drivers/hwmon/emc1403.c
@@ -27,294 +27,47 @@
 enum emc1403_chip { emc1402, emc1403, emc1404 };
 
 struct thermal_data {
+	enum emc1403_chip chip;
 	struct regmap *regmap;
 	struct mutex mutex;
-	const struct attribute_group *groups[4];
 };
 
-static ssize_t temp_show(struct device *dev, struct device_attribute *attr,
-			 char *buf)
+static ssize_t power_state_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	struct sensor_device_attribute *sda = to_sensor_dev_attr(attr);
 	struct thermal_data *data = dev_get_drvdata(dev);
 	unsigned int val;
 	int retval;
 
-	retval = regmap_read(data->regmap, sda->index, &val);
+	retval = regmap_read(data->regmap, 0x03, &val);
 	if (retval < 0)
 		return retval;
-	return sprintf(buf, "%d000\n", val);
+	return sprintf(buf, "%d\n", !!(val & BIT(6)));
 }
 
-static ssize_t bit_show(struct device *dev, struct device_attribute *attr,
-			char *buf)
+static ssize_t power_state_store(struct device *dev, struct device_attribute *attr,
+				 const char *buf, size_t count)
 {
-	struct sensor_device_attribute_2 *sda = to_sensor_dev_attr_2(attr);
-	struct thermal_data *data = dev_get_drvdata(dev);
-	unsigned int val;
-	int retval;
-
-	retval = regmap_read(data->regmap, sda->nr, &val);
-	if (retval < 0)
-		return retval;
-	return sprintf(buf, "%d\n", !!(val & sda->index));
-}
-
-static ssize_t temp_store(struct device *dev, struct device_attribute *attr,
-			  const char *buf, size_t count)
-{
-	struct sensor_device_attribute *sda = to_sensor_dev_attr(attr);
 	struct thermal_data *data = dev_get_drvdata(dev);
 	unsigned long val;
 	int retval;
 
 	if (kstrtoul(buf, 10, &val))
 		return -EINVAL;
-	retval = regmap_write(data->regmap, sda->index,
-			      DIV_ROUND_CLOSEST(val, 1000));
+
+	retval = regmap_update_bits(data->regmap, 0x03, BIT(6),
+				    val ? BIT(6) : 0);
 	if (retval < 0)
 		return retval;
 	return count;
 }
 
-static ssize_t bit_store(struct device *dev, struct device_attribute *attr,
-			 const char *buf, size_t count)
-{
-	struct sensor_device_attribute_2 *sda = to_sensor_dev_attr_2(attr);
-	struct thermal_data *data = dev_get_drvdata(dev);
-	unsigned long val;
-	int retval;
-
-	if (kstrtoul(buf, 10, &val))
-		return -EINVAL;
-
-	retval = regmap_update_bits(data->regmap, sda->nr, sda->index,
-				    val ? sda->index : 0);
-	if (retval < 0)
-		return retval;
-	return count;
-}
-
-static ssize_t show_hyst_common(struct device *dev,
-				struct device_attribute *attr, char *buf,
-				bool is_min)
-{
-	struct sensor_device_attribute *sda = to_sensor_dev_attr(attr);
-	struct thermal_data *data = dev_get_drvdata(dev);
-	struct regmap *regmap = data->regmap;
-	unsigned int limit;
-	unsigned int hyst;
-	int retval;
-
-	retval = regmap_read(regmap, sda->index, &limit);
-	if (retval < 0)
-		return retval;
-
-	retval = regmap_read(regmap, 0x21, &hyst);
-	if (retval < 0)
-		return retval;
-
-	return sprintf(buf, "%d000\n", is_min ? limit + hyst : limit - hyst);
-}
-
-static ssize_t hyst_show(struct device *dev, struct device_attribute *attr,
-			 char *buf)
-{
-	return show_hyst_common(dev, attr, buf, false);
-}
-
-static ssize_t min_hyst_show(struct device *dev,
-			     struct device_attribute *attr, char *buf)
-{
-	return show_hyst_common(dev, attr, buf, true);
-}
-
-static ssize_t hyst_store(struct device *dev, struct device_attribute *attr,
-			  const char *buf, size_t count)
-{
-	struct sensor_device_attribute *sda = to_sensor_dev_attr(attr);
-	struct thermal_data *data = dev_get_drvdata(dev);
-	struct regmap *regmap = data->regmap;
-	unsigned int limit;
-	int retval;
-	int hyst;
-	unsigned long val;
-
-	if (kstrtoul(buf, 10, &val))
-		return -EINVAL;
-
-	mutex_lock(&data->mutex);
-	retval = regmap_read(regmap, sda->index, &limit);
-	if (retval < 0)
-		goto fail;
-
-	hyst = limit * 1000 - val;
-	hyst = clamp_val(DIV_ROUND_CLOSEST(hyst, 1000), 0, 255);
-	retval = regmap_write(regmap, 0x21, hyst);
-	if (retval == 0)
-		retval = count;
-fail:
-	mutex_unlock(&data->mutex);
-	return retval;
-}
-
-/*
- *	Sensors. We pass the actual i2c register to the methods.
- */
-
-static SENSOR_DEVICE_ATTR_RW(temp1_min, temp, 0x06);
-static SENSOR_DEVICE_ATTR_RW(temp1_max, temp, 0x05);
-static SENSOR_DEVICE_ATTR_RW(temp1_crit, temp, 0x20);
-static SENSOR_DEVICE_ATTR_RO(temp1_input, temp, 0x00);
-static SENSOR_DEVICE_ATTR_2_RO(temp1_min_alarm, bit, 0x36, 0x01);
-static SENSOR_DEVICE_ATTR_2_RO(temp1_max_alarm, bit, 0x35, 0x01);
-static SENSOR_DEVICE_ATTR_2_RO(temp1_crit_alarm, bit, 0x37, 0x01);
-static SENSOR_DEVICE_ATTR_RO(temp1_min_hyst, min_hyst, 0x06);
-static SENSOR_DEVICE_ATTR_RO(temp1_max_hyst, hyst, 0x05);
-static SENSOR_DEVICE_ATTR_RW(temp1_crit_hyst, hyst, 0x20);
-
-static SENSOR_DEVICE_ATTR_RW(temp2_min, temp, 0x08);
-static SENSOR_DEVICE_ATTR_RW(temp2_max, temp, 0x07);
-static SENSOR_DEVICE_ATTR_RW(temp2_crit, temp, 0x19);
-static SENSOR_DEVICE_ATTR_RO(temp2_input, temp, 0x01);
-static SENSOR_DEVICE_ATTR_2_RO(temp2_fault, bit, 0x1b, 0x02);
-static SENSOR_DEVICE_ATTR_2_RO(temp2_min_alarm, bit, 0x36, 0x02);
-static SENSOR_DEVICE_ATTR_2_RO(temp2_max_alarm, bit, 0x35, 0x02);
-static SENSOR_DEVICE_ATTR_2_RO(temp2_crit_alarm, bit, 0x37, 0x02);
-static SENSOR_DEVICE_ATTR_RO(temp2_min_hyst, min_hyst, 0x08);
-static SENSOR_DEVICE_ATTR_RO(temp2_max_hyst, hyst, 0x07);
-static SENSOR_DEVICE_ATTR_RO(temp2_crit_hyst, hyst, 0x19);
-
-static SENSOR_DEVICE_ATTR_RW(temp3_min, temp, 0x16);
-static SENSOR_DEVICE_ATTR_RW(temp3_max, temp, 0x15);
-static SENSOR_DEVICE_ATTR_RW(temp3_crit, temp, 0x1A);
-static SENSOR_DEVICE_ATTR_RO(temp3_input, temp, 0x23);
-static SENSOR_DEVICE_ATTR_2_RO(temp3_fault, bit, 0x1b, 0x04);
-static SENSOR_DEVICE_ATTR_2_RO(temp3_min_alarm, bit, 0x36, 0x04);
-static SENSOR_DEVICE_ATTR_2_RO(temp3_max_alarm, bit, 0x35, 0x04);
-static SENSOR_DEVICE_ATTR_2_RO(temp3_crit_alarm, bit, 0x37, 0x04);
-static SENSOR_DEVICE_ATTR_RO(temp3_min_hyst, min_hyst, 0x16);
-static SENSOR_DEVICE_ATTR_RO(temp3_max_hyst, hyst, 0x15);
-static SENSOR_DEVICE_ATTR_RO(temp3_crit_hyst, hyst, 0x1A);
-
-static SENSOR_DEVICE_ATTR_RW(temp4_min, temp, 0x2D);
-static SENSOR_DEVICE_ATTR_RW(temp4_max, temp, 0x2C);
-static SENSOR_DEVICE_ATTR_RW(temp4_crit, temp, 0x30);
-static SENSOR_DEVICE_ATTR_RO(temp4_input, temp, 0x2A);
-static SENSOR_DEVICE_ATTR_2_RO(temp4_fault, bit, 0x1b, 0x08);
-static SENSOR_DEVICE_ATTR_2_RO(temp4_min_alarm, bit, 0x36, 0x08);
-static SENSOR_DEVICE_ATTR_2_RO(temp4_max_alarm, bit, 0x35, 0x08);
-static SENSOR_DEVICE_ATTR_2_RO(temp4_crit_alarm, bit, 0x37, 0x08);
-static SENSOR_DEVICE_ATTR_RO(temp4_min_hyst, min_hyst, 0x2D);
-static SENSOR_DEVICE_ATTR_RO(temp4_max_hyst, hyst, 0x2C);
-static SENSOR_DEVICE_ATTR_RO(temp4_crit_hyst, hyst, 0x30);
-
-static SENSOR_DEVICE_ATTR_2_RW(power_state, bit, 0x03, 0x40);
-
-static struct attribute *emc1402_attrs[] = {
-	&sensor_dev_attr_temp1_min.dev_attr.attr,
-	&sensor_dev_attr_temp1_max.dev_attr.attr,
-	&sensor_dev_attr_temp1_crit.dev_attr.attr,
-	&sensor_dev_attr_temp1_input.dev_attr.attr,
-	&sensor_dev_attr_temp1_min_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
-
-	&sensor_dev_attr_temp2_min.dev_attr.attr,
-	&sensor_dev_attr_temp2_max.dev_attr.attr,
-	&sensor_dev_attr_temp2_crit.dev_attr.attr,
-	&sensor_dev_attr_temp2_input.dev_attr.attr,
-	&sensor_dev_attr_temp2_min_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp2_max_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
-
-	&sensor_dev_attr_power_state.dev_attr.attr,
-	NULL
-};
-
-static const struct attribute_group emc1402_group = {
-		.attrs = emc1402_attrs,
-};
+static DEVICE_ATTR_RW(power_state);
 
 static struct attribute *emc1403_attrs[] = {
-	&sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
-
-	&sensor_dev_attr_temp2_fault.dev_attr.attr,
-	&sensor_dev_attr_temp2_min_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
-
-	&sensor_dev_attr_temp3_min.dev_attr.attr,
-	&sensor_dev_attr_temp3_max.dev_attr.attr,
-	&sensor_dev_attr_temp3_crit.dev_attr.attr,
-	&sensor_dev_attr_temp3_input.dev_attr.attr,
-	&sensor_dev_attr_temp3_fault.dev_attr.attr,
-	&sensor_dev_attr_temp3_min_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp3_min_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp3_max_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
+	&dev_attr_power_state.attr,
 	NULL
 };
-
-static const struct attribute_group emc1403_group = {
-	.attrs = emc1403_attrs,
-};
-
-static struct attribute *emc1404_attrs[] = {
-	&sensor_dev_attr_temp4_min.dev_attr.attr,
-	&sensor_dev_attr_temp4_max.dev_attr.attr,
-	&sensor_dev_attr_temp4_crit.dev_attr.attr,
-	&sensor_dev_attr_temp4_input.dev_attr.attr,
-	&sensor_dev_attr_temp4_fault.dev_attr.attr,
-	&sensor_dev_attr_temp4_min_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp4_max_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp4_crit_alarm.dev_attr.attr,
-	&sensor_dev_attr_temp4_min_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp4_max_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp4_crit_hyst.dev_attr.attr,
-	NULL
-};
-
-static const struct attribute_group emc1404_group = {
-	.attrs = emc1404_attrs,
-};
-
-/*
- * EMC14x2 uses a different register and different bits to report alarm and
- * fault status. For simplicity, provide a separate attribute group for this
- * chip series.
- * Since we can not re-use the same attribute names, create a separate attribute
- * array.
- */
-static struct sensor_device_attribute_2 emc1402_alarms[] = {
-	SENSOR_ATTR_2_RO(temp1_min_alarm, bit, 0x02, 0x20),
-	SENSOR_ATTR_2_RO(temp1_max_alarm, bit, 0x02, 0x40),
-	SENSOR_ATTR_2_RO(temp1_crit_alarm, bit, 0x02, 0x01),
-
-	SENSOR_ATTR_2_RO(temp2_fault, bit, 0x02, 0x04),
-	SENSOR_ATTR_2_RO(temp2_min_alarm, bit, 0x02, 0x08),
-	SENSOR_ATTR_2_RO(temp2_max_alarm, bit, 0x02, 0x10),
-	SENSOR_ATTR_2_RO(temp2_crit_alarm, bit, 0x02, 0x02),
-};
-
-static struct attribute *emc1402_alarm_attrs[] = {
-	&emc1402_alarms[0].dev_attr.attr,
-	&emc1402_alarms[1].dev_attr.attr,
-	&emc1402_alarms[2].dev_attr.attr,
-	&emc1402_alarms[3].dev_attr.attr,
-	&emc1402_alarms[4].dev_attr.attr,
-	&emc1402_alarms[5].dev_attr.attr,
-	&emc1402_alarms[6].dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group emc1402_alarm_group = {
-	.attrs = emc1402_alarm_attrs,
-};
+ATTRIBUTE_GROUPS(emc1403);
 
 static int emc1403_detect(struct i2c_client *client,
 			struct i2c_board_info *info)
@@ -389,51 +142,310 @@ static const struct regmap_config emc1403_regmap_config = {
 	.volatile_reg = emc1403_regmap_is_volatile,
 };
 
-static const struct i2c_device_id emc1403_idtable[];
+enum emc1403_reg_map {temp_min, temp_max, temp_crit, temp_input};
 
-static int emc1403_probe(struct i2c_client *client)
+static u8 ema1403_temp_map[] = {
+	[hwmon_temp_min] = temp_min,
+	[hwmon_temp_max] = temp_max,
+	[hwmon_temp_crit] = temp_crit,
+	[hwmon_temp_input] = temp_input,
+};
+
+static u8 emc1403_temp_regs[][4] = {
+	[0] = {
+		[temp_min] = 0x06,
+		[temp_max] = 0x05,
+		[temp_crit] = 0x20,
+		[temp_input] = 0x00,
+	},
+	[1] = {
+		[temp_min] = 0x08,
+		[temp_max] = 0x07,
+		[temp_crit] = 0x19,
+		[temp_input] = 0x01,
+	},
+	[2] = {
+		[temp_min] = 0x16,
+		[temp_max] = 0x15,
+		[temp_crit] = 0x1a,
+		[temp_input] = 0x23,
+	},
+	[3] = {
+		[temp_min] = 0x2d,
+		[temp_max] = 0x2c,
+		[temp_crit] = 0x30,
+		[temp_input] = 0x2a,
+	},
+};
+
+static int __emc1403_get_temp(struct thermal_data *data, int channel,
+			      enum emc1403_reg_map map, long *val)
 {
-	struct thermal_data *data;
-	struct device *hwmon_dev;
-	const struct i2c_device_id *id = i2c_match_id(emc1403_idtable, client);
+	unsigned int regval;
+	int ret;
 
-	data = devm_kzalloc(&client->dev, sizeof(struct thermal_data),
-			    GFP_KERNEL);
-	if (data == NULL)
-		return -ENOMEM;
+	ret = regmap_read(data->regmap, emc1403_temp_regs[channel][map], &regval);
+	if (ret < 0)
+		return ret;
+	*val = regval * 1000;
 
-	data->regmap = devm_regmap_init_i2c(client, &emc1403_regmap_config);
-	if (IS_ERR(data->regmap))
-		return PTR_ERR(data->regmap);
-
-	mutex_init(&data->mutex);
-
-	switch (id->driver_data) {
-	case emc1404:
-		data->groups[2] = &emc1404_group;
-		fallthrough;
-	case emc1403:
-		data->groups[1] = &emc1403_group;
-		fallthrough;
-	case emc1402:
-		data->groups[0] = &emc1402_group;
-	}
-
-	if (id->driver_data == emc1402)
-		data->groups[1] = &emc1402_alarm_group;
-
-	hwmon_dev = devm_hwmon_device_register_with_groups(&client->dev,
-							   client->name, data,
-							   data->groups);
-	if (IS_ERR(hwmon_dev))
-		return PTR_ERR(hwmon_dev);
-
-	dev_info(&client->dev, "%s Thermal chip found\n", id->name);
 	return 0;
 }
 
-static const unsigned short emc1403_address_list[] = {
-	0x18, 0x1c, 0x29, 0x3c, 0x4c, 0x4d, 0x5c, I2C_CLIENT_END
+static int emc1403_get_temp(struct thermal_data *data, int channel,
+			    enum emc1403_reg_map map, long *val)
+{
+	int ret;
+
+	mutex_lock(&data->mutex);
+	ret = __emc1403_get_temp(data, channel, map, val);
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+
+static int emc1403_get_hyst(struct thermal_data *data, int channel,
+			    enum emc1403_reg_map map, long *val)
+{
+	int hyst, ret;
+	long limit;
+
+	mutex_lock(&data->mutex);
+	ret = __emc1403_get_temp(data, channel, map, &limit);
+	if (ret < 0)
+		goto unlock;
+	ret = regmap_read(data->regmap, 0x21, &hyst);
+	if (ret < 0)
+		goto unlock;
+	if (map == temp_min)
+		*val = limit + hyst * 1000;
+	else
+		*val = limit - hyst * 1000;
+unlock:
+	mutex_unlock(&data->mutex);
+	return ret;
+}
+
+static int emc1403_temp_read(struct thermal_data *data, u32 attr, int channel, long *val)
+{
+	unsigned int regval;
+	int ret;
+
+	switch (attr) {
+	case hwmon_temp_min:
+	case hwmon_temp_max:
+	case hwmon_temp_crit:
+	case hwmon_temp_input:
+		ret = emc1403_get_temp(data, channel, ema1403_temp_map[attr], val);
+		break;
+	case hwmon_temp_min_hyst:
+		ret = emc1403_get_hyst(data, channel, temp_min, val);
+		break;
+	case hwmon_temp_max_hyst:
+		ret = emc1403_get_hyst(data, channel, temp_max, val);
+		break;
+	case hwmon_temp_crit_hyst:
+		ret = emc1403_get_hyst(data, channel, temp_crit, val);
+		break;
+	case hwmon_temp_min_alarm:
+		if (data->chip == emc1402) {
+			ret = regmap_read(data->regmap, 0x02, &regval);
+			if (ret < 0)
+				break;
+			*val = !!(regval & BIT(5 - 2 * channel));
+		} else {
+			ret = regmap_read(data->regmap, 0x36, &regval);
+			if (ret < 0)
+				break;
+			*val = !!(regval & BIT(channel));
+		}
+		break;
+	case hwmon_temp_max_alarm:
+		if (data->chip == emc1402) {
+			ret = regmap_read(data->regmap, 0x02, &regval);
+			if (ret < 0)
+				break;
+			*val = !!(regval & BIT(6 - 2 * channel));
+		} else {
+			ret = regmap_read(data->regmap, 0x35, &regval);
+			if (ret < 0)
+				break;
+			*val = !!(regval & BIT(channel));
+		}
+		break;
+	case hwmon_temp_crit_alarm:
+		if (data->chip == emc1402) {
+			ret = regmap_read(data->regmap, 0x02, &regval);
+			if (ret < 0)
+				break;
+			*val = !!(regval & BIT(channel));
+		} else {
+			ret = regmap_read(data->regmap, 0x37, &regval);
+			if (ret < 0)
+				break;
+			*val = !!(regval & BIT(channel));
+		}
+		break;
+	case hwmon_temp_fault:
+		ret = regmap_read(data->regmap, 0x1b, &regval);
+		if (ret < 0)
+			break;
+		*val = !!(regval & BIT(channel));
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	return ret;
+}
+
+static int emc1403_read(struct device *dev, enum hwmon_sensor_types type,
+			u32 attr, int channel, long *val)
+{
+	struct thermal_data *data = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_temp:
+		return emc1403_temp_read(data, attr, channel, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int emc1403_set_hyst(struct thermal_data *data, long val)
+{
+	int hyst, ret;
+	long limit;
+
+	val = clamp_val(val, 0, 255000);
+
+	mutex_lock(&data->mutex);
+	ret = __emc1403_get_temp(data, 0, temp_crit, &limit);
+	if (ret < 0)
+		goto unlock;
+
+	hyst = limit - val;
+	hyst = clamp_val(DIV_ROUND_CLOSEST(hyst, 1000), 0, 255);
+	ret = regmap_write(data->regmap, 0x21, hyst);
+unlock:
+	mutex_unlock(&data->mutex);
+	return ret;
+}
+
+static int emc1403_set_temp(struct thermal_data *data, int channel,
+			    enum emc1403_reg_map map, long val)
+{
+	unsigned int regval;
+	int ret;
+
+	val = clamp_val(val, 0, 255000);
+
+	mutex_lock(&data->mutex);
+	regval = DIV_ROUND_CLOSEST(val, 1000);
+	ret = regmap_write(data->regmap, emc1403_temp_regs[channel][map], regval);
+	mutex_unlock(&data->mutex);
+	return ret;
+}
+
+static int emc1403_temp_write(struct thermal_data *data, u32 attr, int channel, long val)
+{
+	switch (attr) {
+	case hwmon_temp_min:
+	case hwmon_temp_max:
+	case hwmon_temp_crit:
+		return emc1403_set_temp(data, channel, ema1403_temp_map[attr], val);
+	case hwmon_temp_crit_hyst:
+		return emc1403_set_hyst(data, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int emc1403_write(struct device *dev, enum hwmon_sensor_types type,
+			 u32 attr, int channel, long val)
+{
+	struct thermal_data *data = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_temp:
+		return emc1403_temp_write(data, attr, channel, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static umode_t emc1403_temp_is_visible(const void *_data, u32 attr, int channel)
+{
+	const struct thermal_data *data = _data;
+
+	if (data->chip == emc1402 && channel > 1)
+		return 0;
+	if (data->chip == emc1403 && channel > 2)
+		return 0;
+
+	switch (attr) {
+	case hwmon_temp_input:
+	case hwmon_temp_min_alarm:
+	case hwmon_temp_max_alarm:
+	case hwmon_temp_crit_alarm:
+	case hwmon_temp_fault:
+	case hwmon_temp_min_hyst:
+	case hwmon_temp_max_hyst:
+		return 0444;
+	case hwmon_temp_min:
+	case hwmon_temp_max:
+	case hwmon_temp_crit:
+		return 0644;
+	case hwmon_temp_crit_hyst:
+		if (channel == 0)
+			return 0644;
+		return 0444;
+	default:
+		return 0;
+	}
+}
+
+static umode_t emc1403_is_visible(const void *data, enum hwmon_sensor_types type,
+				  u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_temp:
+		return emc1403_temp_is_visible(data, attr, channel);
+	default:
+		return 0;
+	}
+}
+
+static const struct hwmon_channel_info * const emc1403_info[] = {
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX |
+			   HWMON_T_CRIT | HWMON_T_MIN_HYST | HWMON_T_MAX_HYST |
+			   HWMON_T_CRIT_HYST | HWMON_T_MIN_ALARM |
+			   HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM,
+			   HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX |
+			   HWMON_T_CRIT | HWMON_T_MIN_HYST | HWMON_T_MAX_HYST |
+			   HWMON_T_CRIT_HYST | HWMON_T_MIN_ALARM |
+			   HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM | HWMON_T_FAULT,
+			   HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX |
+			   HWMON_T_CRIT | HWMON_T_MIN_HYST | HWMON_T_MAX_HYST |
+			   HWMON_T_CRIT_HYST | HWMON_T_MIN_ALARM |
+			   HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM | HWMON_T_FAULT,
+			   HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX |
+			   HWMON_T_CRIT | HWMON_T_MIN_HYST | HWMON_T_MAX_HYST |
+			   HWMON_T_CRIT_HYST | HWMON_T_MIN_ALARM |
+			   HWMON_T_MAX_ALARM | HWMON_T_CRIT_ALARM | HWMON_T_FAULT
+			   ),
+	NULL
+};
+
+static const struct hwmon_ops emc1403_hwmon_ops = {
+	.is_visible = emc1403_is_visible,
+	.read = emc1403_read,
+	.write = emc1403_write,
+};
+
+static const struct hwmon_chip_info emc1403_chip_info = {
+	.ops = &emc1403_hwmon_ops,
+	.info = emc1403_info,
 };
 
 /* Last digit of chip name indicates number of channels */
@@ -452,6 +464,35 @@ static const struct i2c_device_id emc1403_idtable[] = {
 };
 MODULE_DEVICE_TABLE(i2c, emc1403_idtable);
 
+static int emc1403_probe(struct i2c_client *client)
+{
+	struct thermal_data *data;
+	struct device *hwmon_dev;
+	const struct i2c_device_id *id = i2c_match_id(emc1403_idtable, client);
+
+	data = devm_kzalloc(&client->dev, sizeof(struct thermal_data),
+			    GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->chip = id->driver_data;
+	data->regmap = devm_regmap_init_i2c(client, &emc1403_regmap_config);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+
+	mutex_init(&data->mutex);
+
+	hwmon_dev = devm_hwmon_device_register_with_info(&client->dev,
+							 client->name, data,
+							 &emc1403_chip_info,
+							 emc1403_groups);
+	return PTR_ERR_OR_ZERO(hwmon_dev);
+}
+
+static const unsigned short emc1403_address_list[] = {
+	0x18, 0x1c, 0x29, 0x3c, 0x4c, 0x4d, 0x5c, I2C_CLIENT_END
+};
+
 static struct i2c_driver sensor_emc1403 = {
 	.class = I2C_CLASS_HWMON,
 	.driver = {
-- 
2.39.2

[PATCH v2 2/3] hwmon: (emc1403) Support 11 bit accuracy

[Guenter Roeck]

Various temperature and limit registers support 11 bit accuracy.
Add support for it.

Cc: Lars Petter Mostad <lars.petter.mostad@appear.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Added patch

 drivers/hwmon/emc1403.c | 55 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
index e656f0432a57..e53bb4d8bc1b 100644
--- a/drivers/hwmon/emc1403.c
+++ b/drivers/hwmon/emc1403.c
@@ -178,17 +178,52 @@ static u8 emc1403_temp_regs[][4] = {
 	},
 };
 
+static s8 emc1403_temp_regs_low[][4] = {
+	[0] = {
+		[temp_min] = -1,
+		[temp_max] = -1,
+		[temp_crit] = -1,
+		[temp_input] = 0x29,
+	},
+	[1] = {
+		[temp_min] = 0x14,
+		[temp_max] = 0x13,
+		[temp_crit] = -1,
+		[temp_input] = 0x10,
+	},
+	[2] = {
+		[temp_min] = 0x18,
+		[temp_max] = 0x17,
+		[temp_crit] = -1,
+		[temp_input] = 0x24,
+	},
+	[3] = {
+		[temp_min] = 0x2f,
+		[temp_max] = 0x2e,
+		[temp_crit] = -1,
+		[temp_input] = 0x2b,
+	},
+};
+
 static int __emc1403_get_temp(struct thermal_data *data, int channel,
 			      enum emc1403_reg_map map, long *val)
 {
 	unsigned int regval;
 	int ret;
+	s8 reg;
 
 	ret = regmap_read(data->regmap, emc1403_temp_regs[channel][map], &regval);
 	if (ret < 0)
 		return ret;
 	*val = regval * 1000;
 
+	reg = emc1403_temp_regs_low[channel][map];
+	if (reg >= 0) {
+		ret = regmap_read(data->regmap, reg, &regval);
+		if (ret < 0)
+			return ret;
+		*val += (regval >> 5) * 125;
+	}
 	return 0;
 }
 
@@ -336,12 +371,26 @@ static int emc1403_set_temp(struct thermal_data *data, int channel,
 {
 	unsigned int regval;
 	int ret;
+	u8 regh;
+	s8 regl;
 
-	val = clamp_val(val, 0, 255000);
+	regh = emc1403_temp_regs[channel][map];
+	regl = emc1403_temp_regs_low[channel][map];
 
 	mutex_lock(&data->mutex);
-	regval = DIV_ROUND_CLOSEST(val, 1000);
-	ret = regmap_write(data->regmap, emc1403_temp_regs[channel][map], regval);
+	if (regl >= 0) {
+		val = clamp_val(val, 0, 255875);
+		regval = DIV_ROUND_CLOSEST(val, 125);
+		ret = regmap_write(data->regmap, regh, regval >> 3);
+		if (ret < 0)
+			goto unlock;
+		ret = regmap_write(data->regmap, regl, (regval & 0x07) << 5);
+	} else {
+		val = clamp_val(val, 0, 255000);
+		regval = DIV_ROUND_CLOSEST(val, 1000);
+		ret = regmap_write(data->regmap, regh, regval);
+	}
+unlock:
 	mutex_unlock(&data->mutex);
 	return ret;
 }
-- 
2.39.2

[PATCH v2 3/3] hwmon: (emc1403) Add support for conversion interval configuration

[Guenter Roeck]

The chips supported by the emc1403 driver support configurable
conversion rates. Add support for it.

Cc: Lars Petter Mostad <lars.petter.mostad@appear.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Added patch

 drivers/hwmon/emc1403.c | 67 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
index e53bb4d8bc1b..43322bb9aaa1 100644
--- a/drivers/hwmon/emc1403.c
+++ b/drivers/hwmon/emc1403.c
@@ -19,6 +19,7 @@
 #include <linux/sysfs.h>
 #include <linux/mutex.h>
 #include <linux/regmap.h>
+#include <linux/util_macros.h>
 
 #define THERMAL_PID_REG		0xfd
 #define THERMAL_SMSC_ID_REG	0xfe
@@ -333,6 +334,31 @@ static int emc1403_temp_read(struct thermal_data *data, u32 attr, int channel, l
 	return ret;
 }
 
+static int emc1403_get_convrate(struct thermal_data *data, long *val)
+{
+	unsigned int convrate;
+	int ret;
+
+	ret = regmap_read(data->regmap, 0x04, &convrate);
+	if (ret < 0)
+		return ret;
+	if (convrate > 10)
+		convrate = 4;
+
+	*val = 16000 >> convrate;
+	return 0;
+}
+
+static int emc1403_chip_read(struct thermal_data *data, u32 attr, long *val)
+{
+	switch (attr) {
+	case hwmon_chip_update_interval:
+		return emc1403_get_convrate(data, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int emc1403_read(struct device *dev, enum hwmon_sensor_types type,
 			u32 attr, int channel, long *val)
 {
@@ -341,6 +367,8 @@ static int emc1403_read(struct device *dev, enum hwmon_sensor_types type,
 	switch (type) {
 	case hwmon_temp:
 		return emc1403_temp_read(data, attr, channel, val);
+	case hwmon_chip:
+		return emc1403_chip_read(data, attr, val);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -409,6 +437,30 @@ static int emc1403_temp_write(struct thermal_data *data, u32 attr, int channel,
 	}
 }
 
+/* Lookup table for temperature conversion times in msec */
+static const u16 ina3221_conv_time[] = {
+	16000, 8000, 4000, 2000, 1000, 500, 250, 125, 62, 31, 16
+};
+
+static int emc1403_set_convrate(struct thermal_data *data, unsigned int interval)
+{
+	int convrate;
+
+	convrate = find_closest_descending(interval, ina3221_conv_time,
+					   ARRAY_SIZE(ina3221_conv_time));
+	return regmap_write(data->regmap, 0x04, convrate);
+}
+
+static int emc1403_chip_write(struct thermal_data *data, u32 attr, long val)
+{
+	switch (attr) {
+	case hwmon_chip_update_interval:
+		return emc1403_set_convrate(data, clamp_val(val, 0, 100000));
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static int emc1403_write(struct device *dev, enum hwmon_sensor_types type,
 			 u32 attr, int channel, long val)
 {
@@ -417,6 +469,8 @@ static int emc1403_write(struct device *dev, enum hwmon_sensor_types type,
 	switch (type) {
 	case hwmon_temp:
 		return emc1403_temp_write(data, attr, channel, val);
+	case hwmon_chip:
+		return emc1403_chip_write(data, attr, val);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -453,18 +507,31 @@ static umode_t emc1403_temp_is_visible(const void *_data, u32 attr, int channel)
 	}
 }
 
+static umode_t emc1403_chip_is_visible(const void *_data, u32 attr)
+{
+	switch (attr) {
+	case hwmon_chip_update_interval:
+		return 0644;
+	default:
+		return 0;
+	}
+}
+
 static umode_t emc1403_is_visible(const void *data, enum hwmon_sensor_types type,
 				  u32 attr, int channel)
 {
 	switch (type) {
 	case hwmon_temp:
 		return emc1403_temp_is_visible(data, attr, channel);
+	case hwmon_chip:
+		return emc1403_chip_is_visible(data, attr);
 	default:
 		return 0;
 	}
 }
 
 static const struct hwmon_channel_info * const emc1403_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
 	HWMON_CHANNEL_INFO(temp,
 			   HWMON_T_INPUT | HWMON_T_MIN | HWMON_T_MAX |
 			   HWMON_T_CRIT | HWMON_T_MIN_HYST | HWMON_T_MAX_HYST |
-- 
2.39.2

Re: [PATCH v2 0/3] hwmon: (emc1403) Various improvements

[Lars Petter Mostad]

I have tested these patches on EMC1438. This has also worked well for me.

Regards,
Lars Petter

Re: [PATCH v2 0/3] hwmon: (emc1403) Various improvements

[Guenter Roeck]

On Mon, May 06, 2024 at 04:44:12PM +0200, Lars Petter Mostad wrote:
> I have tested these patches on EMC1438. This has also worked well for me.
> 

Great, thanks a lot!

It would be great if you can reply to the patches with a
Tested-by: tag.

Also, could you send me a register dump of emc1438 ?
I'd like to use a real register dump for the unit test code if possible.

Thanks,
Guenter

Re: [PATCH v2 0/3] hwmon: (emc1403) Various improvements

[Lars Petter Mostad]

> It would be great if you can reply to the patches with a
> Tested-by: tag.

Tested-by: Lars Petter Mostad <lars.petter.mostad@appear.net>

Of course, I had to do some light modifications to the patches to be able
to test on emc1438. I will post these modifications as an RFC patch once
the above patches are available on the hwmon-next (right?) branch.

Note that checkpatch.pl --strict fails a couple of checks.

> Also, could you send me a register dump of emc1438 ?
> I'd like to use a real register dump for the unit test code if possible.

     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00: 2b 2d 00 80 06 55 00 55 00 80 06 55 00 55 00 00
10: 00 00 00 00 00 55 00 00 00 55 55 00 00 00 00 00
20: 55 0a 70 2d 00 0d 0d 00 00 80 00 00 55 00 00 00
30: 55 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40: 00 2b 60 00 00 2e c0 00 00 00 00 00 00 00 00 00
50: 55 00 00 00 55 00 00 00 55 00 00 00 55 00 00 00
60: 0e 00 00 00 55 55 55 55 00 00 00 00 00 00 00 00
70: 00 0f 0f 00 00 00 00 00 00 00 00 00 00 00 00 00
80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 6b 60 00 00 23 00 3f 13 ad 03 20 02
b0: 20 00 00 00 20 00 20 00 00 00 00 00 12 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 59 5d 00

Regards,
Lars Petter

Re: [PATCH v2 0/3] hwmon: (emc1403) Various improvements

[Guenter Roeck]

Hi Lars,

On Tue, May 07, 2024 at 01:27:44PM +0200, Lars Petter Mostad wrote:
> > It would be great if you can reply to the patches with a
> > Tested-by: tag.
> 
> Tested-by: Lars Petter Mostad <lars.petter.mostad@appear.net>
> 
> Of course, I had to do some light modifications to the patches to be able
> to test on emc1438. I will post these modifications as an RFC patch once
> the above patches are available on the hwmon-next (right?) branch.
> 
Yes. Patches are there now.

> Note that checkpatch.pl --strict fails a couple of checks.
> 
Odd; not for me. I see a couple of CHECK messages when running it on the
driver.

CHECK: struct mutex definition without comment
CHECK: Alignment should match open parenthesis

However I don't see any messages when running it on the patches.
Please send me the log.

> > Also, could you send me a register dump of emc1438 ?
> > I'd like to use a real register dump for the unit test code if possible.
> 
>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
> 00: 2b 2d 00 80 06 55 00 55 00 80 06 55 00 55 00 00
> 10: 00 00 00 00 00 55 00 00 00 55 55 00 00 00 00 00
> 20: 55 0a 70 2d 00 0d 0d 00 00 80 00 00 55 00 00 00
> 30: 55 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 40: 00 2b 60 00 00 2e c0 00 00 00 00 00 00 00 00 00
> 50: 55 00 00 00 55 00 00 00 55 00 00 00 55 00 00 00
> 60: 0e 00 00 00 55 55 55 55 00 00 00 00 00 00 00 00
> 70: 00 0f 0f 00 00 00 00 00 00 00 00 00 00 00 00 00
> 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> a0: 00 00 00 00 6b 60 00 00 23 00 3f 13 ad 03 20 02
> b0: 20 00 00 00 20 00 20 00 00 00 00 00 12 00 00 00
> c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 59 5d 00
> 
Great, thanks!

Guenter