* Re: [PATCH] hwmon:max6697: Allow max6581 to set temperature read offset
2020-06-30 17:43 [PATCH] hwmon:max6697: Allow max6581 to set temperature read offset Chu Lin
@ 2020-06-30 17:59 ` Guenter Roeck
2020-07-01 18:43 ` Chu Lin
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-06-30 17:59 UTC (permalink / raw)
To: Chu Lin
Cc: jdelvare, linux-hwmon, linux-kernel, jasonling, belgaied, zhongqil
On Tue, Jun 30, 2020 at 05:43:50PM +0000, 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.
>
> Testing:
> dts: temperature-read-offset = <0xde 0x0>;
This should not be implemented this way. Instead, the standard tempX_offset
attribute should be used, with appropriate code to determine affected
channels (since there is only one offset value, and since it can be enabled
per channel). A secondary question is if and how to set the temperature
offset via devicetree; that will have to be documented and approved by a
DT maintainer. I would think, though, that the offset should be specified
in milli-degrees C and not as raw chip data.
Thanks,
Guenter
>
> verify: iotools smbus_read8 <vbus> 0x4d 0x4e
> 0x6F
>
> Signed-off-by: Chu Lin <linchuyuan@google.com>
> ---
> drivers/hwmon/max6697.c | 19 +++++++++++++++++--
> include/linux/platform_data/max6697.h | 4 ++++
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
> index 743752a2467a..16c0f0995a12 100644
> --- a/drivers/hwmon/max6697.c
> +++ b/drivers/hwmon/max6697.c
> @@ -493,8 +493,13 @@ 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]);
> + }
> + prop = of_get_property(node, "temperature-read-offset", &len);
> + if (prop && len == 2 * sizeof(u32)) {
> + pdata->offset_mask = be32_to_cpu(prop[0]);
> + pdata->offset_value = be32_to_cpu(prop[1]);
> }
> }
>
> @@ -586,6 +591,16 @@ static int max6697_init_chip(struct max6697_data *data,
> pdata->ideality_mask >> 1);
> if (ret < 0)
> return ret;
> + ret = i2c_smbus_write_byte_data(client,
> + MAX6581_REG_OFFSET,
> + pdata->offset_value);
> + if (ret < 0)
> + return ret;
> + ret = i2c_smbus_write_byte_data(client,
> + MAX6581_REG_OFFSET_SELECT,
> + pdata->offset_mask >> 1);
> + if (ret < 0)
> + return ret;
> }
> done:
> data->update_interval = factor * MAX6697_CONV_TIME;
> diff --git a/include/linux/platform_data/max6697.h b/include/linux/platform_data/max6697.h
> index 6fbb70005541..ff98adfe9d8d 100644
> --- a/include/linux/platform_data/max6697.h
> +++ b/include/linux/platform_data/max6697.h
> @@ -28,6 +28,10 @@ struct max6697_platform_data {
> u8 ideality_value; /* transistor ideality as per
> * MAX6581 datasheet
> */
> + u8 offset_mask; /* set bit to 0 to disable */
> + u8 offset_value; /* temperature read offset as
> + * MAX6581 datasheet
> + */
> };
>
> #endif /* MAX6697_H */
> --
> 2.27.0.212.ge8ba1cc988-goog
>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] hwmon:max6697: Allow max6581 to set temperature read offset
2020-06-30 17:43 [PATCH] hwmon:max6697: Allow max6581 to set temperature read offset Chu Lin
2020-06-30 17:59 ` Guenter Roeck
@ 2020-07-01 18:43 ` Chu Lin
2020-07-01 18:45 ` Chu Lin
2020-07-02 1:42 ` [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset attributes Chu Lin
3 siblings, 0 replies; 10+ messages in thread
From: Chu Lin @ 2020-07-01 18:43 UTC (permalink / raw)
To: linux, jdelvare, linchuyuan, linux-hwmon, linux-kernel
Cc: jasonling, belgaied, zhongqil
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
-sh: echo: write error: Invalid argument
cat temp2_input
37000
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 | 131 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 126 insertions(+), 5 deletions(-)
diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
index 743752a2467a..d9f3f0c41495 100644
--- a/drivers/hwmon/max6697.c
+++ b/drivers/hwmon/max6697.c
@@ -41,6 +41,14 @@ static const u8 MAX6697_REG_CRIT[] = {
#define MAX6697_MAP_BITS(reg) ((((reg) & 0x7e) >> 1) | \
(((reg) & 0x01) << 6) | ((reg) & 0x80))
+#define MAX6581_OFFSET_TO_MILLIC(reg) (16000 * (((reg) & (1<<6)) >> 6) + \
+ 8000 * (((reg) & (1<<5)) >> 5) + \
+ 4000 * (((reg) & (1<<4)) >> 4) + \
+ 2000 * (((reg) & (1<<3)) >> 3) + \
+ 1000 * (((reg) & (1<<2)) >> 2) + \
+ 500 * (((reg) & (1<<1)) >> 1) + \
+ 250 * ((reg) & 1))
+
#define MAX6697_REG_STAT(n) (0x44 + (n))
#define MAX6697_REG_CONFIG 0x41
@@ -56,6 +64,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 -MAX6581_OFFSET_MIN
+
#define MAX6697_CONV_TIME 156 /* ms per channel, worst case */
@@ -316,6 +327,94 @@ 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)
+{
+ long temp;
+ u8 val, select;
+ int i, temp_frac, temp_int, ret, index;
+ bool sign;
+ struct max6697_data *data;
+
+ 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 -ENODATA;
+ ret = kstrtol(buf, 10, &temp);
+ if (ret < 0)
+ return ret;
+ if (temp > MAX6581_OFFSET_MAX || temp < MAX6581_OFFSET_MIN)
+ return -EINVAL;
+
+ /* 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;
+ }
+ sign = temp < 0 ? false : true;
+ temp = abs(temp);
+ temp_int = temp / 1000;
+ temp_frac = temp % 1000;
+ for (i = 4; i >= 0 && temp_int > 0; --i) {
+ if (temp_int >= 1<<i) {
+ temp_int -= 1<<i;
+ val |= 1 << (i+2);
+ }
+ }
+ /* special handle the fraction */
+ if (temp_frac % 250 != 0)
+ return -EINVAL;
+ if (temp_frac == 750)
+ val |= 0x3;
+ else if (temp_frac == 500)
+ val |= 0x2;
+ else if (temp_frac)
+ val |= 0x1;
+ 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,
+ sign?val:(~val + 1));
+ return ret < 0 ? ret : count;
+}
+
+static ssize_t offset_show(struct device *dev, struct device_attribute *devattr,
+ char *buf)
+{
+ int select, ret, index, temp;
+ struct max6697_data *data;
+
+ 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 -ENODATA;
+ if (select & (1<<(index-1))) {
+ ret = i2c_smbus_read_byte_data(data->client,
+ MAX6581_REG_OFFSET);
+ if (ret < 0)
+ return -ENODATA;
+ } else {
+ return sprintf(buf, "%d\n", 0);
+ }
+ if (1<<7 & ret) {
+ ret = ~ret + 1;
+ temp = -MAX6581_OFFSET_TO_MILLIC(ret);
+ } else {
+ temp = MAX6581_OFFSET_TO_MILLIC(ret);
+ }
+ return sprintf(buf, "%d\n", temp);
+}
+
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);
@@ -374,6 +473,16 @@ 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(temp1_offset, offset, 0);
+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,
@@ -382,8 +491,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;
@@ -392,6 +501,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;
}
@@ -408,6 +521,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,
+ &sensor_dev_attr_temp1_offset.dev_attr.attr,
&sensor_dev_attr_temp2_input.dev_attr.attr,
&sensor_dev_attr_temp2_max.dev_attr.attr,
@@ -415,6 +529,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,
@@ -422,6 +537,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,
@@ -429,6 +545,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,
@@ -436,6 +553,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,
@@ -443,6 +561,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,
@@ -450,6 +569,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,
@@ -457,6 +577,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
};
@@ -493,8 +614,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]);
}
}
@@ -689,7 +810,7 @@ MODULE_DEVICE_TABLE(of, max6697_of_match);
static struct i2c_driver max6697_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
- .name = "max6697",
+ .name = "max6697_test",
.of_match_table = of_match_ptr(max6697_of_match),
},
.probe = max6697_probe,
--
2.27.0.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] hwmon:max6697: Allow max6581 to set temperature read offset
2020-06-30 17:43 [PATCH] hwmon:max6697: Allow max6581 to set temperature read offset Chu Lin
2020-06-30 17:59 ` Guenter Roeck
2020-07-01 18:43 ` Chu Lin
@ 2020-07-01 18:45 ` Chu Lin
2020-07-02 1:42 ` [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset attributes Chu Lin
3 siblings, 0 replies; 10+ messages in thread
From: Chu Lin @ 2020-07-01 18:45 UTC (permalink / raw)
To: linux, jdelvare, linchuyuan, linux-hwmon, linux-kernel
Cc: jasonling, belgaied, zhongqil
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
-sh: echo: write error: Invalid argument
cat temp2_input
37000
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 | 129 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 125 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
index 743752a2467a..b11fcb594807 100644
--- a/drivers/hwmon/max6697.c
+++ b/drivers/hwmon/max6697.c
@@ -41,6 +41,14 @@ static const u8 MAX6697_REG_CRIT[] = {
#define MAX6697_MAP_BITS(reg) ((((reg) & 0x7e) >> 1) | \
(((reg) & 0x01) << 6) | ((reg) & 0x80))
+#define MAX6581_OFFSET_TO_MILLIC(reg) (16000 * (((reg) & (1<<6)) >> 6) + \
+ 8000 * (((reg) & (1<<5)) >> 5) + \
+ 4000 * (((reg) & (1<<4)) >> 4) + \
+ 2000 * (((reg) & (1<<3)) >> 3) + \
+ 1000 * (((reg) & (1<<2)) >> 2) + \
+ 500 * (((reg) & (1<<1)) >> 1) + \
+ 250 * ((reg) & 1))
+
#define MAX6697_REG_STAT(n) (0x44 + (n))
#define MAX6697_REG_CONFIG 0x41
@@ -56,6 +64,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 -MAX6581_OFFSET_MIN
+
#define MAX6697_CONV_TIME 156 /* ms per channel, worst case */
@@ -316,6 +327,94 @@ 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)
+{
+ long temp;
+ u8 val, select;
+ int i, temp_frac, temp_int, ret, index;
+ bool sign;
+ struct max6697_data *data;
+
+ 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 -ENODATA;
+ ret = kstrtol(buf, 10, &temp);
+ if (ret < 0)
+ return ret;
+ if (temp > MAX6581_OFFSET_MAX || temp < MAX6581_OFFSET_MIN)
+ return -EINVAL;
+
+ /* 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;
+ }
+ sign = temp < 0 ? false : true;
+ temp = abs(temp);
+ temp_int = temp / 1000;
+ temp_frac = temp % 1000;
+ for (i = 4; i >= 0 && temp_int > 0; --i) {
+ if (temp_int >= 1<<i) {
+ temp_int -= 1<<i;
+ val |= 1 << (i+2);
+ }
+ }
+ /* special handle the fraction */
+ if (temp_frac % 250 != 0)
+ return -EINVAL;
+ if (temp_frac == 750)
+ val |= 0x3;
+ else if (temp_frac == 500)
+ val |= 0x2;
+ else if (temp_frac)
+ val |= 0x1;
+ 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,
+ sign?val:(~val + 1));
+ return ret < 0 ? ret : count;
+}
+
+static ssize_t offset_show(struct device *dev, struct device_attribute *devattr,
+ char *buf)
+{
+ int select, ret, index, temp;
+ struct max6697_data *data;
+
+ 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 -ENODATA;
+ if (select & (1<<(index-1))) {
+ ret = i2c_smbus_read_byte_data(data->client,
+ MAX6581_REG_OFFSET);
+ if (ret < 0)
+ return -ENODATA;
+ } else {
+ return sprintf(buf, "%d\n", 0);
+ }
+ if (1<<7 & ret) {
+ ret = ~ret + 1;
+ temp = -MAX6581_OFFSET_TO_MILLIC(ret);
+ } else {
+ temp = MAX6581_OFFSET_TO_MILLIC(ret);
+ }
+ return sprintf(buf, "%d\n", temp);
+}
+
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);
@@ -374,6 +473,16 @@ 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(temp1_offset, offset, 0);
+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,
@@ -382,8 +491,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;
@@ -392,6 +501,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;
}
@@ -408,6 +521,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,
+ &sensor_dev_attr_temp1_offset.dev_attr.attr,
&sensor_dev_attr_temp2_input.dev_attr.attr,
&sensor_dev_attr_temp2_max.dev_attr.attr,
@@ -415,6 +529,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,
@@ -422,6 +537,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,
@@ -429,6 +545,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,
@@ -436,6 +553,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,
@@ -443,6 +561,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,
@@ -450,6 +569,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,
@@ -457,6 +577,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
};
@@ -493,8 +614,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.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset attributes
2020-06-30 17:43 [PATCH] hwmon:max6697: Allow max6581 to set temperature read offset Chu Lin
` (2 preceding siblings ...)
2020-07-01 18:45 ` Chu Lin
@ 2020-07-02 1:42 ` Chu Lin
2020-07-02 5:14 ` Guenter Roeck
2020-07-03 1:06 ` Guenter Roeck
3 siblings, 2 replies; 10+ messages in thread
From: Chu Lin @ 2020-07-02 1:42 UTC (permalink / raw)
To: linux, jdelvare, linchuyuan, linux-hwmon, linux-kernel
Cc: jasonling, belgaied, zhongqil
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
-sh: echo: write error: Invalid argument
cat temp2_input
37000
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 | 129 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 125 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
index 743752a2467a..b11fcb594807 100644
--- a/drivers/hwmon/max6697.c
+++ b/drivers/hwmon/max6697.c
@@ -41,6 +41,14 @@ static const u8 MAX6697_REG_CRIT[] = {
#define MAX6697_MAP_BITS(reg) ((((reg) & 0x7e) >> 1) | \
(((reg) & 0x01) << 6) | ((reg) & 0x80))
+#define MAX6581_OFFSET_TO_MILLIC(reg) (16000 * (((reg) & (1<<6)) >> 6) + \
+ 8000 * (((reg) & (1<<5)) >> 5) + \
+ 4000 * (((reg) & (1<<4)) >> 4) + \
+ 2000 * (((reg) & (1<<3)) >> 3) + \
+ 1000 * (((reg) & (1<<2)) >> 2) + \
+ 500 * (((reg) & (1<<1)) >> 1) + \
+ 250 * ((reg) & 1))
+
#define MAX6697_REG_STAT(n) (0x44 + (n))
#define MAX6697_REG_CONFIG 0x41
@@ -56,6 +64,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 -MAX6581_OFFSET_MIN
+
#define MAX6697_CONV_TIME 156 /* ms per channel, worst case */
@@ -316,6 +327,94 @@ 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)
+{
+ long temp;
+ u8 val, select;
+ int i, temp_frac, temp_int, ret, index;
+ bool sign;
+ struct max6697_data *data;
+
+ 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 -ENODATA;
+ ret = kstrtol(buf, 10, &temp);
+ if (ret < 0)
+ return ret;
+ if (temp > MAX6581_OFFSET_MAX || temp < MAX6581_OFFSET_MIN)
+ return -EINVAL;
+
+ /* 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;
+ }
+ sign = temp < 0 ? false : true;
+ temp = abs(temp);
+ temp_int = temp / 1000;
+ temp_frac = temp % 1000;
+ for (i = 4; i >= 0 && temp_int > 0; --i) {
+ if (temp_int >= 1<<i) {
+ temp_int -= 1<<i;
+ val |= 1 << (i+2);
+ }
+ }
+ /* special handle the fraction */
+ if (temp_frac % 250 != 0)
+ return -EINVAL;
+ if (temp_frac == 750)
+ val |= 0x3;
+ else if (temp_frac == 500)
+ val |= 0x2;
+ else if (temp_frac)
+ val |= 0x1;
+ 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,
+ sign?val:(~val + 1));
+ return ret < 0 ? ret : count;
+}
+
+static ssize_t offset_show(struct device *dev, struct device_attribute *devattr,
+ char *buf)
+{
+ int select, ret, index, temp;
+ struct max6697_data *data;
+
+ 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 -ENODATA;
+ if (select & (1<<(index-1))) {
+ ret = i2c_smbus_read_byte_data(data->client,
+ MAX6581_REG_OFFSET);
+ if (ret < 0)
+ return -ENODATA;
+ } else {
+ return sprintf(buf, "%d\n", 0);
+ }
+ if (1<<7 & ret) {
+ ret = ~ret + 1;
+ temp = -MAX6581_OFFSET_TO_MILLIC(ret);
+ } else {
+ temp = MAX6581_OFFSET_TO_MILLIC(ret);
+ }
+ return sprintf(buf, "%d\n", temp);
+}
+
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);
@@ -374,6 +473,16 @@ 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(temp1_offset, offset, 0);
+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,
@@ -382,8 +491,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;
@@ -392,6 +501,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;
}
@@ -408,6 +521,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,
+ &sensor_dev_attr_temp1_offset.dev_attr.attr,
&sensor_dev_attr_temp2_input.dev_attr.attr,
&sensor_dev_attr_temp2_max.dev_attr.attr,
@@ -415,6 +529,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,
@@ -422,6 +537,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,
@@ -429,6 +545,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,
@@ -436,6 +553,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,
@@ -443,6 +561,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,
@@ -450,6 +569,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,
@@ -457,6 +577,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
};
@@ -493,8 +614,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.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset attributes
2020-07-02 1:42 ` [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset attributes Chu Lin
@ 2020-07-02 5:14 ` Guenter Roeck
2020-07-02 16:38 ` Chu Lin
2020-07-03 1:06 ` Guenter Roeck
1 sibling, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2020-07-02 5:14 UTC (permalink / raw)
To: Chu Lin
Cc: jdelvare, linux-hwmon, linux-kernel, jasonling, belgaied, zhongqil
On Thu, Jul 02, 2020 at 01:42:23AM +0000, 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.
>
Patchwork received four versions of this patch, with two different
subject lines.
How am I supposed to know which one to look at ?
Guenter
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset attributes
2020-07-02 5:14 ` Guenter Roeck
@ 2020-07-02 16:38 ` Chu Lin
0 siblings, 0 replies; 10+ messages in thread
From: Chu Lin @ 2020-07-02 16:38 UTC (permalink / raw)
To: Guenter Roeck
Cc: jdelvare, linux-hwmon, linux-kernel, Jason Ling, Kais Belgaied,
Zhongqi Li
On Wed, Jul 1, 2020 at 10:14 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Thu, Jul 02, 2020 at 01:42:23AM +0000, 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.
> >
> Patchwork received four versions of this patch, with two different
> subject lines.
>
> How am I supposed to know which one to look at ?
Sorry for the confusion.
I was learning how to send amendments to a patch and forget that
it may create confusion for the reviewers. The latest one with headline
hwmon:max6697: Allow max6581 to create tempX_offset attributes is the one that
should be reviewed.
Chu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset attributes
2020-07-02 1:42 ` [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset attributes Chu Lin
2020-07-02 5:14 ` Guenter Roeck
@ 2020-07-03 1:06 ` Guenter Roeck
2020-07-06 17:54 ` [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset Chu Lin
1 sibling, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2020-07-03 1:06 UTC (permalink / raw)
To: Chu Lin
Cc: jdelvare, linux-hwmon, linux-kernel, jasonling, belgaied, zhongqil
On Thu, Jul 02, 2020 at 01:42:23AM +0000, 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.
>
> 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
> -sh: echo: write error: Invalid argument
>
> cat temp2_input
> 37000
>
> 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>
Please run "checkpatch --strict" and fix everything it reports.
> ---
> drivers/hwmon/max6697.c | 129 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 125 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
> index 743752a2467a..b11fcb594807 100644
> --- a/drivers/hwmon/max6697.c
> +++ b/drivers/hwmon/max6697.c
> @@ -41,6 +41,14 @@ static const u8 MAX6697_REG_CRIT[] = {
> #define MAX6697_MAP_BITS(reg) ((((reg) & 0x7e) >> 1) | \
> (((reg) & 0x01) << 6) | ((reg) & 0x80))
>
> +#define MAX6581_OFFSET_TO_MILLIC(reg) (16000 * (((reg) & (1<<6)) >> 6) + \
> + 8000 * (((reg) & (1<<5)) >> 5) + \
> + 4000 * (((reg) & (1<<4)) >> 4) + \
> + 2000 * (((reg) & (1<<3)) >> 3) + \
> + 1000 * (((reg) & (1<<2)) >> 2) + \
> + 500 * (((reg) & (1<<1)) >> 1) + \
> + 250 * ((reg) & 1))
Why not just the following ?
#define MAX6581_OFFSET_TO_MILLIC(reg) ((reg) * 250)
> +
> #define MAX6697_REG_STAT(n) (0x44 + (n))
>
> #define MAX6697_REG_CONFIG 0x41
> @@ -56,6 +64,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 -MAX6581_OFFSET_MIN
> +
Why the double negation ?
#define MAX6581_OFFSET_MIN -31750
#define MAX6581_OFFSET_MAX 31750
or
#define MAX6581_OFFSET_MAX 31750
#define MAX6581_OFFSET_MIN -MAX6581_OFFSET_MAX
>
> #define MAX6697_CONV_TIME 156 /* ms per channel, worst case */
>
> @@ -316,6 +327,94 @@ 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)
> +{
> + long temp;
> + u8 val, select;
> + int i, temp_frac, temp_int, ret, index;
> + bool sign;
> + struct max6697_data *data;
> +
> + 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 -ENODATA;
Please return the error.
> + ret = kstrtol(buf, 10, &temp);
> + if (ret < 0)
> + return ret;
> + if (temp > MAX6581_OFFSET_MAX || temp < MAX6581_OFFSET_MIN)
> + return -EINVAL;
Please use clamp_val() to limit the range. We don't expect users to know
chip limits / restrictions.
> +
> + /* 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;
> + }
> + sign = temp < 0 ? false : true;
> + temp = abs(temp);
> + temp_int = temp / 1000;
> + temp_frac = temp % 1000;
> + for (i = 4; i >= 0 && temp_int > 0; --i) {
> + if (temp_int >= 1<<i) {
> + temp_int -= 1<<i;
> + val |= 1 << (i+2);
> + }
> + }
> + /* special handle the fraction */
> + if (temp_frac % 250 != 0)
> + return -EINVAL;
> + if (temp_frac == 750)
> + val |= 0x3;
> + else if (temp_frac == 500)
> + val |= 0x2;
> + else if (temp_frac)
> + val |= 0x1;
This is way too complicated.
sign = temp < 0;
temp = clamp_val(abs(temp), 0, MAX6581_OFFSET_MAX);
reg = DIV_ROUND_CLOSEST(temp, 250);
if (sign)
reg |= (1 << 7);
should accomplish the same.
> + 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,
> + sign?val:(~val + 1));
> + return ret < 0 ? ret : count;
Since this function requires multiple i2c accesses, it needs to be
mutex protected.
> +}
> +
> +static ssize_t offset_show(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + int select, ret, index, temp;
> + struct max6697_data *data;
> +
> + 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 -ENODATA;
> + if (select & (1<<(index-1))) {
> + ret = i2c_smbus_read_byte_data(data->client,
> + MAX6581_REG_OFFSET);
> + if (ret < 0)
> + return -ENODATA;
> + } else {
> + return sprintf(buf, "%d\n", 0);
> + }
> + if (1<<7 & ret) {
Not believer in Yoda Programming I am.
> + ret = ~ret + 1;
> + temp = -MAX6581_OFFSET_TO_MILLIC(ret);
> + } else {
> + temp = MAX6581_OFFSET_TO_MILLIC(ret);
> + }
Negative vlaues should be handled in MAX6581_OFFSET_TO_MILLIC().
If necessary / too complex, make it a function.
> + return sprintf(buf, "%d\n", temp);
> +}
> +
> 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);
> @@ -374,6 +473,16 @@ 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 */
If so ...
> +static SENSOR_DEVICE_ATTR_RW(temp1_offset, offset, 0);
then why an attribute for temp1_offset ? Guess it is a dummy,
which is a waste. Just use dummy again.
> +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,
> @@ -382,8 +491,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;
> @@ -392,6 +501,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;
> }
> @@ -408,6 +521,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,
> + &sensor_dev_attr_temp1_offset.dev_attr.attr,
>
> &sensor_dev_attr_temp2_input.dev_attr.attr,
> &sensor_dev_attr_temp2_max.dev_attr.attr,
> @@ -415,6 +529,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,
> @@ -422,6 +537,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,
> @@ -429,6 +545,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,
> @@ -436,6 +553,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,
> @@ -443,6 +561,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,
> @@ -450,6 +569,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,
> @@ -457,6 +577,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
> };
>
> @@ -493,8 +614,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]);
> }
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset
2020-07-03 1:06 ` Guenter Roeck
@ 2020-07-06 17:54 ` Chu Lin
2020-07-06 19:32 ` Guenter Roeck
0 siblings, 1 reply; 10+ messages in thread
From: Chu Lin @ 2020-07-06 17:54 UTC (permalink / raw)
To: linux
Cc: belgaied, jasonling, jdelvare, linchuyuan, linux-hwmon,
linux-kernel, zhongqil
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>
---
drivers/hwmon/max6697.c | 100 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 96 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
index da43f7ae3de1..4b71b3716f22 100644
--- a/drivers/hwmon/max6697.c
+++ b/drivers/hwmon/max6697.c
@@ -65,6 +65,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 */
@@ -180,6 +183,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);
@@ -325,6 +333,69 @@ 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)
+{
+ long temp;
+ u8 val, select;
+ int ret, index;
+ struct max6697_data *data;
+
+ 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)
+{
+ int select, ret, index;
+ struct max6697_data *data;
+
+ 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);
@@ -383,6 +454,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,
@@ -391,8 +471,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;
@@ -401,6 +481,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;
}
@@ -417,6 +501,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,
@@ -424,6 +509,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,
@@ -431,6 +517,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,
@@ -438,6 +525,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,
@@ -445,6 +533,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,
@@ -452,6 +541,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,
@@ -459,6 +549,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,
@@ -466,6 +557,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
};
@@ -502,8 +594,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.212.ge8ba1cc988-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset
2020-07-06 17:54 ` [PATCH] hwmon:max6697: Allow max6581 to create tempX_offset Chu Lin
@ 2020-07-06 19:32 ` Guenter Roeck
0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-07-06 19:32 UTC (permalink / raw)
To: Chu Lin
Cc: belgaied, jasonling, jdelvare, linux-hwmon, linux-kernel, zhongqil
On Mon, Jul 06, 2020 at 05:54:33PM +0000, 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.
>
Please version your patches and provide a log of changes made.
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>
> ---
> drivers/hwmon/max6697.c | 100 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 96 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
> index da43f7ae3de1..4b71b3716f22 100644
> --- a/drivers/hwmon/max6697.c
> +++ b/drivers/hwmon/max6697.c
> @@ -65,6 +65,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 */
>
> @@ -180,6 +183,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);
> @@ -325,6 +333,69 @@ 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)
> +{
> + long temp;
> + u8 val, select;
> + int ret, index;
> + struct max6697_data *data;
> +
> + 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)
> +{
> + int select, ret, index;
> + struct max6697_data *data;
> +
> + 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);
> @@ -383,6 +454,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,
> @@ -391,8 +471,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;
> @@ -401,6 +481,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;
> }
> @@ -417,6 +501,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,
> @@ -424,6 +509,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,
> @@ -431,6 +517,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,
> @@ -438,6 +525,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,
> @@ -445,6 +533,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,
> @@ -452,6 +541,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,
> @@ -459,6 +549,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,
> @@ -466,6 +557,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
> };
>
> @@ -502,8 +594,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.212.ge8ba1cc988-goog
>
^ permalink raw reply [flat|nested] 10+ messages in thread