All of lore.kernel.org
 help / color / mirror / Atom feed
* More hardware monitoring drivers ported to the new sysfs callbacks
@ 2005-06-05 20:08 ` Jean Delvare
  0 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-05 18:09 UTC (permalink / raw)
  To: LM Sensors, LKML; +Cc: Yani Ioannou, Greg KH

Hi all,

I have been modifying three additional hardware monitoring drivers to
take benefit of Yani Ioannou's new, extended sysfs callbacks. These
drivers are lm63, lm83 and lm90. All of these are relatively small when
compared to the first two modified drivers (adm1026 and it87). My goal
was to demonstrate that the new callbacks can also be used in small
drivers, with significant benefits. The result is even smaller drivers
(less memory used when loaded), relying far less on macros, which makes
the code easier to read (and the drivers presumably faster to distribute
using distcc).

Module                Before   After
lm63                   10128    9424 ( -704/ -6%)
lm83                    8784    6864 (-1920/-21%)
lm90                   12420   10628 (-1792/-14%)

Individual patches will follow. Comments welcome. Greg, can you add
these to one of your trees?

Before I go on with driver conversion, there are two points I'd like to
discuss:

First, I don't much like the name of the new header file,
linux/i2c-sysfs.h. It isn't related with i2c at all! It's all about
sensors (or hardware monitoring if you prefer). I think the header file
should be named linux/hwmon-sysfs.h or something similar.

Second, is there a reason why the SENSOR_DEVICE_ATTR macro creates a
stucture named sensor_dev_attr_##_name rather than simply
dev_attr_##_name? As it seems unlikely that SENSOR_DEVICE_ATTR and
DEVICE_ATTR will both be called for the same file, going for the short
form shouldn't cause any problem. This would make the calling code more
readable IMHO.

Thanks,
-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: More hardware monitoring drivers ported to the new sysfs callbacks
  2005-06-05 20:08 ` [lm-sensors] More hardware monitoring drivers ported to the new Jean Delvare
@ 2005-06-05 20:26   ` Yani Ioannou
  -1 siblings, 0 replies; 14+ messages in thread
From: Yani Ioannou @ 2005-06-05 18:25 UTC (permalink / raw)
  To: Jean Delvare; +Cc: LM Sensors, LKML, Greg KH

Hi Jean,

> I have been modifying three additional hardware monitoring drivers to
> take benefit of Yani Ioannou's new, extended sysfs callbacks. These
> drivers are lm63, lm83 and lm90. All of these are relatively small when
> compared to the first two modified drivers (adm1026 and it87). My goal
> was to demonstrate that the new callbacks can also be used in small
> drivers, with significant benefits. The result is even smaller drivers
> (less memory used when loaded), relying far less on macros, which makes
> the code easier to read (and the drivers presumably faster to distribute
> using distcc).
>
> Module                Before   After
> lm63                   10128    9424 ( -704/ -6%)
> lm83                    8784    6864 (-1920/-21%)
> lm90                   12420   10628 (-1792/-14%)
> 

Good stuff :-)

> First, I don't much like the name of the new header file,
> linux/i2c-sysfs.h. It isn't related with i2c at all! It's all about
> sensors (or hardware monitoring if you prefer). I think the header file
> should be named linux/hwmon-sysfs.h or something similar.

hwmon wasn't near being added to -mm at the time so I went with the
status quo and named it i2c-sysfs, I'm all for renaming it to
hwmon-sysfs.h though.

> Second, is there a reason why the SENSOR_DEVICE_ATTR macro creates a
> stucture named sensor_dev_attr_##_name rather than simply
> dev_attr_##_name? As it seems unlikely that SENSOR_DEVICE_ATTR and
> DEVICE_ATTR will both be called for the same file, going for the short
> form shouldn't cause any problem. This would make the calling code more
> readable IMHO.

I know the variable names are a bit long, but my patch against adm1026
for example still uses DEVICE_ATTR for attributes that can't make any
use of the dynamic callbacks, and hence I don't want to waste the
extra space required for a sensor device attribute. So there is still
reason to use both in one file. When it comes to macros that define
'hidden' variable names I'm inclined to err on the side of caution and
use the longer name too.

Thanks,
Yani

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2.6] I2C: (1/3) lm63 uses new sysfs callbacks
  2005-06-05 20:08 ` [lm-sensors] More hardware monitoring drivers ported to the new Jean Delvare
@ 2005-06-05 20:32   ` Jean Delvare
  -1 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-05 18:32 UTC (permalink / raw)
  To: LM Sensors, LKML; +Cc: Greg KH, Yani Ioannou

Hi Greg, all,

I updated the lm63 hardware monitoring driver to take benefit of Yani
Ioannou's new sysfs callback capabilities.

Please apply,
thanks.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

 drivers/i2c/chips/lm63.c |  267 ++++++++++++++++++++++++-----------------------
 1 files changed, 142 insertions(+), 125 deletions(-)

--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm63.c	2005-06-05 19:23:52.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm63.c	2005-06-05 19:37:15.000000000 +0200
@@ -1,7 +1,7 @@
 /*
  * lm63.c - driver for the National Semiconductor LM63 temperature sensor
  *          with integrated fan control
- * Copyright (C) 2004  Jean Delvare <khali@linux-fr.org>
+ * Copyright (C) 2004-2005  Jean Delvare <khali@linux-fr.org>
  * Based on the lm90 driver.
  *
  * The LM63 is a sensor chip made by National Semiconductor. It measures
@@ -43,6 +43,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
+#include <linux/i2c-sysfs.h>
 
 /*
  * Addresses to scan
@@ -157,16 +158,16 @@
 
 	/* registers values */
 	u8 config, config_fan;
-	u16 fan1_input;
-	u16 fan1_low;
+	u16 fan[2];	/* 0: input
+			   1: low limit */
 	u8 pwm1_freq;
 	u8 pwm1_value;
-	s8 temp1_input;
-	s8 temp1_high;
-	s16 temp2_input;
-	s16 temp2_high;
-	s16 temp2_low;
-	s8 temp2_crit;
+	s8 temp8[3];	/* 0: local input
+			   1: local high limit
+			   2: remote critical limit */
+	s16 temp11[3];	/* 0: remote input
+			   1: remote low limit
+			   2: remote high limit */
 	u8 temp2_crit_hyst;
 	u8 alarms;
 };
@@ -175,33 +176,33 @@
  * Sysfs callback functions and files
  */
 
-#define show_fan(value) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm63_data *data = lm63_update_device(dev); \
-	return sprintf(buf, "%d\n", FAN_FROM_REG(data->value)); \
+static ssize_t show_fan(struct device *dev, struct device_attribute *devattr,
+			char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm63_data *data = lm63_update_device(dev);
+	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index]));
 }
-show_fan(fan1_input);
-show_fan(fan1_low);
 
-static ssize_t set_fan1_low(struct device *dev, struct device_attribute *attr, const char *buf,
-	size_t count)
+static ssize_t set_fan(struct device *dev, struct device_attribute *dummy,
+		       const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm63_data *data = i2c_get_clientdata(client);
 	unsigned long val = simple_strtoul(buf, NULL, 10);
 
 	down(&data->update_lock);
-	data->fan1_low = FAN_TO_REG(val);
+	data->fan[1] = FAN_TO_REG(val);
 	i2c_smbus_write_byte_data(client, LM63_REG_TACH_LIMIT_LSB,
-				  data->fan1_low & 0xFF);
+				  data->fan[1] & 0xFF);
 	i2c_smbus_write_byte_data(client, LM63_REG_TACH_LIMIT_MSB,
-				  data->fan1_low >> 8);
+				  data->fan[1] >> 8);
 	up(&data->update_lock);
 	return count;
 }
 
-static ssize_t show_pwm1(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_pwm1(struct device *dev, struct device_attribute *dummy,
+			 char *buf)
 {
 	struct lm63_data *data = lm63_update_device(dev);
 	return sprintf(buf, "%d\n", data->pwm1_value >= 2 * data->pwm1_freq ?
@@ -209,7 +210,8 @@
 		       (2 * data->pwm1_freq));
 }
 
-static ssize_t set_pwm1(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+static ssize_t set_pwm1(struct device *dev, struct device_attribute *dummy,
+			const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm63_data *data = i2c_get_clientdata(client);
@@ -228,77 +230,83 @@
 	return count;
 }
 
-static ssize_t show_pwm1_enable(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_pwm1_enable(struct device *dev, struct device_attribute *dummy,
+				char *buf)
 {
 	struct lm63_data *data = lm63_update_device(dev);
 	return sprintf(buf, "%d\n", data->config_fan & 0x20 ? 1 : 2);
 }
 
-#define show_temp8(value) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm63_data *data = lm63_update_device(dev); \
-	return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->value)); \
-}
-#define show_temp11(value) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm63_data *data = lm63_update_device(dev); \
-	return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->value)); \
-}
-show_temp8(temp1_input);
-show_temp8(temp1_high);
-show_temp11(temp2_input);
-show_temp11(temp2_high);
-show_temp11(temp2_low);
-show_temp8(temp2_crit);
-
-#define set_temp8(value, reg) \
-static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm63_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	data->value = TEMP8_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, reg, data->value); \
-	up(&data->update_lock); \
-	return count; \
-}
-#define set_temp11(value, reg_msb, reg_lsb) \
-static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm63_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	data->value = TEMP11_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, reg_msb, data->value >> 8); \
-	i2c_smbus_write_byte_data(client, reg_lsb, data->value & 0xff); \
-	up(&data->update_lock); \
-	return count; \
-}
-set_temp8(temp1_high, LM63_REG_LOCAL_HIGH);
-set_temp11(temp2_high, LM63_REG_REMOTE_HIGH_MSB, LM63_REG_REMOTE_HIGH_LSB);
-set_temp11(temp2_low, LM63_REG_REMOTE_LOW_MSB, LM63_REG_REMOTE_LOW_LSB);
+static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
+			  char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm63_data *data = lm63_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index]));
+}
+
+static ssize_t set_temp8(struct device *dev, struct device_attribute *dummy,
+			 const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm63_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+
+	down(&data->update_lock);
+	data->temp8[1] = TEMP8_TO_REG(val);
+	i2c_smbus_write_byte_data(client, LM63_REG_LOCAL_HIGH, data->temp8[1]);
+	up(&data->update_lock);
+	return count;
+}
+
+static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
+			   char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm63_data *data = lm63_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index]));
+}
+
+static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
+			  const char *buf, size_t count)
+{
+	static const u8 reg[4] = {
+		LM63_REG_REMOTE_LOW_MSB,
+		LM63_REG_REMOTE_LOW_LSB,
+		LM63_REG_REMOTE_HIGH_MSB,
+		LM63_REG_REMOTE_HIGH_LSB,
+	};
+
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm63_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+	int nr = attr->index;
+
+	down(&data->update_lock);
+	data->temp11[nr] = TEMP11_TO_REG(val);
+	i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2],
+				  data->temp11[nr] >> 8);
+	i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1],
+				  data->temp11[nr] & 0xff);
+	up(&data->update_lock);
+	return count;
+}
 
 /* Hysteresis register holds a relative value, while we want to present
    an absolute to user-space */
-static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute *dummy,
+				    char *buf)
 {
 	struct lm63_data *data = lm63_update_device(dev);
-	return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp2_crit)
+	return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2])
 		       - TEMP8_FROM_REG(data->temp2_crit_hyst));
 }
 
 /* And now the other way around, user-space provides an absolute
    hysteresis value and we have to store a relative one */
-static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute *attr, const char *buf,
-	size_t count)
+static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute *dummy,
+				   const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm63_data *data = i2c_get_clientdata(client);
@@ -306,36 +314,37 @@
 	long hyst;
 
 	down(&data->update_lock);
-	hyst = TEMP8_FROM_REG(data->temp2_crit) - val;
+	hyst = TEMP8_FROM_REG(data->temp8[2]) - val;
 	i2c_smbus_write_byte_data(client, LM63_REG_REMOTE_TCRIT_HYST,
 				  HYST_TO_REG(hyst));
 	up(&data->update_lock);
 	return count;
 }
 
-static ssize_t show_alarms(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_alarms(struct device *dev, struct device_attribute *dummy,
+			   char *buf)
 {
 	struct lm63_data *data = lm63_update_device(dev);
 	return sprintf(buf, "%u\n", data->alarms);
 }
 
-static DEVICE_ATTR(fan1_input, S_IRUGO, show_fan1_input, NULL);
-static DEVICE_ATTR(fan1_min, S_IWUSR | S_IRUGO, show_fan1_low,
-	set_fan1_low);
+static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, show_fan, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan1_min, S_IWUSR | S_IRUGO, show_fan,
+	set_fan, 1);
 
 static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, show_pwm1, set_pwm1);
 static DEVICE_ATTR(pwm1_enable, S_IRUGO, show_pwm1_enable, NULL);
 
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp1_input, NULL);
-static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp1_high,
-	set_temp1_high);
-
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp2_input, NULL);
-static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp2_low,
-	set_temp2_low);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp2_high,
-	set_temp2_high);
-static DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp2_crit, NULL);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp8, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 1);
+
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp11, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11,
+	set_temp11, 1);
+static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11,
+	set_temp11, 2);
+static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp8, NULL, 2);
 static DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_temp2_crit_hyst,
 	set_temp2_crit_hyst);
 
@@ -429,17 +438,25 @@
 
 	/* Register sysfs hooks */
 	if (data->config & 0x04) { /* tachometer enabled */
-		device_create_file(&new_client->dev, &dev_attr_fan1_input);
-		device_create_file(&new_client->dev, &dev_attr_fan1_min);
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_fan1_input.dev_attr);
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_fan1_min.dev_attr);
 	}
 	device_create_file(&new_client->dev, &dev_attr_pwm1);
 	device_create_file(&new_client->dev, &dev_attr_pwm1_enable);
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_min);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_min.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_crit.dev_attr);
 	device_create_file(&new_client->dev, &dev_attr_temp2_crit_hyst);
 	device_create_file(&new_client->dev, &dev_attr_alarms);
 
@@ -510,14 +527,14 @@
 	if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
 		if (data->config & 0x04) { /* tachometer enabled  */
 			/* order matters for fan1_input */
-			data->fan1_input = i2c_smbus_read_byte_data(client,
-					   LM63_REG_TACH_COUNT_LSB) & 0xFC;
-			data->fan1_input |= i2c_smbus_read_byte_data(client,
-					    LM63_REG_TACH_COUNT_MSB) << 8;
-			data->fan1_low = (i2c_smbus_read_byte_data(client,
-					  LM63_REG_TACH_LIMIT_LSB) & 0xFC)
-				       | (i2c_smbus_read_byte_data(client,
-					  LM63_REG_TACH_LIMIT_MSB) << 8);
+			data->fan[0] = i2c_smbus_read_byte_data(client,
+				       LM63_REG_TACH_COUNT_LSB) & 0xFC;
+			data->fan[0] |= i2c_smbus_read_byte_data(client,
+					LM63_REG_TACH_COUNT_MSB) << 8;
+			data->fan[1] = (i2c_smbus_read_byte_data(client,
+					LM63_REG_TACH_LIMIT_LSB) & 0xFC)
+				     | (i2c_smbus_read_byte_data(client,
+					LM63_REG_TACH_LIMIT_MSB) << 8);
 		}
 
 		data->pwm1_freq = i2c_smbus_read_byte_data(client,
@@ -527,26 +544,26 @@
 		data->pwm1_value = i2c_smbus_read_byte_data(client,
 				   LM63_REG_PWM_VALUE);
 
-		data->temp1_input = i2c_smbus_read_byte_data(client,
-				    LM63_REG_LOCAL_TEMP);
-		data->temp1_high = i2c_smbus_read_byte_data(client,
-				   LM63_REG_LOCAL_HIGH);
+		data->temp8[0] = i2c_smbus_read_byte_data(client,
+				 LM63_REG_LOCAL_TEMP);
+		data->temp8[1] = i2c_smbus_read_byte_data(client,
+				 LM63_REG_LOCAL_HIGH);
 
 		/* order matters for temp2_input */
-		data->temp2_input = i2c_smbus_read_byte_data(client,
-				    LM63_REG_REMOTE_TEMP_MSB) << 8;
-		data->temp2_input |= i2c_smbus_read_byte_data(client,
-				     LM63_REG_REMOTE_TEMP_LSB);
-		data->temp2_high = (i2c_smbus_read_byte_data(client,
-				   LM63_REG_REMOTE_HIGH_MSB) << 8)
-				 | i2c_smbus_read_byte_data(client,
-				   LM63_REG_REMOTE_HIGH_LSB);
-		data->temp2_low = (i2c_smbus_read_byte_data(client,
+		data->temp11[0] = i2c_smbus_read_byte_data(client,
+				  LM63_REG_REMOTE_TEMP_MSB) << 8;
+		data->temp11[0] |= i2c_smbus_read_byte_data(client,
+				   LM63_REG_REMOTE_TEMP_LSB);
+		data->temp11[1] = (i2c_smbus_read_byte_data(client,
 				  LM63_REG_REMOTE_LOW_MSB) << 8)
 				| i2c_smbus_read_byte_data(client,
 				  LM63_REG_REMOTE_LOW_LSB);
-		data->temp2_crit = i2c_smbus_read_byte_data(client,
-				   LM63_REG_REMOTE_TCRIT);
+		data->temp11[2] = (i2c_smbus_read_byte_data(client,
+				  LM63_REG_REMOTE_HIGH_MSB) << 8)
+				| i2c_smbus_read_byte_data(client,
+				  LM63_REG_REMOTE_HIGH_LSB);
+		data->temp8[2] = i2c_smbus_read_byte_data(client,
+				 LM63_REG_REMOTE_TCRIT);
 		data->temp2_crit_hyst = i2c_smbus_read_byte_data(client,
 					LM63_REG_REMOTE_TCRIT_HYST);
 


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2.6] I2C: (2/3) lm83 uses new sysfs callbacks
  2005-06-05 20:08 ` [lm-sensors] More hardware monitoring drivers ported to the new Jean Delvare
@ 2005-06-05 21:16   ` Jean Delvare
  -1 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-05 19:16 UTC (permalink / raw)
  To: LM Sensors, LKML; +Cc: Greg KH, Yani Ioannou

Hi Greg, all,

I updated the lm83 hardware monitoring driver to take benefit of Yani
Ioannou's new sysfs callback capabilities.

Please apply,
thanks.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

 drivers/i2c/chips/lm83.c |  157 +++++++++++++++++++++++------------------------
 1 files changed, 77 insertions(+), 80 deletions(-)

--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm83.c	2005-06-05 19:23:53.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm83.c	2005-06-05 21:11:41.000000000 +0200
@@ -1,7 +1,7 @@
 /*
  * lm83.c - Part of lm_sensors, Linux kernel modules for hardware
  *          monitoring
- * Copyright (C) 2003  Jean Delvare <khali@linux-fr.org>
+ * Copyright (C) 2003-2005  Jean Delvare <khali@linux-fr.org>
  *
  * Heavily inspired from the lm78, lm75 and adm1021 drivers. The LM83 is
  * a sensor chip made by National Semiconductor. It reports up to four
@@ -33,6 +33,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
+#include <linux/i2c-sysfs.h>
 
 /*
  * Addresses to scan
@@ -93,21 +94,20 @@
 	LM83_REG_R_LOCAL_TEMP,
 	LM83_REG_R_REMOTE1_TEMP,
 	LM83_REG_R_REMOTE2_TEMP,
-	LM83_REG_R_REMOTE3_TEMP
-};
-
-static const u8 LM83_REG_R_HIGH[] = {
+	LM83_REG_R_REMOTE3_TEMP,
 	LM83_REG_R_LOCAL_HIGH,
 	LM83_REG_R_REMOTE1_HIGH,
 	LM83_REG_R_REMOTE2_HIGH,
-	LM83_REG_R_REMOTE3_HIGH
+	LM83_REG_R_REMOTE3_HIGH,
+	LM83_REG_R_TCRIT,
 };
 
 static const u8 LM83_REG_W_HIGH[] = {
 	LM83_REG_W_LOCAL_HIGH,
 	LM83_REG_W_REMOTE1_HIGH,
 	LM83_REG_W_REMOTE2_HIGH,
-	LM83_REG_W_REMOTE3_HIGH
+	LM83_REG_W_REMOTE3_HIGH,
+	LM83_REG_W_TCRIT,
 };
 
 /*
@@ -143,9 +143,9 @@
 	unsigned long last_updated; /* in jiffies */
 
 	/* registers values */
-	s8 temp_input[4];
-	s8 temp_high[4];
-	s8 temp_crit;
+	s8 temp[9];	/* 0..3: input 1-4,
+			   4..7: high limit 1-4,
+			   8   : critical limit */
 	u16 alarms; /* bitvector, combined */
 };
 
@@ -153,65 +153,55 @@
  * Sysfs stuff
  */
 
-#define show_temp(suffix, value) \
-static ssize_t show_temp_##suffix(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm83_data *data = lm83_update_device(dev); \
-	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->value)); \
+static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
+			 char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm83_data *data = lm83_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[attr->index]));
 }
-show_temp(input1, temp_input[0]);
-show_temp(input2, temp_input[1]);
-show_temp(input3, temp_input[2]);
-show_temp(input4, temp_input[3]);
-show_temp(high1, temp_high[0]);
-show_temp(high2, temp_high[1]);
-show_temp(high3, temp_high[2]);
-show_temp(high4, temp_high[3]);
-show_temp(crit, temp_crit);
-
-#define set_temp(suffix, value, reg) \
-static ssize_t set_temp_##suffix(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm83_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	data->value = TEMP_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, reg, data->value); \
-	up(&data->update_lock); \
-	return count; \
+
+static ssize_t set_temp(struct device *dev, struct device_attribute *devattr,
+			const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm83_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+	int nr = attr->index;
+
+	down(&data->update_lock);
+	data->temp[nr] = TEMP_TO_REG(val);
+	i2c_smbus_write_byte_data(client, LM83_REG_W_HIGH[nr - 4],
+				  data->temp[nr]);
+	up(&data->update_lock);
+	return count;
 }
-set_temp(high1, temp_high[0], LM83_REG_W_LOCAL_HIGH);
-set_temp(high2, temp_high[1], LM83_REG_W_REMOTE1_HIGH);
-set_temp(high3, temp_high[2], LM83_REG_W_REMOTE2_HIGH);
-set_temp(high4, temp_high[3], LM83_REG_W_REMOTE3_HIGH);
-set_temp(crit, temp_crit, LM83_REG_W_TCRIT);
 
-static ssize_t show_alarms(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_alarms(struct device *dev, struct device_attribute *dummy,
+			   char *buf)
 {
 	struct lm83_data *data = lm83_update_device(dev);
 	return sprintf(buf, "%d\n", data->alarms);
 }
 
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_input1, NULL);
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_input2, NULL);
-static DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_input3, NULL);
-static DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_input4, NULL);
-static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_high1,
-    set_temp_high1);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp_high2,
-    set_temp_high2);
-static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_temp_high3,
-    set_temp_high3);
-static DEVICE_ATTR(temp4_max, S_IWUSR | S_IRUGO, show_temp_high4,
-    set_temp_high4);
-static DEVICE_ATTR(temp1_crit, S_IRUGO, show_temp_crit, NULL);
-static DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp_crit, NULL);
-static DEVICE_ATTR(temp3_crit, S_IWUSR | S_IRUGO, show_temp_crit,
-    set_temp_crit);
-static DEVICE_ATTR(temp4_crit, S_IRUGO, show_temp_crit, NULL);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 4);
+static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 5);
+static SENSOR_DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 6);
+static SENSOR_DEVICE_ATTR(temp4_max, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 7);
+static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, show_temp, NULL, 8);
+static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp, NULL, 8);
+static SENSOR_DEVICE_ATTR(temp3_crit, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 8);
+static SENSOR_DEVICE_ATTR(temp4_crit, S_IRUGO, show_temp, NULL, 8);
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
 /*
@@ -322,18 +312,30 @@
 	 */
 
 	/* Register sysfs hooks */
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_input);
-	device_create_file(&new_client->dev, &dev_attr_temp3_input);
-	device_create_file(&new_client->dev, &dev_attr_temp4_input);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_max);
-	device_create_file(&new_client->dev, &dev_attr_temp3_max);
-	device_create_file(&new_client->dev, &dev_attr_temp4_max);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp3_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp4_crit);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp3_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp4_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp3_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp4_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp3_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp4_crit.dev_attr);
 	device_create_file(&new_client->dev, &dev_attr_alarms);
 
 	return 0;
@@ -369,16 +371,11 @@
 		int nr;
 
 		dev_dbg(&client->dev, "Updating lm83 data.\n");
-		for (nr = 0; nr < 4 ; nr++) {
-			data->temp_input[nr] =
+		for (nr = 0; nr < 9; nr++) {
+			data->temp[nr] =
 			    i2c_smbus_read_byte_data(client,
 			    LM83_REG_R_TEMP[nr]);
-			data->temp_high[nr] =
-			    i2c_smbus_read_byte_data(client,
-			    LM83_REG_R_HIGH[nr]);
 		}
-		data->temp_crit =
-		    i2c_smbus_read_byte_data(client, LM83_REG_R_TCRIT);
 		data->alarms =
 		    i2c_smbus_read_byte_data(client, LM83_REG_R_STATUS1)
 		    + (i2c_smbus_read_byte_data(client, LM83_REG_R_STATUS2)


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2.6] I2C: (3/3) lm90 uses new sysfs callbacks
  2005-06-05 20:08 ` [lm-sensors] More hardware monitoring drivers ported to the new Jean Delvare
@ 2005-06-05 21:27   ` Jean Delvare
  -1 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-05 19:27 UTC (permalink / raw)
  To: LM Sensors, LKML; +Cc: Greg KH, Yani Ioannou

Hi Greg, all,

I updated the lm90 hardware monitoring driver to take benefit of Yani
Ioannou's new sysfs callback capabilities.

Please apply,
thanks.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

 drivers/i2c/chips/lm90.c |  270 ++++++++++++++++++++++++++---------------------
 1 files changed, 150 insertions(+), 120 deletions(-)

Index: linux-2.6.12-rc5/drivers/i2c/chips/lm90.c
===================================================================
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm90.c	2005-06-05 14:14:27.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm90.c	2005-06-05 19:29:15.000000000 +0200
@@ -1,7 +1,7 @@
 /*
  * lm90.c - Part of lm_sensors, Linux kernel modules for hardware
  *          monitoring
- * Copyright (C) 2003-2004  Jean Delvare <khali@linux-fr.org>
+ * Copyright (C) 2003-2005  Jean Delvare <khali@linux-fr.org>
  *
  * Based on the lm83 driver. The LM90 is a sensor chip made by National
  * Semiconductor. It reports up to two temperatures (its own plus up to
@@ -76,6 +76,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
+#include <linux/i2c-sysfs.h>
 
 /*
  * Addresses to scan
@@ -205,9 +206,14 @@
 	int kind;
 
 	/* registers values */
-	s8 temp_input1, temp_low1, temp_high1; /* local */
-	s16 temp_input2, temp_low2, temp_high2; /* remote, combined */
-	s8 temp_crit1, temp_crit2;
+	s8 temp8[5];	/* 0: local input
+			   1: local low limit
+			   2: local high limit
+			   3: local critical limit
+			   4: remote critical limit */
+	s16 temp11[3];	/* 0: remote input
+			   1: remote low limit
+			   2: remote high limit */
 	u8 temp_hyst;
 	u8 alarms; /* bitvector */
 };
@@ -216,75 +222,88 @@
  * Sysfs stuff
  */
 
-#define show_temp(value, converter) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm90_data *data = lm90_update_device(dev); \
-	return sprintf(buf, "%d\n", converter(data->value)); \
-}
-show_temp(temp_input1, TEMP1_FROM_REG);
-show_temp(temp_input2, TEMP2_FROM_REG);
-show_temp(temp_low1, TEMP1_FROM_REG);
-show_temp(temp_low2, TEMP2_FROM_REG);
-show_temp(temp_high1, TEMP1_FROM_REG);
-show_temp(temp_high2, TEMP2_FROM_REG);
-show_temp(temp_crit1, TEMP1_FROM_REG);
-show_temp(temp_crit2, TEMP1_FROM_REG);
-
-#define set_temp1(value, reg) \
-static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm90_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	if (data->kind == adt7461) \
-		data->value = TEMP1_TO_REG_ADT7461(val); \
-	else \
-		data->value = TEMP1_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, reg, data->value); \
-	up(&data->update_lock); \
-	return count; \
-}
-#define set_temp2(value, regh, regl) \
-static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm90_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	if (data->kind == adt7461) \
-		data->value = TEMP2_TO_REG_ADT7461(val); \
-	else \
-		data->value = TEMP2_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, regh, data->value >> 8); \
-	i2c_smbus_write_byte_data(client, regl, data->value & 0xff); \
-	up(&data->update_lock); \
-	return count; \
-}
-set_temp1(temp_low1, LM90_REG_W_LOCAL_LOW);
-set_temp2(temp_low2, LM90_REG_W_REMOTE_LOWH, LM90_REG_W_REMOTE_LOWL);
-set_temp1(temp_high1, LM90_REG_W_LOCAL_HIGH);
-set_temp2(temp_high2, LM90_REG_W_REMOTE_HIGHH, LM90_REG_W_REMOTE_HIGHL);
-set_temp1(temp_crit1, LM90_REG_W_LOCAL_CRIT);
-set_temp1(temp_crit2, LM90_REG_W_REMOTE_CRIT);
-
-#define show_temp_hyst(value, basereg) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm90_data *data = lm90_update_device(dev); \
-	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->basereg) \
-		       - TEMP1_FROM_REG(data->temp_hyst)); \
+static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
+			  char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm90_data *data = lm90_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->temp8[attr->index]));
 }
-show_temp_hyst(temp_hyst1, temp_crit1);
-show_temp_hyst(temp_hyst2, temp_crit2);
 
-static ssize_t set_temp_hyst1(struct device *dev, struct device_attribute *attr, const char *buf,
-	size_t count)
+static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
+			 const char *buf, size_t count)
+{
+	static const u8 reg[4] = {
+		LM90_REG_W_LOCAL_LOW,
+		LM90_REG_W_LOCAL_HIGH,
+		LM90_REG_W_LOCAL_CRIT,
+		LM90_REG_W_REMOTE_CRIT,
+	};
+
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm90_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+	int nr = attr->index;
+
+	down(&data->update_lock);
+	if (data->kind == adt7461)
+		data->temp8[nr] = TEMP1_TO_REG_ADT7461(val);
+	else
+		data->temp8[nr] = TEMP1_TO_REG(val);
+	i2c_smbus_write_byte_data(client, reg[nr - 1], data->temp8[nr]);
+	up(&data->update_lock);
+	return count;
+}
+
+static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
+			   char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm90_data *data = lm90_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP2_FROM_REG(data->temp11[attr->index]));
+}
+
+static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
+			  const char *buf, size_t count)
+{
+	static const u8 reg[4] = {
+		LM90_REG_W_REMOTE_LOWH,
+		LM90_REG_W_REMOTE_LOWL,
+		LM90_REG_W_REMOTE_HIGHH,
+		LM90_REG_W_REMOTE_HIGHL,
+	};
+
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm90_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+	int nr = attr->index;
+
+	down(&data->update_lock);
+	if (data->kind == adt7461)
+		data->temp11[nr] = TEMP2_TO_REG_ADT7461(val);
+	else
+		data->temp11[nr] = TEMP2_TO_REG(val);
+	i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2],
+				  data->temp11[nr] >> 8);
+	i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1],
+				  data->temp11[nr] & 0xff);
+	up(&data->update_lock);
+	return count;
+}
+
+static ssize_t show_temphyst(struct device *dev, struct device_attribute *devattr,
+			     char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm90_data *data = lm90_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->temp8[attr->index])
+		       - TEMP1_FROM_REG(data->temp_hyst));
+}
+
+static ssize_t set_temphyst(struct device *dev, struct device_attribute *dummy,
+			    const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm90_data *data = i2c_get_clientdata(client);
@@ -292,36 +311,37 @@
 	long hyst;
 
 	down(&data->update_lock);
-	hyst = TEMP1_FROM_REG(data->temp_crit1) - val;
+	hyst = TEMP1_FROM_REG(data->temp8[3]) - val;
 	i2c_smbus_write_byte_data(client, LM90_REG_W_TCRIT_HYST,
 				  HYST_TO_REG(hyst));
 	up(&data->update_lock);
 	return count;
 }
 
-static ssize_t show_alarms(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_alarms(struct device *dev, struct device_attribute *dummy,
+			   char *buf)
 {
 	struct lm90_data *data = lm90_update_device(dev);
 	return sprintf(buf, "%d\n", data->alarms);
 }
 
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_input1, NULL);
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_input2, NULL);
-static DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp_low1,
-	set_temp_low1);
-static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp_low2,
-	set_temp_low2);
-static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_high1,
-	set_temp_high1);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp_high2,
-	set_temp_high2);
-static DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp_crit1,
-	set_temp_crit1);
-static DEVICE_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_temp_crit2,
-	set_temp_crit2);
-static DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_temp_hyst1,
-	set_temp_hyst1);
-static DEVICE_ATTR(temp2_crit_hyst, S_IRUGO, show_temp_hyst2, NULL);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp8, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp11, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 1);
+static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11,
+	set_temp11, 1);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 2);
+static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11,
+	set_temp11, 2);
+static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 3);
+static SENSOR_DEVICE_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 4);
+static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_temphyst,
+	set_temphyst, 3);
+static SENSOR_DEVICE_ATTR(temp2_crit_hyst, S_IRUGO, show_temphyst, NULL, 4);
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
 /*
@@ -480,16 +500,26 @@
 	lm90_init_client(new_client);
 
 	/* Register sysfs hooks */
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_input);
-	device_create_file(&new_client->dev, &dev_attr_temp1_min);
-	device_create_file(&new_client->dev, &dev_attr_temp2_min);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_max);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit_hyst);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit_hyst);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_min.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_min.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_crit_hyst.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_crit_hyst.dev_attr);
 	device_create_file(&new_client->dev, &dev_attr_alarms);
 
 	return 0;
@@ -540,16 +570,16 @@
 		u8 oldh, newh;
 
 		dev_dbg(&client->dev, "Updating lm90 data.\n");
-		data->temp_input1 = i2c_smbus_read_byte_data(client,
-				    LM90_REG_R_LOCAL_TEMP);
-		data->temp_high1 = i2c_smbus_read_byte_data(client,
-				   LM90_REG_R_LOCAL_HIGH);
-		data->temp_low1 = i2c_smbus_read_byte_data(client,
-				  LM90_REG_R_LOCAL_LOW);
-		data->temp_crit1 = i2c_smbus_read_byte_data(client,
-				   LM90_REG_R_LOCAL_CRIT);
-		data->temp_crit2 = i2c_smbus_read_byte_data(client,
-				   LM90_REG_R_REMOTE_CRIT);
+		data->temp8[0] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_LOCAL_TEMP);
+		data->temp8[1] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_LOCAL_LOW);
+		data->temp8[2] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_LOCAL_HIGH);
+		data->temp8[3] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_LOCAL_CRIT);
+		data->temp8[4] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_REMOTE_CRIT);
 		data->temp_hyst = i2c_smbus_read_byte_data(client,
 				  LM90_REG_R_TCRIT_HYST);
 
@@ -569,13 +599,13 @@
 		 */
 		oldh = i2c_smbus_read_byte_data(client,
 		       LM90_REG_R_REMOTE_TEMPH);
-		data->temp_input2 = i2c_smbus_read_byte_data(client,
-				    LM90_REG_R_REMOTE_TEMPL);
+		data->temp11[0] = i2c_smbus_read_byte_data(client,
+				  LM90_REG_R_REMOTE_TEMPL);
 		newh = i2c_smbus_read_byte_data(client,
 		       LM90_REG_R_REMOTE_TEMPH);
 		if (newh != oldh) {
-			data->temp_input2 = i2c_smbus_read_byte_data(client,
-					    LM90_REG_R_REMOTE_TEMPL);
+			data->temp11[0] = i2c_smbus_read_byte_data(client,
+					  LM90_REG_R_REMOTE_TEMPL);
 #ifdef DEBUG
 			oldh = i2c_smbus_read_byte_data(client,
 			       LM90_REG_R_REMOTE_TEMPH);
@@ -585,16 +615,16 @@
 					 "wrong.\n");
 #endif
 		}
-		data->temp_input2 |= (newh << 8);
+		data->temp11[0] |= (newh << 8);
 
-		data->temp_high2 = (i2c_smbus_read_byte_data(client,
+		data->temp11[1] = (i2c_smbus_read_byte_data(client,
+				   LM90_REG_R_REMOTE_LOWH) << 8) +
+				   i2c_smbus_read_byte_data(client,
+				   LM90_REG_R_REMOTE_LOWL);
+		data->temp11[2] = (i2c_smbus_read_byte_data(client,
 				   LM90_REG_R_REMOTE_HIGHH) << 8) +
 				   i2c_smbus_read_byte_data(client,
 				   LM90_REG_R_REMOTE_HIGHL);
-		data->temp_low2 = (i2c_smbus_read_byte_data(client,
-				  LM90_REG_R_REMOTE_LOWH) << 8) +
-				  i2c_smbus_read_byte_data(client,
-				  LM90_REG_R_REMOTE_LOWL);
 		data->alarms = i2c_smbus_read_byte_data(client,
 			       LM90_REG_R_STATUS);
 


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [lm-sensors] More hardware monitoring drivers ported to the new
@ 2005-06-05 20:08 ` Jean Delvare
  0 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-05 20:08 UTC (permalink / raw)
  To: LM Sensors, LKML; +Cc: Yani Ioannou, Greg KH

Hi all,

I have been modifying three additional hardware monitoring drivers to
take benefit of Yani Ioannou's new, extended sysfs callbacks. These
drivers are lm63, lm83 and lm90. All of these are relatively small when
compared to the first two modified drivers (adm1026 and it87). My goal
was to demonstrate that the new callbacks can also be used in small
drivers, with significant benefits. The result is even smaller drivers
(less memory used when loaded), relying far less on macros, which makes
the code easier to read (and the drivers presumably faster to distribute
using distcc).

Module                Before   After
lm63                   10128    9424 ( -704/ -6%)
lm83                    8784    6864 (-1920/-21%)
lm90                   12420   10628 (-1792/-14%)

Individual patches will follow. Comments welcome. Greg, can you add
these to one of your trees?

Before I go on with driver conversion, there are two points I'd like to
discuss:

First, I don't much like the name of the new header file,
linux/i2c-sysfs.h. It isn't related with i2c at all! It's all about
sensors (or hardware monitoring if you prefer). I think the header file
should be named linux/hwmon-sysfs.h or something similar.

Second, is there a reason why the SENSOR_DEVICE_ATTR macro creates a
stucture named sensor_dev_attr_##_name rather than simply
dev_attr_##_name? As it seems unlikely that SENSOR_DEVICE_ATTR and
DEVICE_ATTR will both be called for the same file, going for the short
form shouldn't cause any problem. This would make the calling code more
readable IMHO.

Thanks,
-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [lm-sensors] Re: More hardware monitoring drivers ported to the new
@ 2005-06-05 20:26   ` Yani Ioannou
  0 siblings, 0 replies; 14+ messages in thread
From: Yani Ioannou @ 2005-06-05 20:26 UTC (permalink / raw)
  To: Jean Delvare; +Cc: LM Sensors, LKML, Greg KH

Hi Jean,

> I have been modifying three additional hardware monitoring drivers to
> take benefit of Yani Ioannou's new, extended sysfs callbacks. These
> drivers are lm63, lm83 and lm90. All of these are relatively small when
> compared to the first two modified drivers (adm1026 and it87). My goal
> was to demonstrate that the new callbacks can also be used in small
> drivers, with significant benefits. The result is even smaller drivers
> (less memory used when loaded), relying far less on macros, which makes
> the code easier to read (and the drivers presumably faster to distribute
> using distcc).
>
> Module                Before   After
> lm63                   10128    9424 ( -704/ -6%)
> lm83                    8784    6864 (-1920/-21%)
> lm90                   12420   10628 (-1792/-14%)
> 

Good stuff :-)

> First, I don't much like the name of the new header file,
> linux/i2c-sysfs.h. It isn't related with i2c at all! It's all about
> sensors (or hardware monitoring if you prefer). I think the header file
> should be named linux/hwmon-sysfs.h or something similar.

hwmon wasn't near being added to -mm at the time so I went with the
status quo and named it i2c-sysfs, I'm all for renaming it to
hwmon-sysfs.h though.

> Second, is there a reason why the SENSOR_DEVICE_ATTR macro creates a
> stucture named sensor_dev_attr_##_name rather than simply
> dev_attr_##_name? As it seems unlikely that SENSOR_DEVICE_ATTR and
> DEVICE_ATTR will both be called for the same file, going for the short
> form shouldn't cause any problem. This would make the calling code more
> readable IMHO.

I know the variable names are a bit long, but my patch against adm1026
for example still uses DEVICE_ATTR for attributes that can't make any
use of the dynamic callbacks, and hence I don't want to waste the
extra space required for a sensor device attribute. So there is still
reason to use both in one file. When it comes to macros that define
'hidden' variable names I'm inclined to err on the side of caution and
use the longer name too.

Thanks,
Yani

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [lm-sensors] [PATCH 2.6] I2C: (1/3) lm63 uses new sysfs callbacks
@ 2005-06-05 20:32   ` Jean Delvare
  0 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-05 20:32 UTC (permalink / raw)
  To: LM Sensors, LKML; +Cc: Greg KH, Yani Ioannou

Hi Greg, all,

I updated the lm63 hardware monitoring driver to take benefit of Yani
Ioannou's new sysfs callback capabilities.

Please apply,
thanks.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

 drivers/i2c/chips/lm63.c |  267 ++++++++++++++++++++++++-----------------------
 1 files changed, 142 insertions(+), 125 deletions(-)

--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm63.c	2005-06-05 19:23:52.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm63.c	2005-06-05 19:37:15.000000000 +0200
@@ -1,7 +1,7 @@
 /*
  * lm63.c - driver for the National Semiconductor LM63 temperature sensor
  *          with integrated fan control
- * Copyright (C) 2004  Jean Delvare <khali@linux-fr.org>
+ * Copyright (C) 2004-2005  Jean Delvare <khali@linux-fr.org>
  * Based on the lm90 driver.
  *
  * The LM63 is a sensor chip made by National Semiconductor. It measures
@@ -43,6 +43,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
+#include <linux/i2c-sysfs.h>
 
 /*
  * Addresses to scan
@@ -157,16 +158,16 @@
 
 	/* registers values */
 	u8 config, config_fan;
-	u16 fan1_input;
-	u16 fan1_low;
+	u16 fan[2];	/* 0: input
+			   1: low limit */
 	u8 pwm1_freq;
 	u8 pwm1_value;
-	s8 temp1_input;
-	s8 temp1_high;
-	s16 temp2_input;
-	s16 temp2_high;
-	s16 temp2_low;
-	s8 temp2_crit;
+	s8 temp8[3];	/* 0: local input
+			   1: local high limit
+			   2: remote critical limit */
+	s16 temp11[3];	/* 0: remote input
+			   1: remote low limit
+			   2: remote high limit */
 	u8 temp2_crit_hyst;
 	u8 alarms;
 };
@@ -175,33 +176,33 @@
  * Sysfs callback functions and files
  */
 
-#define show_fan(value) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm63_data *data = lm63_update_device(dev); \
-	return sprintf(buf, "%d\n", FAN_FROM_REG(data->value)); \
+static ssize_t show_fan(struct device *dev, struct device_attribute *devattr,
+			char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm63_data *data = lm63_update_device(dev);
+	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index]));
 }
-show_fan(fan1_input);
-show_fan(fan1_low);
 
-static ssize_t set_fan1_low(struct device *dev, struct device_attribute *attr, const char *buf,
-	size_t count)
+static ssize_t set_fan(struct device *dev, struct device_attribute *dummy,
+		       const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm63_data *data = i2c_get_clientdata(client);
 	unsigned long val = simple_strtoul(buf, NULL, 10);
 
 	down(&data->update_lock);
-	data->fan1_low = FAN_TO_REG(val);
+	data->fan[1] = FAN_TO_REG(val);
 	i2c_smbus_write_byte_data(client, LM63_REG_TACH_LIMIT_LSB,
-				  data->fan1_low & 0xFF);
+				  data->fan[1] & 0xFF);
 	i2c_smbus_write_byte_data(client, LM63_REG_TACH_LIMIT_MSB,
-				  data->fan1_low >> 8);
+				  data->fan[1] >> 8);
 	up(&data->update_lock);
 	return count;
 }
 
-static ssize_t show_pwm1(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_pwm1(struct device *dev, struct device_attribute *dummy,
+			 char *buf)
 {
 	struct lm63_data *data = lm63_update_device(dev);
 	return sprintf(buf, "%d\n", data->pwm1_value >= 2 * data->pwm1_freq ?
@@ -209,7 +210,8 @@
 		       (2 * data->pwm1_freq));
 }
 
-static ssize_t set_pwm1(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+static ssize_t set_pwm1(struct device *dev, struct device_attribute *dummy,
+			const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm63_data *data = i2c_get_clientdata(client);
@@ -228,77 +230,83 @@
 	return count;
 }
 
-static ssize_t show_pwm1_enable(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_pwm1_enable(struct device *dev, struct device_attribute *dummy,
+				char *buf)
 {
 	struct lm63_data *data = lm63_update_device(dev);
 	return sprintf(buf, "%d\n", data->config_fan & 0x20 ? 1 : 2);
 }
 
-#define show_temp8(value) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm63_data *data = lm63_update_device(dev); \
-	return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->value)); \
-}
-#define show_temp11(value) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm63_data *data = lm63_update_device(dev); \
-	return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->value)); \
-}
-show_temp8(temp1_input);
-show_temp8(temp1_high);
-show_temp11(temp2_input);
-show_temp11(temp2_high);
-show_temp11(temp2_low);
-show_temp8(temp2_crit);
-
-#define set_temp8(value, reg) \
-static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm63_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	data->value = TEMP8_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, reg, data->value); \
-	up(&data->update_lock); \
-	return count; \
-}
-#define set_temp11(value, reg_msb, reg_lsb) \
-static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm63_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	data->value = TEMP11_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, reg_msb, data->value >> 8); \
-	i2c_smbus_write_byte_data(client, reg_lsb, data->value & 0xff); \
-	up(&data->update_lock); \
-	return count; \
-}
-set_temp8(temp1_high, LM63_REG_LOCAL_HIGH);
-set_temp11(temp2_high, LM63_REG_REMOTE_HIGH_MSB, LM63_REG_REMOTE_HIGH_LSB);
-set_temp11(temp2_low, LM63_REG_REMOTE_LOW_MSB, LM63_REG_REMOTE_LOW_LSB);
+static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
+			  char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm63_data *data = lm63_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index]));
+}
+
+static ssize_t set_temp8(struct device *dev, struct device_attribute *dummy,
+			 const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm63_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+
+	down(&data->update_lock);
+	data->temp8[1] = TEMP8_TO_REG(val);
+	i2c_smbus_write_byte_data(client, LM63_REG_LOCAL_HIGH, data->temp8[1]);
+	up(&data->update_lock);
+	return count;
+}
+
+static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
+			   char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm63_data *data = lm63_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index]));
+}
+
+static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
+			  const char *buf, size_t count)
+{
+	static const u8 reg[4] = {
+		LM63_REG_REMOTE_LOW_MSB,
+		LM63_REG_REMOTE_LOW_LSB,
+		LM63_REG_REMOTE_HIGH_MSB,
+		LM63_REG_REMOTE_HIGH_LSB,
+	};
+
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm63_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+	int nr = attr->index;
+
+	down(&data->update_lock);
+	data->temp11[nr] = TEMP11_TO_REG(val);
+	i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2],
+				  data->temp11[nr] >> 8);
+	i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1],
+				  data->temp11[nr] & 0xff);
+	up(&data->update_lock);
+	return count;
+}
 
 /* Hysteresis register holds a relative value, while we want to present
    an absolute to user-space */
-static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute *dummy,
+				    char *buf)
 {
 	struct lm63_data *data = lm63_update_device(dev);
-	return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp2_crit)
+	return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2])
 		       - TEMP8_FROM_REG(data->temp2_crit_hyst));
 }
 
 /* And now the other way around, user-space provides an absolute
    hysteresis value and we have to store a relative one */
-static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute *attr, const char *buf,
-	size_t count)
+static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute *dummy,
+				   const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm63_data *data = i2c_get_clientdata(client);
@@ -306,36 +314,37 @@
 	long hyst;
 
 	down(&data->update_lock);
-	hyst = TEMP8_FROM_REG(data->temp2_crit) - val;
+	hyst = TEMP8_FROM_REG(data->temp8[2]) - val;
 	i2c_smbus_write_byte_data(client, LM63_REG_REMOTE_TCRIT_HYST,
 				  HYST_TO_REG(hyst));
 	up(&data->update_lock);
 	return count;
 }
 
-static ssize_t show_alarms(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_alarms(struct device *dev, struct device_attribute *dummy,
+			   char *buf)
 {
 	struct lm63_data *data = lm63_update_device(dev);
 	return sprintf(buf, "%u\n", data->alarms);
 }
 
-static DEVICE_ATTR(fan1_input, S_IRUGO, show_fan1_input, NULL);
-static DEVICE_ATTR(fan1_min, S_IWUSR | S_IRUGO, show_fan1_low,
-	set_fan1_low);
+static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, show_fan, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan1_min, S_IWUSR | S_IRUGO, show_fan,
+	set_fan, 1);
 
 static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, show_pwm1, set_pwm1);
 static DEVICE_ATTR(pwm1_enable, S_IRUGO, show_pwm1_enable, NULL);
 
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp1_input, NULL);
-static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp1_high,
-	set_temp1_high);
-
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp2_input, NULL);
-static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp2_low,
-	set_temp2_low);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp2_high,
-	set_temp2_high);
-static DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp2_crit, NULL);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp8, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 1);
+
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp11, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11,
+	set_temp11, 1);
+static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11,
+	set_temp11, 2);
+static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp8, NULL, 2);
 static DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_temp2_crit_hyst,
 	set_temp2_crit_hyst);
 
@@ -429,17 +438,25 @@
 
 	/* Register sysfs hooks */
 	if (data->config & 0x04) { /* tachometer enabled */
-		device_create_file(&new_client->dev, &dev_attr_fan1_input);
-		device_create_file(&new_client->dev, &dev_attr_fan1_min);
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_fan1_input.dev_attr);
+		device_create_file(&new_client->dev,
+				   &sensor_dev_attr_fan1_min.dev_attr);
 	}
 	device_create_file(&new_client->dev, &dev_attr_pwm1);
 	device_create_file(&new_client->dev, &dev_attr_pwm1_enable);
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_min);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_min.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_crit.dev_attr);
 	device_create_file(&new_client->dev, &dev_attr_temp2_crit_hyst);
 	device_create_file(&new_client->dev, &dev_attr_alarms);
 
@@ -510,14 +527,14 @@
 	if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
 		if (data->config & 0x04) { /* tachometer enabled  */
 			/* order matters for fan1_input */
-			data->fan1_input = i2c_smbus_read_byte_data(client,
-					   LM63_REG_TACH_COUNT_LSB) & 0xFC;
-			data->fan1_input |= i2c_smbus_read_byte_data(client,
-					    LM63_REG_TACH_COUNT_MSB) << 8;
-			data->fan1_low = (i2c_smbus_read_byte_data(client,
-					  LM63_REG_TACH_LIMIT_LSB) & 0xFC)
-				       | (i2c_smbus_read_byte_data(client,
-					  LM63_REG_TACH_LIMIT_MSB) << 8);
+			data->fan[0] = i2c_smbus_read_byte_data(client,
+				       LM63_REG_TACH_COUNT_LSB) & 0xFC;
+			data->fan[0] |= i2c_smbus_read_byte_data(client,
+					LM63_REG_TACH_COUNT_MSB) << 8;
+			data->fan[1] = (i2c_smbus_read_byte_data(client,
+					LM63_REG_TACH_LIMIT_LSB) & 0xFC)
+				     | (i2c_smbus_read_byte_data(client,
+					LM63_REG_TACH_LIMIT_MSB) << 8);
 		}
 
 		data->pwm1_freq = i2c_smbus_read_byte_data(client,
@@ -527,26 +544,26 @@
 		data->pwm1_value = i2c_smbus_read_byte_data(client,
 				   LM63_REG_PWM_VALUE);
 
-		data->temp1_input = i2c_smbus_read_byte_data(client,
-				    LM63_REG_LOCAL_TEMP);
-		data->temp1_high = i2c_smbus_read_byte_data(client,
-				   LM63_REG_LOCAL_HIGH);
+		data->temp8[0] = i2c_smbus_read_byte_data(client,
+				 LM63_REG_LOCAL_TEMP);
+		data->temp8[1] = i2c_smbus_read_byte_data(client,
+				 LM63_REG_LOCAL_HIGH);
 
 		/* order matters for temp2_input */
-		data->temp2_input = i2c_smbus_read_byte_data(client,
-				    LM63_REG_REMOTE_TEMP_MSB) << 8;
-		data->temp2_input |= i2c_smbus_read_byte_data(client,
-				     LM63_REG_REMOTE_TEMP_LSB);
-		data->temp2_high = (i2c_smbus_read_byte_data(client,
-				   LM63_REG_REMOTE_HIGH_MSB) << 8)
-				 | i2c_smbus_read_byte_data(client,
-				   LM63_REG_REMOTE_HIGH_LSB);
-		data->temp2_low = (i2c_smbus_read_byte_data(client,
+		data->temp11[0] = i2c_smbus_read_byte_data(client,
+				  LM63_REG_REMOTE_TEMP_MSB) << 8;
+		data->temp11[0] |= i2c_smbus_read_byte_data(client,
+				   LM63_REG_REMOTE_TEMP_LSB);
+		data->temp11[1] = (i2c_smbus_read_byte_data(client,
 				  LM63_REG_REMOTE_LOW_MSB) << 8)
 				| i2c_smbus_read_byte_data(client,
 				  LM63_REG_REMOTE_LOW_LSB);
-		data->temp2_crit = i2c_smbus_read_byte_data(client,
-				   LM63_REG_REMOTE_TCRIT);
+		data->temp11[2] = (i2c_smbus_read_byte_data(client,
+				  LM63_REG_REMOTE_HIGH_MSB) << 8)
+				| i2c_smbus_read_byte_data(client,
+				  LM63_REG_REMOTE_HIGH_LSB);
+		data->temp8[2] = i2c_smbus_read_byte_data(client,
+				 LM63_REG_REMOTE_TCRIT);
 		data->temp2_crit_hyst = i2c_smbus_read_byte_data(client,
 					LM63_REG_REMOTE_TCRIT_HYST);
 


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [lm-sensors] [PATCH 2.6] I2C: (2/3) lm83 uses new sysfs callbacks
@ 2005-06-05 21:16   ` Jean Delvare
  0 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-05 21:16 UTC (permalink / raw)
  To: LM Sensors, LKML; +Cc: Greg KH, Yani Ioannou

Hi Greg, all,

I updated the lm83 hardware monitoring driver to take benefit of Yani
Ioannou's new sysfs callback capabilities.

Please apply,
thanks.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

 drivers/i2c/chips/lm83.c |  157 +++++++++++++++++++++++------------------------
 1 files changed, 77 insertions(+), 80 deletions(-)

--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm83.c	2005-06-05 19:23:53.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm83.c	2005-06-05 21:11:41.000000000 +0200
@@ -1,7 +1,7 @@
 /*
  * lm83.c - Part of lm_sensors, Linux kernel modules for hardware
  *          monitoring
- * Copyright (C) 2003  Jean Delvare <khali@linux-fr.org>
+ * Copyright (C) 2003-2005  Jean Delvare <khali@linux-fr.org>
  *
  * Heavily inspired from the lm78, lm75 and adm1021 drivers. The LM83 is
  * a sensor chip made by National Semiconductor. It reports up to four
@@ -33,6 +33,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
+#include <linux/i2c-sysfs.h>
 
 /*
  * Addresses to scan
@@ -93,21 +94,20 @@
 	LM83_REG_R_LOCAL_TEMP,
 	LM83_REG_R_REMOTE1_TEMP,
 	LM83_REG_R_REMOTE2_TEMP,
-	LM83_REG_R_REMOTE3_TEMP
-};
-
-static const u8 LM83_REG_R_HIGH[] = {
+	LM83_REG_R_REMOTE3_TEMP,
 	LM83_REG_R_LOCAL_HIGH,
 	LM83_REG_R_REMOTE1_HIGH,
 	LM83_REG_R_REMOTE2_HIGH,
-	LM83_REG_R_REMOTE3_HIGH
+	LM83_REG_R_REMOTE3_HIGH,
+	LM83_REG_R_TCRIT,
 };
 
 static const u8 LM83_REG_W_HIGH[] = {
 	LM83_REG_W_LOCAL_HIGH,
 	LM83_REG_W_REMOTE1_HIGH,
 	LM83_REG_W_REMOTE2_HIGH,
-	LM83_REG_W_REMOTE3_HIGH
+	LM83_REG_W_REMOTE3_HIGH,
+	LM83_REG_W_TCRIT,
 };
 
 /*
@@ -143,9 +143,9 @@
 	unsigned long last_updated; /* in jiffies */
 
 	/* registers values */
-	s8 temp_input[4];
-	s8 temp_high[4];
-	s8 temp_crit;
+	s8 temp[9];	/* 0..3: input 1-4,
+			   4..7: high limit 1-4,
+			   8   : critical limit */
 	u16 alarms; /* bitvector, combined */
 };
 
@@ -153,65 +153,55 @@
  * Sysfs stuff
  */
 
-#define show_temp(suffix, value) \
-static ssize_t show_temp_##suffix(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm83_data *data = lm83_update_device(dev); \
-	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->value)); \
+static ssize_t show_temp(struct device *dev, struct device_attribute *devattr,
+			 char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm83_data *data = lm83_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[attr->index]));
 }
-show_temp(input1, temp_input[0]);
-show_temp(input2, temp_input[1]);
-show_temp(input3, temp_input[2]);
-show_temp(input4, temp_input[3]);
-show_temp(high1, temp_high[0]);
-show_temp(high2, temp_high[1]);
-show_temp(high3, temp_high[2]);
-show_temp(high4, temp_high[3]);
-show_temp(crit, temp_crit);
-
-#define set_temp(suffix, value, reg) \
-static ssize_t set_temp_##suffix(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm83_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	data->value = TEMP_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, reg, data->value); \
-	up(&data->update_lock); \
-	return count; \
+
+static ssize_t set_temp(struct device *dev, struct device_attribute *devattr,
+			const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm83_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+	int nr = attr->index;
+
+	down(&data->update_lock);
+	data->temp[nr] = TEMP_TO_REG(val);
+	i2c_smbus_write_byte_data(client, LM83_REG_W_HIGH[nr - 4],
+				  data->temp[nr]);
+	up(&data->update_lock);
+	return count;
 }
-set_temp(high1, temp_high[0], LM83_REG_W_LOCAL_HIGH);
-set_temp(high2, temp_high[1], LM83_REG_W_REMOTE1_HIGH);
-set_temp(high3, temp_high[2], LM83_REG_W_REMOTE2_HIGH);
-set_temp(high4, temp_high[3], LM83_REG_W_REMOTE3_HIGH);
-set_temp(crit, temp_crit, LM83_REG_W_TCRIT);
 
-static ssize_t show_alarms(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_alarms(struct device *dev, struct device_attribute *dummy,
+			   char *buf)
 {
 	struct lm83_data *data = lm83_update_device(dev);
 	return sprintf(buf, "%d\n", data->alarms);
 }
 
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_input1, NULL);
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_input2, NULL);
-static DEVICE_ATTR(temp3_input, S_IRUGO, show_temp_input3, NULL);
-static DEVICE_ATTR(temp4_input, S_IRUGO, show_temp_input4, NULL);
-static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_high1,
-    set_temp_high1);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp_high2,
-    set_temp_high2);
-static DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_temp_high3,
-    set_temp_high3);
-static DEVICE_ATTR(temp4_max, S_IWUSR | S_IRUGO, show_temp_high4,
-    set_temp_high4);
-static DEVICE_ATTR(temp1_crit, S_IRUGO, show_temp_crit, NULL);
-static DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp_crit, NULL);
-static DEVICE_ATTR(temp3_crit, S_IWUSR | S_IRUGO, show_temp_crit,
-    set_temp_crit);
-static DEVICE_ATTR(temp4_crit, S_IRUGO, show_temp_crit, NULL);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp3_input, S_IRUGO, show_temp, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp4_input, S_IRUGO, show_temp, NULL, 3);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 4);
+static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 5);
+static SENSOR_DEVICE_ATTR(temp3_max, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 6);
+static SENSOR_DEVICE_ATTR(temp4_max, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 7);
+static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, show_temp, NULL, 8);
+static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp, NULL, 8);
+static SENSOR_DEVICE_ATTR(temp3_crit, S_IWUSR | S_IRUGO, show_temp,
+	set_temp, 8);
+static SENSOR_DEVICE_ATTR(temp4_crit, S_IRUGO, show_temp, NULL, 8);
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
 /*
@@ -322,18 +312,30 @@
 	 */
 
 	/* Register sysfs hooks */
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_input);
-	device_create_file(&new_client->dev, &dev_attr_temp3_input);
-	device_create_file(&new_client->dev, &dev_attr_temp4_input);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_max);
-	device_create_file(&new_client->dev, &dev_attr_temp3_max);
-	device_create_file(&new_client->dev, &dev_attr_temp4_max);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp3_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp4_crit);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp3_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp4_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp3_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp4_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp3_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp4_crit.dev_attr);
 	device_create_file(&new_client->dev, &dev_attr_alarms);
 
 	return 0;
@@ -369,16 +371,11 @@
 		int nr;
 
 		dev_dbg(&client->dev, "Updating lm83 data.\n");
-		for (nr = 0; nr < 4 ; nr++) {
-			data->temp_input[nr] +		for (nr = 0; nr < 9; nr++) {
+			data->temp[nr]  			    i2c_smbus_read_byte_data(client,
 			    LM83_REG_R_TEMP[nr]);
-			data->temp_high[nr] -			    i2c_smbus_read_byte_data(client,
-			    LM83_REG_R_HIGH[nr]);
 		}
-		data->temp_crit -		    i2c_smbus_read_byte_data(client, LM83_REG_R_TCRIT);
 		data->alarms  		    i2c_smbus_read_byte_data(client, LM83_REG_R_STATUS1)
 		    + (i2c_smbus_read_byte_data(client, LM83_REG_R_STATUS2)


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [lm-sensors] [PATCH 2.6] I2C: (3/3) lm90 uses new sysfs callbacks
@ 2005-06-05 21:27   ` Jean Delvare
  0 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-05 21:27 UTC (permalink / raw)
  To: LM Sensors, LKML; +Cc: Greg KH, Yani Ioannou

Hi Greg, all,

I updated the lm90 hardware monitoring driver to take benefit of Yani
Ioannou's new sysfs callback capabilities.

Please apply,
thanks.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

 drivers/i2c/chips/lm90.c |  270 ++++++++++++++++++++++++++---------------------
 1 files changed, 150 insertions(+), 120 deletions(-)

Index: linux-2.6.12-rc5/drivers/i2c/chips/lm90.c
=================================--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm90.c	2005-06-05 14:14:27.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm90.c	2005-06-05 19:29:15.000000000 +0200
@@ -1,7 +1,7 @@
 /*
  * lm90.c - Part of lm_sensors, Linux kernel modules for hardware
  *          monitoring
- * Copyright (C) 2003-2004  Jean Delvare <khali@linux-fr.org>
+ * Copyright (C) 2003-2005  Jean Delvare <khali@linux-fr.org>
  *
  * Based on the lm83 driver. The LM90 is a sensor chip made by National
  * Semiconductor. It reports up to two temperatures (its own plus up to
@@ -76,6 +76,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
+#include <linux/i2c-sysfs.h>
 
 /*
  * Addresses to scan
@@ -205,9 +206,14 @@
 	int kind;
 
 	/* registers values */
-	s8 temp_input1, temp_low1, temp_high1; /* local */
-	s16 temp_input2, temp_low2, temp_high2; /* remote, combined */
-	s8 temp_crit1, temp_crit2;
+	s8 temp8[5];	/* 0: local input
+			   1: local low limit
+			   2: local high limit
+			   3: local critical limit
+			   4: remote critical limit */
+	s16 temp11[3];	/* 0: remote input
+			   1: remote low limit
+			   2: remote high limit */
 	u8 temp_hyst;
 	u8 alarms; /* bitvector */
 };
@@ -216,75 +222,88 @@
  * Sysfs stuff
  */
 
-#define show_temp(value, converter) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm90_data *data = lm90_update_device(dev); \
-	return sprintf(buf, "%d\n", converter(data->value)); \
-}
-show_temp(temp_input1, TEMP1_FROM_REG);
-show_temp(temp_input2, TEMP2_FROM_REG);
-show_temp(temp_low1, TEMP1_FROM_REG);
-show_temp(temp_low2, TEMP2_FROM_REG);
-show_temp(temp_high1, TEMP1_FROM_REG);
-show_temp(temp_high2, TEMP2_FROM_REG);
-show_temp(temp_crit1, TEMP1_FROM_REG);
-show_temp(temp_crit2, TEMP1_FROM_REG);
-
-#define set_temp1(value, reg) \
-static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm90_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	if (data->kind = adt7461) \
-		data->value = TEMP1_TO_REG_ADT7461(val); \
-	else \
-		data->value = TEMP1_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, reg, data->value); \
-	up(&data->update_lock); \
-	return count; \
-}
-#define set_temp2(value, regh, regl) \
-static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, \
-	size_t count) \
-{ \
-	struct i2c_client *client = to_i2c_client(dev); \
-	struct lm90_data *data = i2c_get_clientdata(client); \
-	long val = simple_strtol(buf, NULL, 10); \
- \
-	down(&data->update_lock); \
-	if (data->kind = adt7461) \
-		data->value = TEMP2_TO_REG_ADT7461(val); \
-	else \
-		data->value = TEMP2_TO_REG(val); \
-	i2c_smbus_write_byte_data(client, regh, data->value >> 8); \
-	i2c_smbus_write_byte_data(client, regl, data->value & 0xff); \
-	up(&data->update_lock); \
-	return count; \
-}
-set_temp1(temp_low1, LM90_REG_W_LOCAL_LOW);
-set_temp2(temp_low2, LM90_REG_W_REMOTE_LOWH, LM90_REG_W_REMOTE_LOWL);
-set_temp1(temp_high1, LM90_REG_W_LOCAL_HIGH);
-set_temp2(temp_high2, LM90_REG_W_REMOTE_HIGHH, LM90_REG_W_REMOTE_HIGHL);
-set_temp1(temp_crit1, LM90_REG_W_LOCAL_CRIT);
-set_temp1(temp_crit2, LM90_REG_W_REMOTE_CRIT);
-
-#define show_temp_hyst(value, basereg) \
-static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct lm90_data *data = lm90_update_device(dev); \
-	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->basereg) \
-		       - TEMP1_FROM_REG(data->temp_hyst)); \
+static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr,
+			  char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm90_data *data = lm90_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->temp8[attr->index]));
 }
-show_temp_hyst(temp_hyst1, temp_crit1);
-show_temp_hyst(temp_hyst2, temp_crit2);
 
-static ssize_t set_temp_hyst1(struct device *dev, struct device_attribute *attr, const char *buf,
-	size_t count)
+static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
+			 const char *buf, size_t count)
+{
+	static const u8 reg[4] = {
+		LM90_REG_W_LOCAL_LOW,
+		LM90_REG_W_LOCAL_HIGH,
+		LM90_REG_W_LOCAL_CRIT,
+		LM90_REG_W_REMOTE_CRIT,
+	};
+
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm90_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+	int nr = attr->index;
+
+	down(&data->update_lock);
+	if (data->kind = adt7461)
+		data->temp8[nr] = TEMP1_TO_REG_ADT7461(val);
+	else
+		data->temp8[nr] = TEMP1_TO_REG(val);
+	i2c_smbus_write_byte_data(client, reg[nr - 1], data->temp8[nr]);
+	up(&data->update_lock);
+	return count;
+}
+
+static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr,
+			   char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm90_data *data = lm90_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP2_FROM_REG(data->temp11[attr->index]));
+}
+
+static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
+			  const char *buf, size_t count)
+{
+	static const u8 reg[4] = {
+		LM90_REG_W_REMOTE_LOWH,
+		LM90_REG_W_REMOTE_LOWL,
+		LM90_REG_W_REMOTE_HIGHH,
+		LM90_REG_W_REMOTE_HIGHL,
+	};
+
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct lm90_data *data = i2c_get_clientdata(client);
+	long val = simple_strtol(buf, NULL, 10);
+	int nr = attr->index;
+
+	down(&data->update_lock);
+	if (data->kind = adt7461)
+		data->temp11[nr] = TEMP2_TO_REG_ADT7461(val);
+	else
+		data->temp11[nr] = TEMP2_TO_REG(val);
+	i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2],
+				  data->temp11[nr] >> 8);
+	i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1],
+				  data->temp11[nr] & 0xff);
+	up(&data->update_lock);
+	return count;
+}
+
+static ssize_t show_temphyst(struct device *dev, struct device_attribute *devattr,
+			     char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct lm90_data *data = lm90_update_device(dev);
+	return sprintf(buf, "%d\n", TEMP1_FROM_REG(data->temp8[attr->index])
+		       - TEMP1_FROM_REG(data->temp_hyst));
+}
+
+static ssize_t set_temphyst(struct device *dev, struct device_attribute *dummy,
+			    const char *buf, size_t count)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct lm90_data *data = i2c_get_clientdata(client);
@@ -292,36 +311,37 @@
 	long hyst;
 
 	down(&data->update_lock);
-	hyst = TEMP1_FROM_REG(data->temp_crit1) - val;
+	hyst = TEMP1_FROM_REG(data->temp8[3]) - val;
 	i2c_smbus_write_byte_data(client, LM90_REG_W_TCRIT_HYST,
 				  HYST_TO_REG(hyst));
 	up(&data->update_lock);
 	return count;
 }
 
-static ssize_t show_alarms(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_alarms(struct device *dev, struct device_attribute *dummy,
+			   char *buf)
 {
 	struct lm90_data *data = lm90_update_device(dev);
 	return sprintf(buf, "%d\n", data->alarms);
 }
 
-static DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_input1, NULL);
-static DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_input2, NULL);
-static DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp_low1,
-	set_temp_low1);
-static DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp_low2,
-	set_temp_low2);
-static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_high1,
-	set_temp_high1);
-static DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp_high2,
-	set_temp_high2);
-static DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp_crit1,
-	set_temp_crit1);
-static DEVICE_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_temp_crit2,
-	set_temp_crit2);
-static DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_temp_hyst1,
-	set_temp_hyst1);
-static DEVICE_ATTR(temp2_crit_hyst, S_IRUGO, show_temp_hyst2, NULL);
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp8, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp11, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 1);
+static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11,
+	set_temp11, 1);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 2);
+static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11,
+	set_temp11, 2);
+static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 3);
+static SENSOR_DEVICE_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_temp8,
+	set_temp8, 4);
+static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_temphyst,
+	set_temphyst, 3);
+static SENSOR_DEVICE_ATTR(temp2_crit_hyst, S_IRUGO, show_temphyst, NULL, 4);
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 
 /*
@@ -480,16 +500,26 @@
 	lm90_init_client(new_client);
 
 	/* Register sysfs hooks */
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_input);
-	device_create_file(&new_client->dev, &dev_attr_temp1_min);
-	device_create_file(&new_client->dev, &dev_attr_temp2_min);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_max);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit_hyst);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit_hyst);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_input.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_min.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_min.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_max.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_crit.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp1_crit_hyst.dev_attr);
+	device_create_file(&new_client->dev,
+			   &sensor_dev_attr_temp2_crit_hyst.dev_attr);
 	device_create_file(&new_client->dev, &dev_attr_alarms);
 
 	return 0;
@@ -540,16 +570,16 @@
 		u8 oldh, newh;
 
 		dev_dbg(&client->dev, "Updating lm90 data.\n");
-		data->temp_input1 = i2c_smbus_read_byte_data(client,
-				    LM90_REG_R_LOCAL_TEMP);
-		data->temp_high1 = i2c_smbus_read_byte_data(client,
-				   LM90_REG_R_LOCAL_HIGH);
-		data->temp_low1 = i2c_smbus_read_byte_data(client,
-				  LM90_REG_R_LOCAL_LOW);
-		data->temp_crit1 = i2c_smbus_read_byte_data(client,
-				   LM90_REG_R_LOCAL_CRIT);
-		data->temp_crit2 = i2c_smbus_read_byte_data(client,
-				   LM90_REG_R_REMOTE_CRIT);
+		data->temp8[0] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_LOCAL_TEMP);
+		data->temp8[1] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_LOCAL_LOW);
+		data->temp8[2] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_LOCAL_HIGH);
+		data->temp8[3] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_LOCAL_CRIT);
+		data->temp8[4] = i2c_smbus_read_byte_data(client,
+				 LM90_REG_R_REMOTE_CRIT);
 		data->temp_hyst = i2c_smbus_read_byte_data(client,
 				  LM90_REG_R_TCRIT_HYST);
 
@@ -569,13 +599,13 @@
 		 */
 		oldh = i2c_smbus_read_byte_data(client,
 		       LM90_REG_R_REMOTE_TEMPH);
-		data->temp_input2 = i2c_smbus_read_byte_data(client,
-				    LM90_REG_R_REMOTE_TEMPL);
+		data->temp11[0] = i2c_smbus_read_byte_data(client,
+				  LM90_REG_R_REMOTE_TEMPL);
 		newh = i2c_smbus_read_byte_data(client,
 		       LM90_REG_R_REMOTE_TEMPH);
 		if (newh != oldh) {
-			data->temp_input2 = i2c_smbus_read_byte_data(client,
-					    LM90_REG_R_REMOTE_TEMPL);
+			data->temp11[0] = i2c_smbus_read_byte_data(client,
+					  LM90_REG_R_REMOTE_TEMPL);
 #ifdef DEBUG
 			oldh = i2c_smbus_read_byte_data(client,
 			       LM90_REG_R_REMOTE_TEMPH);
@@ -585,16 +615,16 @@
 					 "wrong.\n");
 #endif
 		}
-		data->temp_input2 |= (newh << 8);
+		data->temp11[0] |= (newh << 8);
 
-		data->temp_high2 = (i2c_smbus_read_byte_data(client,
+		data->temp11[1] = (i2c_smbus_read_byte_data(client,
+				   LM90_REG_R_REMOTE_LOWH) << 8) +
+				   i2c_smbus_read_byte_data(client,
+				   LM90_REG_R_REMOTE_LOWL);
+		data->temp11[2] = (i2c_smbus_read_byte_data(client,
 				   LM90_REG_R_REMOTE_HIGHH) << 8) +
 				   i2c_smbus_read_byte_data(client,
 				   LM90_REG_R_REMOTE_HIGHL);
-		data->temp_low2 = (i2c_smbus_read_byte_data(client,
-				  LM90_REG_R_REMOTE_LOWH) << 8) +
-				  i2c_smbus_read_byte_data(client,
-				  LM90_REG_R_REMOTE_LOWL);
 		data->alarms = i2c_smbus_read_byte_data(client,
 			       LM90_REG_R_STATUS);
 


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: More hardware monitoring drivers ported to the new sysfs callbacks
  2005-06-05 20:08 ` [lm-sensors] More hardware monitoring drivers ported to the new Jean Delvare
@ 2005-06-06  9:17   ` Greg KH
  -1 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2005-06-06  6:22 UTC (permalink / raw)
  To: Jean Delvare; +Cc: LM Sensors, LKML, Yani Ioannou

On Sun, Jun 05, 2005 at 08:09:01PM +0200, Jean Delvare wrote:
> Hi all,
> 
> I have been modifying three additional hardware monitoring drivers to
> take benefit of Yani Ioannou's new, extended sysfs callbacks. These
> drivers are lm63, lm83 and lm90. All of these are relatively small when
> compared to the first two modified drivers (adm1026 and it87). My goal
> was to demonstrate that the new callbacks can also be used in small
> drivers, with significant benefits. The result is even smaller drivers
> (less memory used when loaded), relying far less on macros, which makes
> the code easier to read (and the drivers presumably faster to distribute
> using distcc).
> 
> Module                Before   After
> lm63                   10128    9424 ( -704/ -6%)
> lm83                    8784    6864 (-1920/-21%)
> lm90                   12420   10628 (-1792/-14%)
> 
> Individual patches will follow. Comments welcome. Greg, can you add
> these to one of your trees?

Applied, thanks.

> Before I go on with driver conversion, there are two points I'd like to
> discuss:
> 
> First, I don't much like the name of the new header file,
> linux/i2c-sysfs.h. It isn't related with i2c at all! It's all about
> sensors (or hardware monitoring if you prefer). I think the header file
> should be named linux/hwmon-sysfs.h or something similar.

Sure, that would be fine.

> Second, is there a reason why the SENSOR_DEVICE_ATTR macro creates a
> stucture named sensor_dev_attr_##_name rather than simply
> dev_attr_##_name? As it seems unlikely that SENSOR_DEVICE_ATTR and
> DEVICE_ATTR will both be called for the same file, going for the short
> form shouldn't cause any problem. This would make the calling code more
> readable IMHO.

Hm, I really don't care either way about this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [lm-sensors] Re: More hardware monitoring drivers ported to the new
@ 2005-06-06  9:17   ` Greg KH
  0 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2005-06-06  9:17 UTC (permalink / raw)
  To: Jean Delvare; +Cc: LM Sensors, LKML, Yani Ioannou

On Sun, Jun 05, 2005 at 08:09:01PM +0200, Jean Delvare wrote:
> Hi all,
> 
> I have been modifying three additional hardware monitoring drivers to
> take benefit of Yani Ioannou's new, extended sysfs callbacks. These
> drivers are lm63, lm83 and lm90. All of these are relatively small when
> compared to the first two modified drivers (adm1026 and it87). My goal
> was to demonstrate that the new callbacks can also be used in small
> drivers, with significant benefits. The result is even smaller drivers
> (less memory used when loaded), relying far less on macros, which makes
> the code easier to read (and the drivers presumably faster to distribute
> using distcc).
> 
> Module                Before   After
> lm63                   10128    9424 ( -704/ -6%)
> lm83                    8784    6864 (-1920/-21%)
> lm90                   12420   10628 (-1792/-14%)
> 
> Individual patches will follow. Comments welcome. Greg, can you add
> these to one of your trees?

Applied, thanks.

> Before I go on with driver conversion, there are two points I'd like to
> discuss:
> 
> First, I don't much like the name of the new header file,
> linux/i2c-sysfs.h. It isn't related with i2c at all! It's all about
> sensors (or hardware monitoring if you prefer). I think the header file
> should be named linux/hwmon-sysfs.h or something similar.

Sure, that would be fine.

> Second, is there a reason why the SENSOR_DEVICE_ATTR macro creates a
> stucture named sensor_dev_attr_##_name rather than simply
> dev_attr_##_name? As it seems unlikely that SENSOR_DEVICE_ATTR and
> DEVICE_ATTR will both be called for the same file, going for the short
> form shouldn't cause any problem. This would make the calling code more
> readable IMHO.

Hm, I really don't care either way about this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: More hardware monitoring drivers ported to the new sysfs callbacks
  2005-06-06  9:17   ` [lm-sensors] Re: More hardware monitoring drivers ported to the new Greg KH
@ 2005-06-06 19:34     ` Jean Delvare
  -1 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-06 17:34 UTC (permalink / raw)
  To: Greg KH; +Cc: LM Sensors, LKML, Yani Ioannou

Hi Greg, all,

> > First, I don't much like the name of the new header file,
> > linux/i2c-sysfs.h. It isn't related with i2c at all! It's all about
> > sensors (or hardware monitoring if you prefer). I think the header
> > file should be named linux/hwmon-sysfs.h or something similar.
> 
> Sure, that would be fine.

OK, patch follows.

> > Second, is there a reason why the SENSOR_DEVICE_ATTR macro creates a
> > stucture named sensor_dev_attr_##_name rather than simply
> > dev_attr_##_name? As it seems unlikely that SENSOR_DEVICE_ATTR and
> > DEVICE_ATTR will both be called for the same file, going for the
> > short form shouldn't cause any problem. This would make the calling
> > code more readable IMHO.
> 
> Hm, I really don't care either way about this.

Let's not change it then.

---------------------------------------------------------

This patch renames the new linux/i2c-sysfs.h header file to
linux/hwmon-sysfs.h. This names seems to be more appropriate since this
file defines macros and structures not related to i2c but to hardware
monitoring drivers. The patch also updates the five hardware monitoring
driver which include that header file already.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

 drivers/i2c/chips/adm1026.c |    2 +-
 drivers/i2c/chips/it87.c    |    2 +-
 drivers/i2c/chips/lm63.c    |    2 +-
 drivers/i2c/chips/lm83.c    |    2 +-
 drivers/i2c/chips/lm90.c    |    2 +-
 include/linux/hwmon-sysfs.h |   37 +++++++++++++++++++++++++++++++++++++
 include/linux/i2c-sysfs.h   |   37 -------------------------------------
 7 files changed, 42 insertions(+), 42 deletions(-)

--- linux-2.6.12-rc5.orig/drivers/i2c/chips/adm1026.c	2005-06-05 10:53:57.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/adm1026.c	2005-06-06 19:23:32.000000000 +0200
@@ -29,8 +29,8 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
 #include <linux/i2c-vid.h>
+#include <linux/hwmon-sysfs.h>
 
 /* Addresses to scan */
 static unsigned short normal_i2c[] = { 0x2c, 0x2d, 0x2e, I2C_CLIENT_END };
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/it87.c	2005-06-05 11:51:22.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/it87.c	2005-06-06 19:23:14.000000000 +0200
@@ -37,8 +37,8 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
 #include <linux/i2c-vid.h>
+#include <linux/hwmon-sysfs.h>
 #include <asm/io.h>
 
 
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm63.c	2005-06-06 19:09:36.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm63.c	2005-06-06 19:23:01.000000000 +0200
@@ -43,7 +43,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
+#include <linux/hwmon-sysfs.h>
 
 /*
  * Addresses to scan
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm83.c	2005-06-06 19:09:36.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm83.c	2005-06-06 19:22:55.000000000 +0200
@@ -33,7 +33,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
+#include <linux/hwmon-sysfs.h>
 
 /*
  * Addresses to scan
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm90.c	2005-06-05 19:35:05.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm90.c	2005-06-06 19:22:48.000000000 +0200
@@ -76,7 +76,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
+#include <linux/hwmon-sysfs.h>
 
 /*
  * Addresses to scan
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.12-rc5/include/linux/hwmon-sysfs.h	2005-06-06 19:21:02.000000000 +0200
@@ -0,0 +1,37 @@
+/*
+ *  hwmon-sysfs.h - hardware monitoring chip driver sysfs defines
+ *
+ *  Copyright (C) 2005 Yani Ioannou <yani.ioannou@gmail.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+#ifndef _LINUX_HWMON_SYSFS_H
+#define _LINUX_HWMON_SYSFS_H
+
+struct sensor_device_attribute{
+	struct device_attribute dev_attr;
+	int index;
+};
+
+#define to_sensor_dev_attr(_dev_attr) \
+container_of(_dev_attr, struct sensor_device_attribute, dev_attr)
+
+#define SENSOR_DEVICE_ATTR(_name,_mode,_show,_store,_index)	\
+struct sensor_device_attribute sensor_dev_attr_##_name = {	\
+	.dev_attr=__ATTR(_name,_mode,_show,_store),		\
+	.index=_index,						\
+}
+
+#endif /* _LINUX_HWMON_SYSFS_H */
--- linux-2.6.12-rc5.orig/include/linux/i2c-sysfs.h	2005-06-05 10:53:57.000000000 +0200
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,37 +0,0 @@
-/*
- *  i2c-sysfs.h - i2c chip driver sysfs defines
- *
- *  Copyright (C) 2005 Yani Ioannou <yani.ioannou@gmail.com>
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-#ifndef _LINUX_I2C_SYSFS_H
-#define _LINUX_I2C_SYSFS_H
-
-struct sensor_device_attribute{
-	struct device_attribute dev_attr;
-	int index;
-};
-
-#define to_sensor_dev_attr(_dev_attr) \
-container_of(_dev_attr, struct sensor_device_attribute, dev_attr)
-
-#define SENSOR_DEVICE_ATTR(_name,_mode,_show,_store,_index)	\
-struct sensor_device_attribute sensor_dev_attr_##_name = {	\
-	.dev_attr=__ATTR(_name,_mode,_show,_store),		\
-	.index=_index,						\
-}
-
-#endif /* _LINUX_I2C_SYSFS_H */


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [lm-sensors] Re: More hardware monitoring drivers ported to the new
@ 2005-06-06 19:34     ` Jean Delvare
  0 siblings, 0 replies; 14+ messages in thread
From: Jean Delvare @ 2005-06-06 19:34 UTC (permalink / raw)
  To: Greg KH; +Cc: LM Sensors, LKML, Yani Ioannou

Hi Greg, all,

> > First, I don't much like the name of the new header file,
> > linux/i2c-sysfs.h. It isn't related with i2c at all! It's all about
> > sensors (or hardware monitoring if you prefer). I think the header
> > file should be named linux/hwmon-sysfs.h or something similar.
> 
> Sure, that would be fine.

OK, patch follows.

> > Second, is there a reason why the SENSOR_DEVICE_ATTR macro creates a
> > stucture named sensor_dev_attr_##_name rather than simply
> > dev_attr_##_name? As it seems unlikely that SENSOR_DEVICE_ATTR and
> > DEVICE_ATTR will both be called for the same file, going for the
> > short form shouldn't cause any problem. This would make the calling
> > code more readable IMHO.
> 
> Hm, I really don't care either way about this.

Let's not change it then.

---------------------------------------------------------

This patch renames the new linux/i2c-sysfs.h header file to
linux/hwmon-sysfs.h. This names seems to be more appropriate since this
file defines macros and structures not related to i2c but to hardware
monitoring drivers. The patch also updates the five hardware monitoring
driver which include that header file already.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

 drivers/i2c/chips/adm1026.c |    2 +-
 drivers/i2c/chips/it87.c    |    2 +-
 drivers/i2c/chips/lm63.c    |    2 +-
 drivers/i2c/chips/lm83.c    |    2 +-
 drivers/i2c/chips/lm90.c    |    2 +-
 include/linux/hwmon-sysfs.h |   37 +++++++++++++++++++++++++++++++++++++
 include/linux/i2c-sysfs.h   |   37 -------------------------------------
 7 files changed, 42 insertions(+), 42 deletions(-)

--- linux-2.6.12-rc5.orig/drivers/i2c/chips/adm1026.c	2005-06-05 10:53:57.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/adm1026.c	2005-06-06 19:23:32.000000000 +0200
@@ -29,8 +29,8 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
 #include <linux/i2c-vid.h>
+#include <linux/hwmon-sysfs.h>
 
 /* Addresses to scan */
 static unsigned short normal_i2c[] = { 0x2c, 0x2d, 0x2e, I2C_CLIENT_END };
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/it87.c	2005-06-05 11:51:22.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/it87.c	2005-06-06 19:23:14.000000000 +0200
@@ -37,8 +37,8 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
 #include <linux/i2c-vid.h>
+#include <linux/hwmon-sysfs.h>
 #include <asm/io.h>
 
 
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm63.c	2005-06-06 19:09:36.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm63.c	2005-06-06 19:23:01.000000000 +0200
@@ -43,7 +43,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
+#include <linux/hwmon-sysfs.h>
 
 /*
  * Addresses to scan
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm83.c	2005-06-06 19:09:36.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm83.c	2005-06-06 19:22:55.000000000 +0200
@@ -33,7 +33,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
+#include <linux/hwmon-sysfs.h>
 
 /*
  * Addresses to scan
--- linux-2.6.12-rc5.orig/drivers/i2c/chips/lm90.c	2005-06-05 19:35:05.000000000 +0200
+++ linux-2.6.12-rc5/drivers/i2c/chips/lm90.c	2005-06-06 19:22:48.000000000 +0200
@@ -76,7 +76,7 @@
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
 #include <linux/i2c-sensor.h>
-#include <linux/i2c-sysfs.h>
+#include <linux/hwmon-sysfs.h>
 
 /*
  * Addresses to scan
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.12-rc5/include/linux/hwmon-sysfs.h	2005-06-06 19:21:02.000000000 +0200
@@ -0,0 +1,37 @@
+/*
+ *  hwmon-sysfs.h - hardware monitoring chip driver sysfs defines
+ *
+ *  Copyright (C) 2005 Yani Ioannou <yani.ioannou@gmail.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+#ifndef _LINUX_HWMON_SYSFS_H
+#define _LINUX_HWMON_SYSFS_H
+
+struct sensor_device_attribute{
+	struct device_attribute dev_attr;
+	int index;
+};
+
+#define to_sensor_dev_attr(_dev_attr) \
+container_of(_dev_attr, struct sensor_device_attribute, dev_attr)
+
+#define SENSOR_DEVICE_ATTR(_name,_mode,_show,_store,_index)	\
+struct sensor_device_attribute sensor_dev_attr_##_name = {	\
+	.dev_attr=__ATTR(_name,_mode,_show,_store),		\
+	.index=_index,						\
+}
+
+#endif /* _LINUX_HWMON_SYSFS_H */
--- linux-2.6.12-rc5.orig/include/linux/i2c-sysfs.h	2005-06-05 10:53:57.000000000 +0200
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,37 +0,0 @@
-/*
- *  i2c-sysfs.h - i2c chip driver sysfs defines
- *
- *  Copyright (C) 2005 Yani Ioannou <yani.ioannou@gmail.com>
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- */
-#ifndef _LINUX_I2C_SYSFS_H
-#define _LINUX_I2C_SYSFS_H
-
-struct sensor_device_attribute{
-	struct device_attribute dev_attr;
-	int index;
-};
-
-#define to_sensor_dev_attr(_dev_attr) \
-container_of(_dev_attr, struct sensor_device_attribute, dev_attr)
-
-#define SENSOR_DEVICE_ATTR(_name,_mode,_show,_store,_index)	\
-struct sensor_device_attribute sensor_dev_attr_##_name = {	\
-	.dev_attr=__ATTR(_name,_mode,_show,_store),		\
-	.index=_index,						\
-}
-
-#endif /* _LINUX_I2C_SYSFS_H */


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2005-06-06 19:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-06-05 18:09 More hardware monitoring drivers ported to the new sysfs callbacks Jean Delvare
2005-06-05 20:08 ` [lm-sensors] More hardware monitoring drivers ported to the new Jean Delvare
2005-06-05 18:25 ` More hardware monitoring drivers ported to the new sysfs callbacks Yani Ioannou
2005-06-05 20:26   ` [lm-sensors] Re: More hardware monitoring drivers ported to the new Yani Ioannou
2005-06-05 18:32 ` [PATCH 2.6] I2C: (1/3) lm63 uses new sysfs callbacks Jean Delvare
2005-06-05 20:32   ` [lm-sensors] " Jean Delvare
2005-06-05 19:16 ` [PATCH 2.6] I2C: (2/3) lm83 " Jean Delvare
2005-06-05 21:16   ` [lm-sensors] " Jean Delvare
2005-06-05 19:27 ` [PATCH 2.6] I2C: (3/3) lm90 " Jean Delvare
2005-06-05 21:27   ` [lm-sensors] " Jean Delvare
2005-06-06  6:22 ` More hardware monitoring drivers ported to the " Greg KH
2005-06-06  9:17   ` [lm-sensors] Re: More hardware monitoring drivers ported to the new Greg KH
2005-06-06 17:34   ` More hardware monitoring drivers ported to the new sysfs callbacks Jean Delvare
2005-06-06 19:34     ` [lm-sensors] Re: More hardware monitoring drivers ported to the new 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.