* [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros
@ 2010-11-09 9:33 Davide Rizzo
2010-11-09 22:14 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
` (9 more replies)
0 siblings, 10 replies; 19+ messages in thread
From: Davide Rizzo @ 2010-11-09 9:33 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1.1: Type: text/plain, Size: 19518 bytes --]
From: Davide Rizzo <elpa.rizzo@gmail.com>
Rewriting of driver/hwmon/lm95241.c to avoid using macros
Now it uses SENSOR_DEVICE_ATTR to distinguish similar attr.
Signed-off-by: Davide Rizzo <elpa.rizzo@gmail.com>
---
--- linux-2.6.37-rc1/drivers/hwmon/lm95241.c 2010-11-01
12:54:12.000000000 +0100
+++ linux-2.6.37-rc1.elpa/drivers/hwmon/lm95241.c 2010-11-09
10:00:47.464820382 +0100
@@ -1,13 +1,9 @@
/*
- * lm95241.c - Part of lm_sensors, Linux kernel modules for hardware
- * monitoring
- * Copyright (C) 2008 Davide Rizzo <elpa-rizzo@gmail.com>
+ * Copyright (C) 2008, 2010 Davide Rizzo <elpa.rizzo@gmail.com>
*
- * Based on the max1619 driver. The LM95241 is a sensor chip made by
National
- * Semiconductors.
- * It reports up to three temperatures (its own plus up to
- * two external ones). Complete datasheet can be
- * obtained from National's website at:
+ * The LM95241 is a sensor chip made by National Semiconductors.
+ * It reports up to three temperatures (its own plus up to two external
ones).
+ * Complete datasheet can be obtained from National's website at:
* http://www.national.com/ds.cgi/LM/LM95241.pdf
*
* This program is free software; you can redistribute it and/or modify
@@ -36,6 +32,8 @@
#include <linux/mutex.h>
#include <linux/sysfs.h>
+#define DEVNAME "lm95241"
+
static const unsigned short normal_i2c[] = {
0x19, 0x2a, 0x2b, I2C_CLIENT_END};
@@ -79,14 +77,6 @@ static const unsigned short normal_i2c[]
#define MANUFACTURER_ID 0x01
#define DEFAULT_REVISION 0xA4
-/* Conversions and various macros */
-#define TEMP_FROM_REG(val_h, val_l) (((val_h) & 0x80 ? (val_h) - 0x100 : \
- (val_h)) * 1000 + (val_l) * 1000 / 256)
-
-/* Functions declaration */
-static void lm95241_init_client(struct i2c_client *client);
-static struct lm95241_data *lm95241_update_device(struct device *dev);
-
/* Client data (each client gets its own) */
struct lm95241_data {
struct device *hwmon_dev;
@@ -94,29 +84,208 @@ struct lm95241_data {
unsigned long last_updated, interval; /* in jiffies */
char valid; /* zero until following fields are valid */
/* registers values */
- u8 local_h, local_l; /* local */
- u8 remote1_h, remote1_l; /* remote1 */
- u8 remote2_h, remote2_l; /* remote2 */
+ u8 temp[6];
u8 config, model, trutherm;
};
+/* Conversions */
+static int TempFromReg(u8 val_h, u8 val_l)
+{
+ if (val_h & 0x80)
+ return val_h - 0x100;
+ return val_h * 1000 + val_l * 1000 / 256;
+}
+
+static const u8 address[] = {
+ LM95241_REG_R_LOCAL_TEMPH,
+ LM95241_REG_R_LOCAL_TEMPL,
+ LM95241_REG_R_REMOTE1_TEMPH,
+ LM95241_REG_R_REMOTE1_TEMPL,
+ LM95241_REG_R_REMOTE2_TEMPH,
+ LM95241_REG_R_REMOTE2_TEMPL
+};
+
+static struct lm95241_data *lm95241_update_device(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ mutex_lock(&data->update_lock);
+
+ if (time_after(jiffies, data->last_updated + data->interval) ||
+ !data->valid) {
+ int i;
+
+ dev_dbg(&client->dev, "Updating lm95241 data.\n");
+ for(i = 0; i < 6; i++)
+ data->temp[i] = i2c_smbus_read_byte_data(client,
+ address[i]);
+ data->last_updated = jiffies;
+ data->valid = 1;
+ }
+
+ mutex_unlock(&data->update_lock);
+
+ return data;
+}
+
/* Sysfs stuff */
-#define show_temp(value) \
-static ssize_t show_##value(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct lm95241_data *data = lm95241_update_device(dev); \
- snprintf(buf, PAGE_SIZE - 1, "%d\n", \
- TEMP_FROM_REG(data->value##_h, data->value##_l)); \
- return strlen(buf); \
-}
-show_temp(local);
-show_temp(remote1);
-show_temp(remote2);
+static ssize_t show_input(struct device *dev, struct device_attribute
*attr,
+ char *buf)
+{
+ struct lm95241_data *data = lm95241_update_device(dev);
-static ssize_t show_interval(struct device *dev, struct device_attribute
*attr,
+ snprintf(buf, PAGE_SIZE - 1, "%d\n",
+ TempFromReg(data->temp[to_sensor_dev_attr(attr)->index],
+ data->temp[to_sensor_dev_attr(attr)->index + 1]));
+ return strlen(buf);
+}
+
+static SENSOR_DEVICE_ATTR(locale_input, S_IRUGO, show_input, NULL, 0);
+static SENSOR_DEVICE_ATTR(remote1_input, S_IRUGO, show_input, NULL, 2);
+static SENSOR_DEVICE_ATTR(remote2_input, S_IRUGO, show_input, NULL, 4);
+
+static ssize_t show_type(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ snprintf(buf, PAGE_SIZE - 1,
+ data->model & to_sensor_dev_attr(attr)->index ? "1\n" : "2\n");
+ return strlen(buf);
+}
+
+static ssize_t set_type(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ unsigned long val;
+
+ if (strict_strtoul(buf, 10, &val) < 0)
+ return -EINVAL;
+
+ if (val == 1 || val == 2) {
+ int shift = (to_sensor_dev_attr(attr)->index == R1MS_MASK) ?
+ TT1_SHIFT : TT2_SHIFT;
+
+ mutex_lock(&data->update_lock);
+
+ data->trutherm &= ~(TT_MASK << shift);
+ if (val == 1) {
+ data->model |= to_sensor_dev_attr(attr)->index;
+ data->trutherm |= (TT_ON << shift);
+ } else {
+ data->model &= ~to_sensor_dev_attr(attr)->index;
+ data->trutherm |= (TT_OFF << shift);
+ }
+
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
+ data->model);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
+ data->trutherm);
+
+ mutex_unlock(&data->update_lock);
+
+ }
+ return count;
+}
+
+static SENSOR_DEVICE_ATTR(remote1_type, S_IWUSR | S_IRUGO, show_type,
set_type,
+ R1MS_MASK);
+static SENSOR_DEVICE_ATTR(remote2_type, S_IWUSR | S_IRUGO, show_type,
set_type,
+ R2MS_MASK);
+
+static ssize_t show_min(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ snprintf(buf, PAGE_SIZE - 1,
+ data->config & to_sensor_dev_attr(attr)->index ?
+ "-127000\n" : "0\n");
+ return strlen(buf);
+}
+
+static ssize_t set_min(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ long val;
+
+ if (strict_strtol(buf, 10, &val) < 0)
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ if (val < 0)
+ data->config |= to_sensor_dev_attr(attr)->index;
+ else
+ data->config &= ~to_sensor_dev_attr(attr)->index;
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static SENSOR_DEVICE_ATTR(remote1_min, S_IWUSR | S_IRUGO, show_min,
set_min,
+ R1DF_MASK);
+static SENSOR_DEVICE_ATTR(remote2_min, S_IWUSR | S_IRUGO, show_min,
set_min,
+ R2DF_MASK);
+
+static ssize_t show_max(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ snprintf(buf, PAGE_SIZE - 1,
+ data->config & to_sensor_dev_attr(attr)->index ?
+ "127000\n" : "255000\n");
+ return strlen(buf);
+}
+
+static ssize_t set_max(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ long val;
+
+ if (strict_strtol(buf, 10, &val) < 0)
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ if (val <= 127000)
+ data->config |= to_sensor_dev_attr(attr)->index;
+ else
+ data->config &= ~to_sensor_dev_attr(attr)->index;
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static SENSOR_DEVICE_ATTR(remote1_max, S_IWUSR | S_IRUGO, show_max,
set_max,
+ R1DF_MASK);
+static SENSOR_DEVICE_ATTR(remote2_max, S_IWUSR | S_IRUGO, show_max,
set_max,
+ R2DF_MASK);
+
+static ssize_t show_interval(struct device *dev, struct device_attribute
*attr,
+ char *buf)
+{
struct lm95241_data *data = lm95241_update_device(dev);
snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval / HZ);
@@ -124,181 +293,33 @@ static ssize_t show_interval(struct devi
}
static ssize_t set_interval(struct device *dev, struct device_attribute
*attr,
- const char *buf, size_t count)
+ const char *buf, size_t count)
{
struct i2c_client *client = to_i2c_client(dev);
struct lm95241_data *data = i2c_get_clientdata(client);
+ unsigned long val;
+
+ if (strict_strtoul(buf, 10, &val) < 0)
+ return -EINVAL;
- strict_strtol(buf, 10, &data->interval);
- data->interval = data->interval * HZ / 1000;
+ data->interval = val * HZ / 1000;
return count;
}
-#define show_type(flag) \
-static ssize_t show_type##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->model & R##flag##MS_MASK ? "1\n" : "2\n"); \
- return strlen(buf); \
-}
-show_type(1);
-show_type(2);
-
-#define show_min(flag) \
-static ssize_t show_min##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->config & R##flag##DF_MASK ? \
- "-127000\n" : "0\n"); \
- return strlen(buf); \
-}
-show_min(1);
-show_min(2);
-
-#define show_max(flag) \
-static ssize_t show_max##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->config & R##flag##DF_MASK ? \
- "127000\n" : "255000\n"); \
- return strlen(buf); \
-}
-show_max(1);
-show_max(2);
-
-#define set_type(flag) \
-static ssize_t set_type##flag(struct device *dev, \
- struct device_attribute *attr, \
- const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
- strict_strtol(buf, 10, &val); \
-\
- if ((val == 1) || (val == 2)) { \
-\
- mutex_lock(&data->update_lock); \
-\
- data->trutherm &= ~(TT_MASK << TT##flag##_SHIFT); \
- if (val == 1) { \
- data->model |= R##flag##MS_MASK; \
- data->trutherm |= (TT_ON << TT##flag##_SHIFT); \
- } \
- else { \
- data->model &= ~R##flag##MS_MASK; \
- data->trutherm |= (TT_OFF << TT##flag##_SHIFT); \
- } \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL, \
- data->model); \
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM, \
- data->trutherm); \
-\
- mutex_unlock(&data->update_lock); \
-\
- } \
- return count; \
-}
-set_type(1);
-set_type(2);
-
-#define set_min(flag) \
-static ssize_t set_min##flag(struct device *dev, \
- struct device_attribute *devattr, const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
- strict_strtol(buf, 10, &val); \
-\
- mutex_lock(&data->update_lock); \
-\
- if (val < 0) \
- data->config |= R##flag##DF_MASK; \
- else \
- data->config &= ~R##flag##DF_MASK; \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
- data->config); \
-\
- mutex_unlock(&data->update_lock); \
-\
- return count; \
-}
-set_min(1);
-set_min(2);
-
-#define set_max(flag) \
-static ssize_t set_max##flag(struct device *dev, \
- struct device_attribute *devattr, const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
- strict_strtol(buf, 10, &val); \
-\
- mutex_lock(&data->update_lock); \
-\
- if (val <= 127000) \
- data->config |= R##flag##DF_MASK; \
- else \
- data->config &= ~R##flag##DF_MASK; \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
- data->config); \
-\
- mutex_unlock(&data->update_lock); \
-\
- return count; \
-}
-set_max(1);
-set_max(2);
-
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_local, NULL);
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_remote1, NULL);
-static DEVICE_ATTR(temp3_input, S_IRUGO, show_remote2, NULL);
-static DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type1, set_type1);
-static DEVICE_ATTR(temp3_type, S_IWUSR | S_IRUGO, show_type2, set_type2);
-static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min1, set_min1);
-static DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min2, set_min2);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_max1, set_max1);
-static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_max2, set_max2);
static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_interval,
set_interval);
static struct attribute *lm95241_attributes[] = {
- &dev_attr_temp1_input.attr,
- &dev_attr_temp2_input.attr,
- &dev_attr_temp3_input.attr,
- &dev_attr_temp2_type.attr,
- &dev_attr_temp3_type.attr,
- &dev_attr_temp2_min.attr,
- &dev_attr_temp3_min.attr,
- &dev_attr_temp2_max.attr,
- &dev_attr_temp3_max.attr,
+ &sensor_dev_attr_locale_input.dev_attr.attr,
+ &sensor_dev_attr_remote1_input.dev_attr.attr,
+ &sensor_dev_attr_remote2_input.dev_attr.attr,
+ &sensor_dev_attr_remote1_type.dev_attr.attr,
+ &sensor_dev_attr_remote2_type.dev_attr.attr,
+ &sensor_dev_attr_remote1_min.dev_attr.attr,
+ &sensor_dev_attr_remote2_min.dev_attr.attr,
+ &sensor_dev_attr_remote1_max.dev_attr.attr,
+ &sensor_dev_attr_remote2_max.dev_attr.attr,
&dev_attr_update_interval.attr,
NULL
};
@@ -322,7 +343,7 @@ static int lm95241_detect(struct i2c_cli
== MANUFACTURER_ID)
&& (i2c_smbus_read_byte_data(new_client, LM95241_REG_R_CHIP_ID)
>= DEFAULT_REVISION)) {
- name = "lm95241";
+ name = DEVNAME;
} else {
dev_dbg(&adapter->dev, "LM95241 detection failed at 0x%02x\n",
address);
@@ -334,6 +355,26 @@ static int lm95241_detect(struct i2c_cli
return 0;
}
+static void lm95241_init_client(struct i2c_client *client)
+{
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ data->interval = HZ; /* 1 sec default */
+ data->valid = 0;
+ data->config = CFG_CR0076;
+ data->model = 0;
+ data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
+ data->config);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
+ R1FE_MASK | R2FE_MASK);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
+ data->trutherm);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
+ data->model);
+}
+
static int lm95241_probe(struct i2c_client *new_client,
const struct i2c_device_id *id)
{
@@ -373,26 +414,6 @@ exit:
return err;
}
-static void lm95241_init_client(struct i2c_client *client)
-{
- struct lm95241_data *data = i2c_get_clientdata(client);
-
- data->interval = HZ; /* 1 sec default */
- data->valid = 0;
- data->config = CFG_CR0076;
- data->model = 0;
- data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
-
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
- data->config);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
- R1FE_MASK | R2FE_MASK);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
- data->trutherm);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
- data->model);
-}
-
static int lm95241_remove(struct i2c_client *client)
{
struct lm95241_data *data = i2c_get_clientdata(client);
@@ -404,46 +425,9 @@ static int lm95241_remove(struct i2c_cli
return 0;
}
-static struct lm95241_data *lm95241_update_device(struct device *dev)
-{
- struct i2c_client *client = to_i2c_client(dev);
- struct lm95241_data *data = i2c_get_clientdata(client);
-
- mutex_lock(&data->update_lock);
-
- if (time_after(jiffies, data->last_updated + data->interval) ||
- !data->valid) {
- dev_dbg(&client->dev, "Updating lm95241 data.\n");
- data->local_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_LOCAL_TEMPH);
- data->local_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_LOCAL_TEMPL);
- data->remote1_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE1_TEMPH);
- data->remote1_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE1_TEMPL);
- data->remote2_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE2_TEMPH);
- data->remote2_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE2_TEMPL);
- data->last_updated = jiffies;
- data->valid = 1;
- }
-
- mutex_unlock(&data->update_lock);
-
- return data;
-}
-
/* Driver data (common to all clients) */
static const struct i2c_device_id lm95241_id[] = {
- { "lm95241", 0 },
+ { DEVNAME, 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, lm95241_id);
@@ -451,7 +435,7 @@ MODULE_DEVICE_TABLE(i2c, lm95241_id);
static struct i2c_driver lm95241_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
- .name = "lm95241",
+ .name = DEVNAME,
},
.probe = lm95241_probe,
.remove = lm95241_remove,
@@ -470,9 +454,10 @@ static void __exit sensors_lm95241_exit(
i2c_del_driver(&lm95241_driver);
}
-MODULE_AUTHOR("Davide Rizzo <elpa-rizzo@gmail.com>");
+MODULE_AUTHOR("Davide Rizzo <elpa.rizzo@gmail.com>");
MODULE_DESCRIPTION("LM95241 sensor driver");
MODULE_LICENSE("GPL");
module_init(sensors_lm95241_init);
module_exit(sensors_lm95241_exit);
+
[-- Attachment #1.2: Type: text/html, Size: 22848 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
@ 2010-11-09 22:14 ` Guenter Roeck
2010-11-11 18:18 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
` (8 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2010-11-09 22:14 UTC (permalink / raw)
To: lm-sensors
On Tue, 2010-11-09 at 04:33 -0500, Davide Rizzo wrote:
> From: Davide Rizzo <elpa.rizzo@gmail.com>
>
> Rewriting of driver/hwmon/lm95241.c to avoid using macros
> Now it uses SENSOR_DEVICE_ATTR to distinguish similar attr.
> Signed-off-by: Davide Rizzo <elpa.rizzo@gmail.com>
> ---
>
Hi Davide,
this looks much better, and gives me a chance for a more complete
review. Please see below.
Unfortunately, I got hit with the MS Exchange server bug, meaning I can
not apply your patch. So I might still miss something. Next time please
copy lkml - that will give me a chance to retrieve the patch from
patchwork.kernel.org.
Thanks,
Guenter
> --- linux-2.6.37-rc1/drivers/hwmon/lm95241.c 2010-11-01
> 12:54:12.000000000 +0100
> +++ linux-2.6.37-rc1.elpa/drivers/hwmon/lm95241.c 2010-11-09
> 10:00:47.464820382 +0100
> @@ -1,13 +1,9 @@
> /*
> - * lm95241.c - Part of lm_sensors, Linux kernel modules for hardware
> - * monitoring
> - * Copyright (C) 2008 Davide Rizzo <elpa-rizzo@gmail.com>
> + * Copyright (C) 2008, 2010 Davide Rizzo <elpa.rizzo@gmail.com>
> *
> - * Based on the max1619 driver. The LM95241 is a sensor chip made by
> National
> - * Semiconductors.
> - * It reports up to three temperatures (its own plus up to
> - * two external ones). Complete datasheet can be
> - * obtained from National's website at:
> + * The LM95241 is a sensor chip made by National Semiconductors.
> + * It reports up to three temperatures (its own plus up to two
> external ones).
> + * Complete datasheet can be obtained from National's website at:
> * http://www.national.com/ds.cgi/LM/LM95241.pdf
> *
> * This program is free software; you can redistribute it and/or
> modify
> @@ -36,6 +32,8 @@
> #include <linux/mutex.h>
> #include <linux/sysfs.h>
>
> +#define DEVNAME "lm95241"
> +
> static const unsigned short normal_i2c[] = {
> 0x19, 0x2a, 0x2b, I2C_CLIENT_END};
>
> @@ -79,14 +77,6 @@ static const unsigned short normal_i2c[]
> #define MANUFACTURER_ID 0x01
> #define DEFAULT_REVISION 0xA4
>
> -/* Conversions and various macros */
> -#define TEMP_FROM_REG(val_h, val_l) (((val_h) & 0x80 ? (val_h) -
> 0x100 : \
> - (val_h)) * 1000 + (val_l) * 1000 / 256)
> -
> -/* Functions declaration */
> -static void lm95241_init_client(struct i2c_client *client);
> -static struct lm95241_data *lm95241_update_device(struct device
> *dev);
> -
> /* Client data (each client gets its own) */
> struct lm95241_data {
> struct device *hwmon_dev;
> @@ -94,29 +84,208 @@ struct lm95241_data {
> unsigned long last_updated, interval; /* in jiffies */
> char valid; /* zero until following fields are valid */
> /* registers values */
> - u8 local_h, local_l; /* local */
> - u8 remote1_h, remote1_l; /* remote1 */
> - u8 remote2_h, remote2_l; /* remote2 */
> + u8 temp[6];
> u8 config, model, trutherm;
> };
>
> +/* Conversions */
> +static int TempFromReg(u8 val_h, u8 val_l)
> +{
> + if (val_h & 0x80)
> + return val_h - 0x100;
> + return val_h * 1000 + val_l * 1000 / 256;
> +}
> +
> +static const u8 address[] = {
Please use something like lm95241_reg_address. Also, if you move this
array above the definition of struct lm95241_data, you can use
u8 temp[ARRAY_SIZE(lm95241_reg_address)];
in struct lm95241_data, instead of the hardcoded array size.
> + LM95241_REG_R_LOCAL_TEMPH,
> + LM95241_REG_R_LOCAL_TEMPL,
> + LM95241_REG_R_REMOTE1_TEMPH,
> + LM95241_REG_R_REMOTE1_TEMPL,
> + LM95241_REG_R_REMOTE2_TEMPH,
> + LM95241_REG_R_REMOTE2_TEMPL
> +};
> +
> +static struct lm95241_data *lm95241_update_device(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lm95241_data *data = i2c_get_clientdata(client);
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->last_updated + data->interval) ||
> + !data->valid) {
> + int i;
> +
> + dev_dbg(&client->dev, "Updating lm95241 data.\n");
> + for(i = 0; i < 6; i++)
i < ARRAY_SIZE(data->temp);
> + data->temp[i] = i2c_smbus_read_byte_data(client,
> + address[i]);
> + data->last_updated = jiffies;
> + data->valid = 1;
> + }
> +
> + mutex_unlock(&data->update_lock);
> +
> + return data;
> +}
> +
> /* Sysfs stuff */
> -#define show_temp(value) \
> -static ssize_t show_##value(struct device *dev, \
> - struct device_attribute *attr, char *buf) \
> -{ \
> - struct lm95241_data *data = lm95241_update_device(dev); \
> - snprintf(buf, PAGE_SIZE - 1, "%d\n", \
> - TEMP_FROM_REG(data->value##_h, data->value##_l)); \
> - return strlen(buf); \
> -}
> -show_temp(local);
> -show_temp(remote1);
> -show_temp(remote2);
> +static ssize_t show_input(struct device *dev, struct device_attribute
> *attr,
> + char *buf)
> +{
> + struct lm95241_data *data = lm95241_update_device(dev);
>
> -static ssize_t show_interval(struct device *dev, struct
> device_attribute *attr,
> + snprintf(buf, PAGE_SIZE - 1, "%d\n",
> + TempFromReg(data->temp[to_sensor_dev_attr(attr)->index],
> + data->temp[to_sensor_dev_attr(attr)->index + 1]));
> + return strlen(buf);
> +}
> +
> +static SENSOR_DEVICE_ATTR(locale_input, S_IRUGO, show_input, NULL,
> 0);
> +static SENSOR_DEVICE_ATTR(remote1_input, S_IRUGO, show_input, NULL,
> 2);
> +static SENSOR_DEVICE_ATTR(remote2_input, S_IRUGO, show_input, NULL,
> 4);
> +
You renamed all the attributes from the standard ABI (tempX_input) to
something proprietary, which no one will understand. Please stick with
the standard attribute names.
I would suggest to keep all SENSOR_DEVICE_ATTR definitions at a single
place, as it used to be. Makes the code look cleaner (and issues like
attribute renames are easier to catch).
> +static ssize_t show_type(struct device *dev, struct device_attribute
> *attr,
> char *buf)
> {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lm95241_data *data = i2c_get_clientdata(client);
> +
> + snprintf(buf, PAGE_SIZE - 1,
> + data->model & to_sensor_dev_attr(attr)->index ? "1\n" : "2
> \n");
> + return strlen(buf);
Please use
return snprintf(...);
> +}
> +
> +static ssize_t set_type(struct device *dev, struct device_attribute
> *attr,
> + const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lm95241_data *data = i2c_get_clientdata(client);
> + unsigned long val;
Add
u8 mask = to_sensor_dev_attr(attr)->index;
Then you can use it below instead of accessing
to_sensor_dev_attr(attr)->index several times.
> +
> + if (strict_strtoul(buf, 10, &val) < 0)
> + return -EINVAL;
> +
Since numbers other than 1 and 2 are invalid, the following would be
better.
if (val != 1 && val != 2)
return -EINVAL;
shift = mask = R1MS_MASK ? TT1_SHIFT : TT2_SHIFT;
...
> + if (val = 1 || val = 2) {
> + int shift = (to_sensor_dev_attr(attr)->index = R1MS_MASK) ?
> + TT1_SHIFT : TT2_SHIFT;
> +
> + mutex_lock(&data->update_lock);
> +
> + data->trutherm &= ~(TT_MASK << shift);
> + if (val = 1) {
> + data->model |= to_sensor_dev_attr(attr)->index;
> + data->trutherm |= (TT_ON << shift);
> + } else {
> + data->model &= ~to_sensor_dev_attr(attr)->index;
> + data->trutherm |= (TT_OFF << shift);
> + }
> +
> + data->valid = 0;
> +
> + i2c_smbus_write_byte_data(client,
> LM95241_REG_RW_REMOTE_MODEL,
> + data->model);
> + i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
> + data->trutherm);
> +
> + mutex_unlock(&data->update_lock);
> +
> + }
> + return count;
> +}
> +
> +static SENSOR_DEVICE_ATTR(remote1_type, S_IWUSR | S_IRUGO, show_type,
> set_type,
> + R1MS_MASK);
> +static SENSOR_DEVICE_ATTR(remote2_type, S_IWUSR | S_IRUGO, show_type,
> set_type,
> + R2MS_MASK);
> +
Move, and fix attribute names.
> +static ssize_t show_min(struct device *dev, struct device_attribute
> *attr,
> + char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lm95241_data *data = i2c_get_clientdata(client);
> +
> + snprintf(buf, PAGE_SIZE - 1,
> + data->config & to_sensor_dev_attr(attr)->index ?
> + "-127000\n" : "0\n");
> + return strlen(buf);
return snprintf(...);
> +}
> +
> +static ssize_t set_min(struct device *dev, struct device_attribute
> *attr,
> + const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lm95241_data *data = i2c_get_clientdata(client);
> + long val;
> +
> + if (strict_strtol(buf, 10, &val) < 0)
> + return -EINVAL;
> +
Is there a valid range, or is it just <0 vs. >= 0 ?
If there is a valid range, it would make sense to check it.
> + mutex_lock(&data->update_lock);
> +
> + if (val < 0)
> + data->config |= to_sensor_dev_attr(attr)->index;
> + else
> + data->config &= ~to_sensor_dev_attr(attr)->index;
> + data->valid = 0;
> +
> + i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
> data->config);
> +
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +static SENSOR_DEVICE_ATTR(remote1_min, S_IWUSR | S_IRUGO, show_min,
> set_min,
> + R1DF_MASK);
> +static SENSOR_DEVICE_ATTR(remote2_min, S_IWUSR | S_IRUGO, show_min,
> set_min,
> + R2DF_MASK);
> +
Move, and fix attribute names.
> +static ssize_t show_max(struct device *dev, struct device_attribute
> *attr,
> + char *buf)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lm95241_data *data = i2c_get_clientdata(client);
> +
> + snprintf(buf, PAGE_SIZE - 1,
> + data->config & to_sensor_dev_attr(attr)->index ?
> + "127000\n" : "255000\n");
> + return strlen(buf);
return snprintf(...);
> +}
> +
> +static ssize_t set_max(struct device *dev, struct device_attribute
> *attr,
> + const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct lm95241_data *data = i2c_get_clientdata(client);
> + long val;
> +
> + if (strict_strtol(buf, 10, &val) < 0)
> + return -EINVAL;
> +
Is there a valid range ? If so, please check it.
> + mutex_lock(&data->update_lock);
> +
> + if (val <= 127000)
> + data->config |= to_sensor_dev_attr(attr)->index;
> + else
> + data->config &= ~to_sensor_dev_attr(attr)->index;
> + data->valid = 0;
> +
> + i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
> data->config);
> +
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +static SENSOR_DEVICE_ATTR(remote1_max, S_IWUSR | S_IRUGO, show_max,
> set_max,
> + R1DF_MASK);
> +static SENSOR_DEVICE_ATTR(remote2_max, S_IWUSR | S_IRUGO, show_max,
> set_max,
> + R2DF_MASK);
> +
Move, and fix attribute names.
> +static ssize_t show_interval(struct device *dev, struct
> device_attribute *attr,
> + char *buf)
> +{
> struct lm95241_data *data = lm95241_update_device(dev);
>
> snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval /
> HZ);
return snprintf(...);
> @@ -124,181 +293,33 @@ static ssize_t show_interval(struct devi
> }
>
> static ssize_t set_interval(struct device *dev, struct
> device_attribute *attr,
> - const char *buf, size_t count)
> + const char *buf, size_t count)
> {
> struct i2c_client *client = to_i2c_client(dev);
> struct lm95241_data *data = i2c_get_clientdata(client);
> + unsigned long val;
> +
> + if (strict_strtoul(buf, 10, &val) < 0)
> + return -EINVAL;
>
> - strict_strtol(buf, 10, &data->interval);
> - data->interval = data->interval * HZ / 1000;
> + data->interval = val * HZ / 1000;
>
> return count;
> }
>
> -#define show_type(flag) \
> -static ssize_t show_type##flag(struct device *dev, \
> - struct device_attribute *attr, char *buf) \
> -{ \
> - struct i2c_client *client = to_i2c_client(dev); \
> - struct lm95241_data *data = i2c_get_clientdata(client); \
> -\
> - snprintf(buf, PAGE_SIZE - 1, \
> - data->model & R##flag##MS_MASK ? "1\n" : "2\n"); \
> - return strlen(buf); \
> -}
> -show_type(1);
> -show_type(2);
> -
> -#define show_min(flag) \
> -static ssize_t show_min##flag(struct device *dev, \
> - struct device_attribute *attr, char *buf) \
> -{ \
> - struct i2c_client *client = to_i2c_client(dev); \
> - struct lm95241_data *data = i2c_get_clientdata(client); \
> -\
> - snprintf(buf, PAGE_SIZE - 1, \
> - data->config & R##flag##DF_MASK ? \
> - "-127000\n" : "0\n"); \
> - return strlen(buf); \
> -}
> -show_min(1);
> -show_min(2);
> -
> -#define show_max(flag) \
> -static ssize_t show_max##flag(struct device *dev, \
> - struct device_attribute *attr, char *buf) \
> -{ \
> - struct i2c_client *client = to_i2c_client(dev); \
> - struct lm95241_data *data = i2c_get_clientdata(client); \
> -\
> - snprintf(buf, PAGE_SIZE - 1, \
> - data->config & R##flag##DF_MASK ? \
> - "127000\n" : "255000\n"); \
> - return strlen(buf); \
> -}
> -show_max(1);
> -show_max(2);
> -
> -#define set_type(flag) \
> -static ssize_t set_type##flag(struct device *dev, \
> - struct device_attribute *attr, \
> - const char *buf, size_t count) \
> -{ \
> - struct i2c_client *client = to_i2c_client(dev); \
> - struct lm95241_data *data = i2c_get_clientdata(client); \
> -\
> - long val; \
> - strict_strtol(buf, 10, &val); \
> -\
> - if ((val = 1) || (val = 2)) { \
> -\
> - mutex_lock(&data->update_lock); \
> -\
> - data->trutherm &= ~(TT_MASK << TT##flag##_SHIFT); \
> - if (val = 1) { \
> - data->model |= R##flag##MS_MASK; \
> - data->trutherm |= (TT_ON << TT##flag##_SHIFT); \
> - } \
> - else { \
> - data->model &= ~R##flag##MS_MASK; \
> - data->trutherm |= (TT_OFF << TT##flag##_SHIFT); \
> - } \
> -\
> - data->valid = 0; \
> -\
> - i2c_smbus_write_byte_data(client,
> LM95241_REG_RW_REMOTE_MODEL, \
> - data->model); \
> - i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM, \
> - data->trutherm); \
> -\
> - mutex_unlock(&data->update_lock); \
> -\
> - } \
> - return count; \
> -}
> -set_type(1);
> -set_type(2);
> -
> -#define set_min(flag) \
> -static ssize_t set_min##flag(struct device *dev, \
> - struct device_attribute *devattr, const char *buf, size_t count)
> \
> -{ \
> - struct i2c_client *client = to_i2c_client(dev); \
> - struct lm95241_data *data = i2c_get_clientdata(client); \
> -\
> - long val; \
> - strict_strtol(buf, 10, &val); \
> -\
> - mutex_lock(&data->update_lock); \
> -\
> - if (val < 0) \
> - data->config |= R##flag##DF_MASK; \
> - else \
> - data->config &= ~R##flag##DF_MASK; \
> -\
> - data->valid = 0; \
> -\
> - i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
> - data->config); \
> -\
> - mutex_unlock(&data->update_lock); \
> -\
> - return count; \
> -}
> -set_min(1);
> -set_min(2);
> -
> -#define set_max(flag) \
> -static ssize_t set_max##flag(struct device *dev, \
> - struct device_attribute *devattr, const char *buf, size_t count)
> \
> -{ \
> - struct i2c_client *client = to_i2c_client(dev); \
> - struct lm95241_data *data = i2c_get_clientdata(client); \
> -\
> - long val; \
> - strict_strtol(buf, 10, &val); \
> -\
> - mutex_lock(&data->update_lock); \
> -\
> - if (val <= 127000) \
> - data->config |= R##flag##DF_MASK; \
> - else \
> - data->config &= ~R##flag##DF_MASK; \
> -\
> - data->valid = 0; \
> -\
> - i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
> - data->config); \
> -\
> - mutex_unlock(&data->update_lock); \
> -\
> - return count; \
> -}
> -set_max(1);
> -set_max(2);
> -
> -static DEVICE_ATTR(temp1_input, S_IRUGO, show_local, NULL);
> -static DEVICE_ATTR(temp2_input, S_IRUGO, show_remote1, NULL);
> -static DEVICE_ATTR(temp3_input, S_IRUGO, show_remote2, NULL);
> -static DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type1,
> set_type1);
> -static DEVICE_ATTR(temp3_type, S_IWUSR | S_IRUGO, show_type2,
> set_type2);
> -static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min1,
> set_min1);
> -static DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min2,
> set_min2);
> -static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_max1,
> set_max1);
> -static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_max2,
> set_max2);
> static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_interval,
> set_interval);
>
> static struct attribute *lm95241_attributes[] = {
> - &dev_attr_temp1_input.attr,
> - &dev_attr_temp2_input.attr,
> - &dev_attr_temp3_input.attr,
> - &dev_attr_temp2_type.attr,
> - &dev_attr_temp3_type.attr,
> - &dev_attr_temp2_min.attr,
> - &dev_attr_temp3_min.attr,
> - &dev_attr_temp2_max.attr,
> - &dev_attr_temp3_max.attr,
> + &sensor_dev_attr_locale_input.dev_attr.attr,
> + &sensor_dev_attr_remote1_input.dev_attr.attr,
> + &sensor_dev_attr_remote2_input.dev_attr.attr,
> + &sensor_dev_attr_remote1_type.dev_attr.attr,
> + &sensor_dev_attr_remote2_type.dev_attr.attr,
> + &sensor_dev_attr_remote1_min.dev_attr.attr,
> + &sensor_dev_attr_remote2_min.dev_attr.attr,
> + &sensor_dev_attr_remote1_max.dev_attr.attr,
> + &sensor_dev_attr_remote2_max.dev_attr.attr,
> &dev_attr_update_interval.attr,
> NULL
> };
> @@ -322,7 +343,7 @@ static int lm95241_detect(struct i2c_cli
> = MANUFACTURER_ID)
> && (i2c_smbus_read_byte_data(new_client, LM95241_REG_R_CHIP_ID)
> >= DEFAULT_REVISION)) {
> - name = "lm95241";
> + name = DEVNAME;
> } else {
> dev_dbg(&adapter->dev, "LM95241 detection failed at 0x%02x
> \n",
> address);
> @@ -334,6 +355,26 @@ static int lm95241_detect(struct i2c_cli
> return 0;
> }
>
> +static void lm95241_init_client(struct i2c_client *client)
> +{
> + struct lm95241_data *data = i2c_get_clientdata(client);
> +
> + data->interval = HZ; /* 1 sec default */
> + data->valid = 0;
> + data->config = CFG_CR0076;
> + data->model = 0;
> + data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
> +
> + i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
> + data->config);
> + i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
> + R1FE_MASK | R2FE_MASK);
> + i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
> + data->trutherm);
> + i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
> + data->model);
> +}
> +
> static int lm95241_probe(struct i2c_client *new_client,
> const struct i2c_device_id *id)
> {
> @@ -373,26 +414,6 @@ exit:
> return err;
> }
>
> -static void lm95241_init_client(struct i2c_client *client)
> -{
> - struct lm95241_data *data = i2c_get_clientdata(client);
> -
> - data->interval = HZ; /* 1 sec default */
> - data->valid = 0;
> - data->config = CFG_CR0076;
> - data->model = 0;
> - data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
> -
> - i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
> - data->config);
> - i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
> - R1FE_MASK | R2FE_MASK);
> - i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
> - data->trutherm);
> - i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
> - data->model);
> -}
> -
> static int lm95241_remove(struct i2c_client *client)
> {
> struct lm95241_data *data = i2c_get_clientdata(client);
> @@ -404,46 +425,9 @@ static int lm95241_remove(struct i2c_cli
> return 0;
> }
>
> -static struct lm95241_data *lm95241_update_device(struct device *dev)
> -{
> - struct i2c_client *client = to_i2c_client(dev);
> - struct lm95241_data *data = i2c_get_clientdata(client);
> -
> - mutex_lock(&data->update_lock);
> -
> - if (time_after(jiffies, data->last_updated + data->interval) ||
> - !data->valid) {
> - dev_dbg(&client->dev, "Updating lm95241 data.\n");
> - data->local_h > - i2c_smbus_read_byte_data(client,
> - LM95241_REG_R_LOCAL_TEMPH);
> - data->local_l > - i2c_smbus_read_byte_data(client,
> - LM95241_REG_R_LOCAL_TEMPL);
> - data->remote1_h > - i2c_smbus_read_byte_data(client,
> - LM95241_REG_R_REMOTE1_TEMPH);
> - data->remote1_l > - i2c_smbus_read_byte_data(client,
> - LM95241_REG_R_REMOTE1_TEMPL);
> - data->remote2_h > - i2c_smbus_read_byte_data(client,
> - LM95241_REG_R_REMOTE2_TEMPH);
> - data->remote2_l > - i2c_smbus_read_byte_data(client,
> - LM95241_REG_R_REMOTE2_TEMPL);
> - data->last_updated = jiffies;
> - data->valid = 1;
> - }
> -
> - mutex_unlock(&data->update_lock);
> -
> - return data;
> -}
> -
> /* Driver data (common to all clients) */
> static const struct i2c_device_id lm95241_id[] = {
> - { "lm95241", 0 },
> + { DEVNAME, 0 },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, lm95241_id);
> @@ -451,7 +435,7 @@ MODULE_DEVICE_TABLE(i2c, lm95241_id);
> static struct i2c_driver lm95241_driver = {
> .class = I2C_CLASS_HWMON,
> .driver = {
> - .name = "lm95241",
> + .name = DEVNAME,
> },
> .probe = lm95241_probe,
> .remove = lm95241_remove,
> @@ -470,9 +454,10 @@ static void __exit sensors_lm95241_exit(
> i2c_del_driver(&lm95241_driver);
> }
>
> -MODULE_AUTHOR("Davide Rizzo <elpa-rizzo@gmail.com>");
> +MODULE_AUTHOR("Davide Rizzo <elpa.rizzo@gmail.com>");
> MODULE_DESCRIPTION("LM95241 sensor driver");
> MODULE_LICENSE("GPL");
>
> module_init(sensors_lm95241_init);
> module_exit(sensors_lm95241_exit);
> +
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
2010-11-09 22:14 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
@ 2010-11-11 18:18 ` Davide Rizzo
2010-11-11 18:29 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
` (7 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Davide Rizzo @ 2010-11-11 18:18 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1.1: Type: text/plain, Size: 20276 bytes --]
> Hi Davide,
>
> this looks much better, and gives me a chance for a more complete
> review. Please see below.
>
> Unfortunately, I got hit with the MS Exchange server bug, meaning I can
> not apply your patch. So I might still miss something. Next time please
> copy lkml - that will give me a chance to retrieve the patch from
> patchwork.kernel.org.
>
> Thanks,
> Guenter
Hi Guenther, here is the driver again with requested modifies.
What do you mean with "copy lkml" ?
Regards
Davide
From: Davide Rizzo <elpa.rizzo@gmail.com>
Rewriting of driver/hwmon/lm95241.c to avoid using macros
Now it uses SENSOR_DEVICE_ATTR to distinguish similar attr.
Signed-off-by: Davide Rizzo <elpa.rizzo@gmail.com>
---
--- linux-2.6.37-rc1/drivers/hwmon/lm95241.c 2010-11-01
12:54:12.000000000 +0100
+++ linux-2.6.37-rc1.elpa/drivers/hwmon/lm95241.c 2010-11-11
19:12:28.392515642 +0100
@@ -1,13 +1,9 @@
/*
- * lm95241.c - Part of lm_sensors, Linux kernel modules for hardware
- * monitoring
- * Copyright (C) 2008 Davide Rizzo <elpa-rizzo@gmail.com>
+ * Copyright (C) 2008, 2010 Davide Rizzo <elpa.rizzo@gmail.com>
*
- * Based on the max1619 driver. The LM95241 is a sensor chip made by
National
- * Semiconductors.
- * It reports up to three temperatures (its own plus up to
- * two external ones). Complete datasheet can be
- * obtained from National's website at:
+ * The LM95241 is a sensor chip made by National Semiconductors.
+ * It reports up to three temperatures (its own plus up to two external
ones).
+ * Complete datasheet can be obtained from National's website at:
* http://www.national.com/ds.cgi/LM/LM95241.pdf
*
* This program is free software; you can redistribute it and/or modify
@@ -25,16 +21,13 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/
-#include <linux/module.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/mutex.h>
-#include <linux/sysfs.h>
+
+#define DEVNAME "lm95241"
static const unsigned short normal_i2c[] = {
0x19, 0x2a, 0x2b, I2C_CLIENT_END};
@@ -79,13 +72,14 @@ static const unsigned short normal_i2c[]
#define MANUFACTURER_ID 0x01
#define DEFAULT_REVISION 0xA4
-/* Conversions and various macros */
-#define TEMP_FROM_REG(val_h, val_l) (((val_h) & 0x80 ? (val_h) - 0x100 : \
- (val_h)) * 1000 + (val_l) * 1000 / 256)
-
-/* Functions declaration */
-static void lm95241_init_client(struct i2c_client *client);
-static struct lm95241_data *lm95241_update_device(struct device *dev);
+static const u8 lm95241_reg_address[] = {
+ LM95241_REG_R_LOCAL_TEMPH,
+ LM95241_REG_R_LOCAL_TEMPL,
+ LM95241_REG_R_REMOTE1_TEMPH,
+ LM95241_REG_R_REMOTE1_TEMPL,
+ LM95241_REG_R_REMOTE2_TEMPH,
+ LM95241_REG_R_REMOTE2_TEMPL
+};
/* Client data (each client gets its own) */
struct lm95241_data {
@@ -94,211 +88,230 @@ struct lm95241_data {
unsigned long last_updated, interval; /* in jiffies */
char valid; /* zero until following fields are valid */
/* registers values */
- u8 local_h, local_l; /* local */
- u8 remote1_h, remote1_l; /* remote1 */
- u8 remote2_h, remote2_l; /* remote2 */
+ u8 temp[ARRAY_SIZE(lm95241_reg_address)];
u8 config, model, trutherm;
};
+/* Conversions */
+static int TempFromReg(u8 val_h, u8 val_l)
+{
+ if (val_h & 0x80)
+ return val_h - 0x100;
+ return val_h * 1000 + val_l * 1000 / 256;
+}
+
+static struct lm95241_data *lm95241_update_device(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ mutex_lock(&data->update_lock);
+
+ if (time_after(jiffies, data->last_updated + data->interval) ||
+ !data->valid) {
+ int i;
+
+ dev_dbg(&client->dev, "Updating lm95241 data.\n");
+ for (i = 0; i < ARRAY_SIZE(lm95241_reg_address); i++)
+ data->temp[i] = i2c_smbus_read_byte_data(client,
+ lm95241_reg_address[i]);
+ data->last_updated = jiffies;
+ data->valid = 1;
+ }
+
+ mutex_unlock(&data->update_lock);
+
+ return data;
+}
+
/* Sysfs stuff */
-#define show_temp(value) \
-static ssize_t show_##value(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct lm95241_data *data = lm95241_update_device(dev); \
- snprintf(buf, PAGE_SIZE - 1, "%d\n", \
- TEMP_FROM_REG(data->value##_h, data->value##_l)); \
- return strlen(buf); \
-}
-show_temp(local);
-show_temp(remote1);
-show_temp(remote2);
+static ssize_t show_input(struct device *dev, struct device_attribute
*attr,
+ char *buf)
+{
+ struct lm95241_data *data = lm95241_update_device(dev);
-static ssize_t show_interval(struct device *dev, struct device_attribute
*attr,
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n",
+ TempFromReg(data->temp[to_sensor_dev_attr(attr)->index],
+ data->temp[to_sensor_dev_attr(attr)->index + 1]));
+}
+
+static ssize_t show_type(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->model & to_sensor_dev_attr(attr)->index ? "1\n" : "2\n");
+}
+
+static ssize_t set_type(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ unsigned long val;
+ int shift;
+ u8 mask = to_sensor_dev_attr(attr)->index;
+
+ if (strict_strtoul(buf, 10, &val) < 0)
+ return -EINVAL;
+
+ if (val != 1 && val != 2)
+ return -EINVAL;
+ shift = mask == R1MS_MASK ? TT1_SHIFT : TT2_SHIFT;
+
+ mutex_lock(&data->update_lock);
+
+ data->trutherm &= ~(TT_MASK << shift);
+ if (val == 1) {
+ data->model |= mask;
+ data->trutherm |= (TT_ON << shift);
+ } else {
+ data->model &= ~mask;
+ data->trutherm |= (TT_OFF << shift);
+ }
+
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
+ data->model);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
+ data->trutherm);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_min(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->config & to_sensor_dev_attr(attr)->index ?
+ "-127000\n" : "0\n");
+}
+
+static ssize_t set_min(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ long val;
+
+ if (strict_strtol(buf, 10, &val) < 0)
+ return -EINVAL;
+ if (val < -128000)
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ if (val < 0)
+ data->config |= to_sensor_dev_attr(attr)->index;
+ else
+ data->config &= ~to_sensor_dev_attr(attr)->index;
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_max(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->config & to_sensor_dev_attr(attr)->index ?
+ "127000\n" : "255000\n");
+}
+
+static ssize_t set_max(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ long val;
+
+ if (strict_strtol(buf, 10, &val) < 0)
+ return -EINVAL;
+ if (val >= 256000)
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ if (val <= 127000)
+ data->config |= to_sensor_dev_attr(attr)->index;
+ else
+ data->config &= ~to_sensor_dev_attr(attr)->index;
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_interval(struct device *dev, struct device_attribute
*attr,
+ char *buf)
+{
struct lm95241_data *data = lm95241_update_device(dev);
- snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval / HZ);
- return strlen(buf);
+ return snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval
+ / HZ);
}
static ssize_t set_interval(struct device *dev, struct device_attribute
*attr,
- const char *buf, size_t count)
+ const char *buf, size_t count)
{
struct i2c_client *client = to_i2c_client(dev);
struct lm95241_data *data = i2c_get_clientdata(client);
+ unsigned long val;
+
+ if (strict_strtoul(buf, 10, &val) < 0)
+ return -EINVAL;
- strict_strtol(buf, 10, &data->interval);
- data->interval = data->interval * HZ / 1000;
+ data->interval = val * HZ / 1000;
return count;
}
-#define show_type(flag) \
-static ssize_t show_type##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->model & R##flag##MS_MASK ? "1\n" : "2\n"); \
- return strlen(buf); \
-}
-show_type(1);
-show_type(2);
-
-#define show_min(flag) \
-static ssize_t show_min##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->config & R##flag##DF_MASK ? \
- "-127000\n" : "0\n"); \
- return strlen(buf); \
-}
-show_min(1);
-show_min(2);
-
-#define show_max(flag) \
-static ssize_t show_max##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->config & R##flag##DF_MASK ? \
- "127000\n" : "255000\n"); \
- return strlen(buf); \
-}
-show_max(1);
-show_max(2);
-
-#define set_type(flag) \
-static ssize_t set_type##flag(struct device *dev, \
- struct device_attribute *attr, \
- const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
- strict_strtol(buf, 10, &val); \
-\
- if ((val == 1) || (val == 2)) { \
-\
- mutex_lock(&data->update_lock); \
-\
- data->trutherm &= ~(TT_MASK << TT##flag##_SHIFT); \
- if (val == 1) { \
- data->model |= R##flag##MS_MASK; \
- data->trutherm |= (TT_ON << TT##flag##_SHIFT); \
- } \
- else { \
- data->model &= ~R##flag##MS_MASK; \
- data->trutherm |= (TT_OFF << TT##flag##_SHIFT); \
- } \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL, \
- data->model); \
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM, \
- data->trutherm); \
-\
- mutex_unlock(&data->update_lock); \
-\
- } \
- return count; \
-}
-set_type(1);
-set_type(2);
-
-#define set_min(flag) \
-static ssize_t set_min##flag(struct device *dev, \
- struct device_attribute *devattr, const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
- strict_strtol(buf, 10, &val); \
-\
- mutex_lock(&data->update_lock); \
-\
- if (val < 0) \
- data->config |= R##flag##DF_MASK; \
- else \
- data->config &= ~R##flag##DF_MASK; \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
- data->config); \
-\
- mutex_unlock(&data->update_lock); \
-\
- return count; \
-}
-set_min(1);
-set_min(2);
-
-#define set_max(flag) \
-static ssize_t set_max##flag(struct device *dev, \
- struct device_attribute *devattr, const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
- strict_strtol(buf, 10, &val); \
-\
- mutex_lock(&data->update_lock); \
-\
- if (val <= 127000) \
- data->config |= R##flag##DF_MASK; \
- else \
- data->config &= ~R##flag##DF_MASK; \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
- data->config); \
-\
- mutex_unlock(&data->update_lock); \
-\
- return count; \
-}
-set_max(1);
-set_max(2);
-
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_local, NULL);
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_remote1, NULL);
-static DEVICE_ATTR(temp3_input, S_IRUGO, show_remote2, NULL);
-static DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type1, set_type1);
-static DEVICE_ATTR(temp3_type, S_IWUSR | S_IRUGO, show_type2, set_type2);
-static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min1, set_min1);
-static DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min2, set_min2);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_max1, set_max1);
-static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_max2, set_max2);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_input, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_input, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_input, NULL, 4);
+static SENSOR_DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type,
set_type,
+ R1MS_MASK);
+static SENSOR_DEVICE_ATTR(temp3_type, S_IWUSR | S_IRUGO, show_type,
set_type,
+ R2MS_MASK);
+static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min, set_min,
+ R1DF_MASK);
+static SENSOR_DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min, set_min,
+ R2DF_MASK);
+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 DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_interval,
set_interval);
static struct attribute *lm95241_attributes[] = {
- &dev_attr_temp1_input.attr,
- &dev_attr_temp2_input.attr,
- &dev_attr_temp3_input.attr,
- &dev_attr_temp2_type.attr,
- &dev_attr_temp3_type.attr,
- &dev_attr_temp2_min.attr,
- &dev_attr_temp3_min.attr,
- &dev_attr_temp2_max.attr,
- &dev_attr_temp3_max.attr,
+ &sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_input.dev_attr.attr,
+ &sensor_dev_attr_temp3_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_type.dev_attr.attr,
+ &sensor_dev_attr_temp3_type.dev_attr.attr,
+ &sensor_dev_attr_temp2_min.dev_attr.attr,
+ &sensor_dev_attr_temp3_min.dev_attr.attr,
+ &sensor_dev_attr_temp2_max.dev_attr.attr,
+ &sensor_dev_attr_temp3_max.dev_attr.attr,
&dev_attr_update_interval.attr,
NULL
};
@@ -322,7 +335,7 @@ static int lm95241_detect(struct i2c_cli
== MANUFACTURER_ID)
&& (i2c_smbus_read_byte_data(new_client, LM95241_REG_R_CHIP_ID)
>= DEFAULT_REVISION)) {
- name = "lm95241";
+ name = DEVNAME;
} else {
dev_dbg(&adapter->dev, "LM95241 detection failed at 0x%02x\n",
address);
@@ -334,6 +347,26 @@ static int lm95241_detect(struct i2c_cli
return 0;
}
+static void lm95241_init_client(struct i2c_client *client)
+{
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ data->interval = HZ; /* 1 sec default */
+ data->valid = 0;
+ data->config = CFG_CR0076;
+ data->model = 0;
+ data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
+ data->config);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
+ R1FE_MASK | R2FE_MASK);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
+ data->trutherm);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
+ data->model);
+}
+
static int lm95241_probe(struct i2c_client *new_client,
const struct i2c_device_id *id)
{
@@ -373,26 +406,6 @@ exit:
return err;
}
-static void lm95241_init_client(struct i2c_client *client)
-{
- struct lm95241_data *data = i2c_get_clientdata(client);
-
- data->interval = HZ; /* 1 sec default */
- data->valid = 0;
- data->config = CFG_CR0076;
- data->model = 0;
- data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
-
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
- data->config);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
- R1FE_MASK | R2FE_MASK);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
- data->trutherm);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
- data->model);
-}
-
static int lm95241_remove(struct i2c_client *client)
{
struct lm95241_data *data = i2c_get_clientdata(client);
@@ -404,46 +417,9 @@ static int lm95241_remove(struct i2c_cli
return 0;
}
-static struct lm95241_data *lm95241_update_device(struct device *dev)
-{
- struct i2c_client *client = to_i2c_client(dev);
- struct lm95241_data *data = i2c_get_clientdata(client);
-
- mutex_lock(&data->update_lock);
-
- if (time_after(jiffies, data->last_updated + data->interval) ||
- !data->valid) {
- dev_dbg(&client->dev, "Updating lm95241 data.\n");
- data->local_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_LOCAL_TEMPH);
- data->local_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_LOCAL_TEMPL);
- data->remote1_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE1_TEMPH);
- data->remote1_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE1_TEMPL);
- data->remote2_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE2_TEMPH);
- data->remote2_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE2_TEMPL);
- data->last_updated = jiffies;
- data->valid = 1;
- }
-
- mutex_unlock(&data->update_lock);
-
- return data;
-}
-
/* Driver data (common to all clients) */
static const struct i2c_device_id lm95241_id[] = {
- { "lm95241", 0 },
+ { DEVNAME, 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, lm95241_id);
@@ -451,7 +427,7 @@ MODULE_DEVICE_TABLE(i2c, lm95241_id);
static struct i2c_driver lm95241_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
- .name = "lm95241",
+ .name = DEVNAME,
},
.probe = lm95241_probe,
.remove = lm95241_remove,
@@ -470,9 +446,10 @@ static void __exit sensors_lm95241_exit(
i2c_del_driver(&lm95241_driver);
}
-MODULE_AUTHOR("Davide Rizzo <elpa-rizzo@gmail.com>");
+MODULE_AUTHOR("Davide Rizzo <elpa.rizzo@gmail.com>");
MODULE_DESCRIPTION("LM95241 sensor driver");
MODULE_LICENSE("GPL");
module_init(sensors_lm95241_init);
module_exit(sensors_lm95241_exit);
+
[-- Attachment #1.2: Type: text/html, Size: 23891 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
2010-11-09 22:14 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
2010-11-11 18:18 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
@ 2010-11-11 18:29 ` Guenter Roeck
2010-11-11 19:07 ` Jean Delvare
` (6 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2010-11-11 18:29 UTC (permalink / raw)
To: lm-sensors
On Thu, 2010-11-11 at 13:18 -0500, Davide Rizzo wrote:
>
> > Hi Davide,
> >
> > this looks much better, and gives me a chance for a more complete
> > review. Please see below.
> >
> > Unfortunately, I got hit with the MS Exchange server bug, meaning I
> can
> > not apply your patch. So I might still miss something. Next time
> please
> > copy lkml - that will give me a chance to retrieve the patch from
> > patchwork.kernel.org.
> >
> > Thanks,
> > Guenter
> Hi Guenther, here is the driver again with requested modifies.
> What do you mean with "copy lkml" ?
lkml: linux kernel mailing list, or linux-kernel@vger.kernel.org.
Patches sent to that list show up on patchwork.kernel.org, where I can
pull it from if our mailer acts up.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
` (2 preceding siblings ...)
2010-11-11 18:29 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
@ 2010-11-11 19:07 ` Jean Delvare
2010-11-11 19:29 ` Guenter Roeck
` (5 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Jean Delvare @ 2010-11-11 19:07 UTC (permalink / raw)
To: lm-sensors
Hi Guenter,
On Thu, 11 Nov 2010 10:29:01 -0800, Guenter Roeck wrote:
> On Thu, 2010-11-11 at 13:18 -0500, Davide Rizzo wrote:
> >
> > > Hi Davide,
> > >
> > > this looks much better, and gives me a chance for a more complete
> > > review. Please see below.
> > >
> > > Unfortunately, I got hit with the MS Exchange server bug, meaning I
> > > can not apply your patch. So I might still miss something. Next time
> > > please copy lkml - that will give me a chance to retrieve the patch from
> > > patchwork.kernel.org.
> >
> > Hi Guenther, here is the driver again with requested modifies.
> > What do you mean with "copy lkml" ?
>
> lkml: linux kernel mailing list, or linux-kernel@vger.kernel.org.
> Patches sent to that list show up on patchwork.kernel.org, where I can
> pull it from if our mailer acts up.
You can also get the raw messages from marc.info:
http://marc.info/?l=lm-sensors
(Click on "Download message RAW".)
From what I can see, Davide's message is messed up at the source (long
lines are folded.) Davide, you'll have to sort it out. It may be a
simple e-mail client option to switch.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
` (3 preceding siblings ...)
2010-11-11 19:07 ` Jean Delvare
@ 2010-11-11 19:29 ` Guenter Roeck
2010-11-11 19:33 ` Jean Delvare
` (4 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2010-11-11 19:29 UTC (permalink / raw)
To: lm-sensors
On Thu, 2010-11-11 at 14:07 -0500, Jean Delvare wrote:
> Hi Guenter,
>
> On Thu, 11 Nov 2010 10:29:01 -0800, Guenter Roeck wrote:
> > On Thu, 2010-11-11 at 13:18 -0500, Davide Rizzo wrote:
> > >
> > > > Hi Davide,
> > > >
> > > > this looks much better, and gives me a chance for a more complete
> > > > review. Please see below.
> > > >
> > > > Unfortunately, I got hit with the MS Exchange server bug, meaning I
> > > > can not apply your patch. So I might still miss something. Next time
> > > > please copy lkml - that will give me a chance to retrieve the patch from
> > > > patchwork.kernel.org.
> > >
> > > Hi Guenther, here is the driver again with requested modifies.
> > > What do you mean with "copy lkml" ?
> >
> > lkml: linux kernel mailing list, or linux-kernel@vger.kernel.org.
> > Patches sent to that list show up on patchwork.kernel.org, where I can
> > pull it from if our mailer acts up.
>
> You can also get the raw messages from marc.info:
> http://marc.info/?l=lm-sensors
> (Click on "Download message RAW".)
>
Ok, found it. That should help for the future. Sorry for the noise.
> From what I can see, Davide's message is messed up at the source (long
> lines are folded.) Davide, you'll have to sort it out. It may be a
> simple e-mail client option to switch.
>
Apparently. Also, even the raw version pulled from marc.info has tabs
replaced with blanks. So maybe it wasn't our mailer this time.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
` (4 preceding siblings ...)
2010-11-11 19:29 ` Guenter Roeck
@ 2010-11-11 19:33 ` Jean Delvare
2010-11-12 20:49 ` Guenter Roeck
` (3 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Jean Delvare @ 2010-11-11 19:33 UTC (permalink / raw)
To: lm-sensors
On Thu, 11 Nov 2010 11:29:11 -0800, Guenter Roeck wrote:
> On Thu, 2010-11-11 at 14:07 -0500, Jean Delvare wrote:
> > From what I can see, Davide's message is messed up at the source (long
> > lines are folded.) Davide, you'll have to sort it out. It may be a
> > simple e-mail client option to switch.
> >
> Apparently. Also, even the raw version pulled from marc.info has tabs
> replaced with blanks. So maybe it wasn't our mailer this time.
Indeed, I see this problem here as well.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
` (5 preceding siblings ...)
2010-11-11 19:33 ` Jean Delvare
@ 2010-11-12 20:49 ` Guenter Roeck
2010-11-16 15:03 ` Guenter Roeck
` (2 subsequent siblings)
9 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2010-11-12 20:49 UTC (permalink / raw)
To: lm-sensors
On Thu, 2010-11-11 at 13:18 -0500, Davide Rizzo wrote:
>
> > Hi Davide,
> >
> > this looks much better, and gives me a chance for a more complete
> > review. Please see below.
> >
> > Unfortunately, I got hit with the MS Exchange server bug, meaning I
> can
> > not apply your patch. So I might still miss something. Next time
> please
> > copy lkml - that will give me a chance to retrieve the patch from
> > patchwork.kernel.org.
> >
> > Thanks,
> > Guenter
> Hi Guenther, here is the driver again with requested modifies.
> What do you mean with "copy lkml" ?
> Regards
> Davide
>
> From: Davide Rizzo <elpa.rizzo@gmail.com>
>
Hi Davide,
in case it got lost - I am still waiting for a non-corrupted version of
your patch.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
` (6 preceding siblings ...)
2010-11-12 20:49 ` Guenter Roeck
@ 2010-11-16 15:03 ` Guenter Roeck
2010-11-17 11:05 ` [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
2011-01-17 8:31 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Davide Rizzo
2011-01-17 8:49 ` Jean Delvare
9 siblings, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2010-11-16 15:03 UTC (permalink / raw)
To: lm-sensors
On Thu, Nov 11, 2010 at 01:18:56PM -0500, Davide Rizzo wrote:
> > Hi Davide,
> >
> > this looks much better, and gives me a chance for a more complete
> > review. Please see below.
> >
> > Unfortunately, I got hit with the MS Exchange server bug, meaning I can
> > not apply your patch. So I might still miss something. Next time please
> > copy lkml - that will give me a chance to retrieve the patch from
> > patchwork.kernel.org<http://patchwork.kernel.org/>.
> >
> > Thanks,
> > Guenter
> Hi Guenther, here is the driver again with requested modifies.
> What do you mean with "copy lkml" ?
> Regards
> Davide
>
> From: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
>
> Rewriting of driver/hwmon/lm95241.c to avoid using macros
> Now it uses SENSOR_DEVICE_ATTR to distinguish similar attr.
> Signed-off-by: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
> ---
> --- linux-2.6.37-rc1/drivers/hwmon/lm95241.c 2010-11-01 12:54:12.000000000 +0100
> +++ linux-2.6.37-rc1.elpa/drivers/hwmon/lm95241.c 2010-11-11 19:12:28.392515642 +0100
> @@ -1,13 +1,9 @@
> /*
> - * lm95241.c - Part of lm_sensors, Linux kernel modules for hardware
> - * monitoring
> - * Copyright (C) 2008 Davide Rizzo <elpa-rizzo@gmail.com<mailto:elpa-rizzo@gmail.com>>
> + * Copyright (C) 2008, 2010 Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
> *
> - * Based on the max1619 driver. The LM95241 is a sensor chip made by National
> - * Semiconductors.
> - * It reports up to three temperatures (its own plus up to
> - * two external ones). Complete datasheet can be
> - * obtained from National's website at:
> + * The LM95241 is a sensor chip made by National Semiconductors.
> + * It reports up to three temperatures (its own plus up to two external ones).
> + * Complete datasheet can be obtained from National's website at:
> * http://www.national.com/ds.cgi/LM/LM95241.pdf
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -25,16 +21,13 @@
> * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
> */
>
> -#include <linux/module.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/mutex.h>
> -#include <linux/sysfs.h>
Removing the above includes violates SubmitChecklist rule #1. Also, please reparent to Linus'
latest tree.
Other than that, there are a only few formatting issues, but I can take care of those myself.
So please re-submit with the above changes, and we should be ready to go.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-16 15:03 ` Guenter Roeck
@ 2010-11-17 11:05 ` Davide Rizzo
0 siblings, 0 replies; 19+ messages in thread
From: Davide Rizzo @ 2010-11-17 11:05 UTC (permalink / raw)
To: Guenter Roeck, LM Sensors, linux-kernel; +Cc: Jean Delvare
> Removing the above includes violates SubmitChecklist rule #1. Also, please reparent to Linus'
> latest tree.
>
> Other than that, there are a only few formatting issues, but I can take care of those myself.
> So please re-submit with the above changes, and we should be ready to go.
>
> Thanks,
> Guenter
Here it is as requested.
Regards,
Davide Rizzo
From: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
Rewriting of driver/hwmon/lm95241.c to avoid using macros
Signed-off-by: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
---
--- linux-2.6.37-rc2/drivers/hwmon/lm95241.c 2010-11-16 03:31:02.000000000 +0100
+++ linux-2.6.37-rc2.elpa/drivers/hwmon/lm95241.c 2010-11-17
11:47:21.911752940 +0100
@@ -1,13 +1,9 @@
/*
- * lm95241.c - Part of lm_sensors, Linux kernel modules for hardware
- * monitoring
- * Copyright (C) 2008 Davide Rizzo <elpa-rizzo@gmail.com>
+ * Copyright (C) 2008, 2010 Davide Rizzo <elpa.rizzo@gmail.com>
*
- * Based on the max1619 driver. The LM95241 is a sensor chip made by National
- * Semiconductors.
- * It reports up to three temperatures (its own plus up to
- * two external ones). Complete datasheet can be
- * obtained from National's website at:
+ * The LM95241 is a sensor chip made by National Semiconductors.
+ * It reports up to three temperatures (its own plus up to two external ones).
+ * Complete datasheet can be obtained from National's website at:
* http://www.national.com/ds.cgi/LM/LM95241.pdf
*
* This program is free software; you can redistribute it and/or modify
@@ -27,14 +23,17 @@
#include <linux/module.h>
#include <linux/init.h>
-#include <linux/slab.h>
#include <linux/jiffies.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+
+#include <linux/slab.h>
#include <linux/i2c.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
#include <linux/err.h>
-#include <linux/mutex.h>
-#include <linux/sysfs.h>
+
+#define DEVNAME "lm95241"
static const unsigned short normal_i2c[] = {
0x19, 0x2a, 0x2b, I2C_CLIENT_END};
@@ -79,13 +78,14 @@ static const unsigned short normal_i2c[]
#define MANUFACTURER_ID 0x01
#define DEFAULT_REVISION 0xA4
-/* Conversions and various macros */
-#define TEMP_FROM_REG(val_h, val_l) (((val_h) & 0x80 ? (val_h) - 0x100 : \
- (val_h)) * 1000 + (val_l) * 1000 / 256)
-
-/* Functions declaration */
-static void lm95241_init_client(struct i2c_client *client);
-static struct lm95241_data *lm95241_update_device(struct device *dev);
+static const u8 lm95241_reg_address[] = {
+ LM95241_REG_R_LOCAL_TEMPH,
+ LM95241_REG_R_LOCAL_TEMPL,
+ LM95241_REG_R_REMOTE1_TEMPH,
+ LM95241_REG_R_REMOTE1_TEMPL,
+ LM95241_REG_R_REMOTE2_TEMPH,
+ LM95241_REG_R_REMOTE2_TEMPL
+};
/* Client data (each client gets its own) */
struct lm95241_data {
@@ -94,37 +94,189 @@ struct lm95241_data {
unsigned long last_updated, interval; /* in jiffies */
char valid; /* zero until following fields are valid */
/* registers values */
- u8 local_h, local_l; /* local */
- u8 remote1_h, remote1_l; /* remote1 */
- u8 remote2_h, remote2_l; /* remote2 */
+ u8 temp[ARRAY_SIZE(lm95241_reg_address)];
u8 config, model, trutherm;
};
+/* Conversions */
+static int TempFromReg(u8 val_h, u8 val_l)
+{
+ if (val_h & 0x80)
+ return val_h - 0x100;
+ return val_h * 1000 + val_l * 1000 / 256;
+}
+
+static struct lm95241_data *lm95241_update_device(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ mutex_lock(&data->update_lock);
+
+ if (time_after(jiffies, data->last_updated + data->interval) ||
+ !data->valid) {
+ int i;
+
+ dev_dbg(&client->dev, "Updating lm95241 data.\n");
+ for (i = 0; i < ARRAY_SIZE(lm95241_reg_address); i++)
+ data->temp[i] = i2c_smbus_read_byte_data(client,
+ lm95241_reg_address[i]);
+ data->last_updated = jiffies;
+ data->valid = 1;
+ }
+
+ mutex_unlock(&data->update_lock);
+
+ return data;
+}
+
/* Sysfs stuff */
-#define show_temp(value) \
-static ssize_t show_##value(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct lm95241_data *data = lm95241_update_device(dev); \
- snprintf(buf, PAGE_SIZE - 1, "%d\n", \
- TEMP_FROM_REG(data->value##_h, data->value##_l)); \
- return strlen(buf); \
-}
-show_temp(local);
-show_temp(remote1);
-show_temp(remote2);
+static ssize_t show_input(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct lm95241_data *data = lm95241_update_device(dev);
-static ssize_t show_interval(struct device *dev, struct device_attribute *attr,
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n",
+ TempFromReg(data->temp[to_sensor_dev_attr(attr)->index],
+ data->temp[to_sensor_dev_attr(attr)->index + 1]));
+}
+
+static ssize_t show_type(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->model & to_sensor_dev_attr(attr)->index ? "1\n" : "2\n");
+}
+
+static ssize_t set_type(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ unsigned long val;
+ int shift;
+ u8 mask = to_sensor_dev_attr(attr)->index;
+
+ if (strict_strtoul(buf, 10, &val) < 0)
+ return -EINVAL;
+
+ if (val != 1 && val != 2)
+ return -EINVAL;
+ shift = mask = R1MS_MASK ? TT1_SHIFT : TT2_SHIFT;
+
+ mutex_lock(&data->update_lock);
+
+ data->trutherm &= ~(TT_MASK << shift);
+ if (val = 1) {
+ data->model |= mask;
+ data->trutherm |= (TT_ON << shift);
+ } else {
+ data->model &= ~mask;
+ data->trutherm |= (TT_OFF << shift);
+ }
+
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
+ data->model);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
+ data->trutherm);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_min(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->config & to_sensor_dev_attr(attr)->index ?
+ "-127000\n" : "0\n");
+}
+
+static ssize_t set_min(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ long val;
+
+ if (strict_strtol(buf, 10, &val) < 0)
+ return -EINVAL;
+ if (val < -128000)
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ if (val < 0)
+ data->config |= to_sensor_dev_attr(attr)->index;
+ else
+ data->config &= ~to_sensor_dev_attr(attr)->index;
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_max(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->config & to_sensor_dev_attr(attr)->index ?
+ "127000\n" : "255000\n");
+}
+
+static ssize_t set_max(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ long val;
+
+ if (strict_strtol(buf, 10, &val) < 0)
+ return -EINVAL;
+ if (val >= 256000)
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ if (val <= 127000)
+ data->config |= to_sensor_dev_attr(attr)->index;
+ else
+ data->config &= ~to_sensor_dev_attr(attr)->index;
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_interval(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
struct lm95241_data *data = lm95241_update_device(dev);
- snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval / HZ);
- return strlen(buf);
+ return snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval
+ / HZ);
}
static ssize_t set_interval(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
+ const char *buf, size_t count)
{
struct i2c_client *client = to_i2c_client(dev);
struct lm95241_data *data = i2c_get_clientdata(client);
@@ -138,176 +290,34 @@ static ssize_t set_interval(struct devic
return count;
}
-#define show_type(flag) \
-static ssize_t show_type##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->model & R##flag##MS_MASK ? "1\n" : "2\n"); \
- return strlen(buf); \
-}
-show_type(1);
-show_type(2);
-
-#define show_min(flag) \
-static ssize_t show_min##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->config & R##flag##DF_MASK ? \
- "-127000\n" : "0\n"); \
- return strlen(buf); \
-}
-show_min(1);
-show_min(2);
-
-#define show_max(flag) \
-static ssize_t show_max##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->config & R##flag##DF_MASK ? \
- "127000\n" : "255000\n"); \
- return strlen(buf); \
-}
-show_max(1);
-show_max(2);
-
-#define set_type(flag) \
-static ssize_t set_type##flag(struct device *dev, \
- struct device_attribute *attr, \
- const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
-\
- if (strict_strtol(buf, 10, &val) < 0) \
- return -EINVAL; \
-\
- if ((val = 1) || (val = 2)) { \
-\
- mutex_lock(&data->update_lock); \
-\
- data->trutherm &= ~(TT_MASK << TT##flag##_SHIFT); \
- if (val = 1) { \
- data->model |= R##flag##MS_MASK; \
- data->trutherm |= (TT_ON << TT##flag##_SHIFT); \
- } \
- else { \
- data->model &= ~R##flag##MS_MASK; \
- data->trutherm |= (TT_OFF << TT##flag##_SHIFT); \
- } \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL, \
- data->model); \
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM, \
- data->trutherm); \
-\
- mutex_unlock(&data->update_lock); \
-\
- } \
- return count; \
-}
-set_type(1);
-set_type(2);
-
-#define set_min(flag) \
-static ssize_t set_min##flag(struct device *dev, \
- struct device_attribute *devattr, const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
-\
- if (strict_strtol(buf, 10, &val) < 0) \
- return -EINVAL;\
-\
- mutex_lock(&data->update_lock); \
-\
- if (val < 0) \
- data->config |= R##flag##DF_MASK; \
- else \
- data->config &= ~R##flag##DF_MASK; \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
- data->config); \
-\
- mutex_unlock(&data->update_lock); \
-\
- return count; \
-}
-set_min(1);
-set_min(2);
-
-#define set_max(flag) \
-static ssize_t set_max##flag(struct device *dev, \
- struct device_attribute *devattr, const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
-\
- if (strict_strtol(buf, 10, &val) < 0) \
- return -EINVAL; \
-\
- mutex_lock(&data->update_lock); \
-\
- if (val <= 127000) \
- data->config |= R##flag##DF_MASK; \
- else \
- data->config &= ~R##flag##DF_MASK; \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
- data->config); \
-\
- mutex_unlock(&data->update_lock); \
-\
- return count; \
-}
-set_max(1);
-set_max(2);
-
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_local, NULL);
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_remote1, NULL);
-static DEVICE_ATTR(temp3_input, S_IRUGO, show_remote2, NULL);
-static DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type1, set_type1);
-static DEVICE_ATTR(temp3_type, S_IWUSR | S_IRUGO, show_type2, set_type2);
-static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min1, set_min1);
-static DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min2, set_min2);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_max1, set_max1);
-static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_max2, set_max2);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_input, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_input, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_input, NULL, 4);
+static SENSOR_DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type, set_type,
+ R1MS_MASK);
+static SENSOR_DEVICE_ATTR(temp3_type, S_IWUSR | S_IRUGO, show_type, set_type,
+ R2MS_MASK);
+static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min, set_min,
+ R1DF_MASK);
+static SENSOR_DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min, set_min,
+ R2DF_MASK);
+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 DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_interval,
set_interval);
static struct attribute *lm95241_attributes[] = {
- &dev_attr_temp1_input.attr,
- &dev_attr_temp2_input.attr,
- &dev_attr_temp3_input.attr,
- &dev_attr_temp2_type.attr,
- &dev_attr_temp3_type.attr,
- &dev_attr_temp2_min.attr,
- &dev_attr_temp3_min.attr,
- &dev_attr_temp2_max.attr,
- &dev_attr_temp3_max.attr,
+ &sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_input.dev_attr.attr,
+ &sensor_dev_attr_temp3_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_type.dev_attr.attr,
+ &sensor_dev_attr_temp3_type.dev_attr.attr,
+ &sensor_dev_attr_temp2_min.dev_attr.attr,
+ &sensor_dev_attr_temp3_min.dev_attr.attr,
+ &sensor_dev_attr_temp2_max.dev_attr.attr,
+ &sensor_dev_attr_temp3_max.dev_attr.attr,
&dev_attr_update_interval.attr,
NULL
};
@@ -331,7 +341,7 @@ static int lm95241_detect(struct i2c_cli
= MANUFACTURER_ID)
&& (i2c_smbus_read_byte_data(new_client, LM95241_REG_R_CHIP_ID)
>= DEFAULT_REVISION)) {
- name = "lm95241";
+ name = DEVNAME;
} else {
dev_dbg(&adapter->dev, "LM95241 detection failed at 0x%02x\n",
address);
@@ -343,6 +353,26 @@ static int lm95241_detect(struct i2c_cli
return 0;
}
+static void lm95241_init_client(struct i2c_client *client)
+{
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ data->interval = HZ; /* 1 sec default */
+ data->valid = 0;
+ data->config = CFG_CR0076;
+ data->model = 0;
+ data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
+ data->config);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
+ R1FE_MASK | R2FE_MASK);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
+ data->trutherm);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
+ data->model);
+}
+
static int lm95241_probe(struct i2c_client *new_client,
const struct i2c_device_id *id)
{
@@ -382,26 +412,6 @@ exit:
return err;
}
-static void lm95241_init_client(struct i2c_client *client)
-{
- struct lm95241_data *data = i2c_get_clientdata(client);
-
- data->interval = HZ; /* 1 sec default */
- data->valid = 0;
- data->config = CFG_CR0076;
- data->model = 0;
- data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
-
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
- data->config);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
- R1FE_MASK | R2FE_MASK);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
- data->trutherm);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
- data->model);
-}
-
static int lm95241_remove(struct i2c_client *client)
{
struct lm95241_data *data = i2c_get_clientdata(client);
@@ -413,46 +423,9 @@ static int lm95241_remove(struct i2c_cli
return 0;
}
-static struct lm95241_data *lm95241_update_device(struct device *dev)
-{
- struct i2c_client *client = to_i2c_client(dev);
- struct lm95241_data *data = i2c_get_clientdata(client);
-
- mutex_lock(&data->update_lock);
-
- if (time_after(jiffies, data->last_updated + data->interval) ||
- !data->valid) {
- dev_dbg(&client->dev, "Updating lm95241 data.\n");
- data->local_h - i2c_smbus_read_byte_data(client,
- LM95241_REG_R_LOCAL_TEMPH);
- data->local_l - i2c_smbus_read_byte_data(client,
- LM95241_REG_R_LOCAL_TEMPL);
- data->remote1_h - i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE1_TEMPH);
- data->remote1_l - i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE1_TEMPL);
- data->remote2_h - i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE2_TEMPH);
- data->remote2_l - i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE2_TEMPL);
- data->last_updated = jiffies;
- data->valid = 1;
- }
-
- mutex_unlock(&data->update_lock);
-
- return data;
-}
-
/* Driver data (common to all clients) */
static const struct i2c_device_id lm95241_id[] = {
- { "lm95241", 0 },
+ { DEVNAME, 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, lm95241_id);
@@ -460,7 +433,7 @@ MODULE_DEVICE_TABLE(i2c, lm95241_id);
static struct i2c_driver lm95241_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
- .name = "lm95241",
+ .name = DEVNAME,
},
.probe = lm95241_probe,
.remove = lm95241_remove,
@@ -479,9 +452,10 @@ static void __exit sensors_lm95241_exit(
i2c_del_driver(&lm95241_driver);
}
-MODULE_AUTHOR("Davide Rizzo <elpa-rizzo@gmail.com>");
+MODULE_AUTHOR("Davide Rizzo <elpa.rizzo@gmail.com>");
MODULE_DESCRIPTION("LM95241 sensor driver");
MODULE_LICENSE("GPL");
module_init(sensors_lm95241_init);
module_exit(sensors_lm95241_exit);
+
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] hwmon: (lm95241) Rewritten without using macros
@ 2010-11-17 11:05 ` Davide Rizzo
0 siblings, 0 replies; 19+ messages in thread
From: Davide Rizzo @ 2010-11-17 11:05 UTC (permalink / raw)
To: Guenter Roeck, LM Sensors, linux-kernel; +Cc: Jean Delvare
> Removing the above includes violates SubmitChecklist rule #1. Also, please reparent to Linus'
> latest tree.
>
> Other than that, there are a only few formatting issues, but I can take care of those myself.
> So please re-submit with the above changes, and we should be ready to go.
>
> Thanks,
> Guenter
Here it is as requested.
Regards,
Davide Rizzo
From: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
Rewriting of driver/hwmon/lm95241.c to avoid using macros
Signed-off-by: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
---
--- linux-2.6.37-rc2/drivers/hwmon/lm95241.c 2010-11-16 03:31:02.000000000 +0100
+++ linux-2.6.37-rc2.elpa/drivers/hwmon/lm95241.c 2010-11-17
11:47:21.911752940 +0100
@@ -1,13 +1,9 @@
/*
- * lm95241.c - Part of lm_sensors, Linux kernel modules for hardware
- * monitoring
- * Copyright (C) 2008 Davide Rizzo <elpa-rizzo@gmail.com>
+ * Copyright (C) 2008, 2010 Davide Rizzo <elpa.rizzo@gmail.com>
*
- * Based on the max1619 driver. The LM95241 is a sensor chip made by National
- * Semiconductors.
- * It reports up to three temperatures (its own plus up to
- * two external ones). Complete datasheet can be
- * obtained from National's website at:
+ * The LM95241 is a sensor chip made by National Semiconductors.
+ * It reports up to three temperatures (its own plus up to two external ones).
+ * Complete datasheet can be obtained from National's website at:
* http://www.national.com/ds.cgi/LM/LM95241.pdf
*
* This program is free software; you can redistribute it and/or modify
@@ -27,14 +23,17 @@
#include <linux/module.h>
#include <linux/init.h>
-#include <linux/slab.h>
#include <linux/jiffies.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+
+#include <linux/slab.h>
#include <linux/i2c.h>
#include <linux/hwmon.h>
#include <linux/hwmon-sysfs.h>
#include <linux/err.h>
-#include <linux/mutex.h>
-#include <linux/sysfs.h>
+
+#define DEVNAME "lm95241"
static const unsigned short normal_i2c[] = {
0x19, 0x2a, 0x2b, I2C_CLIENT_END};
@@ -79,13 +78,14 @@ static const unsigned short normal_i2c[]
#define MANUFACTURER_ID 0x01
#define DEFAULT_REVISION 0xA4
-/* Conversions and various macros */
-#define TEMP_FROM_REG(val_h, val_l) (((val_h) & 0x80 ? (val_h) - 0x100 : \
- (val_h)) * 1000 + (val_l) * 1000 / 256)
-
-/* Functions declaration */
-static void lm95241_init_client(struct i2c_client *client);
-static struct lm95241_data *lm95241_update_device(struct device *dev);
+static const u8 lm95241_reg_address[] = {
+ LM95241_REG_R_LOCAL_TEMPH,
+ LM95241_REG_R_LOCAL_TEMPL,
+ LM95241_REG_R_REMOTE1_TEMPH,
+ LM95241_REG_R_REMOTE1_TEMPL,
+ LM95241_REG_R_REMOTE2_TEMPH,
+ LM95241_REG_R_REMOTE2_TEMPL
+};
/* Client data (each client gets its own) */
struct lm95241_data {
@@ -94,37 +94,189 @@ struct lm95241_data {
unsigned long last_updated, interval; /* in jiffies */
char valid; /* zero until following fields are valid */
/* registers values */
- u8 local_h, local_l; /* local */
- u8 remote1_h, remote1_l; /* remote1 */
- u8 remote2_h, remote2_l; /* remote2 */
+ u8 temp[ARRAY_SIZE(lm95241_reg_address)];
u8 config, model, trutherm;
};
+/* Conversions */
+static int TempFromReg(u8 val_h, u8 val_l)
+{
+ if (val_h & 0x80)
+ return val_h - 0x100;
+ return val_h * 1000 + val_l * 1000 / 256;
+}
+
+static struct lm95241_data *lm95241_update_device(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ mutex_lock(&data->update_lock);
+
+ if (time_after(jiffies, data->last_updated + data->interval) ||
+ !data->valid) {
+ int i;
+
+ dev_dbg(&client->dev, "Updating lm95241 data.\n");
+ for (i = 0; i < ARRAY_SIZE(lm95241_reg_address); i++)
+ data->temp[i] = i2c_smbus_read_byte_data(client,
+ lm95241_reg_address[i]);
+ data->last_updated = jiffies;
+ data->valid = 1;
+ }
+
+ mutex_unlock(&data->update_lock);
+
+ return data;
+}
+
/* Sysfs stuff */
-#define show_temp(value) \
-static ssize_t show_##value(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct lm95241_data *data = lm95241_update_device(dev); \
- snprintf(buf, PAGE_SIZE - 1, "%d\n", \
- TEMP_FROM_REG(data->value##_h, data->value##_l)); \
- return strlen(buf); \
-}
-show_temp(local);
-show_temp(remote1);
-show_temp(remote2);
+static ssize_t show_input(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct lm95241_data *data = lm95241_update_device(dev);
-static ssize_t show_interval(struct device *dev, struct device_attribute *attr,
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n",
+ TempFromReg(data->temp[to_sensor_dev_attr(attr)->index],
+ data->temp[to_sensor_dev_attr(attr)->index + 1]));
+}
+
+static ssize_t show_type(struct device *dev, struct device_attribute *attr,
char *buf)
{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->model & to_sensor_dev_attr(attr)->index ? "1\n" : "2\n");
+}
+
+static ssize_t set_type(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ unsigned long val;
+ int shift;
+ u8 mask = to_sensor_dev_attr(attr)->index;
+
+ if (strict_strtoul(buf, 10, &val) < 0)
+ return -EINVAL;
+
+ if (val != 1 && val != 2)
+ return -EINVAL;
+ shift = mask == R1MS_MASK ? TT1_SHIFT : TT2_SHIFT;
+
+ mutex_lock(&data->update_lock);
+
+ data->trutherm &= ~(TT_MASK << shift);
+ if (val == 1) {
+ data->model |= mask;
+ data->trutherm |= (TT_ON << shift);
+ } else {
+ data->model &= ~mask;
+ data->trutherm |= (TT_OFF << shift);
+ }
+
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
+ data->model);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
+ data->trutherm);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_min(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->config & to_sensor_dev_attr(attr)->index ?
+ "-127000\n" : "0\n");
+}
+
+static ssize_t set_min(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ long val;
+
+ if (strict_strtol(buf, 10, &val) < 0)
+ return -EINVAL;
+ if (val < -128000)
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ if (val < 0)
+ data->config |= to_sensor_dev_attr(attr)->index;
+ else
+ data->config &= ~to_sensor_dev_attr(attr)->index;
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_max(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ return snprintf(buf, PAGE_SIZE - 1,
+ data->config & to_sensor_dev_attr(attr)->index ?
+ "127000\n" : "255000\n");
+}
+
+static ssize_t set_max(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ struct lm95241_data *data = i2c_get_clientdata(client);
+ long val;
+
+ if (strict_strtol(buf, 10, &val) < 0)
+ return -EINVAL;
+ if (val >= 256000)
+ return -EINVAL;
+
+ mutex_lock(&data->update_lock);
+
+ if (val <= 127000)
+ data->config |= to_sensor_dev_attr(attr)->index;
+ else
+ data->config &= ~to_sensor_dev_attr(attr)->index;
+ data->valid = 0;
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, data->config);
+
+ mutex_unlock(&data->update_lock);
+
+ return count;
+}
+
+static ssize_t show_interval(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
struct lm95241_data *data = lm95241_update_device(dev);
- snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval / HZ);
- return strlen(buf);
+ return snprintf(buf, PAGE_SIZE - 1, "%lu\n", 1000 * data->interval
+ / HZ);
}
static ssize_t set_interval(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
+ const char *buf, size_t count)
{
struct i2c_client *client = to_i2c_client(dev);
struct lm95241_data *data = i2c_get_clientdata(client);
@@ -138,176 +290,34 @@ static ssize_t set_interval(struct devic
return count;
}
-#define show_type(flag) \
-static ssize_t show_type##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->model & R##flag##MS_MASK ? "1\n" : "2\n"); \
- return strlen(buf); \
-}
-show_type(1);
-show_type(2);
-
-#define show_min(flag) \
-static ssize_t show_min##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->config & R##flag##DF_MASK ? \
- "-127000\n" : "0\n"); \
- return strlen(buf); \
-}
-show_min(1);
-show_min(2);
-
-#define show_max(flag) \
-static ssize_t show_max##flag(struct device *dev, \
- struct device_attribute *attr, char *buf) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- snprintf(buf, PAGE_SIZE - 1, \
- data->config & R##flag##DF_MASK ? \
- "127000\n" : "255000\n"); \
- return strlen(buf); \
-}
-show_max(1);
-show_max(2);
-
-#define set_type(flag) \
-static ssize_t set_type##flag(struct device *dev, \
- struct device_attribute *attr, \
- const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
-\
- if (strict_strtol(buf, 10, &val) < 0) \
- return -EINVAL; \
-\
- if ((val == 1) || (val == 2)) { \
-\
- mutex_lock(&data->update_lock); \
-\
- data->trutherm &= ~(TT_MASK << TT##flag##_SHIFT); \
- if (val == 1) { \
- data->model |= R##flag##MS_MASK; \
- data->trutherm |= (TT_ON << TT##flag##_SHIFT); \
- } \
- else { \
- data->model &= ~R##flag##MS_MASK; \
- data->trutherm |= (TT_OFF << TT##flag##_SHIFT); \
- } \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL, \
- data->model); \
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM, \
- data->trutherm); \
-\
- mutex_unlock(&data->update_lock); \
-\
- } \
- return count; \
-}
-set_type(1);
-set_type(2);
-
-#define set_min(flag) \
-static ssize_t set_min##flag(struct device *dev, \
- struct device_attribute *devattr, const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
-\
- if (strict_strtol(buf, 10, &val) < 0) \
- return -EINVAL;\
-\
- mutex_lock(&data->update_lock); \
-\
- if (val < 0) \
- data->config |= R##flag##DF_MASK; \
- else \
- data->config &= ~R##flag##DF_MASK; \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
- data->config); \
-\
- mutex_unlock(&data->update_lock); \
-\
- return count; \
-}
-set_min(1);
-set_min(2);
-
-#define set_max(flag) \
-static ssize_t set_max##flag(struct device *dev, \
- struct device_attribute *devattr, const char *buf, size_t count) \
-{ \
- struct i2c_client *client = to_i2c_client(dev); \
- struct lm95241_data *data = i2c_get_clientdata(client); \
-\
- long val; \
-\
- if (strict_strtol(buf, 10, &val) < 0) \
- return -EINVAL; \
-\
- mutex_lock(&data->update_lock); \
-\
- if (val <= 127000) \
- data->config |= R##flag##DF_MASK; \
- else \
- data->config &= ~R##flag##DF_MASK; \
-\
- data->valid = 0; \
-\
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG, \
- data->config); \
-\
- mutex_unlock(&data->update_lock); \
-\
- return count; \
-}
-set_max(1);
-set_max(2);
-
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_local, NULL);
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_remote1, NULL);
-static DEVICE_ATTR(temp3_input, S_IRUGO, show_remote2, NULL);
-static DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type1, set_type1);
-static DEVICE_ATTR(temp3_type, S_IWUSR | S_IRUGO, show_type2, set_type2);
-static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min1, set_min1);
-static DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min2, set_min2);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_max1, set_max1);
-static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_max2, set_max2);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_input, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_input, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_input, NULL, 4);
+static SENSOR_DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type, set_type,
+ R1MS_MASK);
+static SENSOR_DEVICE_ATTR(temp3_type, S_IWUSR | S_IRUGO, show_type, set_type,
+ R2MS_MASK);
+static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_min, set_min,
+ R1DF_MASK);
+static SENSOR_DEVICE_ATTR(temp3_min, S_IWUSR | S_IRUGO, show_min, set_min,
+ R2DF_MASK);
+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 DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_interval,
set_interval);
static struct attribute *lm95241_attributes[] = {
- &dev_attr_temp1_input.attr,
- &dev_attr_temp2_input.attr,
- &dev_attr_temp3_input.attr,
- &dev_attr_temp2_type.attr,
- &dev_attr_temp3_type.attr,
- &dev_attr_temp2_min.attr,
- &dev_attr_temp3_min.attr,
- &dev_attr_temp2_max.attr,
- &dev_attr_temp3_max.attr,
+ &sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_input.dev_attr.attr,
+ &sensor_dev_attr_temp3_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_type.dev_attr.attr,
+ &sensor_dev_attr_temp3_type.dev_attr.attr,
+ &sensor_dev_attr_temp2_min.dev_attr.attr,
+ &sensor_dev_attr_temp3_min.dev_attr.attr,
+ &sensor_dev_attr_temp2_max.dev_attr.attr,
+ &sensor_dev_attr_temp3_max.dev_attr.attr,
&dev_attr_update_interval.attr,
NULL
};
@@ -331,7 +341,7 @@ static int lm95241_detect(struct i2c_cli
== MANUFACTURER_ID)
&& (i2c_smbus_read_byte_data(new_client, LM95241_REG_R_CHIP_ID)
>= DEFAULT_REVISION)) {
- name = "lm95241";
+ name = DEVNAME;
} else {
dev_dbg(&adapter->dev, "LM95241 detection failed at 0x%02x\n",
address);
@@ -343,6 +353,26 @@ static int lm95241_detect(struct i2c_cli
return 0;
}
+static void lm95241_init_client(struct i2c_client *client)
+{
+ struct lm95241_data *data = i2c_get_clientdata(client);
+
+ data->interval = HZ; /* 1 sec default */
+ data->valid = 0;
+ data->config = CFG_CR0076;
+ data->model = 0;
+ data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
+
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
+ data->config);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
+ R1FE_MASK | R2FE_MASK);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
+ data->trutherm);
+ i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
+ data->model);
+}
+
static int lm95241_probe(struct i2c_client *new_client,
const struct i2c_device_id *id)
{
@@ -382,26 +412,6 @@ exit:
return err;
}
-static void lm95241_init_client(struct i2c_client *client)
-{
- struct lm95241_data *data = i2c_get_clientdata(client);
-
- data->interval = HZ; /* 1 sec default */
- data->valid = 0;
- data->config = CFG_CR0076;
- data->model = 0;
- data->trutherm = (TT_OFF << TT1_SHIFT) | (TT_OFF << TT2_SHIFT);
-
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_CONFIG,
- data->config);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REM_FILTER,
- R1FE_MASK | R2FE_MASK);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_TRUTHERM,
- data->trutherm);
- i2c_smbus_write_byte_data(client, LM95241_REG_RW_REMOTE_MODEL,
- data->model);
-}
-
static int lm95241_remove(struct i2c_client *client)
{
struct lm95241_data *data = i2c_get_clientdata(client);
@@ -413,46 +423,9 @@ static int lm95241_remove(struct i2c_cli
return 0;
}
-static struct lm95241_data *lm95241_update_device(struct device *dev)
-{
- struct i2c_client *client = to_i2c_client(dev);
- struct lm95241_data *data = i2c_get_clientdata(client);
-
- mutex_lock(&data->update_lock);
-
- if (time_after(jiffies, data->last_updated + data->interval) ||
- !data->valid) {
- dev_dbg(&client->dev, "Updating lm95241 data.\n");
- data->local_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_LOCAL_TEMPH);
- data->local_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_LOCAL_TEMPL);
- data->remote1_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE1_TEMPH);
- data->remote1_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE1_TEMPL);
- data->remote2_h =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE2_TEMPH);
- data->remote2_l =
- i2c_smbus_read_byte_data(client,
- LM95241_REG_R_REMOTE2_TEMPL);
- data->last_updated = jiffies;
- data->valid = 1;
- }
-
- mutex_unlock(&data->update_lock);
-
- return data;
-}
-
/* Driver data (common to all clients) */
static const struct i2c_device_id lm95241_id[] = {
- { "lm95241", 0 },
+ { DEVNAME, 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, lm95241_id);
@@ -460,7 +433,7 @@ MODULE_DEVICE_TABLE(i2c, lm95241_id);
static struct i2c_driver lm95241_driver = {
.class = I2C_CLASS_HWMON,
.driver = {
- .name = "lm95241",
+ .name = DEVNAME,
},
.probe = lm95241_probe,
.remove = lm95241_remove,
@@ -479,9 +452,10 @@ static void __exit sensors_lm95241_exit(
i2c_del_driver(&lm95241_driver);
}
-MODULE_AUTHOR("Davide Rizzo <elpa-rizzo@gmail.com>");
+MODULE_AUTHOR("Davide Rizzo <elpa.rizzo@gmail.com>");
MODULE_DESCRIPTION("LM95241 sensor driver");
MODULE_LICENSE("GPL");
module_init(sensors_lm95241_init);
module_exit(sensors_lm95241_exit);
+
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-17 11:05 ` [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
@ 2010-11-18 15:38 ` Guenter Roeck
-1 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2010-11-18 15:38 UTC (permalink / raw)
To: Davide Rizzo; +Cc: LM Sensors, linux-kernel@vger.kernel.org, Jean Delvare
Hi Davide,
On Wed, Nov 17, 2010 at 06:05:43AM -0500, Davide Rizzo wrote:
> > Removing the above includes violates SubmitChecklist rule #1. Also, please reparent to Linus'
> > latest tree.
> >
> > Other than that, there are a only few formatting issues, but I can take care of those myself.
> > So please re-submit with the above changes, and we should be ready to go.
> >
> > Thanks,
> > Guenter
>
> Here it is as requested.
> Regards,
> Davide Rizzo
>
> From: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
>
> Rewriting of driver/hwmon/lm95241.c to avoid using macros
>
> Signed-off-by: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
> ---
> --- linux-2.6.37-rc2/drivers/hwmon/lm95241.c 2010-11-16 03:31:02.000000000 +0100
> +++ linux-2.6.37-rc2.elpa/drivers/hwmon/lm95241.c 2010-11-17
> 11:47:21.911752940 +0100
Looks like you still have trouble with line wraps. You might want to get this fixed;
having to figure out why the patch does not apply is a bit annoying.
> @@ -1,13 +1,9 @@
> /*
> - * lm95241.c - Part of lm_sensors, Linux kernel modules for hardware
> - * monitoring
> - * Copyright (C) 2008 Davide Rizzo <elpa-rizzo@gmail.com>
> + * Copyright (C) 2008, 2010 Davide Rizzo <elpa.rizzo@gmail.com>
> *
> - * Based on the max1619 driver. The LM95241 is a sensor chip made by National
> - * Semiconductors.
> - * It reports up to three temperatures (its own plus up to
> - * two external ones). Complete datasheet can be
> - * obtained from National's website at:
> + * The LM95241 is a sensor chip made by National Semiconductors.
> + * It reports up to three temperatures (its own plus up to two external ones).
> + * Complete datasheet can be obtained from National's website at:
> * http://www.national.com/ds.cgi/LM/LM95241.pdf
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -27,14 +23,17 @@
>
> #include <linux/module.h>
> #include <linux/init.h>
> -#include <linux/slab.h>
> #include <linux/jiffies.h>
> +#include <linux/mutex.h>
> +#include <linux/sysfs.h>
> +
> +#include <linux/slab.h>
> #include <linux/i2c.h>
> #include <linux/hwmon.h>
> #include <linux/hwmon-sysfs.h>
> #include <linux/err.h>
> -#include <linux/mutex.h>
> -#include <linux/sysfs.h>
> +
Moving include statements around is just a whitespace change and should be avoided.
Also, you added a blank line at the end of the file which causes a git whitespace error
when applying the patch.
No need to retransmit. I fixed both and applied the patch with a couple of additional
formatting changes to make the file checkpatch clean.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] hwmon: (lm95241) Rewritten without using macros
@ 2010-11-18 15:38 ` Guenter Roeck
0 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2010-11-18 15:38 UTC (permalink / raw)
To: Davide Rizzo; +Cc: LM Sensors, linux-kernel@vger.kernel.org, Jean Delvare
Hi Davide,
On Wed, Nov 17, 2010 at 06:05:43AM -0500, Davide Rizzo wrote:
> > Removing the above includes violates SubmitChecklist rule #1. Also, please reparent to Linus'
> > latest tree.
> >
> > Other than that, there are a only few formatting issues, but I can take care of those myself.
> > So please re-submit with the above changes, and we should be ready to go.
> >
> > Thanks,
> > Guenter
>
> Here it is as requested.
> Regards,
> Davide Rizzo
>
> From: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
>
> Rewriting of driver/hwmon/lm95241.c to avoid using macros
>
> Signed-off-by: Davide Rizzo <elpa.rizzo@gmail.com<mailto:elpa.rizzo@gmail.com>>
> ---
> --- linux-2.6.37-rc2/drivers/hwmon/lm95241.c 2010-11-16 03:31:02.000000000 +0100
> +++ linux-2.6.37-rc2.elpa/drivers/hwmon/lm95241.c 2010-11-17
> 11:47:21.911752940 +0100
Looks like you still have trouble with line wraps. You might want to get this fixed;
having to figure out why the patch does not apply is a bit annoying.
> @@ -1,13 +1,9 @@
> /*
> - * lm95241.c - Part of lm_sensors, Linux kernel modules for hardware
> - * monitoring
> - * Copyright (C) 2008 Davide Rizzo <elpa-rizzo@gmail.com>
> + * Copyright (C) 2008, 2010 Davide Rizzo <elpa.rizzo@gmail.com>
> *
> - * Based on the max1619 driver. The LM95241 is a sensor chip made by National
> - * Semiconductors.
> - * It reports up to three temperatures (its own plus up to
> - * two external ones). Complete datasheet can be
> - * obtained from National's website at:
> + * The LM95241 is a sensor chip made by National Semiconductors.
> + * It reports up to three temperatures (its own plus up to two external ones).
> + * Complete datasheet can be obtained from National's website at:
> * http://www.national.com/ds.cgi/LM/LM95241.pdf
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -27,14 +23,17 @@
>
> #include <linux/module.h>
> #include <linux/init.h>
> -#include <linux/slab.h>
> #include <linux/jiffies.h>
> +#include <linux/mutex.h>
> +#include <linux/sysfs.h>
> +
> +#include <linux/slab.h>
> #include <linux/i2c.h>
> #include <linux/hwmon.h>
> #include <linux/hwmon-sysfs.h>
> #include <linux/err.h>
> -#include <linux/mutex.h>
> -#include <linux/sysfs.h>
> +
Moving include statements around is just a whitespace change and should be avoided.
Also, you added a blank line at the end of the file which causes a git whitespace error
when applying the patch.
No need to retransmit. I fixed both and applied the patch with a couple of additional
formatting changes to make the file checkpatch clean.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-18 15:38 ` [PATCH] hwmon: (lm95241) Rewritten without using macros Guenter Roeck
@ 2010-11-18 15:54 ` Davide Rizzo
-1 siblings, 0 replies; 19+ messages in thread
From: Davide Rizzo @ 2010-11-18 15:54 UTC (permalink / raw)
To: Guenter Roeck; +Cc: LM Sensors, linux-kernel@vger.kernel.org, Jean Delvare
2010/11/18 Guenter Roeck <guenter.roeck@ericsson.com>:
> Hi Davide,
>
Hi Guenter,
> Looks like you still have trouble with line wraps. You might want to get this fixed;
> having to figure out why the patch does not apply is a bit annoying.
>
You said me that attaching the patch to the mail is not correct.
First times I copy/pasted from kate, that broke tabs.
The last time I copy/pasted from kwrite, that seemed me correct (but
you're saying me that it isn't).
I'm using gmail, please explain me how to attach a patch to the e-mail.
> Moving include statements around is just a whitespace change and should be avoided.
> Also, you added a blank line at the end of the file which causes a git whitespace error
> when applying the patch.
>
> No need to retransmit. I fixed both and applied the patch with a couple of additional
> formatting changes to make the file checkpatch clean.
>
> Thanks,
> Guenter
>
I thank you,
Davide.
--
All difficult problems have a simple solution. That is wrong. [A. Einstein]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] hwmon: (lm95241) Rewritten without using macros
@ 2010-11-18 15:54 ` Davide Rizzo
0 siblings, 0 replies; 19+ messages in thread
From: Davide Rizzo @ 2010-11-18 15:54 UTC (permalink / raw)
To: Guenter Roeck; +Cc: LM Sensors, linux-kernel@vger.kernel.org, Jean Delvare
2010/11/18 Guenter Roeck <guenter.roeck@ericsson.com>:
> Hi Davide,
>
Hi Guenter,
> Looks like you still have trouble with line wraps. You might want to get this fixed;
> having to figure out why the patch does not apply is a bit annoying.
>
You said me that attaching the patch to the mail is not correct.
First times I copy/pasted from kate, that broke tabs.
The last time I copy/pasted from kwrite, that seemed me correct (but
you're saying me that it isn't).
I'm using gmail, please explain me how to attach a patch to the e-mail.
> Moving include statements around is just a whitespace change and should be avoided.
> Also, you added a blank line at the end of the file which causes a git whitespace error
> when applying the patch.
>
> No need to retransmit. I fixed both and applied the patch with a couple of additional
> formatting changes to make the file checkpatch clean.
>
> Thanks,
> Guenter
>
I thank you,
Davide.
--
All difficult problems have a simple solution. That is wrong. [A. Einstein]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-18 15:54 ` [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
@ 2010-11-18 16:10 ` Guenter Roeck
-1 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2010-11-18 16:10 UTC (permalink / raw)
To: Davide Rizzo; +Cc: LM Sensors, linux-kernel@vger.kernel.org, Jean Delvare
On Thu, Nov 18, 2010 at 10:54:05AM -0500, Davide Rizzo wrote:
> 2010/11/18 Guenter Roeck <guenter.roeck@ericsson.com>:
> > Hi Davide,
> >
>
> Hi Guenter,
>
> > Looks like you still have trouble with line wraps. You might want to get this fixed;
> > having to figure out why the patch does not apply is a bit annoying.
> >
>
> You said me that attaching the patch to the mail is not correct.
> First times I copy/pasted from kate, that broke tabs.
> The last time I copy/pasted from kwrite, that seemed me correct (but
> you're saying me that it isn't).
> I'm using gmail, please explain me how to attach a patch to the e-mail.
>
Cut-and-paste is never a good idea. I avoid such problems by generating patches
with "git format-patch" and sending them with "git send-email" from my linux system.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] hwmon: (lm95241) Rewritten without using macros
@ 2010-11-18 16:10 ` Guenter Roeck
0 siblings, 0 replies; 19+ messages in thread
From: Guenter Roeck @ 2010-11-18 16:10 UTC (permalink / raw)
To: Davide Rizzo; +Cc: LM Sensors, linux-kernel@vger.kernel.org, Jean Delvare
On Thu, Nov 18, 2010 at 10:54:05AM -0500, Davide Rizzo wrote:
> 2010/11/18 Guenter Roeck <guenter.roeck@ericsson.com>:
> > Hi Davide,
> >
>
> Hi Guenter,
>
> > Looks like you still have trouble with line wraps. You might want to get this fixed;
> > having to figure out why the patch does not apply is a bit annoying.
> >
>
> You said me that attaching the patch to the mail is not correct.
> First times I copy/pasted from kate, that broke tabs.
> The last time I copy/pasted from kwrite, that seemed me correct (but
> you're saying me that it isn't).
> I'm using gmail, please explain me how to attach a patch to the e-mail.
>
Cut-and-paste is never a good idea. I avoid such problems by generating patches
with "git format-patch" and sending them with "git send-email" from my linux system.
Guenter
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
` (7 preceding siblings ...)
2010-11-16 15:03 ` Guenter Roeck
@ 2011-01-17 8:31 ` Davide Rizzo
2011-01-17 8:49 ` Jean Delvare
9 siblings, 0 replies; 19+ messages in thread
From: Davide Rizzo @ 2011-01-17 8:31 UTC (permalink / raw)
To: lm-sensors
[-- Attachment #1.1: Type: text/plain, Size: 646 bytes --]
2010/11/18 Guenter Roeck <guenter.roeck@ericsson.com>
> Hi Davide,
>
>
> Moving include statements around is just a whitespace change and should be
> avoided.
> Also, you added a blank line at the end of the file which causes a git
> whitespace error
> when applying the patch.
>
> No need to retransmit. I fixed both and applied the patch with a couple of
> additional
> formatting changes to make the file checkpatch clean.
>
> Thanks,
> Guenter
>
>
Hi,
Any news about this ? I cannot find it in 2.6.37, did we miss the update
window or is it locked somewhere ?
--
All difficult problems have a simple solution. That is wrong. [A. Einstein]
[-- Attachment #1.2: Type: text/html, Size: 1033 bytes --]
[-- Attachment #2: Type: text/plain, Size: 153 bytes --]
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
` (8 preceding siblings ...)
2011-01-17 8:31 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Davide Rizzo
@ 2011-01-17 8:49 ` Jean Delvare
9 siblings, 0 replies; 19+ messages in thread
From: Jean Delvare @ 2011-01-17 8:49 UTC (permalink / raw)
To: lm-sensors
Hi Davide,
On Mon, 17 Jan 2011 09:31:03 +0100, Davide Rizzo wrote:
> 2010/11/18 Guenter Roeck <guenter.roeck@ericsson.com>
>
> > Hi Davide,
> >
> >
> > Moving include statements around is just a whitespace change and should be
> > avoided.
> > Also, you added a blank line at the end of the file which causes a git
> > whitespace error
> > when applying the patch.
> >
> > No need to retransmit. I fixed both and applied the patch with a couple of
> > additional
> > formatting changes to make the file checkpatch clean.
> >
> > Thanks,
> > Guenter
>
> Any news about this ? I cannot find it in 2.6.37, did we miss the update
> window or is it locked somewhere ?
Your patch is in Linus' tree and will be part of kernel 2.6.38:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h\x0f1deb4b820cfacf22492abd7b17e891dafc51ae
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2011-01-17 8:49 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-09 9:33 [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
2010-11-09 22:14 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
2010-11-11 18:18 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
2010-11-11 18:29 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
2010-11-11 19:07 ` Jean Delvare
2010-11-11 19:29 ` Guenter Roeck
2010-11-11 19:33 ` Jean Delvare
2010-11-12 20:49 ` Guenter Roeck
2010-11-16 15:03 ` Guenter Roeck
2010-11-17 11:05 ` Davide Rizzo
2010-11-17 11:05 ` [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
2010-11-18 15:38 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
2010-11-18 15:38 ` [PATCH] hwmon: (lm95241) Rewritten without using macros Guenter Roeck
2010-11-18 15:54 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Davide Rizzo
2010-11-18 15:54 ` [PATCH] hwmon: (lm95241) Rewritten without using macros Davide Rizzo
2010-11-18 16:10 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Guenter Roeck
2010-11-18 16:10 ` [PATCH] hwmon: (lm95241) Rewritten without using macros Guenter Roeck
2011-01-17 8:31 ` [lm-sensors] [PATCH] hwmon: (lm95241) Rewritten without using Davide Rizzo
2011-01-17 8:49 ` Jean Delvare
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.