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

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

[Guenter Roeck]

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

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

[Guenter Roeck]

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

[PATCH 3/6] hwmon: (lm95241) Order include files alphabetically

[Guenter Roeck]

Simplify detecting duplicate include files and finding the right place
for adding new ones.

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

diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c
index e4e7bf169b07..c2da2b161996 100644
--- a/drivers/hwmon/lm95241.c
+++ b/drivers/hwmon/lm95241.c
@@ -21,15 +21,15 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
-#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
 #include <linux/init.h>
-#include <linux/slab.h>
 #include <linux/jiffies.h>
-#include <linux/i2c.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
-#include <linux/err.h>
+#include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/slab.h>
 #include <linux/sysfs.h>
 
 #define DEVNAME "lm95241"
-- 
2.5.0

[PATCH 4/6] hwmon: (lm95241) Drop FSF address

[Guenter Roeck]

The FSF address may change, and providing it does not add any value.

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

diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c
index c2da2b161996..507b32b67974 100644
--- a/drivers/hwmon/lm95241.c
+++ b/drivers/hwmon/lm95241.c
@@ -15,10 +15,6 @@
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
 #include <linux/err.h>
-- 
2.5.0

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

[Guenter Roeck]

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

[PATCH 6/6] hwmon: (lm95241) Use more accurate limits

[Guenter Roeck]

The lower temperature limit is -128 degrees C. The supported upper limits
are 127.875 or 255.875 degrees C. Also, don't fail if a value outside
the supported range is provided when setting a temperature limit.
Instead, clamp the provided value to the available value range.

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

diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c
index df94f486b21c..3d96c3fcba9b 100644
--- a/drivers/hwmon/lm95241.c
+++ b/drivers/hwmon/lm95241.c
@@ -205,7 +205,7 @@ static ssize_t show_min(struct device *dev, struct device_attribute *attr,
 
 	return snprintf(buf, PAGE_SIZE - 1,
 			data->config & to_sensor_dev_attr(attr)->index ?
-			"-127000\n" : "0\n");
+			"-128000\n" : "0\n");
 }
 
 static ssize_t set_min(struct device *dev, struct device_attribute *attr,
@@ -216,8 +216,6 @@ static ssize_t set_min(struct device *dev, struct device_attribute *attr,
 
 	if (kstrtol(buf, 10, &val) < 0)
 		return -EINVAL;
-	if (val < -128000)
-		return -EINVAL;
 
 	mutex_lock(&data->update_lock);
 
@@ -242,7 +240,7 @@ static ssize_t show_max(struct device *dev, struct device_attribute *attr,
 
 	return snprintf(buf, PAGE_SIZE - 1,
 			data->config & to_sensor_dev_attr(attr)->index ?
-			"127000\n" : "255000\n");
+			"127875\n" : "255875\n");
 }
 
 static ssize_t set_max(struct device *dev, struct device_attribute *attr,
@@ -253,8 +251,6 @@ static ssize_t set_max(struct device *dev, struct device_attribute *attr,
 
 	if (kstrtol(buf, 10, &val) < 0)
 		return -EINVAL;
-	if (val >= 256000)
-		return -EINVAL;
 
 	mutex_lock(&data->update_lock);
 
-- 
2.5.0