Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1457502 > unrolled thread

[PATCH 1/6] hwmon: (lm95241) Fix overflow problems, write conversion rate to chip

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/6] hwmon: (lm95241) Fix overflow problems, write conversion rate to chip Guenter Roeck <linux@roeck-us.net> - 2016-08-08 01:40 +0200
    [PATCH 5/6] hwmon: (lm95241) Use BIT macro where appropriate Guenter Roeck <linux@roeck-us.net> - 2016-08-08 01:40 +0200
    [PATCH 2/6] hwmon: (lm95241) Add support for fault attributes Guenter Roeck <linux@roeck-us.net> - 2016-08-08 01:40 +0200

#1457502 — [PATCH 1/6] hwmon: (lm95241) Fix overflow problems, write conversion rate to chip

Writing the update_interval attribute could result in an overflow if
a number close to the maximum unsigned long was written. At the same
time, even though the chip supports setting the conversion rate,
the selected conversion rate was not actually written to the chip.

Fix the second problem by selecting valid (supported) conversion rates,
and writing the selected conversion rate to the chip. This also fixes the
first problem, since arbitrary conversion rates are now converted to
actually supported conversion rates.

Also, set the default chip conversion rate to 1 second. Previously, the
chip was configured for continuous conversion, but readings were only
retrieved every seond, which doesn't make much sense. If we only read a
value from the chip every second, we can as well save some power and only
convert in one-second intervals.

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

diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c
index cdf19adaec79..a8cf666fe661 100644
--- a/drivers/hwmon/lm95241.c
+++ b/drivers/hwmon/lm95241.c
@@ -59,6 +59,7 @@ static const unsigned short normal_i2c[] = {
 #define CFG_CR0182 0x10
 #define CFG_CR1000 0x20
 #define CFG_CR2700 0x30
+#define CFG_CRMASK 0x30
 #define R1MS_SHIFT 0
 #define R2MS_SHIFT 2
 #define R1MS_MASK (0x01 << (R1MS_SHIFT))
@@ -91,7 +92,8 @@ static const u8 lm95241_reg_address[] = {
 struct lm95241_data {
 	struct i2c_client *client;
 	struct mutex update_lock;
-	unsigned long last_updated, interval;	/* in jiffies */
+	unsigned long last_updated;	/* in jiffies */
+	unsigned long interval;		/* in milli-seconds */
 	char valid;		/* zero until following fields are valid */
 	/* registers values */
 	u8 temp[ARRAY_SIZE(lm95241_reg_address)];
@@ -118,7 +120,8 @@ static struct lm95241_data *lm95241_update_device(struct device *dev)
 
 	mutex_lock(&data->update_lock);
 
-	if (time_after(jiffies, data->last_updated + data->interval) ||
+	if (time_after(jiffies, data->last_updated
+		       + msecs_to_jiffies(data->interval)) ||
 	    !data->valid) {
 		int i;
 
@@ -276,8 +279,7 @@ static ssize_t show_interval(struct device *dev, struct device_attribute *attr,
 {
 	struct lm95241_data *data = lm95241_update_device(dev);
 
-	return snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval
-			/ HZ);
+	return snprintf(buf, PAGE_SIZE - 1, "%lu\n", data->interval);
 }
 
 static ssize_t set_interval(struct device *dev, struct device_attribute *attr,
@@ -285,11 +287,35 @@ static ssize_t set_interval(struct device *dev, struct device_attribute *attr,
 {
 	struct lm95241_data *data = dev_get_drvdata(dev);
 	unsigned long val;
+	int convrate;
+	u8 config;
 
 	if (kstrtoul(buf, 10, &val) < 0)
 		return -EINVAL;
 
-	data->interval = val * HZ / 1000;
+	mutex_lock(&data->update_lock);
+
+	config = data->config & ~CFG_CRMASK;
+
+	if (val < 130) {
+		convrate = 76;
+		config |= CFG_CR0076;
+	} else if (val < 590) {
+		convrate = 182;
+		config |= CFG_CR0182;
+	} else if (val < 1850) {
+		convrate = 1000;
+		config |= CFG_CR1000;
+	} else {
+		convrate = 2700;
+		config |= CFG_CR2700;
+	}
+
+	data->interval = convrate;
+	data->config = config;
+	i2c_smbus_write_byte_data(data->client, LM95241_REG_RW_CONFIG,
+				  config);
+	mutex_unlock(&data->update_lock);
 
 	return count;
 }
@@ -362,8 +388,8 @@ static int lm95241_detect(struct i2c_client *new_client,
 static void lm95241_init_client(struct i2c_client *client,
 				struct lm95241_data *data)
 {
-	data->interval = HZ;	/* 1 sec default */
-	data->config = CFG_CR0076;
+	data->interval = 1000;
+	data->config = CFG_CR1000;
 	data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
 
 	i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
-- 
2.5.0

[toc] | [next] | [standalone]

#1457503 — [PATCH 5/6] hwmon: (lm95241) Use BIT macro where appropriate

Drop some of the SHIFT defines since shift is implied with BIT().

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

diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c
index 507b32b67974..df94f486b21c 100644
--- a/drivers/hwmon/lm95241.c
+++ b/drivers/hwmon/lm95241.c
@@ -17,6 +17,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/bitops.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
@@ -50,29 +51,25 @@ static const unsigned short normal_i2c[] = {
 #define LM95241_REG_RW_REMOTE_MODEL	0x30
 
 /* LM95241 specific bitfields */
-#define CFG_STOP 0x40
-#define CFG_CR0076 0x00
-#define CFG_CR0182 0x10
-#define CFG_CR1000 0x20
-#define CFG_CR2700 0x30
-#define CFG_CRMASK 0x30
-#define R1MS_SHIFT 0
-#define R2MS_SHIFT 2
-#define R1MS_MASK (0x01 << (R1MS_SHIFT))
-#define R2MS_MASK (0x01 << (R2MS_SHIFT))
-#define R1DF_SHIFT 1
-#define R2DF_SHIFT 2
-#define R1DF_MASK (0x01 << (R1DF_SHIFT))
-#define R2DF_MASK (0x01 << (R2DF_SHIFT))
-#define R1FE_MASK 0x01
-#define R2FE_MASK 0x05
-#define R1DM 0x01
-#define R2DM 0x02
-#define TT1_SHIFT 0
-#define TT2_SHIFT 4
-#define TT_OFF 0
-#define TT_ON 1
-#define TT_MASK 7
+#define CFG_STOP	BIT(6)
+#define CFG_CR0076	0x00
+#define CFG_CR0182	BIT(4)
+#define CFG_CR1000	BIT(5)
+#define CFG_CR2700	(BIT(4) | BIT(5))
+#define CFG_CRMASK	(BIT(4) | BIT(5))
+#define R1MS_MASK	BIT(0)
+#define R2MS_MASK	BIT(2)
+#define R1DF_MASK	BIT(1)
+#define R2DF_MASK	BIT(2)
+#define R1FE_MASK	BIT(0)
+#define R2FE_MASK	BIT(2)
+#define R1DM		BIT(0)
+#define R2DM		BIT(1)
+#define TT1_SHIFT	0
+#define TT2_SHIFT	4
+#define TT_OFF		0
+#define TT_ON		1
+#define TT_MASK		7
 #define NATSEMI_MAN_ID	0x01
 #define LM95231_CHIP_ID	0xA1
 #define LM95241_CHIP_ID	0xA4
@@ -148,7 +145,7 @@ static ssize_t show_input(struct device *dev, struct device_attribute *attr,
 	int index = to_sensor_dev_attr(attr)->index;
 
 	return snprintf(buf, PAGE_SIZE - 1, "%d\n",
-			index == 0 || (data->config & (1 << (index / 2))) ?
+			index == 0 || (data->config & BIT(index / 2)) ?
 		temp_from_reg_signed(data->temp[index], data->temp[index + 1]) :
 		temp_from_reg_unsigned(data->temp[index],
 				       data->temp[index + 1]));
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1457504 — [PATCH 2/6] hwmon: (lm95241) Add support for fault attributes

The chip reports if remote diodes are present, which can be used for
the fault attrributes.

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

diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c
index a8cf666fe661..e4e7bf169b07 100644
--- a/drivers/hwmon/lm95241.c
+++ b/drivers/hwmon/lm95241.c
@@ -70,6 +70,8 @@ static const unsigned short normal_i2c[] = {
 #define R2DF_MASK (0x01 << (R2DF_SHIFT))
 #define R1FE_MASK 0x01
 #define R2FE_MASK 0x05
+#define R1DM 0x01
+#define R2DM 0x02
 #define TT1_SHIFT 0
 #define TT2_SHIFT 4
 #define TT_OFF 0
@@ -97,7 +99,7 @@ struct lm95241_data {
 	char valid;		/* zero until following fields are valid */
 	/* registers values */
 	u8 temp[ARRAY_SIZE(lm95241_reg_address)];
-	u8 config, model, trutherm;
+	u8 status, config, model, trutherm;
 };
 
 /* Conversions */
@@ -130,6 +132,9 @@ static struct lm95241_data *lm95241_update_device(struct device *dev)
 			data->temp[i]
 			  = i2c_smbus_read_byte_data(client,
 						     lm95241_reg_address[i]);
+
+		data->status = i2c_smbus_read_byte_data(client,
+							LM95241_REG_R_STATUS);
 		data->last_updated = jiffies;
 		data->valid = 1;
 	}
@@ -274,6 +279,15 @@ static ssize_t set_max(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static ssize_t show_fault(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	struct lm95241_data *data = lm95241_update_device(dev);
+
+	return snprintf(buf, PAGE_SIZE - 1, "%d",
+			!!(data->status & to_sensor_dev_attr(attr)->index));
+}
+
 static ssize_t show_interval(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
@@ -335,6 +349,8 @@ static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_max, set_max,
 			  R1DF_MASK);
 static SENSOR_DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_max, set_max,
 			  R2DF_MASK);
+static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_fault, NULL, R1DM);
+static SENSOR_DEVICE_ATTR(temp3_fault, S_IRUGO, show_fault, NULL, R2DM);
 static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_interval,
 		   set_interval);
 
@@ -348,6 +364,8 @@ static struct attribute *lm95241_attrs[] = {
 	&sensor_dev_attr_temp3_min.dev_attr.attr,
 	&sensor_dev_attr_temp2_max.dev_attr.attr,
 	&sensor_dev_attr_temp3_max.dev_attr.attr,
+	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp3_fault.dev_attr.attr,
 	&dev_attr_update_interval.attr,
 	NULL
 };
-- 
2.5.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web