[PATCH v3 0/1] hwmon:max6697: Allow max6581 to create tempX_offset

[PATCH v3 0/1] hwmon:max6697: Allow max6581 to create tempX_offset

[Chu Lin]

Per max6581, reg 4d and reg 4e is used for temperature read offset.
This patch will let the user specify the temperature read offset for
max6581. This patch is tested on max6581 and only applies to max6581.

Testing:
echo 16250 > temp2_offset
cat temp2_offset
16250

echo 17500 > temp3_offset
cat temp3_offset
17500
cat temp4_offset
0
cat temp2_offset
17500

echo 0 > temp2_offset
cat temp2_offset
0
cat temp3_offset
17500

echo -0 > temp2_offset
cat temp2_offset
0

echo -100000 > temp2_offset
cat temp2_input
4875

echo 10000 > temp2_offset
cat temp2_input
47125

echo -2000 > temp2_offset
cat temp2_input
34875

echo -0 > temp2_offset
cat temp2_input
37000

Signed-off-by: Chu Lin <linchuyuan@google.com>
---
ChangeLog v2 -> v3:
  - Use reverse christmas tree order convension
  - Fix the type issue where comparision is always true
  - Change the line limit to 100 char instead of 80 char

ChangeLog v1 -> v2:
  - Simplify the offset reg raw value to milli ceisus conversion
  - Substitute the temp1_offset with dummy attr
  - Avoid using double negative in the macro definition
  - Return the actual error when i2c read/write is failed
  - clamp the value to MAX or MIN respectively if an out of range input is given
  - Provide mux protection when multiple i2c accesses is required

Chu Lin (1):
  hwmon:max6697: Allow max6581 to create tempX_offset attributes

 drivers/hwmon/max6697.c | 92 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 4 deletions(-)

-- 
2.27.0.383.g050319c2ae-goog

[PATCH v3 1/1] hwmon:max6697: Allows max6581 to create tempX_offset attributes

[Chu Lin]

Testing:
echo 16250 > temp2_offset
cat temp2_offset
16250

echo 17500 > temp3_offset
cat temp3_offset
17500
cat temp4_offset
0
cat temp2_offset
17500

echo 0 > temp2_offset
cat temp2_offset
0
cat temp3_offset
17500

echo -0 > temp2_offset
cat temp2_offset
0

echo -100000 > temp2_offset
cat temp2_input
4875

echo 10000 > temp2_offset
cat temp2_input
47125

echo -2000 > temp2_offset
cat temp2_input
34875

echo -0 > temp2_offset
cat temp2_input
37000

Signed-off-by: Chu Lin <linchuyuan@google.com>
---
 drivers/hwmon/max6697.c | 92 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
index 64122eb38060..6b213e146fbe 100644
--- a/drivers/hwmon/max6697.c
+++ b/drivers/hwmon/max6697.c
@@ -57,6 +57,9 @@ static const u8 MAX6697_REG_CRIT[] = {
 #define MAX6581_REG_IDEALITY_SELECT	0x4c
 #define MAX6581_REG_OFFSET		0x4d
 #define MAX6581_REG_OFFSET_SELECT	0x4e
+#define MAX6581_OFFSET_MIN		-31750
+#define MAX6581_OFFSET_MAX		31750
+
 
 #define MAX6697_CONV_TIME		156	/* ms per channel, worst case */
 
@@ -172,6 +175,11 @@ static const struct max6697_chip_data max6697_chip_data[] = {
 	},
 };
 
+static inline int max6581_offset_to_millic(int val)
+{
+	return val & (1 << 7) ? (val | 0xffffff00) * 250 : val * 250;
+}
+
 static struct max6697_data *max6697_update_device(struct device *dev)
 {
 	struct max6697_data *data = dev_get_drvdata(dev);
@@ -317,6 +325,61 @@ static ssize_t temp_store(struct device *dev,
 	return ret < 0 ? ret : count;
 }
 
+static ssize_t offset_store(struct device *dev,
+			    struct device_attribute *devattr, const char *buf,
+			    size_t count)
+{
+	int val, ret, index, select;
+	struct max6697_data *data;
+	long temp;
+
+	index = to_sensor_dev_attr(devattr)->index;
+	data = dev_get_drvdata(dev);
+	select = i2c_smbus_read_byte_data(data->client, MAX6581_REG_OFFSET_SELECT);
+	if (select < 0)
+		return select;
+	ret = kstrtol(buf, 10, &temp);
+	if (ret < 0)
+		return ret;
+	/* disable the offset for channel */
+	if (temp == 0) {
+		ret = i2c_smbus_write_byte_data(data->client, MAX6581_REG_OFFSET_SELECT,
+						select & ~(1 << (index - 1)));
+		return ret < 0 ? ret : count;
+	}
+	temp = clamp_val(temp, MAX6581_OFFSET_MIN, MAX6581_OFFSET_MAX);
+	val = DIV_ROUND_CLOSEST(temp, 250);
+	mutex_lock(&data->update_lock);
+	ret = i2c_smbus_write_byte_data(data->client, MAX6581_REG_OFFSET_SELECT,
+					select | (1 << (index - 1)));
+	if (ret < 0)
+		return ret;
+	ret = i2c_smbus_write_byte_data(data->client, MAX6581_REG_OFFSET, val);
+	mutex_unlock(&data->update_lock);
+	return ret < 0 ? ret : count;
+}
+
+static ssize_t offset_show(struct device *dev, struct device_attribute *devattr,
+			   char *buf)
+{
+	struct max6697_data *data;
+	int select, ret, index;
+
+	index = to_sensor_dev_attr(devattr)->index;
+	data = dev_get_drvdata(dev);
+	select = i2c_smbus_read_byte_data(data->client, MAX6581_REG_OFFSET_SELECT);
+	if (select < 0)
+		return select;
+	if (select & (1 << (index - 1))) {
+		ret = i2c_smbus_read_byte_data(data->client, MAX6581_REG_OFFSET);
+		if (ret < 0)
+			return ret;
+	} else {
+		return sprintf(buf, "%d\n", 0);
+	}
+	return sprintf(buf, "%d\n", max6581_offset_to_millic(ret));
+}
+
 static SENSOR_DEVICE_ATTR_RO(temp1_input, temp_input, 0);
 static SENSOR_DEVICE_ATTR_2_RW(temp1_max, temp, 0, MAX6697_TEMP_MAX);
 static SENSOR_DEVICE_ATTR_2_RW(temp1_crit, temp, 0, MAX6697_TEMP_CRIT);
@@ -375,6 +438,15 @@ static SENSOR_DEVICE_ATTR_RO(temp6_fault, alarm, 5);
 static SENSOR_DEVICE_ATTR_RO(temp7_fault, alarm, 6);
 static SENSOR_DEVICE_ATTR_RO(temp8_fault, alarm, 7);
 
+/* There is no offset for local temperature so starting from temp2 */
+static SENSOR_DEVICE_ATTR_RW(temp2_offset, offset, 1);
+static SENSOR_DEVICE_ATTR_RW(temp3_offset, offset, 2);
+static SENSOR_DEVICE_ATTR_RW(temp4_offset, offset, 3);
+static SENSOR_DEVICE_ATTR_RW(temp5_offset, offset, 4);
+static SENSOR_DEVICE_ATTR_RW(temp6_offset, offset, 5);
+static SENSOR_DEVICE_ATTR_RW(temp7_offset, offset, 6);
+static SENSOR_DEVICE_ATTR_RW(temp8_offset, offset, 7);
+
 static DEVICE_ATTR(dummy, 0, NULL, NULL);
 
 static umode_t max6697_is_visible(struct kobject *kobj, struct attribute *attr,
@@ -383,8 +455,8 @@ static umode_t max6697_is_visible(struct kobject *kobj, struct attribute *attr,
 	struct device *dev = container_of(kobj, struct device, kobj);
 	struct max6697_data *data = dev_get_drvdata(dev);
 	const struct max6697_chip_data *chip = data->chip;
-	int channel = index / 6;	/* channel number */
-	int nr = index % 6;		/* attribute index within channel */
+	int channel = index / 8;	/* channel number */
+	int nr = index % 7;		/* attribute index within channel */
 
 	if (channel >= chip->channels)
 		return 0;
@@ -393,6 +465,10 @@ static umode_t max6697_is_visible(struct kobject *kobj, struct attribute *attr,
 		return 0;
 	if (nr == 5 && !(chip->have_fault & (1 << channel)))
 		return 0;
+	/* offset reg is only supported on max6581 remote channels */
+	if (nr == 6)
+		if (data->type != max6581 || channel == 0)
+			return 0;
 
 	return attr->mode;
 }
@@ -409,6 +485,7 @@ static struct attribute *max6697_attributes[] = {
 	&sensor_dev_attr_temp1_crit.dev_attr.attr,
 	&sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
 	&dev_attr_dummy.attr,
+	&dev_attr_dummy.attr,
 
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_max.dev_attr.attr,
@@ -416,6 +493,7 @@ static struct attribute *max6697_attributes[] = {
 	&sensor_dev_attr_temp2_crit.dev_attr.attr,
 	&sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_offset.dev_attr.attr,
 
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_max.dev_attr.attr,
@@ -423,6 +501,7 @@ static struct attribute *max6697_attributes[] = {
 	&sensor_dev_attr_temp3_crit.dev_attr.attr,
 	&sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_offset.dev_attr.attr,
 
 	&sensor_dev_attr_temp4_input.dev_attr.attr,
 	&sensor_dev_attr_temp4_max.dev_attr.attr,
@@ -430,6 +509,7 @@ static struct attribute *max6697_attributes[] = {
 	&sensor_dev_attr_temp4_crit.dev_attr.attr,
 	&sensor_dev_attr_temp4_crit_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp4_fault.dev_attr.attr,
+	&sensor_dev_attr_temp4_offset.dev_attr.attr,
 
 	&sensor_dev_attr_temp5_input.dev_attr.attr,
 	&sensor_dev_attr_temp5_max.dev_attr.attr,
@@ -437,6 +517,7 @@ static struct attribute *max6697_attributes[] = {
 	&sensor_dev_attr_temp5_crit.dev_attr.attr,
 	&sensor_dev_attr_temp5_crit_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp5_fault.dev_attr.attr,
+	&sensor_dev_attr_temp5_offset.dev_attr.attr,
 
 	&sensor_dev_attr_temp6_input.dev_attr.attr,
 	&sensor_dev_attr_temp6_max.dev_attr.attr,
@@ -444,6 +525,7 @@ static struct attribute *max6697_attributes[] = {
 	&sensor_dev_attr_temp6_crit.dev_attr.attr,
 	&sensor_dev_attr_temp6_crit_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp6_fault.dev_attr.attr,
+	&sensor_dev_attr_temp6_offset.dev_attr.attr,
 
 	&sensor_dev_attr_temp7_input.dev_attr.attr,
 	&sensor_dev_attr_temp7_max.dev_attr.attr,
@@ -451,6 +533,7 @@ static struct attribute *max6697_attributes[] = {
 	&sensor_dev_attr_temp7_crit.dev_attr.attr,
 	&sensor_dev_attr_temp7_crit_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp7_fault.dev_attr.attr,
+	&sensor_dev_attr_temp7_offset.dev_attr.attr,
 
 	&sensor_dev_attr_temp8_input.dev_attr.attr,
 	&sensor_dev_attr_temp8_max.dev_attr.attr,
@@ -458,6 +541,7 @@ static struct attribute *max6697_attributes[] = {
 	&sensor_dev_attr_temp8_crit.dev_attr.attr,
 	&sensor_dev_attr_temp8_crit_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp8_fault.dev_attr.attr,
+	&sensor_dev_attr_temp8_offset.dev_attr.attr,
 	NULL
 };
 
@@ -494,8 +578,8 @@ static void max6697_get_config_of(struct device_node *node,
 	}
 	prop = of_get_property(node, "transistor-ideality", &len);
 	if (prop && len == 2 * sizeof(u32)) {
-			pdata->ideality_mask = be32_to_cpu(prop[0]);
-			pdata->ideality_value = be32_to_cpu(prop[1]);
+		pdata->ideality_mask = be32_to_cpu(prop[0]);
+		pdata->ideality_value = be32_to_cpu(prop[1]);
 	}
 }
 
-- 
2.27.0.383.g050319c2ae-goog

Re: [PATCH v3 1/1] hwmon:max6697: Allows max6581 to create tempX_offset attributes

[Guenter Roeck]

On 7/6/20 6:18 PM, Chu Lin wrote:
> Testing:
> echo 16250 > temp2_offset
> cat temp2_offset
> 16250
> 
> echo 17500 > temp3_offset
> cat temp3_offset
> 17500
> cat temp4_offset
> 0
> cat temp2_offset
> 17500
> 
> echo 0 > temp2_offset
> cat temp2_offset
> 0
> cat temp3_offset
> 17500
> 
> echo -0 > temp2_offset
> cat temp2_offset
> 0
> 
> echo -100000 > temp2_offset
> cat temp2_input
> 4875
> 
> echo 10000 > temp2_offset
> cat temp2_input
> 47125
> 
> echo -2000 > temp2_offset
> cat temp2_input
> 34875
> 
> echo -0 > temp2_offset
> cat temp2_input
> 37000
> 
> Signed-off-by: Chu Lin <linchuyuan@google.com>

Technically this is patch v4, but never mind.

> ---
>  drivers/hwmon/max6697.c | 92 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 88 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
> index 64122eb38060..6b213e146fbe 100644
> --- a/drivers/hwmon/max6697.c
> +++ b/drivers/hwmon/max6697.c
> @@ -57,6 +57,9 @@ static const u8 MAX6697_REG_CRIT[] = {
>  #define MAX6581_REG_IDEALITY_SELECT	0x4c
>  #define MAX6581_REG_OFFSET		0x4d
>  #define MAX6581_REG_OFFSET_SELECT	0x4e
> +#define MAX6581_OFFSET_MIN		-31750
> +#define MAX6581_OFFSET_MAX		31750
> +
>  
Pease no double empty lines.

>  #define MAX6697_CONV_TIME		156	/* ms per channel, worst case */
>  
> @@ -172,6 +175,11 @@ static const struct max6697_chip_data max6697_chip_data[] = {
>  	},
>  };
>  
> +static inline int max6581_offset_to_millic(int val)
> +{
> +	return val & (1 << 7) ? (val | 0xffffff00) * 250 : val * 250;

	return sign_extend32(val, 7) * 250;

> +}
> +
>  static struct max6697_data *max6697_update_device(struct device *dev)
>  {
>  	struct max6697_data *data = dev_get_drvdata(dev);
> @@ -317,6 +325,61 @@ static ssize_t temp_store(struct device *dev,
>  	return ret < 0 ? ret : count;
>  }
>  
> +static ssize_t offset_store(struct device *dev,
> +			    struct device_attribute *devattr, const char *buf,
> +			    size_t count)

I don't think this needs three lines with the new line length limit.

> +{
> +	int val, ret, index, select;
> +	struct max6697_data *data;
> +	long temp;
> +
> +	index = to_sensor_dev_attr(devattr)->index;
> +	data = dev_get_drvdata(dev);
> +	select = i2c_smbus_read_byte_data(data->client, MAX6581_REG_OFFSET_SELECT);
> +	if (select < 0)
> +		return select;

Unfortunately, this is racy: The select register can be changed by another write
after it was read here. The kstrtol() can be outside the lock, but the entire
sequence of i2c reads and writes has to be mutex protected.

> +	ret = kstrtol(buf, 10, &temp);
> +	if (ret < 0)
> +		return ret;
> +	/* disable the offset for channel */

                                          ... if the new offset is 0.

> +	if (temp == 0) {

This now has to be after the assignment of val below, and must be "if (val == 0)".

> +		ret = i2c_smbus_write_byte_data(data->client, MAX6581_REG_OFFSET_SELECT,
> +						select & ~(1 << (index - 1)));
> +		return ret < 0 ? ret : count;
> +	}
> +	temp = clamp_val(temp, MAX6581_OFFSET_MIN, MAX6581_OFFSET_MAX);
> +	val = DIV_ROUND_CLOSEST(temp, 250);
> +	mutex_lock(&data->update_lock);
> +	ret = i2c_smbus_write_byte_data(data->client, MAX6581_REG_OFFSET_SELECT,> +					select | (1 << (index - 1)));

It might make sense to update the select register only if it actually changes
its value. The cost for checking this is much less than the cost of the additional
i2c write.

> +	if (ret < 0)
> +		return ret;

Forgot to release the lock - needs goto

> +	ret = i2c_smbus_write_byte_data(data->client, MAX6581_REG_OFFSET, val);
> +	mutex_unlock(&data->update_lock);
> +	return ret < 0 ? ret : count;
> +}
> +
> +static ssize_t offset_show(struct device *dev, struct device_attribute *devattr,
> +			   char *buf)

Single line should be sufficient.

> +{
> +	struct max6697_data *data;
> +	int select, ret, index;
> +
> +	index = to_sensor_dev_attr(devattr)->index;
> +	data = dev_get_drvdata(dev);
> +	select = i2c_smbus_read_byte_data(data->client, MAX6581_REG_OFFSET_SELECT);
> +	if (select < 0)
> +		return select;
> +	if (select & (1 << (index - 1))) {
> +		ret = i2c_smbus_read_byte_data(data->client, MAX6581_REG_OFFSET);
> +		if (ret < 0)
> +			return ret;
> +	} else {
> +		return sprintf(buf, "%d\n", 0);

Better make this "ret = 0;".

Also, this is racy: select can be changed after the first read.
Not sure if that matters much, though.

> +	}
> +	return sprintf(buf, "%d\n", max6581_offset_to_millic(ret));
> +}
> +
>  static SENSOR_DEVICE_ATTR_RO(temp1_input, temp_input, 0);
>  static SENSOR_DEVICE_ATTR_2_RW(temp1_max, temp, 0, MAX6697_TEMP_MAX);
>  static SENSOR_DEVICE_ATTR_2_RW(temp1_crit, temp, 0, MAX6697_TEMP_CRIT);
> @@ -375,6 +438,15 @@ static SENSOR_DEVICE_ATTR_RO(temp6_fault, alarm, 5);
>  static SENSOR_DEVICE_ATTR_RO(temp7_fault, alarm, 6);
>  static SENSOR_DEVICE_ATTR_RO(temp8_fault, alarm, 7);
>  
> +/* There is no offset for local temperature so starting from temp2 */
> +static SENSOR_DEVICE_ATTR_RW(temp2_offset, offset, 1);
> +static SENSOR_DEVICE_ATTR_RW(temp3_offset, offset, 2);
> +static SENSOR_DEVICE_ATTR_RW(temp4_offset, offset, 3);
> +static SENSOR_DEVICE_ATTR_RW(temp5_offset, offset, 4);
> +static SENSOR_DEVICE_ATTR_RW(temp6_offset, offset, 5);
> +static SENSOR_DEVICE_ATTR_RW(temp7_offset, offset, 6);
> +static SENSOR_DEVICE_ATTR_RW(temp8_offset, offset, 7);
> +
>  static DEVICE_ATTR(dummy, 0, NULL, NULL);
>  
>  static umode_t max6697_is_visible(struct kobject *kobj, struct attribute *attr,
> @@ -383,8 +455,8 @@ static umode_t max6697_is_visible(struct kobject *kobj, struct attribute *attr,
>  	struct device *dev = container_of(kobj, struct device, kobj);
>  	struct max6697_data *data = dev_get_drvdata(dev);
>  	const struct max6697_chip_data *chip = data->chip;
> -	int channel = index / 6;	/* channel number */
> -	int nr = index % 6;		/* attribute index within channel */
> +	int channel = index / 8;	/* channel number */
> +	int nr = index % 7;		/* attribute index within channel */

Something is wrong here: "/ 6" and "% 6" changed to "/ 8" and "% 7".
Why "/ 8" ?

>  
>  	if (channel >= chip->channels)
>  		return 0;
> @@ -393,6 +465,10 @@ static umode_t max6697_is_visible(struct kobject *kobj, struct attribute *attr,
>  		return 0;
>  	if (nr == 5 && !(chip->have_fault & (1 << channel)))
>  		return 0;
> +	/* offset reg is only supported on max6581 remote channels */
> +	if (nr == 6)
> +		if (data->type != max6581 || channel == 0)
> +			return 0;
>  
>  	return attr->mode;
>  }
> @@ -409,6 +485,7 @@ static struct attribute *max6697_attributes[] = {
>  	&sensor_dev_attr_temp1_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
>  	&dev_attr_dummy.attr,
> +	&dev_attr_dummy.attr,
>  
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_max.dev_attr.attr,
> @@ -416,6 +493,7 @@ static struct attribute *max6697_attributes[] = {
>  	&sensor_dev_attr_temp2_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
>  	&sensor_dev_attr_temp2_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp2_offset.dev_attr.attr,
>  
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_max.dev_attr.attr,
> @@ -423,6 +501,7 @@ static struct attribute *max6697_attributes[] = {
>  	&sensor_dev_attr_temp3_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp3_offset.dev_attr.attr,
>  
>  	&sensor_dev_attr_temp4_input.dev_attr.attr,
>  	&sensor_dev_attr_temp4_max.dev_attr.attr,
> @@ -430,6 +509,7 @@ static struct attribute *max6697_attributes[] = {
>  	&sensor_dev_attr_temp4_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp4_crit_alarm.dev_attr.attr,
>  	&sensor_dev_attr_temp4_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp4_offset.dev_attr.attr,
>  
>  	&sensor_dev_attr_temp5_input.dev_attr.attr,
>  	&sensor_dev_attr_temp5_max.dev_attr.attr,
> @@ -437,6 +517,7 @@ static struct attribute *max6697_attributes[] = {
>  	&sensor_dev_attr_temp5_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp5_crit_alarm.dev_attr.attr,
>  	&sensor_dev_attr_temp5_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp5_offset.dev_attr.attr,
>  
>  	&sensor_dev_attr_temp6_input.dev_attr.attr,
>  	&sensor_dev_attr_temp6_max.dev_attr.attr,
> @@ -444,6 +525,7 @@ static struct attribute *max6697_attributes[] = {
>  	&sensor_dev_attr_temp6_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp6_crit_alarm.dev_attr.attr,
>  	&sensor_dev_attr_temp6_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp6_offset.dev_attr.attr,
>  
>  	&sensor_dev_attr_temp7_input.dev_attr.attr,
>  	&sensor_dev_attr_temp7_max.dev_attr.attr,
> @@ -451,6 +533,7 @@ static struct attribute *max6697_attributes[] = {
>  	&sensor_dev_attr_temp7_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp7_crit_alarm.dev_attr.attr,
>  	&sensor_dev_attr_temp7_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp7_offset.dev_attr.attr,
>  
>  	&sensor_dev_attr_temp8_input.dev_attr.attr,
>  	&sensor_dev_attr_temp8_max.dev_attr.attr,
> @@ -458,6 +541,7 @@ static struct attribute *max6697_attributes[] = {
>  	&sensor_dev_attr_temp8_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp8_crit_alarm.dev_attr.attr,
>  	&sensor_dev_attr_temp8_fault.dev_attr.attr,
> +	&sensor_dev_attr_temp8_offset.dev_attr.attr,
>  	NULL
>  };
>  
> @@ -494,8 +578,8 @@ static void max6697_get_config_of(struct device_node *node,
>  	}
>  	prop = of_get_property(node, "transistor-ideality", &len);
>  	if (prop && len == 2 * sizeof(u32)) {
> -			pdata->ideality_mask = be32_to_cpu(prop[0]);
> -			pdata->ideality_value = be32_to_cpu(prop[1]);
> +		pdata->ideality_mask = be32_to_cpu(prop[0]);
> +		pdata->ideality_value = be32_to_cpu(prop[1]);

Unrelated change. I don't mind if you want to make that change,
but it would have to be in a separate patch.

>  	}
>  }
>  
> 

Re: [PATCH v3 0/1] hwmon:max6697: Allow max6581 to create tempX_offset

[Guenter Roeck]

On 7/6/20 6:18 PM, Chu Lin wrote:
> Per max6581, reg 4d and reg 4e is used for temperature read offset.
> This patch will let the user specify the temperature read offset for
> max6581. This patch is tested on max6581 and only applies to max6581.
> 

Since this is a single patch, you don't need patch 0.
Just add the change log after "---" to the actual patch.

Thanks,
Guenter

> Testing:
> echo 16250 > temp2_offset
> cat temp2_offset
> 16250
> 
> echo 17500 > temp3_offset
> cat temp3_offset
> 17500
> cat temp4_offset
> 0
> cat temp2_offset
> 17500
> 
> echo 0 > temp2_offset
> cat temp2_offset
> 0
> cat temp3_offset
> 17500
> 
> echo -0 > temp2_offset
> cat temp2_offset
> 0
> 
> echo -100000 > temp2_offset
> cat temp2_input
> 4875
> 
> echo 10000 > temp2_offset
> cat temp2_input
> 47125
> 
> echo -2000 > temp2_offset
> cat temp2_input
> 34875
> 
> echo -0 > temp2_offset
> cat temp2_input
> 37000
> 
> Signed-off-by: Chu Lin <linchuyuan@google.com>
> ---
> ChangeLog v2 -> v3:
>   - Use reverse christmas tree order convension
>   - Fix the type issue where comparision is always true
>   - Change the line limit to 100 char instead of 80 char
> 
> ChangeLog v1 -> v2:
>   - Simplify the offset reg raw value to milli ceisus conversion
>   - Substitute the temp1_offset with dummy attr
>   - Avoid using double negative in the macro definition
>   - Return the actual error when i2c read/write is failed
>   - clamp the value to MAX or MIN respectively if an out of range input is given
>   - Provide mux protection when multiple i2c accesses is required
> 
> Chu Lin (1):
>   hwmon:max6697: Allow max6581 to create tempX_offset attributes
> 
>  drivers/hwmon/max6697.c | 92 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 88 insertions(+), 4 deletions(-)
>