Skip to contentSkip to conversation

de-macro-fy w83627hf.c

PATCH In review Jim Cromie 2 participants 4 messages Jul 8, 2007 – Jul 14, 2007 Hardware monitoring

lore
Share & tools
lore

Jim Cromie

author
#0 lore raw

PATCH 1/1 de-macro-fy w83627hf.c

Hi All, Mark,

w83627hf.c has lots of macros which generate repetetive sysfs callback functions.

Attached patch replaces most of them, and upgrades several DEVICE_ATTR declarations with SENSOR_DEVICE_ATTRs as needed to determine which the callbacks need to determine which sensor-number is being accessed.

Patched driver works on my W83627HF equipped AMD-Barton MOBO.

If you have one of the other parts supported by this board, please test this patch and let me know. (ie Ack/Nack this and subsequent revisions)

WRT Submission:

1 - the patch is getting big, so Id like to hear whether its acceptable in one big lump.

diffstat diff.hwmon-w83627hf-demacro-sysfs-callbacks
w83627hf.c | 516 ++++++++++++++++++++++++++++++-------------------------------

1 file changed, 254 insertions(+), 262 deletions(-)

If I should break it up, Id probably do it on VIN, FAN, PWM, TEMP boundaries, unless you have other thoughts.

2 - checkpatch complains about a few things - and fixing them would enlarge the patch, and thus may qualify for separation (or ignoring)

line over 80 characters
#27: FILE: drivers/hwmon/w83627hf.c:422:
+static ssize_t show_in_input(struct device *dev, struct device_attribute *devattr, char *buf)

IIUC, Linus thinks this check is somewhat nazi-ish (his word), and I recall that some people prefer to have the entire function-signature on a single line (grep friendly)

Macros with multiple statements should be enclosed in a do - while loop #76: FILE: drivers/hwmon/w83627hf.c:471:
+#define vin_decl(offset) \
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, \

Ive stuffed repetetive declarations and initializations into macros, and this warning is unavoidable for those uses.

drivers/hwmon/w83627hf.c +254 -262
diff -ruNp -X dontdiff -X exclude-diffs lx-22rc7-hwmon/drivers/hwmon/w83627hf.c lx-22rc7-hwmon-slim/drivers/hwmon/w83627hf.c
@@ -45,6 +45,7 @@
 #include <linux/jiffies.h>
 #include <linux/platform_device.h>
 #include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 #include <linux/hwmon-vid.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
@@ -347,75 +348,73 @@ static struct platform_driver w83627hf_d
 	.remove		= __devexit_p(w83627hf_remove),
 };
 
-/* following are the sysfs callback functions */
-#define show_in_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \
+static ssize_t show_in_input(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long)IN_FROM_REG(data->in[attr->index]));
+}
+static ssize_t show_in_min(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long)IN_FROM_REG(data->in_min[attr->index]));
+}
+static ssize_t show_in_max(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long)IN_FROM_REG(data->in_max[attr->index]));
 }
-show_in_reg(in)
-show_in_reg(in_min)
-show_in_reg(in_max)
 
-#define store_in_reg(REG, reg) \
-static ssize_t \
-store_in_##reg (struct device *dev, const char *buf, size_t count, int nr) \
-{ \
-	struct w83627hf_data *data = dev_get_drvdata(dev); \
-	u32 val; \
-	 \
-	val = simple_strtoul(buf, NULL, 10); \
-	 \
-	mutex_lock(&data->update_lock); \
-	data->in_##reg[nr] = IN_TO_REG(val); \
-	w83627hf_write_value(data, W83781D_REG_IN_##REG(nr), \
-			    data->in_##reg[nr]); \
-	 \
-	mutex_unlock(&data->update_lock); \
-	return count; \
+static ssize_t store_in_min(struct device *dev, struct device_attribute *devattr, const char *buf,
+			  size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	long val = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	data->in_min[attr->index] = IN_TO_REG(val);
+	w83627hf_write_value(data, W83781D_REG_IN_MIN(attr->index), data->in_min[attr->index]);
+
+	mutex_unlock(&data->update_lock);
+	return count;
 }
-store_in_reg(MIN, min)
-store_in_reg(MAX, max)
 
-#define sysfs_in_offset(offset) \
-static ssize_t \
-show_regs_in_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-        return show_in(dev, buf, offset); \
-} \
-static DEVICE_ATTR(in##offset##_input, S_IRUGO, show_regs_in_##offset, NULL);
+static ssize_t store_in_max(struct device *dev, struct device_attribute *devattr, const char *buf,
+			  size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	long val = simple_strtol(buf, NULL, 10);
 
-#define sysfs_in_reg_offset(reg, offset) \
-static ssize_t show_regs_in_##reg##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_in_##reg (dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_in_##reg##offset (struct device *dev, struct device_attribute *attr, \
-			    const char *buf, size_t count) \
-{ \
-	return store_in_##reg (dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(in##offset##_##reg, S_IRUGO| S_IWUSR, \
-		  show_regs_in_##reg##offset, store_regs_in_##reg##offset);
+	mutex_lock(&data->update_lock);
+	data->in_max[attr->index] = IN_TO_REG(val);
+	w83627hf_write_value(data, W83781D_REG_IN_MAX(attr->index), data->in_max[attr->index]);
+	mutex_unlock(&data->update_lock);
+	return count;
+}
 
-#define sysfs_in_offsets(offset) \
-sysfs_in_offset(offset) \
-sysfs_in_reg_offset(min, offset) \
-sysfs_in_reg_offset(max, offset)
-
-sysfs_in_offsets(1);
-sysfs_in_offsets(2);
-sysfs_in_offsets(3);
-sysfs_in_offsets(4);
-sysfs_in_offsets(5);
-sysfs_in_offsets(6);
-sysfs_in_offsets(7);
-sysfs_in_offsets(8);
+#define vin_decl(offset)	\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO, 		\
+			  show_in_input, NULL, offset);		\
+static SENSOR_DEVICE_ATTR(in##offset##_min, S_IRUGO | S_IWUSR,	\
+			  show_in_min, store_in_min, offset);	\
+static SENSOR_DEVICE_ATTR(in##offset##_max, S_IRUGO | S_IWUSR,	\
+			  show_in_max, store_in_max, offset);
+
+vin_decl(1);
+vin_decl(2);
+vin_decl(3);
+vin_decl(4);
+vin_decl(5);
+vin_decl(6);
+vin_decl(7);
+vin_decl(8);
 
 /* use a different set of functions for in0 */
-static ssize_t show_in_0(struct w83627hf_data *data, char *buf, u8 reg)
+static ssize_t _show_in_0(struct w83627hf_data *data, char *buf, u8 reg)
 {
 	long in0;
 
@@ -432,26 +431,26 @@ static ssize_t show_in_0(struct w83627hf
 	return sprintf(buf,"%ld\n", in0);
 }
 
-static ssize_t show_regs_in_0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_0(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in[0]);
+	return _show_in_0(data, buf, data->in[0]);
 }
 
-static ssize_t show_regs_in_min0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_min0(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in_min[0]);
+	return _show_in_0(data, buf, data->in_min[0]);
 }
 
-static ssize_t show_regs_in_max0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_max0(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in_max[0]);
+	return _show_in_0(data, buf, data->in_max[0]);
 }
 
-static ssize_t store_regs_in_min0(struct device *dev, struct device_attribute *attr,
-	const char *buf, size_t count)
+static ssize_t store_in_min0(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	u32 val;
@@ -477,8 +476,8 @@ static ssize_t store_regs_in_min0(struct
 	return count;
 }
 
-static ssize_t store_regs_in_max0(struct device *dev, struct device_attribute *attr,
-	const char *buf, size_t count)
+static ssize_t store_in_max0(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	u32 val;
@@ -504,70 +503,58 @@ static ssize_t store_regs_in_max0(struct
 	return count;
 }
 
-static DEVICE_ATTR(in0_input, S_IRUGO, show_regs_in_0, NULL);
+static DEVICE_ATTR(in0_input, S_IRUGO, show_in_0, NULL);
 static DEVICE_ATTR(in0_min, S_IRUGO | S_IWUSR,
-	show_regs_in_min0, store_regs_in_min0);
+	show_in_min0, store_in_min0);
 static DEVICE_ATTR(in0_max, S_IRUGO | S_IWUSR,
-	show_regs_in_max0, store_regs_in_max0);
+	show_in_max0, store_in_max0);
 
-#define show_fan_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	return sprintf(buf,"%ld\n", \
-		FAN_FROM_REG(data->reg[nr-1], \
-			    (long)DIV_FROM_REG(data->fan_div[nr-1]))); \
+static ssize_t show_fan_input(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long)FAN_FROM_REG(data->fan[attr->index-1],
+					(long)DIV_FROM_REG(data->fan_div[attr->index-1])));
+}
+static ssize_t show_fan_min(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long)FAN_FROM_REG(data->fan_min[attr->index-1],
+					(long)DIV_FROM_REG(data->fan_div[attr->index-1])));
 }
-show_fan_reg(fan);
-show_fan_reg(fan_min);
-
 static ssize_t
-store_fan_min(struct device *dev, const char *buf, size_t count, int nr)
+store_fan_min (struct device *dev, struct device_attribute *devattr,
+	       const char *buf, size_t count)
 {
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	u32 val;
 
 	val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
-	data->fan_min[nr - 1] =
-	    FAN_TO_REG(val, DIV_FROM_REG(data->fan_div[nr - 1]));
-	w83627hf_write_value(data, W83781D_REG_FAN_MIN(nr),
-			    data->fan_min[nr - 1]);
+	data->fan_min[attr->index - 1] =
+	    FAN_TO_REG(val, DIV_FROM_REG(data->fan_div[attr->index - 1]));
+	w83627hf_write_value(data, W83781D_REG_FAN_MIN(attr->index),
+			    data->fan_min[attr->index - 1]);
 
 	mutex_unlock(&data->update_lock);
 	return count;
 }
 
-#define sysfs_fan_offset(offset) \
-static ssize_t show_regs_fan_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan(dev, buf, offset); \
-} \
-static DEVICE_ATTR(fan##offset##_input, S_IRUGO, show_regs_fan_##offset, NULL);
-
-#define sysfs_fan_min_offset(offset) \
-static ssize_t show_regs_fan_min##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan_min(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_fan_min##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_fan_min(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(fan##offset##_min, S_IRUGO | S_IWUSR, \
-		  show_regs_fan_min##offset, store_regs_fan_min##offset);
-
-sysfs_fan_offset(1);
-sysfs_fan_min_offset(1);
-sysfs_fan_offset(2);
-sysfs_fan_min_offset(2);
-sysfs_fan_offset(3);
-sysfs_fan_min_offset(3);
+#define sysfs_fan_decl(offset)	\
+static SENSOR_DEVICE_ATTR(fan##offset##_input, S_IRUGO,			\
+			  show_fan_input, NULL, offset);		\
+static SENSOR_DEVICE_ATTR(fan##offset##_min, S_IRUGO | S_IWUSR,		\
+			  show_fan_min, store_fan_min, offset);
+
+sysfs_fan_decl(1);
+sysfs_fan_decl(2);
+sysfs_fan_decl(3);
 
 #define show_temp_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
+static ssize_t _show_##reg (struct device *dev, char *buf, int nr) \
 { \
 	struct w83627hf_data *data = w83627hf_update_device(dev); \
 	if (nr >= 2) {	/* TEMP2 and TEMP3 */ \
@@ -583,7 +570,7 @@ show_temp_reg(temp_max_hyst);
 
 #define store_temp_reg(REG, reg) \
 static ssize_t \
-store_temp_##reg (struct device *dev, const char *buf, size_t count, int nr) \
+_store_temp_##reg (struct device *dev, const char *buf, size_t count, int nr) \
 { \
 	struct w83627hf_data *data = dev_get_drvdata(dev); \
 	u32 val; \
@@ -608,36 +595,52 @@ store_temp_##reg (struct device *dev, co
 store_temp_reg(OVER, max);
 store_temp_reg(HYST, max_hyst);
 
-#define sysfs_temp_offset(offset) \
-static ssize_t \
-show_regs_temp_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_temp(dev, buf, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_input, S_IRUGO, show_regs_temp_##offset, NULL);
+static ssize_t
+show_temp (struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return _show_temp(dev, buf, attr->index);
+}
 
-#define sysfs_temp_reg_offset(reg, offset) \
-static ssize_t show_regs_temp_##reg##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_temp_##reg (dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_temp_##reg##offset (struct device *dev, struct device_attribute *attr, \
-			      const char *buf, size_t count) \
-{ \
-	return store_temp_##reg (dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_##reg, S_IRUGO| S_IWUSR, \
-		  show_regs_temp_##reg##offset, store_regs_temp_##reg##offset);
+static ssize_t show_temp_max (struct device *dev,
+				   struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return _show_temp_max (dev, buf, attr->index);
+}
+static ssize_t show_temp_max_hyst (struct device *dev,
+					struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return _show_temp_max_hyst (dev, buf, attr->index);
+}
+
+static ssize_t
+store_temp_max (struct device *dev, struct device_attribute *devattr,
+		     const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return _store_temp_max (dev, buf, count, attr->index);
+}
+static ssize_t
+store_temp_max_hyst (struct device *dev, struct device_attribute *devattr,
+			  const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return _store_temp_max_hyst (dev, buf, count, attr->index);
+}
 
-#define sysfs_temp_offsets(offset) \
-sysfs_temp_offset(offset) \
-sysfs_temp_reg_offset(max, offset) \
-sysfs_temp_reg_offset(max_hyst, offset)
-
-sysfs_temp_offsets(1);
-sysfs_temp_offsets(2);
-sysfs_temp_offsets(3);
+#define temp_decl(offset) \
+static SENSOR_DEVICE_ATTR(temp##offset##_input, S_IRUGO, 		\
+			  show_temp, NULL, offset);			\
+static SENSOR_DEVICE_ATTR(temp##offset##_max, S_IRUGO | S_IWUSR, 	\
+			  show_temp_max, store_temp_max, offset);	\
+static SENSOR_DEVICE_ATTR(temp##offset##_max_hyst, S_IRUGO | S_IWUSR,	\
+			  show_temp_max_hyst, store_temp_max_hyst, offset);
+
+temp_decl(1);
+temp_decl(2);
+temp_decl(3);
 
 static ssize_t
 show_vid_reg(struct device *dev, struct device_attribute *attr, char *buf)
@@ -654,7 +657,8 @@ show_vrm_reg(struct device *dev, struct 
 	return sprintf(buf, "%ld\n", (long) data->vrm);
 }
 static ssize_t
-store_vrm_reg(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+store_vrm_reg(struct device *dev, struct device_attribute *attr,
+	      const char *buf, size_t count)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	u32 val;
@@ -780,23 +784,25 @@ store_fan_div_reg(struct device *dev, co
 	return count;
 }
 
-#define sysfs_fan_div(offset) \
-static ssize_t show_regs_fan_div_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan_div_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_fan_div_##offset (struct device *dev, struct device_attribute *attr, \
-			    const char *buf, size_t count) \
-{ \
-	return store_fan_div_reg(dev, buf, count, offset - 1); \
-} \
-static DEVICE_ATTR(fan##offset##_div, S_IRUGO | S_IWUSR, \
-		  show_regs_fan_div_##offset, store_regs_fan_div_##offset);
+static ssize_t show_fan_div (struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return show_fan_div_reg(dev, buf, attr->index);
+}
+static ssize_t
+store_fan_div (struct device *dev, struct device_attribute *devattr,
+		    const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return store_fan_div_reg(dev, buf, count, attr->index - 1);
+}
 
-sysfs_fan_div(1);
-sysfs_fan_div(2);
-sysfs_fan_div(3);
+static SENSOR_DEVICE_ATTR(fan1_div, S_IRUGO | S_IWUSR, \
+		   show_fan_div, store_fan_div, 1);
+static SENSOR_DEVICE_ATTR(fan2_div, S_IRUGO | S_IWUSR, \
+		   show_fan_div, store_fan_div, 2);
+static SENSOR_DEVICE_ATTR(fan3_div, S_IRUGO | S_IWUSR, \
+		   show_fan_div, store_fan_div, 3);
 
 static ssize_t
 show_pwm_reg(struct device *dev, char *buf, int nr)
@@ -834,22 +840,24 @@ store_pwm_reg(struct device *dev, const 
 	return count;
 }
 
-#define sysfs_pwm(offset) \
-static ssize_t show_regs_pwm_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_pwm_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_pwm_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_pwm_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR, \
-		  show_regs_pwm_##offset, store_regs_pwm_##offset);
+static ssize_t show_pwm (struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return show_pwm_reg(dev, buf, attr->index);
+}
+static ssize_t
+store_pwm (struct device *dev, struct device_attribute *devattr, const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return store_pwm_reg(dev, buf, count, attr->index);
+}
 
-sysfs_pwm(1);
-sysfs_pwm(2);
-sysfs_pwm(3);
+static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, \
+			  show_pwm, store_pwm, 1);
+static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, \
+			  show_pwm, store_pwm, 2);
+static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, \
+			  show_pwm, store_pwm, 3);
 
 static ssize_t
 show_sensor_reg(struct device *dev, char *buf, int nr)
@@ -904,22 +912,25 @@ store_sensor_reg(struct device *dev, con
 	return count;
 }
 
-#define sysfs_sensor(offset) \
-static ssize_t show_regs_sensor_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-    return show_sensor_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_sensor_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-    return store_sensor_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_type, S_IRUGO | S_IWUSR, \
-		  show_regs_sensor_##offset, store_regs_sensor_##offset);
+static ssize_t show_sensor (struct device *dev, struct device_attribute *devattr, char *buf)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return show_sensor_reg(dev, buf, attr->index);
+}
+static ssize_t
+store_sensor (struct device *dev, struct device_attribute *devattr, const char *buf, size_t count)
+{
+	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+	return store_sensor_reg(dev, buf, count, attr->index);
+}
+
+#define sensor_decl(offset) \
+static SENSOR_DEVICE_ATTR(temp##offset##_type, S_IRUGO | S_IWUSR, \
+			  show_sensor, store_sensor, offset);
 
-sysfs_sensor(1);
-sysfs_sensor(2);
-sysfs_sensor(3);
+sensor_decl(1);
+sensor_decl(2);
+sensor_decl(3);
 
 static ssize_t show_name(struct device *dev, struct device_attribute
 			 *devattr, char *buf)
@@ -1004,48 +1015,43 @@ static int __init w83627hf_find(int sioa
 	return err;
 }
 
+#define VIN_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_in##X##_input.dev_attr.attr,	    \
+	&sensor_dev_attr_in##X##_min.dev_attr.attr,	    \
+	&sensor_dev_attr_in##X##_max.dev_attr.attr
+
+#define FAN_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_fan##X##_input.dev_attr.attr,	    \
+	&sensor_dev_attr_fan##X##_min.dev_attr.attr,	    \
+	&sensor_dev_attr_fan##X##_div.dev_attr.attr
+
+#define TEMP_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_temp##X##_input.dev_attr.attr,		\
+	&sensor_dev_attr_temp##X##_max.dev_attr.attr,		\
+	&sensor_dev_attr_temp##X##_max_hyst.dev_attr.attr,	\
+	&sensor_dev_attr_temp##X##_type.dev_attr.attr
+
 static struct attribute *w83627hf_attributes[] = {
 	&dev_attr_in0_input.attr,
 	&dev_attr_in0_min.attr,
 	&dev_attr_in0_max.attr,
-	&dev_attr_in2_input.attr,
-	&dev_attr_in2_min.attr,
-	&dev_attr_in2_max.attr,
-	&dev_attr_in3_input.attr,
-	&dev_attr_in3_min.attr,
-	&dev_attr_in3_max.attr,
-	&dev_attr_in4_input.attr,
-	&dev_attr_in4_min.attr,
-	&dev_attr_in4_max.attr,
-	&dev_attr_in7_input.attr,
-	&dev_attr_in7_min.attr,
-	&dev_attr_in7_max.attr,
-	&dev_attr_in8_input.attr,
-	&dev_attr_in8_min.attr,
-	&dev_attr_in8_max.attr,
-
-	&dev_attr_fan1_input.attr,
-	&dev_attr_fan1_min.attr,
-	&dev_attr_fan1_div.attr,
-	&dev_attr_fan2_input.attr,
-	&dev_attr_fan2_min.attr,
-	&dev_attr_fan2_div.attr,
-
-	&dev_attr_temp1_input.attr,
-	&dev_attr_temp1_max.attr,
-	&dev_attr_temp1_max_hyst.attr,
-	&dev_attr_temp1_type.attr,
-	&dev_attr_temp2_input.attr,
-	&dev_attr_temp2_max.attr,
-	&dev_attr_temp2_max_hyst.attr,
-	&dev_attr_temp2_type.attr,
+
+	VIN_UNIT_ATTRS(2),
+	VIN_UNIT_ATTRS(3),
+	VIN_UNIT_ATTRS(4),
+	VIN_UNIT_ATTRS(7),
+	VIN_UNIT_ATTRS(8),
+	FAN_UNIT_ATTRS(1),
+	FAN_UNIT_ATTRS(2),
+	TEMP_UNIT_ATTRS(1),
+	TEMP_UNIT_ATTRS(2),
 
 	&dev_attr_alarms.attr,
 	&dev_attr_beep_enable.attr,
 	&dev_attr_beep_mask.attr,
 
-	&dev_attr_pwm1.attr,
-	&dev_attr_pwm2.attr,
+	&sensor_dev_attr_pwm1.dev_attr.attr,
+	&sensor_dev_attr_pwm2.dev_attr.attr,
 
 	&dev_attr_name.attr,
 	NULL
@@ -1056,27 +1062,13 @@ static const struct attribute_group w836
 };
 
 static struct attribute *w83627hf_attributes_opt[] = {
-	&dev_attr_in1_input.attr,
-	&dev_attr_in1_min.attr,
-	&dev_attr_in1_max.attr,
-	&dev_attr_in5_input.attr,
-	&dev_attr_in5_min.attr,
-	&dev_attr_in5_max.attr,
-	&dev_attr_in6_input.attr,
-	&dev_attr_in6_min.attr,
-	&dev_attr_in6_max.attr,
-
-	&dev_attr_fan3_input.attr,
-	&dev_attr_fan3_min.attr,
-	&dev_attr_fan3_div.attr,
-
-	&dev_attr_temp3_input.attr,
-	&dev_attr_temp3_max.attr,
-	&dev_attr_temp3_max_hyst.attr,
-	&dev_attr_temp3_type.attr,
-
-	&dev_attr_pwm3.attr,
+	VIN_UNIT_ATTRS(1),
+	VIN_UNIT_ATTRS(5),
+	VIN_UNIT_ATTRS(6),
+	FAN_UNIT_ATTRS(3),
+	TEMP_UNIT_ATTRS(3),
 
+	&sensor_dev_attr_pwm3.dev_attr.attr,
 	NULL
 };
 
@@ -1134,25 +1126,25 @@ static int __devinit w83627hf_probe(stru
 
 	/* Register chip-specific device attributes */
 	if (data->type == w83627hf || data->type == w83697hf)
-		if ((err = device_create_file(dev, &dev_attr_in5_input))
-		 || (err = device_create_file(dev, &dev_attr_in5_min))
-		 || (err = device_create_file(dev, &dev_attr_in5_max))
-		 || (err = device_create_file(dev, &dev_attr_in6_input))
-		 || (err = device_create_file(dev, &dev_attr_in6_min))
-		 || (err = device_create_file(dev, &dev_attr_in6_max)))
+		if ((err = device_create_file(dev, &sensor_dev_attr_in5_input.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_in5_min.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_in5_max.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_in6_input.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_in6_min.dev_attr))
+		    || (err = device_create_file(dev, &sensor_dev_attr_in6_max.dev_attr)))
 			goto ERROR4;
 
 	if (data->type != w83697hf)
-		if ((err = device_create_file(dev, &dev_attr_in1_input))
-		 || (err = device_create_file(dev, &dev_attr_in1_min))
-		 || (err = device_create_file(dev, &dev_attr_in1_max))
-		 || (err = device_create_file(dev, &dev_attr_fan3_input))
-		 || (err = device_create_file(dev, &dev_attr_fan3_min))
-		 || (err = device_create_file(dev, &dev_attr_fan3_div))
-		 || (err = device_create_file(dev, &dev_attr_temp3_input))
-		 || (err = device_create_file(dev, &dev_attr_temp3_max))
-		 || (err = device_create_file(dev, &dev_attr_temp3_max_hyst))
-		 || (err = device_create_file(dev, &dev_attr_temp3_type)))
+		if ((err = device_create_file(dev, &sensor_dev_attr_in1_input.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_in1_min.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_in1_max.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_fan3_input.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_fan3_min.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_fan3_div.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_temp3_input.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_temp3_max.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_temp3_max_hyst.dev_attr))
+		 || (err = device_create_file(dev, &sensor_dev_attr_temp3_type.dev_attr)))
 			goto ERROR4;
 
 	if (data->type != w83697hf && data->vid != 0xff) {
@@ -1166,7 +1158,7 @@ static int __devinit w83627hf_probe(stru
 
 	if (data->type == w83627thf || data->type == w83637hf
 	 || data->type == w83687thf)
-		if ((err = device_create_file(dev, &dev_attr_pwm3)))
+		if ((err = device_create_file(dev, &sensor_dev_attr_pwm3.dev_attr)))
 			goto ERROR4;
 
 	data->class_dev = hwmon_device_register(dev);

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#1 lore raw

Hi Jim,

Quoted 8 lines · Jim Cromie

w83627hf.c has lots of macros which generate repetetive sysfs callback functions.

Attached patch replaces most of them, and upgrades several DEVICE_ATTR declarations with SENSOR_DEVICE_ATTRs as needed to determine which the callbacks need to determine which sensor-number is being accessed.

Patched driver works on my W83627HF equipped AMD-Barton MOBO.

Great, thanks for working on this, this is very needed.

Quoted 3 lines

If you have one of the other parts supported by this board, please test this patch
and let me know. (ie Ack/Nack this and subsequent revisions)

I have a W83627THF and will be testing your patch (but see below.)

Quoted 5 lines

WRT Submission:

1 - the patch is getting big, so Id like to hear whether its acceptable in one big lump.

Quoted 7 lines

diffstat diff.hwmon-w83627hf-demacro-sysfs-callbacks

 w83627hf.c |  516 
++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 254 insertions(+), 262 deletions(-)

If I should break it up, Id probably do it on VIN, FAN, PWM, TEMP boundaries, unless you have other thoughts.

The patch is big but it does only one thing, so it's perfectly OK. Please don't waste your time splitting it up.

OTOH, I have a problem applying your patch, because it's based on Linus' tree rather than Mark M. Hoffman's hwmon testing branch. Mark's branch has 3 additional patches affecting the w83627hf driver, and at least one of them conflicts with your patch. You can see these patches here:

http://lm-sensors.org/kernel?p=kernel/mhoffman/hwmon-2.6.git;a=history;f=drivers/hwmon/w83627hf.c;hb=testing

So I would like you to rebase your patch on this branch, otherwise Mark won't be able to apply it anyway.

Quoted 9 lines

2 - checkpatch complains about a few things - and fixing them would enlarge the patch,
and thus may qualify for separation (or ignoring)

line over 80 characters
#27: FILE: drivers/hwmon/w83627hf.c:422:
+static ssize_t show_in_input(struct device *dev, struct device_attribute *devattr, char *buf)

Quoted 4 lines

IIUC, Linus thinks this check is somewhat nazi-ish (his word), and I recall that some people
prefer to have the entire function-signature on a single line (grep friendly)

Documentation/CodingStyle says: "The limit on the length of lines is 80 columns and this is a hard limit." It could hardly be clearer, and if Linus wasn't happy with this, I guess that this would have been updated by now. So, yes, such long lines need to be folded. The line you quote above is not in the original driver, BTW, it's added by your patch so you should fix it in your patch. If there are other occurrences that are not introduced by your patch, then they can be fixed in a later patch, if you care.

Quoted 7 lines

Macros with multiple statements should be enclosed in a do - while loop #76: FILE: drivers/hwmon/w83627hf.c:471:

drivers/hwmon/w83627hf.c +2 -0
+#define vin_decl(offset)       \
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO,                 \

Ive stuffed repetetive declarations and initializations into macros, and this warning is unavoidable for those uses.

This isn't really a problem as long as these macros are only used in the source file where they are defined. So I'd say leave them as is, it's OK.

Thanks,

Signature · 6 lines
Jean Delvare

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
author
#2 lore raw
In reply to Jim Cromie

Thank you Krzysztof for your review.

This 2nd rev does:

- rebases on Marks GIT tree (thought I had, sorry)
- fixes all 80-column probs in last, plus a few others
- adopts new fashion int nr = to_sensor_dev_attr(devattr)->index;
- folds many one-caller functions into the caller
- converts remaining fn-generating macros to plain old functions

WRT types and conversions in the *_FROM/TO_REG macros, thanks for the detailed review, tracing thru the conversions is time consuming, and feels error prone..

- I dropped the (long) casts in some cases,
- and pushed them down into the macros in others
- Im disappointed that sparse/C=1 didnt identify these things

Im reluctant to do more here (Do No Harm, leave well enough alone). It seems that a full review, and conversion to static inline functions is warranted, and probably should be done separately.

<update>

Ive now seen Jean's comments, and have manually (I hope fully) backed out the three 'types and conversions' fixes above, reverting to original form. I have a set of interim diffs, so I *could* go back and reconstruct things, but I would *really* like to avoid doing so; its tedious and error prone.

FWIW - the patch results in a smaller object.

14627 2464 36 17127 42e7
lx-git-hwmon/drivers/hwmon/w83627hf.ko
12895 2676 36 15607 3cf7
lx-git-hwmon-demacro/drivers/hwmon/w83627hf.ko

---

Convert hwmon/w83627hf to use dynamic sysfs callbacks, replacing the function-generating macros.

[jimc@groucho lx]$ diffstat diff.hwmon-w83627hf-demacro-sysfs-callbacks w83627hf.c | 887 ++++++++++++++++++++++++++++++-------------------------------

1 file changed, 449 insertions(+), 438 deletions(-)

PS: attached file contains this same intro, hth.

Thank you Krzysztof for your review.

This 2nd rev does:

- rebases on Marks GIT tree (thought I had, sorry)
- fixes all 80-column probs in last, plus a few others
- adopts new fashion int nr = to_sensor_dev_attr(devattr)->index;
- folds many one-caller functions into the caller
- converts remaining fn-generating macros to plain old functions

WRT types and conversions in the *_FROM/TO_REG macros, thanks for the detailed review, tracing thru the conversions is time consuming, and feels error prone..

- I dropped the (long) casts in some cases,
- and pushed them down into the macros in others
- Im disappointed that sparse/C=1 didnt identify these things

Im reluctant to do more here (Do No Harm, leave well enough alone). It seems that a full review, and conversion to static inline functions is warranted, and probably should be done separately.

<update>

Ive now seen Jean's comments, and have manually (I hope fully) backed out the three 'types and conversions' fixes above, reverting to original form. I have a set of interim diffs, so I *could* go back and reconstruct things, but I would *really* like to avoid doing so; its tedious and error prone.

FWIW - the patch results in a smaller object.

  14627    2464      36   17127    42e7 lx-git-hwmon/drivers/hwmon/w83627hf.ko
  12895    2676      36   15607    3cf7 lx-git-hwmon-demacro/drivers/hwmon/w83627hf.ko

---

Convert hwmon/w83627hf to use dynamic sysfs callbacks, replacing the function-generating macros.

[jimc@groucho lx]$ diffstat diff.hwmon-w83627hf-demacro-sysfs-callbacks

 w83627hf.c |  887 ++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 449 insertions(+), 438 deletions(-)
drivers/hwmon/w83627hf.c +449 -438
diff -ruNp -X dontdiff -X exclude-diffs lx-git-hwmon/drivers/hwmon/w83627hf.c lx-git-hwmon-demacro/drivers/hwmon/w83627hf.c
@@ -45,6 +45,7 @@
 #include <linux/jiffies.h>
 #include <linux/platform_device.h>
 #include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 #include <linux/hwmon-vid.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
@@ -403,75 +404,79 @@ static struct platform_driver w83627hf_d
 	.remove		= __devexit_p(w83627hf_remove),
 };
 
-/* following are the sysfs callback functions */
-#define show_in_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \
-}
-show_in_reg(in)
-show_in_reg(in_min)
-show_in_reg(in_max)
-
-#define store_in_reg(REG, reg) \
-static ssize_t \
-store_in_##reg (struct device *dev, const char *buf, size_t count, int nr) \
-{ \
-	struct w83627hf_data *data = dev_get_drvdata(dev); \
-	u32 val; \
-	 \
-	val = simple_strtoul(buf, NULL, 10); \
-	 \
-	mutex_lock(&data->update_lock); \
-	data->in_##reg[nr] = IN_TO_REG(val); \
-	w83627hf_write_value(data, W83781D_REG_IN_##REG(nr), \
-			    data->in_##reg[nr]); \
-	 \
-	mutex_unlock(&data->update_lock); \
-	return count; \
-}
-store_in_reg(MIN, min)
-store_in_reg(MAX, max)
-
-#define sysfs_in_offset(offset) \
-static ssize_t \
-show_regs_in_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-        return show_in(dev, buf, offset); \
-} \
-static DEVICE_ATTR(in##offset##_input, S_IRUGO, show_regs_in_##offset, NULL);
-
-#define sysfs_in_reg_offset(reg, offset) \
-static ssize_t show_regs_in_##reg##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_in_##reg (dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_in_##reg##offset (struct device *dev, struct device_attribute *attr, \
-			    const char *buf, size_t count) \
-{ \
-	return store_in_##reg (dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(in##offset##_##reg, S_IRUGO| S_IWUSR, \
-		  show_regs_in_##reg##offset, store_regs_in_##reg##offset);
-
-#define sysfs_in_offsets(offset) \
-sysfs_in_offset(offset) \
-sysfs_in_reg_offset(min, offset) \
-sysfs_in_reg_offset(max, offset)
-
-sysfs_in_offsets(1);
-sysfs_in_offsets(2);
-sysfs_in_offsets(3);
-sysfs_in_offsets(4);
-sysfs_in_offsets(5);
-sysfs_in_offsets(6);
-sysfs_in_offsets(7);
-sysfs_in_offsets(8);
+static ssize_t show_in_input(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long) IN_FROM_REG(data->in[nr]));
+}
+static ssize_t show_in_min(struct device *dev,
+			   struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long) IN_FROM_REG(data->in_min[nr]));
+}
+static ssize_t show_in_max(struct device *dev,
+			   struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long) IN_FROM_REG(data->in_max[nr]));
+}
+
+static ssize_t store_in_min(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	long val = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	data->in_min[nr] = IN_TO_REG(val);
+	w83627hf_write_value(data, W83781D_REG_IN_MIN(nr), data->in_min[nr]);
+
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+static ssize_t store_in_max(struct device *dev,
+			    struct device_attribute *devattr, const char *buf,
+			    size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	u32 val = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+	data->in_max[nr] = IN_TO_REG(val);
+	w83627hf_write_value(data, W83781D_REG_IN_MAX(nr),
+			     data->in_max[nr]);
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define vin_decl(offset)	\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO,		\
+			  show_in_input, NULL, offset);		\
+static SENSOR_DEVICE_ATTR(in##offset##_min, S_IRUGO | S_IWUSR,	\
+			  show_in_min, store_in_min, offset);	\
+static SENSOR_DEVICE_ATTR(in##offset##_max, S_IRUGO | S_IWUSR,	\
+			  show_in_max, store_in_max, offset);
+
+vin_decl(1);
+vin_decl(2);
+vin_decl(3);
+vin_decl(4);
+vin_decl(5);
+vin_decl(6);
+vin_decl(7);
+vin_decl(8);
 
 /* use a different set of functions for in0 */
-static ssize_t show_in_0(struct w83627hf_data *data, char *buf, u8 reg)
+static ssize_t _show_in_0(struct w83627hf_data *data, char *buf, u8 reg)
 {
 	long in0;
 
@@ -488,31 +493,33 @@ static ssize_t show_in_0(struct w83627hf
 	return sprintf(buf,"%ld\n", in0);
 }
 
-static ssize_t show_regs_in_0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_0(struct device *dev,
+			 struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in[0]);
+	return _show_in_0(data, buf, data->in[0]);
 }
 
-static ssize_t show_regs_in_min0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_min0(struct device *dev,
+			    struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in_min[0]);
+	return _show_in_0(data, buf, data->in_min[0]);
 }
 
-static ssize_t show_regs_in_max0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_max0(struct device *dev,
+			    struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in_max[0]);
+	return _show_in_0(data, buf, data->in_max[0]);
 }
 
-static ssize_t store_regs_in_min0(struct device *dev, struct device_attribute *attr,
-	const char *buf, size_t count)
+static ssize_t store_in_min0(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t count)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
 	
@@ -533,13 +540,12 @@ static ssize_t store_regs_in_min0(struct
 	return count;
 }
 
-static ssize_t store_regs_in_max0(struct device *dev, struct device_attribute *attr,
-	const char *buf, size_t count)
+static ssize_t store_in_max0(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t count)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
 
@@ -560,192 +566,195 @@ static ssize_t store_regs_in_max0(struct
 	return count;
 }
 
-static DEVICE_ATTR(in0_input, S_IRUGO, show_regs_in_0, NULL);
+static DEVICE_ATTR(in0_input, S_IRUGO, show_in_0, NULL);
 static DEVICE_ATTR(in0_min, S_IRUGO | S_IWUSR,
-	show_regs_in_min0, store_regs_in_min0);
+		   show_in_min0, store_in_min0);
 static DEVICE_ATTR(in0_max, S_IRUGO | S_IWUSR,
-	show_regs_in_max0, store_regs_in_max0);
+		   show_in_max0, store_in_max0);
 
-#define show_fan_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	return sprintf(buf,"%ld\n", \
-		FAN_FROM_REG(data->reg[nr-1], \
-			    (long)DIV_FROM_REG(data->fan_div[nr-1]))); \
+static ssize_t show_fan_input(struct device *dev,
+			      struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", FAN_FROM_REG(data->fan[nr],
+				(long) DIV_FROM_REG(data->fan_div[nr])));
 }
-show_fan_reg(fan);
-show_fan_reg(fan_min);
 
-static ssize_t
-store_fan_min(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t show_fan_min(struct device *dev,
+			    struct device_attribute *devattr, char *buf)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", FAN_FROM_REG(data->fan_min[nr],
+				(long) DIV_FROM_REG(data->fan_div[nr])));
+}
+
+static ssize_t store_fan_min(struct device *dev,
+			     struct device_attribute *devattr,
+			     const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
+	u32 val = simple_strtoul(buf, NULL, 10);
 
-	val = simple_strtoul(buf, NULL, 10);
+	mutex_lock(&data->update_lock);
+	data->fan_min[nr] = FAN_TO_REG(val, DIV_FROM_REG(data->fan_div[nr]));
+	w83627hf_write_value(data, W83781D_REG_FAN_MIN(nr+1),
+			    data->fan_min[nr]);
+
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define fan_decl(offset)	\
+static SENSOR_DEVICE_ATTR(fan##offset##_input, S_IRUGO,			\
+			  show_fan_input, NULL, offset - 1);		\
+static SENSOR_DEVICE_ATTR(fan##offset##_min, S_IRUGO | S_IWUSR,		\
+			  show_fan_min, store_fan_min, offset - 1);
+
+fan_decl(1);
+fan_decl(2);
+fan_decl(3);
+
+static ssize_t show_temp(struct device *dev,
+			 struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		return sprintf(buf, "%ld\n",
+			(long)LM75_TEMP_FROM_REG(data->temp_add[nr-2]));
+	} else {	/* TEMP1 */
+		return sprintf(buf, "%ld\n", (long) TEMP_FROM_REG(data->temp));
+	}
+}
+static ssize_t show_temp_max(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		return sprintf(buf, "%ld\n",
+			(long)LM75_TEMP_FROM_REG(data->temp_max_add[nr-2]));
+	} else {	/* TEMP1 */
+		return sprintf(buf, "%ld\n",
+			       (long) TEMP_FROM_REG(data->temp_max));
+	}
+}
+static ssize_t show_temp_max_hyst(struct device *dev,
+				  struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		return sprintf(buf, "%ld\n", (long) 
+			LM75_TEMP_FROM_REG(data->temp_max_hyst_add[nr-2]));
+	} else {	/* TEMP1 */
+		return sprintf(buf, "%ld\n",
+			       (long) TEMP_FROM_REG(data->temp_max_hyst));
+	}
+}
+
+static ssize_t store_temp_max(struct device *dev,
+			      struct device_attribute *devattr,
+			      const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	u32 val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
-	data->fan_min[nr - 1] =
-	    FAN_TO_REG(val, DIV_FROM_REG(data->fan_div[nr - 1]));
-	w83627hf_write_value(data, W83781D_REG_FAN_MIN(nr),
-			    data->fan_min[nr - 1]);
+
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		data->temp_max_add[nr-2] = LM75_TEMP_TO_REG(val);
+		w83627hf_write_value(data, W83781D_REG_TEMP_OVER(nr),
+				data->temp_max_add[nr-2]);
+	} else {	/* TEMP1 */
+		data->temp_max = TEMP_TO_REG(val);
+		w83627hf_write_value(data, W83781D_REG_TEMP_OVER(nr),
+			data->temp_max);
+	}
 
 	mutex_unlock(&data->update_lock);
 	return count;
 }
 
-#define sysfs_fan_offset(offset) \
-static ssize_t show_regs_fan_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan(dev, buf, offset); \
-} \
-static DEVICE_ATTR(fan##offset##_input, S_IRUGO, show_regs_fan_##offset, NULL);
-
-#define sysfs_fan_min_offset(offset) \
-static ssize_t show_regs_fan_min##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan_min(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_fan_min##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_fan_min(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(fan##offset##_min, S_IRUGO | S_IWUSR, \
-		  show_regs_fan_min##offset, store_regs_fan_min##offset);
-
-sysfs_fan_offset(1);
-sysfs_fan_min_offset(1);
-sysfs_fan_offset(2);
-sysfs_fan_min_offset(2);
-sysfs_fan_offset(3);
-sysfs_fan_min_offset(3);
-
-#define show_temp_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	if (nr >= 2) {	/* TEMP2 and TEMP3 */ \
-		return sprintf(buf,"%ld\n", \
-			(long)LM75_TEMP_FROM_REG(data->reg##_add[nr-2])); \
-	} else {	/* TEMP1 */ \
-		return sprintf(buf,"%ld\n", (long)TEMP_FROM_REG(data->reg)); \
-	} \
-}
-show_temp_reg(temp);
-show_temp_reg(temp_max);
-show_temp_reg(temp_max_hyst);
-
-#define store_temp_reg(REG, reg) \
-static ssize_t \
-store_temp_##reg (struct device *dev, const char *buf, size_t count, int nr) \
-{ \
-	struct w83627hf_data *data = dev_get_drvdata(dev); \
-	u32 val; \
-	 \
-	val = simple_strtoul(buf, NULL, 10); \
-	 \
-	mutex_lock(&data->update_lock); \
-	 \
-	if (nr >= 2) {	/* TEMP2 and TEMP3 */ \
-		data->temp_##reg##_add[nr-2] = LM75_TEMP_TO_REG(val); \
-		w83627hf_write_value(data, W83781D_REG_TEMP_##REG(nr), \
-				data->temp_##reg##_add[nr-2]); \
-	} else {	/* TEMP1 */ \
-		data->temp_##reg = TEMP_TO_REG(val); \
-		w83627hf_write_value(data, W83781D_REG_TEMP_##REG(nr), \
-			data->temp_##reg); \
-	} \
-	 \
-	mutex_unlock(&data->update_lock); \
-	return count; \
-}
-store_temp_reg(OVER, max);
-store_temp_reg(HYST, max_hyst);
-
-#define sysfs_temp_offset(offset) \
-static ssize_t \
-show_regs_temp_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_temp(dev, buf, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_input, S_IRUGO, show_regs_temp_##offset, NULL);
-
-#define sysfs_temp_reg_offset(reg, offset) \
-static ssize_t show_regs_temp_##reg##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_temp_##reg (dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_temp_##reg##offset (struct device *dev, struct device_attribute *attr, \
-			      const char *buf, size_t count) \
-{ \
-	return store_temp_##reg (dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_##reg, S_IRUGO| S_IWUSR, \
-		  show_regs_temp_##reg##offset, store_regs_temp_##reg##offset);
-
-#define sysfs_temp_offsets(offset) \
-sysfs_temp_offset(offset) \
-sysfs_temp_reg_offset(max, offset) \
-sysfs_temp_reg_offset(max_hyst, offset)
-
-sysfs_temp_offsets(1);
-sysfs_temp_offsets(2);
-sysfs_temp_offsets(3);
+static ssize_t store_temp_max_hyst(struct device *dev,
+				   struct device_attribute *devattr,
+				   const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	u32 val = simple_strtoul(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
 
-static ssize_t
-show_vid_reg(struct device *dev, struct device_attribute *attr, char *buf)
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		data->temp_max_hyst_add[nr-2] = LM75_TEMP_TO_REG(val);
+		w83627hf_write_value(data, W83781D_REG_TEMP_HYST(nr),
+				data->temp_max_hyst_add[nr-2]);
+	} else {	/* TEMP1 */
+		data->temp_max_hyst = TEMP_TO_REG(val);
+		w83627hf_write_value(data, W83781D_REG_TEMP_HYST(nr),
+			data->temp_max_hyst);
+	}
+
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define temp_decl(offset) \
+static SENSOR_DEVICE_ATTR(temp##offset##_input, S_IRUGO, 		\
+			  show_temp, NULL, offset);			\
+static SENSOR_DEVICE_ATTR(temp##offset##_max, S_IRUGO | S_IWUSR, 	\
+			  show_temp_max, store_temp_max, offset);	\
+static SENSOR_DEVICE_ATTR(temp##offset##_max_hyst, S_IRUGO | S_IWUSR,	\
+			  show_temp_max_hyst, store_temp_max_hyst, offset);
+
+temp_decl(1);
+temp_decl(2);
+temp_decl(3);
+
+static ssize_t show_vid_reg(struct device *dev,
+			    struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	return sprintf(buf, "%ld\n", (long) vid_from_reg(data->vid, data->vrm));
 }
 static DEVICE_ATTR(cpu0_vid, S_IRUGO, show_vid_reg, NULL);
 
-static ssize_t
-show_vrm_reg(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_vrm_reg(struct device *dev,
+			    struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	return sprintf(buf, "%ld\n", (long) data->vrm);
 }
-static ssize_t
-store_vrm_reg(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+static ssize_t store_vrm_reg(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t count)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);
 	data->vrm = val;
-
 	return count;
 }
 static DEVICE_ATTR(vrm, S_IRUGO | S_IWUSR, show_vrm_reg, store_vrm_reg);
 
-static ssize_t
-show_alarms_reg(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_alarms_reg(struct device *dev,
+			       struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	return sprintf(buf, "%ld\n", (long) data->alarms);
 }
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms_reg, NULL);
 
-#define show_beep_reg(REG, reg) \
-static ssize_t show_beep_##reg (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	return sprintf(buf,"%ld\n", \
-		      (long)BEEP_##REG##_FROM_REG(data->beep_##reg)); \
-}
-show_beep_reg(ENABLE, enable)
-show_beep_reg(MASK, mask)
-
 #define BEEP_ENABLE			0	/* Store beep_enable */
 #define BEEP_MASK			1	/* Store beep_mask */
 
-static ssize_t
-store_beep_reg(struct device *dev, const char *buf, size_t count,
-	       int update_mask)
+static ssize_t store_beep_reg(struct device *dev,
+			      const char *buf, size_t count,
+			      int update_mask)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	u32 val, val2;
@@ -774,36 +783,44 @@ store_beep_reg(struct device *dev, const
 	return count;
 }
 
-#define sysfs_beep(REG, reg) \
-static ssize_t show_regs_beep_##reg (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_beep_##reg(dev, attr, buf); \
-} \
-static ssize_t \
-store_regs_beep_##reg (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_beep_reg(dev, buf, count, BEEP_##REG); \
-} \
-static DEVICE_ATTR(beep_##reg, S_IRUGO | S_IWUSR, \
-		  show_regs_beep_##reg, store_regs_beep_##reg);
-
-sysfs_beep(ENABLE, enable);
-sysfs_beep(MASK, mask);
+static ssize_t show_beep_enable(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n",
+		      (long)BEEP_ENABLE_FROM_REG(data->beep_enable));
+}
+static ssize_t store_beep_enable(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	return store_beep_reg(dev, buf, count, BEEP_ENABLE);
+}
+static DEVICE_ATTR(beep_enable, S_IRUGO | S_IWUSR, \
+		   show_beep_enable, store_beep_enable);
 
-static ssize_t
-show_fan_div_reg(struct device *dev, char *buf, int nr)
+static ssize_t show_beep_mask(struct device *dev,
+			      struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	return sprintf(buf, "%ld\n",
-		       (long) DIV_FROM_REG(data->fan_div[nr - 1]));
+		      (long)BEEP_MASK_FROM_REG(data->beep_mask));
+}
+static ssize_t store_beep_mask(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	return store_beep_reg(dev, buf, count, BEEP_MASK);
 }
+static DEVICE_ATTR(beep_mask, S_IRUGO | S_IWUSR, \
+		  show_beep_mask, store_beep_mask);
 
 /* Note: we save and restore the fan minimum here, because its value is
    determined in part by the fan divisor.  This follows the principle of
    least surprise; the user doesn't expect the fan minimum to change just
    because the divisor changed. */
-static ssize_t
-store_fan_div_reg(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t store_fan_div_reg(struct device *dev,
+				 const char *buf, size_t count, int nr)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	unsigned long min;
@@ -818,10 +835,12 @@ store_fan_div_reg(struct device *dev, co
 
 	data->fan_div[nr] = DIV_TO_REG(val);
 
-	reg = (w83627hf_read_value(data, nr==2 ? W83781D_REG_PIN : W83781D_REG_VID_FANDIV)
+	reg = (w83627hf_read_value(data, nr == 2 ?
+			W83781D_REG_PIN : W83781D_REG_VID_FANDIV)
 	       & (nr==0 ? 0xcf : 0x3f))
 	    | ((data->fan_div[nr] & 0x03) << (nr==0 ? 4 : 6));
-	w83627hf_write_value(data, nr==2 ? W83781D_REG_PIN : W83781D_REG_VID_FANDIV, reg);
+	w83627hf_write_value(data, nr == 2 ?
+			W83781D_REG_PIN : W83781D_REG_VID_FANDIV, reg);
 
 	reg = (w83627hf_read_value(data, W83781D_REG_VBAT)
 	       & ~(1 << (5 + nr)))
@@ -836,94 +855,85 @@ store_fan_div_reg(struct device *dev, co
 	return count;
 }
 
-#define sysfs_fan_div(offset) \
-static ssize_t show_regs_fan_div_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan_div_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_fan_div_##offset (struct device *dev, struct device_attribute *attr, \
-			    const char *buf, size_t count) \
-{ \
-	return store_fan_div_reg(dev, buf, count, offset - 1); \
-} \
-static DEVICE_ATTR(fan##offset##_div, S_IRUGO | S_IWUSR, \
-		  show_regs_fan_div_##offset, store_regs_fan_div_##offset);
-
-sysfs_fan_div(1);
-sysfs_fan_div(2);
-sysfs_fan_div(3);
-
-static ssize_t
-show_pwm_reg(struct device *dev, char *buf, int nr)
+static ssize_t show_fan_div(struct device *dev,
+			    struct device_attribute *devattr, char *buf)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return sprintf(buf, "%ld\n", (long) data->pwm[nr - 1]);
+	return sprintf(buf, "%ld\n",
+		       (long) DIV_FROM_REG(data->fan_div[nr]));
 }
+static ssize_t store_fan_div(struct device *dev,
+			     struct device_attribute *devattr,
+			     const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	return store_fan_div_reg(dev, buf, count, nr);
+}
+
+static SENSOR_DEVICE_ATTR(fan1_div, S_IRUGO | S_IWUSR,
+			  show_fan_div, store_fan_div, 0);
+static SENSOR_DEVICE_ATTR(fan2_div, S_IRUGO | S_IWUSR,
+			  show_fan_div, store_fan_div, 1);
+static SENSOR_DEVICE_ATTR(fan3_div, S_IRUGO | S_IWUSR,
+			  show_fan_div, store_fan_div, 2);
 
-static ssize_t
-store_pwm_reg(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t show_pwm(struct device *dev,
+			struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long) data->pwm[nr-1]);
+}
+static ssize_t store_pwm(struct device *dev,
+			 struct device_attribute *devattr,
+			 const char *buf, size_t count)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
 
 	if (data->type == w83627thf) {
 		/* bits 0-3 are reserved  in 627THF */
-		data->pwm[nr - 1] = PWM_TO_REG(val) & 0xf0;
+		data->pwm[nr-1] = PWM_TO_REG(val) & 0xf0;
 		w83627hf_write_value(data,
 				     W836X7HF_REG_PWM(data->type, nr),
-				     data->pwm[nr - 1] |
+				     data->pwm[nr-1] |
 				     (w83627hf_read_value(data,
 				     W836X7HF_REG_PWM(data->type, nr)) & 0x0f));
 	} else {
-		data->pwm[nr - 1] = PWM_TO_REG(val);
+		data->pwm[nr-1] = PWM_TO_REG(val);
 		w83627hf_write_value(data,
 				     W836X7HF_REG_PWM(data->type, nr),
-				     data->pwm[nr - 1]);
+				     data->pwm[nr-1]);
 	}
 
 	mutex_unlock(&data->update_lock);
 	return count;
 }
 
-#define sysfs_pwm(offset) \
-static ssize_t show_regs_pwm_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_pwm_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_pwm_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_pwm_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR, \
-		  show_regs_pwm_##offset, store_regs_pwm_##offset);
-
-sysfs_pwm(1);
-sysfs_pwm(2);
-sysfs_pwm(3);
-
-static ssize_t
-show_pwm_freq_reg(struct device *dev, char *buf, int nr)
+static ssize_t show_pwm_freq(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	if (data->type == w83627hf)
 		return sprintf(buf, "%ld\n",
-			pwm_freq_from_reg_627hf(data->pwm_freq[nr - 1]));
+			pwm_freq_from_reg_627hf(data->pwm_freq[nr-1]));
 	else
 		return sprintf(buf, "%ld\n",
-			pwm_freq_from_reg(data->pwm_freq[nr - 1]));
+			pwm_freq_from_reg(data->pwm_freq[nr-1]));
 }
 
-static ssize_t
-store_pwm_freq_reg(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t store_pwm_freq(struct device *dev,
+			      struct device_attribute *devattr,
+			      const char *buf, size_t count)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	static const u8 mask[]={0xF8, 0x8F};
+	static const u8 mask[] = {0xF8, 0x8F};
 	u32 val;
 
 	val = simple_strtoul(buf, NULL, 10);
@@ -931,49 +941,39 @@ store_pwm_freq_reg(struct device *dev, c
 	mutex_lock(&data->update_lock);
 
 	if (data->type == w83627hf) {
-		data->pwm_freq[nr - 1] = pwm_freq_to_reg_627hf(val);
+		data->pwm_freq[nr-1] = pwm_freq_to_reg_627hf(val);
 		w83627hf_write_value(data, W83627HF_REG_PWM_FREQ,
-				(data->pwm_freq[nr - 1] << ((nr - 1)*4)) |
+				(data->pwm_freq[nr-1] << ((nr-1)*4)) |
 				(w83627hf_read_value(data,
-				W83627HF_REG_PWM_FREQ) & mask[nr - 1]));
+				W83627HF_REG_PWM_FREQ) & mask[nr-1]));
 	} else {
-		data->pwm_freq[nr - 1] = pwm_freq_to_reg(val);
-		w83627hf_write_value(data, W83637HF_REG_PWM_FREQ[nr - 1],
-				data->pwm_freq[nr - 1]);
+		data->pwm_freq[nr-1] = pwm_freq_to_reg(val);
+		w83627hf_write_value(data, W83637HF_REG_PWM_FREQ[nr-1],
+				data->pwm_freq[nr-1]);
 	}
 
 	mutex_unlock(&data->update_lock);
 	return count;
 }
 
-#define sysfs_pwm_freq(offset) \
-static ssize_t show_regs_pwm_freq_##offset(struct device *dev, \
-		struct device_attribute *attr, char *buf) \
-{ \
-	return show_pwm_freq_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_pwm_freq_##offset(struct device *dev, \
-		struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_pwm_freq_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR, \
-		  show_regs_pwm_freq_##offset, store_regs_pwm_freq_##offset);
-
-sysfs_pwm_freq(1);
-sysfs_pwm_freq(2);
-sysfs_pwm_freq(3);
+#define pwm_decl(offset)	\
+static SENSOR_DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR,		\
+			  show_pwm, store_pwm, offset);			\
+static SENSOR_DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR,	\
+			  show_pwm_freq, store_pwm_freq, offset);
+
+pwm_decl(1);
+pwm_decl(2);
+pwm_decl(3);
 
-static ssize_t
-show_sensor_reg(struct device *dev, char *buf, int nr)
+static ssize_t show_sensor_reg(struct device *dev, char *buf, int nr)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return sprintf(buf, "%ld\n", (long) data->sens[nr - 1]);
+	return sprintf(buf, "%ld\n", (long) data->sens[nr-1]);
 }
 
-static ssize_t
-store_sensor_reg(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t store_sensor_reg(struct device *dev, const char *buf,
+				size_t count, int nr)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	u32 val, tmp;
@@ -986,26 +986,26 @@ store_sensor_reg(struct device *dev, con
 	case 1:		/* PII/Celeron diode */
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG1);
 		w83627hf_write_value(data, W83781D_REG_SCFG1,
-				    tmp | BIT_SCFG1[nr - 1]);
+				    tmp | BIT_SCFG1[nr-1]);
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG2);
 		w83627hf_write_value(data, W83781D_REG_SCFG2,
-				    tmp | BIT_SCFG2[nr - 1]);
-		data->sens[nr - 1] = val;
+				    tmp | BIT_SCFG2[nr-1]);
+		data->sens[nr-1] = val;
 		break;
 	case 2:		/* 3904 */
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG1);
 		w83627hf_write_value(data, W83781D_REG_SCFG1,
-				    tmp | BIT_SCFG1[nr - 1]);
+				    tmp | BIT_SCFG1[nr-1]);
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG2);
 		w83627hf_write_value(data, W83781D_REG_SCFG2,
-				    tmp & ~BIT_SCFG2[nr - 1]);
-		data->sens[nr - 1] = val;
+				    tmp & ~BIT_SCFG2[nr-1]);
+		data->sens[nr-1] = val;
 		break;
 	case W83781D_DEFAULT_BETA:	/* thermistor */
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG1);
 		w83627hf_write_value(data, W83781D_REG_SCFG1,
-				    tmp & ~BIT_SCFG1[nr - 1]);
-		data->sens[nr - 1] = val;
+				    tmp & ~BIT_SCFG1[nr-1]);
+		data->sens[nr-1] = val;
 		break;
 	default:
 		dev_err(dev,
@@ -1018,22 +1018,27 @@ store_sensor_reg(struct device *dev, con
 	return count;
 }
 
-#define sysfs_sensor(offset) \
-static ssize_t show_regs_sensor_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-    return show_sensor_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_sensor_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-    return store_sensor_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_type, S_IRUGO | S_IWUSR, \
-		  show_regs_sensor_##offset, store_regs_sensor_##offset);
-
-sysfs_sensor(1);
-sysfs_sensor(2);
-sysfs_sensor(3);
+static ssize_t show_sensor(struct device *dev,
+			   struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	return show_sensor_reg(dev, buf, nr);
+}
+static ssize_t store_sensor(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	return store_sensor_reg(dev, buf, count, nr);
+}
+
+#define sensor_decl(offset) \
+static SENSOR_DEVICE_ATTR(temp##offset##_type, S_IRUGO | S_IWUSR, \
+			  show_sensor, store_sensor, offset);
+
+sensor_decl(1);
+sensor_decl(2);
+sensor_decl(3);
 
 static ssize_t show_name(struct device *dev, struct device_attribute
 			 *devattr, char *buf)
@@ -1098,14 +1103,15 @@ static int __init w83627hf_find(int sioa
 	       superio_inb(WINB_BASE_REG + 1);
 	*addr = val & WINB_ALIGNMENT;
 	if (*addr == 0) {
-		printk(KERN_WARNING DRVNAME ": Base address not set, "
-		       "skipping\n");
+		printk(KERN_WARNING DRVNAME
+		       ": Base address not set, skipping\n");
 		goto exit;
 	}
 
 	val = superio_inb(WINB_ACT_REG);
 	if (!(val & 0x01)) {
-		printk(KERN_WARNING DRVNAME ": Enabling HWM logical device\n");
+		printk(KERN_WARNING DRVNAME
+		       ": Enabling HWM logical device\n");
 		superio_outb(WINB_ACT_REG, val | 0x01);
 	}
 
@@ -1118,48 +1124,43 @@ static int __init w83627hf_find(int sioa
 	return err;
 }
 
+#define VIN_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_in##X##_input.dev_attr.attr,	    \
+	&sensor_dev_attr_in##X##_min.dev_attr.attr,	    \
+	&sensor_dev_attr_in##X##_max.dev_attr.attr
+
+#define FAN_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_fan##X##_input.dev_attr.attr,	    \
+	&sensor_dev_attr_fan##X##_min.dev_attr.attr,	    \
+	&sensor_dev_attr_fan##X##_div.dev_attr.attr
+
+#define TEMP_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_temp##X##_input.dev_attr.attr,		\
+	&sensor_dev_attr_temp##X##_max.dev_attr.attr,		\
+	&sensor_dev_attr_temp##X##_max_hyst.dev_attr.attr,	\
+	&sensor_dev_attr_temp##X##_type.dev_attr.attr
+
 static struct attribute *w83627hf_attributes[] = {
 	&dev_attr_in0_input.attr,
 	&dev_attr_in0_min.attr,
 	&dev_attr_in0_max.attr,
-	&dev_attr_in2_input.attr,
-	&dev_attr_in2_min.attr,
-	&dev_attr_in2_max.attr,
-	&dev_attr_in3_input.attr,
-	&dev_attr_in3_min.attr,
-	&dev_attr_in3_max.attr,
-	&dev_attr_in4_input.attr,
-	&dev_attr_in4_min.attr,
-	&dev_attr_in4_max.attr,
-	&dev_attr_in7_input.attr,
-	&dev_attr_in7_min.attr,
-	&dev_attr_in7_max.attr,
-	&dev_attr_in8_input.attr,
-	&dev_attr_in8_min.attr,
-	&dev_attr_in8_max.attr,
-
-	&dev_attr_fan1_input.attr,
-	&dev_attr_fan1_min.attr,
-	&dev_attr_fan1_div.attr,
-	&dev_attr_fan2_input.attr,
-	&dev_attr_fan2_min.attr,
-	&dev_attr_fan2_div.attr,
-
-	&dev_attr_temp1_input.attr,
-	&dev_attr_temp1_max.attr,
-	&dev_attr_temp1_max_hyst.attr,
-	&dev_attr_temp1_type.attr,
-	&dev_attr_temp2_input.attr,
-	&dev_attr_temp2_max.attr,
-	&dev_attr_temp2_max_hyst.attr,
-	&dev_attr_temp2_type.attr,
+
+	VIN_UNIT_ATTRS(2),
+	VIN_UNIT_ATTRS(3),
+	VIN_UNIT_ATTRS(4),
+	VIN_UNIT_ATTRS(7),
+	VIN_UNIT_ATTRS(8),
+	FAN_UNIT_ATTRS(1),
+	FAN_UNIT_ATTRS(2),
+	TEMP_UNIT_ATTRS(1),
+	TEMP_UNIT_ATTRS(2),
 
 	&dev_attr_alarms.attr,
 	&dev_attr_beep_enable.attr,
 	&dev_attr_beep_mask.attr,
 
-	&dev_attr_pwm1.attr,
-	&dev_attr_pwm2.attr,
+	&sensor_dev_attr_pwm1.dev_attr.attr,
+	&sensor_dev_attr_pwm2.dev_attr.attr,
 
 	&dev_attr_name.attr,
 	NULL
@@ -1170,30 +1171,17 @@ static const struct attribute_group w836
 };
 
 static struct attribute *w83627hf_attributes_opt[] = {
-	&dev_attr_in1_input.attr,
-	&dev_attr_in1_min.attr,
-	&dev_attr_in1_max.attr,
-	&dev_attr_in5_input.attr,
-	&dev_attr_in5_min.attr,
-	&dev_attr_in5_max.attr,
-	&dev_attr_in6_input.attr,
-	&dev_attr_in6_min.attr,
-	&dev_attr_in6_max.attr,
-
-	&dev_attr_fan3_input.attr,
-	&dev_attr_fan3_min.attr,
-	&dev_attr_fan3_div.attr,
-
-	&dev_attr_temp3_input.attr,
-	&dev_attr_temp3_max.attr,
-	&dev_attr_temp3_max_hyst.attr,
-	&dev_attr_temp3_type.attr,
-
-	&dev_attr_pwm3.attr,
-
-	&dev_attr_pwm1_freq.attr,
-	&dev_attr_pwm2_freq.attr,
-	&dev_attr_pwm3_freq.attr,
+	VIN_UNIT_ATTRS(1),
+	VIN_UNIT_ATTRS(5),
+	VIN_UNIT_ATTRS(6),
+	FAN_UNIT_ATTRS(3),
+	TEMP_UNIT_ATTRS(3),
+
+	&sensor_dev_attr_pwm3.dev_attr.attr,
+
+	&sensor_dev_attr_pwm1_freq.dev_attr.attr,
+	&sensor_dev_attr_pwm2_freq.dev_attr.attr,
+	&sensor_dev_attr_pwm3_freq.dev_attr.attr,
 	NULL
 };
 
@@ -1251,27 +1239,46 @@ static int __devinit w83627hf_probe(stru
 
 	/* Register chip-specific device attributes */
 	if (data->type == w83627hf || data->type == w83697hf)
-		if ((err = device_create_file(dev, &dev_attr_in5_input))
-		 || (err = device_create_file(dev, &dev_attr_in5_min))
-		 || (err = device_create_file(dev, &dev_attr_in5_max))
-		 || (err = device_create_file(dev, &dev_attr_in6_input))
-		 || (err = device_create_file(dev, &dev_attr_in6_min))
-		 || (err = device_create_file(dev, &dev_attr_in6_max))
-		 || (err = device_create_file(dev, &dev_attr_pwm1_freq))
-		 || (err = device_create_file(dev, &dev_attr_pwm2_freq)))
+		if ((err = device_create_file(dev,
+				&sensor_dev_attr_in5_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in5_min.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in5_max.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in6_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in6_min.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in6_max.dev_attr))
+		 /* tbd */
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_pwm1_freq.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_pwm2_freq.dev_attr)))
 			goto ERROR4;
 
 	if (data->type != w83697hf)
-		if ((err = device_create_file(dev, &dev_attr_in1_input))
-		 || (err = device_create_file(dev, &dev_attr_in1_min))
-		 || (err = device_create_file(dev, &dev_attr_in1_max))
-		 || (err = device_create_file(dev, &dev_attr_fan3_input))
-		 || (err = device_create_file(dev, &dev_attr_fan3_min))
-		 || (err = device_create_file(dev, &dev_attr_fan3_div))
-		 || (err = device_create_file(dev, &dev_attr_temp3_input))
-		 || (err = device_create_file(dev, &dev_attr_temp3_max))
-		 || (err = device_create_file(dev, &dev_attr_temp3_max_hyst))
-		 || (err = device_create_file(dev, &dev_attr_temp3_type)))
+		if ((err = device_create_file(dev,
+				&sensor_dev_attr_in1_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in1_min.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in1_max.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_fan3_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_fan3_min.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_fan3_div.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_temp3_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_temp3_max.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_temp3_max_hyst.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_temp3_type.dev_attr)))
 			goto ERROR4;
 
 	if (data->type != w83697hf && data->vid != 0xff) {
@@ -1285,13 +1292,17 @@ static int __devinit w83627hf_probe(stru
 
 	if (data->type == w83627thf || data->type == w83637hf
 	 || data->type == w83687thf)
-		if ((err = device_create_file(dev, &dev_attr_pwm3)))
+		if ((err = device_create_file(
+				dev, &sensor_dev_attr_pwm3.dev_attr)))
 			goto ERROR4;
 
 	if (data->type == w83637hf || data->type == w83687thf)
-		if ((err = device_create_file(dev, &dev_attr_pwm1_freq))
-		 || (err = device_create_file(dev, &dev_attr_pwm2_freq))
-		 || (err = device_create_file(dev, &dev_attr_pwm3_freq)))
+		if ((err = device_create_file(dev,
+				&sensor_dev_attr_pwm1_freq.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_pwm1_freq.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_pwm1_freq.dev_attr)))
 			goto ERROR4;
 
 	data->class_dev = hwmon_device_register(dev);

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#3 lore raw
In reply to Jim Cromie

Hi Jim,

Quoted 31 lines · Jim Cromie

Thank you Krzysztof for your review.

This 2nd rev does:

- rebases on Marks GIT tree (thought I had, sorry)
- fixes all 80-column probs in last, plus a few others
- adopts new fashion int nr = to_sensor_dev_attr(devattr)->index;
- folds many one-caller functions into the caller
- converts remaining fn-generating macros to plain old functions

WRT types and conversions in the *_FROM/TO_REG macros, thanks for the detailed review, tracing thru the conversions is time consuming, and feels error prone..

- I dropped the (long) casts in some cases,
- and pushed them down into the macros in others
- Im disappointed that sparse/C=1 didnt identify these things

Im reluctant to do more here (Do No Harm, leave well enough alone). It seems that a full review, and conversion to static inline functions is warranted, and probably should be done separately.

<update>

Ive now seen Jean's comments, and have manually (I hope fully) backed out the three 'types and conversions' fixes above, reverting to original form. I have a set of interim diffs, so I *could* go back and reconstruct things, but I would *really* like to avoid doing so; its tedious and error prone.

Quoted 4 lines

FWIW - the patch results in a smaller object.

  14627    2464      36   17127    42e7  lx-git-hwmon/drivers/hwmon/w83627hf.ko
  12895    2676      36   15607    3cf7  lx-git-hwmon-demacro/drivers/hwmon/w83627hf.ko

Yes, the size reduction is really nice.

Quoted 11 lines

---

Convert hwmon/w83627hf to use dynamic sysfs callbacks, replacing the function-generating macros.

[jimc@groucho lx]$ diffstat diff.hwmon-w83627hf-demacro-sysfs-callbacks

 w83627hf.c |  887 ++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 449 insertions(+), 438 deletions(-)

There still are many changes in your patch, which do not belong there. Things like:

Review

Quoted 4 lines
(quoted diff) +1 -3
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);

and:

Review

Quoted 2 lines
(quoted diff) +1 -1
-		data->pwm[nr - 1] = PWM_TO_REG(val) & 0xf0;
+		data->pwm[nr-1] = PWM_TO_REG(val) & 0xf0;

and:

Review

Quoted 4 lines
(quoted diff) +2 -2
-		printk(KERN_WARNING DRVNAME ": Base address not set, "
-		       "skipping\n");
+		printk(KERN_WARNING DRVNAME
+		       ": Base address not set, skipping\n");

should all be reverted. Your patch will be big enough with only the needed changes.

Quoted 100 lines

diff -ruNp -X dontdiff -X exclude-diffs lx-git-hwmon/drivers/hwmon/w83627hf.c lx-git-hwmon-demacro/drivers/hwmon/w83627hf.c

drivers/hwmon/w83627hf.c +20 -66
@@ -45,6 +45,7 @@
 #include <linux/jiffies.h>
 #include <linux/platform_device.h>
 #include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 #include <linux/hwmon-vid.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
@@ -403,75 +404,79 @@ static struct platform_driver w83627hf_d
 	.remove		= __devexit_p(w83627hf_remove),
 };
 
-/* following are the sysfs callback functions */
-#define show_in_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	return sprintf(buf,"%ld\n", (long)IN_FROM_REG(data->reg[nr])); \
-}
-show_in_reg(in)
-show_in_reg(in_min)
-show_in_reg(in_max)
-
-#define store_in_reg(REG, reg) \
-static ssize_t \
-store_in_##reg (struct device *dev, const char *buf, size_t count, int nr) \
-{ \
-	struct w83627hf_data *data = dev_get_drvdata(dev); \
-	u32 val; \
-	 \
-	val = simple_strtoul(buf, NULL, 10); \
-	 \
-	mutex_lock(&data->update_lock); \
-	data->in_##reg[nr] = IN_TO_REG(val); \
-	w83627hf_write_value(data, W83781D_REG_IN_##REG(nr), \
-			    data->in_##reg[nr]); \
-	 \
-	mutex_unlock(&data->update_lock); \
-	return count; \
-}
-store_in_reg(MIN, min)
-store_in_reg(MAX, max)
-
-#define sysfs_in_offset(offset) \
-static ssize_t \
-show_regs_in_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-        return show_in(dev, buf, offset); \
-} \
-static DEVICE_ATTR(in##offset##_input, S_IRUGO, show_regs_in_##offset, NULL);
-
-#define sysfs_in_reg_offset(reg, offset) \
-static ssize_t show_regs_in_##reg##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_in_##reg (dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_in_##reg##offset (struct device *dev, struct device_attribute *attr, \
-			    const char *buf, size_t count) \
-{ \
-	return store_in_##reg (dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(in##offset##_##reg, S_IRUGO| S_IWUSR, \
-		  show_regs_in_##reg##offset, store_regs_in_##reg##offset);
-
-#define sysfs_in_offsets(offset) \
-sysfs_in_offset(offset) \
-sysfs_in_reg_offset(min, offset) \
-sysfs_in_reg_offset(max, offset)
-
-sysfs_in_offsets(1);
-sysfs_in_offsets(2);
-sysfs_in_offsets(3);
-sysfs_in_offsets(4);
-sysfs_in_offsets(5);
-sysfs_in_offsets(6);
-sysfs_in_offsets(7);
-sysfs_in_offsets(8);
+static ssize_t show_in_input(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long) IN_FROM_REG(data->in[nr]));
+}
+static ssize_t show_in_min(struct device *dev,
+			   struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long) IN_FROM_REG(data->in_min[nr]));
+}
+static ssize_t show_in_max(struct device *dev,
+			   struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);

drivers/hwmon/w83627hf.c static struct platform_driver w83627hf_d · line 435

Quoted 10 lines
drivers/hwmon/w83627hf.c +10 -0
+	return sprintf(buf, "%ld\n", (long) IN_FROM_REG(data->in_max[nr]));
+}
+
+static ssize_t store_in_min(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	long val = simple_strtol(buf, NULL, 10);

The original code has u32 and simple_strtoul.

Quoted 6 lines
drivers/hwmon/w83627hf.c +6 -0
+
+	mutex_lock(&data->update_lock);
+	data->in_min[nr] = IN_TO_REG(val);
+	w83627hf_write_value(data, W83781D_REG_IN_MIN(nr), data->in_min[nr]);
+
+	mutex_unlock(&data->update_lock);

drivers/hwmon/w83627hf.c static struct platform_driver w83627hf_d · line 451

Quoted 10 lines
drivers/hwmon/w83627hf.c +10 -0
+	return count;
+}
+
+static ssize_t store_in_max(struct device *dev,
+			    struct device_attribute *devattr, const char *buf,
+			    size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	u32 val = simple_strtol(buf, NULL, 10);

The original code has simple_strtoul.

drivers/hwmon/w83627hf.c static struct platform_driver w83627hf_d · line 456

Quoted 5 lines
drivers/hwmon/w83627hf.c +5 -0
+
+	mutex_lock(&data->update_lock);
+	data->in_max[nr] = IN_TO_REG(val);
+	w83627hf_write_value(data, W83781D_REG_IN_MAX(nr),
+			     data->in_max[nr]);

Would fit on a single line, as you did in store_in_min above.

Quoted 92 lines
drivers/hwmon/w83627hf.c +38 -17
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define vin_decl(offset)	\
+static SENSOR_DEVICE_ATTR(in##offset##_input, S_IRUGO,		\
+			  show_in_input, NULL, offset);		\
+static SENSOR_DEVICE_ATTR(in##offset##_min, S_IRUGO | S_IWUSR,	\
+			  show_in_min, store_in_min, offset);	\
+static SENSOR_DEVICE_ATTR(in##offset##_max, S_IRUGO | S_IWUSR,	\
+			  show_in_max, store_in_max, offset);
+
+vin_decl(1);
+vin_decl(2);
+vin_decl(3);
+vin_decl(4);
+vin_decl(5);
+vin_decl(6);
+vin_decl(7);
+vin_decl(8);
 
 /* use a different set of functions for in0 */
-static ssize_t show_in_0(struct w83627hf_data *data, char *buf, u8 reg)
+static ssize_t _show_in_0(struct w83627hf_data *data, char *buf, u8 reg)
 {
 	long in0;
 
@@ -488,31 +493,33 @@ static ssize_t show_in_0(struct w83627hf
 	return sprintf(buf,"%ld\n", in0);
 }
 
-static ssize_t show_regs_in_0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_0(struct device *dev,
+			 struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in[0]);
+	return _show_in_0(data, buf, data->in[0]);
 }
 
-static ssize_t show_regs_in_min0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_min0(struct device *dev,
+			    struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in_min[0]);
+	return _show_in_0(data, buf, data->in_min[0]);
 }
 
-static ssize_t show_regs_in_max0(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_in_max0(struct device *dev,
+			    struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return show_in_0(data, buf, data->in_max[0]);
+	return _show_in_0(data, buf, data->in_max[0]);
 }
 
-static ssize_t store_regs_in_min0(struct device *dev, struct device_attribute *attr,
-	const char *buf, size_t count)
+static ssize_t store_in_min0(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t count)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
 
@@ -533,13 +540,12 @@ static ssize_t store_regs_in_min0(struct
 	return count;
 }
 
-static ssize_t store_regs_in_max0(struct device *dev, struct device_attribute *attr,
-	const char *buf, size_t count)
+static ssize_t store_in_max0(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t count)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
 
@@ -560,192 +566,195 @@ static ssize_t store_regs_in_max0(struct
 	return count;

drivers/hwmon/w83627hf.c static ssize_t store_regs_in_max0(struct · line 573

Quoted 10 lines
drivers/hwmon/w83627hf.c +3 -3
 }
 
-static DEVICE_ATTR(in0_input, S_IRUGO, show_regs_in_0, NULL);
+static DEVICE_ATTR(in0_input, S_IRUGO, show_in_0, NULL);
 static DEVICE_ATTR(in0_min, S_IRUGO | S_IWUSR,
-	show_regs_in_min0, store_regs_in_min0);
+		   show_in_min0, store_in_min0);
 static DEVICE_ATTR(in0_max, S_IRUGO | S_IWUSR,
-	show_regs_in_max0, store_regs_in_max0);
+		   show_in_max0, store_in_max0);

If you really want to rename these callbacks, I suggest show_in0_input, show_in0_min, store_in0_min, show_in0_max and show_in0_max, to stick to the sysfs file names. But quite frankly, I wouldn't bother renaming them at all.

Quoted 255 lines
drivers/hwmon/w83627hf.c +124 -117
-#define show_fan_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	return sprintf(buf,"%ld\n", \
-		FAN_FROM_REG(data->reg[nr-1], \
-			    (long)DIV_FROM_REG(data->fan_div[nr-1]))); \
+static ssize_t show_fan_input(struct device *dev,
+			      struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", FAN_FROM_REG(data->fan[nr],
+				(long) DIV_FROM_REG(data->fan_div[nr])));
 }
-show_fan_reg(fan);
-show_fan_reg(fan_min);
 
-static ssize_t
-store_fan_min(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t show_fan_min(struct device *dev,
+			    struct device_attribute *devattr, char *buf)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", FAN_FROM_REG(data->fan_min[nr],
+				(long) DIV_FROM_REG(data->fan_div[nr])));
+}
+
+static ssize_t store_fan_min(struct device *dev,
+			     struct device_attribute *devattr,
+			     const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
+	u32 val = simple_strtoul(buf, NULL, 10);
 
-	val = simple_strtoul(buf, NULL, 10);
+	mutex_lock(&data->update_lock);
+	data->fan_min[nr] = FAN_TO_REG(val, DIV_FROM_REG(data->fan_div[nr]));
+	w83627hf_write_value(data, W83781D_REG_FAN_MIN(nr+1),
+			    data->fan_min[nr]);
+
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define fan_decl(offset)	\
+static SENSOR_DEVICE_ATTR(fan##offset##_input, S_IRUGO,			\
+			  show_fan_input, NULL, offset - 1);		\
+static SENSOR_DEVICE_ATTR(fan##offset##_min, S_IRUGO | S_IWUSR,		\
+			  show_fan_min, store_fan_min, offset - 1);
+
+fan_decl(1);
+fan_decl(2);
+fan_decl(3);
+
+static ssize_t show_temp(struct device *dev,
+			 struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		return sprintf(buf, "%ld\n",
+			(long)LM75_TEMP_FROM_REG(data->temp_add[nr-2]));
+	} else {	/* TEMP1 */
+		return sprintf(buf, "%ld\n", (long) TEMP_FROM_REG(data->temp));
+	}
+}
+static ssize_t show_temp_max(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		return sprintf(buf, "%ld\n",
+			(long)LM75_TEMP_FROM_REG(data->temp_max_add[nr-2]));
+	} else {	/* TEMP1 */
+		return sprintf(buf, "%ld\n",
+			       (long) TEMP_FROM_REG(data->temp_max));
+	}
+}
+static ssize_t show_temp_max_hyst(struct device *dev,
+				  struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		return sprintf(buf, "%ld\n", (long) 
+			LM75_TEMP_FROM_REG(data->temp_max_hyst_add[nr-2]));
+	} else {	/* TEMP1 */
+		return sprintf(buf, "%ld\n",
+			       (long) TEMP_FROM_REG(data->temp_max_hyst));
+	}
+}
+
+static ssize_t store_temp_max(struct device *dev,
+			      struct device_attribute *devattr,
+			      const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	u32 val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
-	data->fan_min[nr - 1] > -	    FAN_TO_REG(val, DIV_FROM_REG(data->fan_div[nr - 1]));
-	w83627hf_write_value(data, W83781D_REG_FAN_MIN(nr),
-			    data->fan_min[nr - 1]);
+
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		data->temp_max_add[nr-2] = LM75_TEMP_TO_REG(val);
+		w83627hf_write_value(data, W83781D_REG_TEMP_OVER(nr),
+				data->temp_max_add[nr-2]);
+	} else {	/* TEMP1 */
+		data->temp_max = TEMP_TO_REG(val);
+		w83627hf_write_value(data, W83781D_REG_TEMP_OVER(nr),
+			data->temp_max);
+	}
 
 	mutex_unlock(&data->update_lock);
 	return count;
 }
 
-#define sysfs_fan_offset(offset) \
-static ssize_t show_regs_fan_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan(dev, buf, offset); \
-} \
-static DEVICE_ATTR(fan##offset##_input, S_IRUGO, show_regs_fan_##offset, NULL);
-
-#define sysfs_fan_min_offset(offset) \
-static ssize_t show_regs_fan_min##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan_min(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_fan_min##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_fan_min(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(fan##offset##_min, S_IRUGO | S_IWUSR, \
-		  show_regs_fan_min##offset, store_regs_fan_min##offset);
-
-sysfs_fan_offset(1);
-sysfs_fan_min_offset(1);
-sysfs_fan_offset(2);
-sysfs_fan_min_offset(2);
-sysfs_fan_offset(3);
-sysfs_fan_min_offset(3);
-
-#define show_temp_reg(reg) \
-static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	if (nr >= 2) {	/* TEMP2 and TEMP3 */ \
-		return sprintf(buf,"%ld\n", \
-			(long)LM75_TEMP_FROM_REG(data->reg##_add[nr-2])); \
-	} else {	/* TEMP1 */ \
-		return sprintf(buf,"%ld\n", (long)TEMP_FROM_REG(data->reg)); \
-	} \
-}
-show_temp_reg(temp);
-show_temp_reg(temp_max);
-show_temp_reg(temp_max_hyst);
-
-#define store_temp_reg(REG, reg) \
-static ssize_t \
-store_temp_##reg (struct device *dev, const char *buf, size_t count, int nr) \
-{ \
-	struct w83627hf_data *data = dev_get_drvdata(dev); \
-	u32 val; \
-	 \
-	val = simple_strtoul(buf, NULL, 10); \
-	 \
-	mutex_lock(&data->update_lock); \
-	 \
-	if (nr >= 2) {	/* TEMP2 and TEMP3 */ \
-		data->temp_##reg##_add[nr-2] = LM75_TEMP_TO_REG(val); \
-		w83627hf_write_value(data, W83781D_REG_TEMP_##REG(nr), \
-				data->temp_##reg##_add[nr-2]); \
-	} else {	/* TEMP1 */ \
-		data->temp_##reg = TEMP_TO_REG(val); \
-		w83627hf_write_value(data, W83781D_REG_TEMP_##REG(nr), \
-			data->temp_##reg); \
-	} \
-	 \
-	mutex_unlock(&data->update_lock); \
-	return count; \
-}
-store_temp_reg(OVER, max);
-store_temp_reg(HYST, max_hyst);
-
-#define sysfs_temp_offset(offset) \
-static ssize_t \
-show_regs_temp_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_temp(dev, buf, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_input, S_IRUGO, show_regs_temp_##offset, NULL);
-
-#define sysfs_temp_reg_offset(reg, offset) \
-static ssize_t show_regs_temp_##reg##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_temp_##reg (dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_temp_##reg##offset (struct device *dev, struct device_attribute *attr, \
-			      const char *buf, size_t count) \
-{ \
-	return store_temp_##reg (dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_##reg, S_IRUGO| S_IWUSR, \
-		  show_regs_temp_##reg##offset, store_regs_temp_##reg##offset);
-
-#define sysfs_temp_offsets(offset) \
-sysfs_temp_offset(offset) \
-sysfs_temp_reg_offset(max, offset) \
-sysfs_temp_reg_offset(max_hyst, offset)
-
-sysfs_temp_offsets(1);
-sysfs_temp_offsets(2);
-sysfs_temp_offsets(3);
+static ssize_t store_temp_max_hyst(struct device *dev,
+				   struct device_attribute *devattr,
+				   const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	u32 val = simple_strtoul(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
 
-static ssize_t
-show_vid_reg(struct device *dev, struct device_attribute *attr, char *buf)
+	if (nr >= 2) {	/* TEMP2 and TEMP3 */
+		data->temp_max_hyst_add[nr-2] = LM75_TEMP_TO_REG(val);
+		w83627hf_write_value(data, W83781D_REG_TEMP_HYST(nr),
+				data->temp_max_hyst_add[nr-2]);
+	} else {	/* TEMP1 */
+		data->temp_max_hyst = TEMP_TO_REG(val);
+		w83627hf_write_value(data, W83781D_REG_TEMP_HYST(nr),
+			data->temp_max_hyst);
+	}
+
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define temp_decl(offset) \
+static SENSOR_DEVICE_ATTR(temp##offset##_input, S_IRUGO, 		\
+			  show_temp, NULL, offset);			\
+static SENSOR_DEVICE_ATTR(temp##offset##_max, S_IRUGO | S_IWUSR, 	\
+			  show_temp_max, store_temp_max, offset);	\

drivers/hwmon/w83627hf.c static ssize_t store_regs_in_max0(struct · line 720

Quoted 10 lines
drivers/hwmon/w83627hf.c +9 -0
+static SENSOR_DEVICE_ATTR(temp##offset##_max_hyst, S_IRUGO | S_IWUSR,	\
+			  show_temp_max_hyst, store_temp_max_hyst, offset);
+
+temp_decl(1);
+temp_decl(2);
+temp_decl(3);
+
 
+static ssize_t show_vid_reg(struct device *dev,
+			    struct device_attribute *attr, char *buf)

Please revert, this change is not needed.

drivers/hwmon/w83627hf.c static ssize_t store_regs_in_max0(struct · line 728

Quoted 10 lines
drivers/hwmon/w83627hf.c +2 -2
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	return sprintf(buf, "%ld\n", (long) vid_from_reg(data->vid, data->vrm));
 }
 static DEVICE_ATTR(cpu0_vid, S_IRUGO, show_vid_reg, NULL);
 
-static ssize_t
-show_vrm_reg(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_vrm_reg(struct device *dev,
+			    struct device_attribute *attr, char *buf)

Same here.

drivers/hwmon/w83627hf.c static ssize_t store_regs_in_max0(struct · line 735

Quoted 9 lines
drivers/hwmon/w83627hf.c +3 -2
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	return sprintf(buf, "%ld\n", (long) data->vrm);
 }
-static ssize_t
-store_vrm_reg(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
+static ssize_t store_vrm_reg(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t count)

And same here.

Quoted 6 lines
drivers/hwmon/w83627hf.c +1 -3
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);

drivers/hwmon/w83627hf.c static ssize_t store_regs_in_max0(struct · line 745

Quoted 10 lines
drivers/hwmon/w83627hf.c +2 -3
 	data->vrm = val;
-
 	return count;
 }
 static DEVICE_ATTR(vrm, S_IRUGO | S_IWUSR, show_vrm_reg, store_vrm_reg);
 
-static ssize_t
-show_alarms_reg(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t show_alarms_reg(struct device *dev,
+			       struct device_attribute *attr, char *buf)

And here again.

Quoted 72 lines
drivers/hwmon/w83627hf.c +21 -31
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	return sprintf(buf, "%ld\n", (long) data->alarms);
 }
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms_reg, NULL);
 
-#define show_beep_reg(REG, reg) \
-static ssize_t show_beep_##reg (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	struct w83627hf_data *data = w83627hf_update_device(dev); \
-	return sprintf(buf,"%ld\n", \
-		      (long)BEEP_##REG##_FROM_REG(data->beep_##reg)); \
-}
-show_beep_reg(ENABLE, enable)
-show_beep_reg(MASK, mask)
-
 #define BEEP_ENABLE			0	/* Store beep_enable */
 #define BEEP_MASK			1	/* Store beep_mask */
 
-static ssize_t
-store_beep_reg(struct device *dev, const char *buf, size_t count,
-	       int update_mask)
+static ssize_t store_beep_reg(struct device *dev,
+			      const char *buf, size_t count,
+			      int update_mask)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	u32 val, val2;
@@ -774,36 +783,44 @@ store_beep_reg(struct device *dev, const
 	return count;
 }
 
-#define sysfs_beep(REG, reg) \
-static ssize_t show_regs_beep_##reg (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_beep_##reg(dev, attr, buf); \
-} \
-static ssize_t \
-store_regs_beep_##reg (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_beep_reg(dev, buf, count, BEEP_##REG); \
-} \
-static DEVICE_ATTR(beep_##reg, S_IRUGO | S_IWUSR, \
-		  show_regs_beep_##reg, store_regs_beep_##reg);
-
-sysfs_beep(ENABLE, enable);
-sysfs_beep(MASK, mask);
+static ssize_t show_beep_enable(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n",
+		      (long)BEEP_ENABLE_FROM_REG(data->beep_enable));
+}
+static ssize_t store_beep_enable(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	return store_beep_reg(dev, buf, count, BEEP_ENABLE);
+}
+static DEVICE_ATTR(beep_enable, S_IRUGO | S_IWUSR, \
+		   show_beep_enable, store_beep_enable);
 
-static ssize_t
-show_fan_div_reg(struct device *dev, char *buf, int nr)
+static ssize_t show_beep_mask(struct device *dev,
+			      struct device_attribute *attr, char *buf)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	return sprintf(buf, "%ld\n",
-		       (long) DIV_FROM_REG(data->fan_div[nr - 1]));
+		      (long)BEEP_MASK_FROM_REG(data->beep_mask));

drivers/hwmon/w83627hf.c store_beep_reg(struct device *dev, const · line 817

Quoted 10 lines
drivers/hwmon/w83627hf.c +8 -0
+}
+static ssize_t store_beep_mask(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	return store_beep_reg(dev, buf, count, BEEP_MASK);
 }
+static DEVICE_ATTR(beep_mask, S_IRUGO | S_IWUSR, \
+		  show_beep_mask, store_beep_mask);
 

This change on beep_mask and beep_enable doesn't help at all. You are simply expanding the macros, there's no benefit in doing this. There would be an interest if you were using the index value to share the callbacks between both sysfs files, but you don't. So please either revert this change, or make use of the index to actually share some code.

Quoted 13 lines
drivers/hwmon/w83627hf.c +2 -2
 /* Note: we save and restore the fan minimum here, because its value is
    determined in part by the fan divisor.  This follows the principle of
    least surprise; the user doesn't expect the fan minimum to change just
    because the divisor changed. */
-static ssize_t
-store_fan_div_reg(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t store_fan_div_reg(struct device *dev,
+				 const char *buf, size_t count, int nr)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	unsigned long min;
@@ -818,10 +835,12 @@ store_fan_div_reg(struct device *dev, co
 

drivers/hwmon/w83627hf.c store_fan_div_reg(struct device *dev, co · line 843

Quoted 10 lines
drivers/hwmon/w83627hf.c +4 -2
 	data->fan_div[nr] = DIV_TO_REG(val);
 
-	reg = (w83627hf_read_value(data, nr=2 ? W83781D_REG_PIN : W83781D_REG_VID_FANDIV)
+	reg = (w83627hf_read_value(data, nr = 2 ?
+			W83781D_REG_PIN : W83781D_REG_VID_FANDIV)
 	       & (nr=0 ? 0xcf : 0x3f))
 	    | ((data->fan_div[nr] & 0x03) << (nr=0 ? 4 : 6));
-	w83627hf_write_value(data, nr=2 ? W83781D_REG_PIN : W83781D_REG_VID_FANDIV, reg);
+	w83627hf_write_value(data, nr = 2 ?
+			W83781D_REG_PIN : W83781D_REG_VID_FANDIV, reg);

You have no reason to touch these lines.

Quoted 33 lines
drivers/hwmon/w83627hf.c +3 -21
 	reg = (w83627hf_read_value(data, W83781D_REG_VBAT)
 	       & ~(1 << (5 + nr)))
@@ -836,94 +855,85 @@ store_fan_div_reg(struct device *dev, co
 	return count;
 }
 
-#define sysfs_fan_div(offset) \
-static ssize_t show_regs_fan_div_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_fan_div_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_fan_div_##offset (struct device *dev, struct device_attribute *attr, \
-			    const char *buf, size_t count) \
-{ \
-	return store_fan_div_reg(dev, buf, count, offset - 1); \
-} \
-static DEVICE_ATTR(fan##offset##_div, S_IRUGO | S_IWUSR, \
-		  show_regs_fan_div_##offset, store_regs_fan_div_##offset);
-
-sysfs_fan_div(1);
-sysfs_fan_div(2);
-sysfs_fan_div(3);
-
-static ssize_t
-show_pwm_reg(struct device *dev, char *buf, int nr)
+static ssize_t show_fan_div(struct device *dev,
+			    struct device_attribute *devattr, char *buf)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return sprintf(buf, "%ld\n", (long) data->pwm[nr - 1]);

drivers/hwmon/w83627hf.c store_fan_div_reg(struct device *dev, co · line 872

Quoted 10 lines
drivers/hwmon/w83627hf.c +9 -0
+	return sprintf(buf, "%ld\n",
+		       (long) DIV_FROM_REG(data->fan_div[nr]));
 }
+static ssize_t store_fan_div(struct device *dev,
+			     struct device_attribute *devattr,
+			     const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	return store_fan_div_reg(dev, buf, count, nr);
+}

Your cleanup is insufficient. You should move all the code from store_fan_div_reg directly here.

Quoted 42 lines
drivers/hwmon/w83627hf.c +21 -8
+
+static SENSOR_DEVICE_ATTR(fan1_div, S_IRUGO | S_IWUSR,
+			  show_fan_div, store_fan_div, 0);
+static SENSOR_DEVICE_ATTR(fan2_div, S_IRUGO | S_IWUSR,
+			  show_fan_div, store_fan_div, 1);
+static SENSOR_DEVICE_ATTR(fan3_div, S_IRUGO | S_IWUSR,
+			  show_fan_div, store_fan_div, 2);
 
-static ssize_t
-store_pwm_reg(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t show_pwm(struct device *dev,
+			struct device_attribute *devattr, char *buf)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	return sprintf(buf, "%ld\n", (long) data->pwm[nr-1]);
+}
+static ssize_t store_pwm(struct device *dev,
+			 struct device_attribute *devattr,
+			 const char *buf, size_t count)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	u32 val;
-
-	val = simple_strtoul(buf, NULL, 10);
+	u32 val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
 
 	if (data->type = w83627thf) {
 		/* bits 0-3 are reserved  in 627THF */
-		data->pwm[nr - 1] = PWM_TO_REG(val) & 0xf0;
+		data->pwm[nr-1] = PWM_TO_REG(val) & 0xf0;
 		w83627hf_write_value(data,
 				     W836X7HF_REG_PWM(data->type, nr),
-				     data->pwm[nr - 1] |
+				     data->pwm[nr-1] |
 				     (w83627hf_read_value(data,
 				     W836X7HF_REG_PWM(data->type, nr)) & 0x0f));
 	} else {
-		data->pwm[nr - 1] = PWM_TO_REG(val);

drivers/hwmon/w83627hf.c store_fan_div_reg(struct device *dev, co · line 915

Quoted 10 lines
drivers/hwmon/w83627hf.c +2 -1
+		data->pwm[nr-1] = PWM_TO_REG(val);
 		w83627hf_write_value(data,
 				     W836X7HF_REG_PWM(data->type, nr),
-				     data->pwm[nr - 1]);
+				     data->pwm[nr-1]);
 	}
 
 	mutex_unlock(&data->update_lock);
 	return count;
 }

All the changes inside this function are cosmetic and should be reverted.

Quoted 35 lines
drivers/hwmon/w83627hf.c +5 -21
-#define sysfs_pwm(offset) \
-static ssize_t show_regs_pwm_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-	return show_pwm_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_pwm_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_pwm_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR, \
-		  show_regs_pwm_##offset, store_regs_pwm_##offset);
-
-sysfs_pwm(1);
-sysfs_pwm(2);
-sysfs_pwm(3);
-
-static ssize_t
-show_pwm_freq_reg(struct device *dev, char *buf, int nr)
+static ssize_t show_pwm_freq(struct device *dev,
+			     struct device_attribute *devattr, char *buf)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = w83627hf_update_device(dev);
 	if (data->type = w83627hf)
 		return sprintf(buf, "%ld\n",
-			pwm_freq_from_reg_627hf(data->pwm_freq[nr - 1]));
+			pwm_freq_from_reg_627hf(data->pwm_freq[nr-1]));
 	else
 		return sprintf(buf, "%ld\n",
-			pwm_freq_from_reg(data->pwm_freq[nr - 1]));
+			pwm_freq_from_reg(data->pwm_freq[nr-1]));
 }
 

drivers/hwmon/w83627hf.c store_fan_div_reg(struct device *dev, co · line 935

Quoted 10 lines
drivers/hwmon/w83627hf.c +5 -3
-static ssize_t
-store_pwm_freq_reg(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t store_pwm_freq(struct device *dev,
+			      struct device_attribute *devattr,
+			      const char *buf, size_t count)
 {
+	int nr = to_sensor_dev_attr(devattr)->index;
 	struct w83627hf_data *data = dev_get_drvdata(dev);
-	static const u8 mask[]={0xF8, 0x8F};
+	static const u8 mask[] = {0xF8, 0x8F};

Revert.

Quoted 41 lines
drivers/hwmon/w83627hf.c +6 -19
 	u32 val;
 
 	val = simple_strtoul(buf, NULL, 10);
@@ -931,49 +941,39 @@ store_pwm_freq_reg(struct device *dev, c
 	mutex_lock(&data->update_lock);
 
 	if (data->type = w83627hf) {
-		data->pwm_freq[nr - 1] = pwm_freq_to_reg_627hf(val);
+		data->pwm_freq[nr-1] = pwm_freq_to_reg_627hf(val);
 		w83627hf_write_value(data, W83627HF_REG_PWM_FREQ,
-				(data->pwm_freq[nr - 1] << ((nr - 1)*4)) |
+				(data->pwm_freq[nr-1] << ((nr-1)*4)) |
 				(w83627hf_read_value(data,
-				W83627HF_REG_PWM_FREQ) & mask[nr - 1]));
+				W83627HF_REG_PWM_FREQ) & mask[nr-1]));
 	} else {
-		data->pwm_freq[nr - 1] = pwm_freq_to_reg(val);
-		w83627hf_write_value(data, W83637HF_REG_PWM_FREQ[nr - 1],
-				data->pwm_freq[nr - 1]);
+		data->pwm_freq[nr-1] = pwm_freq_to_reg(val);
+		w83627hf_write_value(data, W83637HF_REG_PWM_FREQ[nr-1],
+				data->pwm_freq[nr-1]);
 	}
 
 	mutex_unlock(&data->update_lock);
 	return count;
 }
 
-#define sysfs_pwm_freq(offset) \
-static ssize_t show_regs_pwm_freq_##offset(struct device *dev, \
-		struct device_attribute *attr, char *buf) \
-{ \
-	return show_pwm_freq_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_pwm_freq_##offset(struct device *dev, \
-		struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-	return store_pwm_freq_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR, \

drivers/hwmon/w83627hf.c store_pwm_freq_reg(struct device *dev, c · line 963

Quoted 10 lines
drivers/hwmon/w83627hf.c +5 -5
-		  show_regs_pwm_freq_##offset, store_regs_pwm_freq_##offset);
-
-sysfs_pwm_freq(1);
-sysfs_pwm_freq(2);
-sysfs_pwm_freq(3);
+#define pwm_decl(offset)	\
+static SENSOR_DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR,		\
+			  show_pwm, store_pwm, offset);			\
+static SENSOR_DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR,	\
+			  show_pwm_freq, store_pwm_freq, offset);

Inside the callbacks, you always use "nr - 1", so you could pass "offset - 1" here, and simplify the code inside the callbacks, same you did for fan callbacks.

Quoted 46 lines
drivers/hwmon/w83627hf.c +14 -11
+
+pwm_decl(1);
+pwm_decl(2);
+pwm_decl(3);
 
-static ssize_t
-show_sensor_reg(struct device *dev, char *buf, int nr)
+static ssize_t show_sensor_reg(struct device *dev, char *buf, int nr)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
-	return sprintf(buf, "%ld\n", (long) data->sens[nr - 1]);
+	return sprintf(buf, "%ld\n", (long) data->sens[nr-1]);
 }
 
-static ssize_t
-store_sensor_reg(struct device *dev, const char *buf, size_t count, int nr)
+static ssize_t store_sensor_reg(struct device *dev, const char *buf,
+				size_t count, int nr)
 {
 	struct w83627hf_data *data = dev_get_drvdata(dev);
 	u32 val, tmp;
@@ -986,26 +986,26 @@ store_sensor_reg(struct device *dev, con
 	case 1:		/* PII/Celeron diode */
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG1);
 		w83627hf_write_value(data, W83781D_REG_SCFG1,
-				    tmp | BIT_SCFG1[nr - 1]);
+				    tmp | BIT_SCFG1[nr-1]);
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG2);
 		w83627hf_write_value(data, W83781D_REG_SCFG2,
-				    tmp | BIT_SCFG2[nr - 1]);
-		data->sens[nr - 1] = val;
+				    tmp | BIT_SCFG2[nr-1]);
+		data->sens[nr-1] = val;
 		break;
 	case 2:		/* 3904 */
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG1);
 		w83627hf_write_value(data, W83781D_REG_SCFG1,
-				    tmp | BIT_SCFG1[nr - 1]);
+				    tmp | BIT_SCFG1[nr-1]);
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG2);
 		w83627hf_write_value(data, W83781D_REG_SCFG2,
-				    tmp & ~BIT_SCFG2[nr - 1]);
-		data->sens[nr - 1] = val;
+				    tmp & ~BIT_SCFG2[nr-1]);
+		data->sens[nr-1] = val;
 		break;

drivers/hwmon/w83627hf.c store_sensor_reg(struct device *dev, con · line 1011

Quoted 10 lines
drivers/hwmon/w83627hf.c +2 -2
 	case W83781D_DEFAULT_BETA:	/* thermistor */
 		tmp = w83627hf_read_value(data, W83781D_REG_SCFG1);
 		w83627hf_write_value(data, W83781D_REG_SCFG1,
-				    tmp & ~BIT_SCFG1[nr - 1]);
-		data->sens[nr - 1] = val;
+				    tmp & ~BIT_SCFG1[nr-1]);
+		data->sens[nr-1] = val;
 		break;
 	default:
 		dev_err(dev,

Either revert all these, of change them to just "nr" and adjust the sysfs file declarations accordingly.

Quoted 23 lines
drivers/hwmon/w83627hf.c +3 -16
@@ -1018,22 +1018,27 @@ store_sensor_reg(struct device *dev, con
 	return count;
 }
 
-#define sysfs_sensor(offset) \
-static ssize_t show_regs_sensor_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
-{ \
-    return show_sensor_reg(dev, buf, offset); \
-} \
-static ssize_t \
-store_regs_sensor_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
-{ \
-    return store_sensor_reg(dev, buf, count, offset); \
-} \
-static DEVICE_ATTR(temp##offset##_type, S_IRUGO | S_IWUSR, \
-		  show_regs_sensor_##offset, store_regs_sensor_##offset);
-
-sysfs_sensor(1);
-sysfs_sensor(2);
-sysfs_sensor(3);
+static ssize_t show_sensor(struct device *dev,
+			   struct device_attribute *devattr, char *buf)
+{

drivers/hwmon/w83627hf.c store_sensor_reg(struct device *dev, con · line 1033

Quoted 10 lines
drivers/hwmon/w83627hf.c +10 -0
+	int nr = to_sensor_dev_attr(devattr)->index;
+	return show_sensor_reg(dev, buf, nr);
+}
+static ssize_t store_sensor(struct device *dev,
+			    struct device_attribute *devattr,
+			    const char *buf, size_t count)
+{
+	int nr = to_sensor_dev_attr(devattr)->index;
+	return store_sensor_reg(dev, buf, count, nr);
+}

Here again, you should move all the code from store_sensor_reg directly inside this function. You no longer need a wrapper.

drivers/hwmon/w83627hf.c store_sensor_reg(struct device *dev, con · line 933

Quoted 8 lines
drivers/hwmon/w83627hf.c +8 -0
+
+#define sensor_decl(offset) \
+static SENSOR_DEVICE_ATTR(temp##offset##_type, S_IRUGO | S_IWUSR, \
+			  show_sensor, store_sensor, offset);
+
+sensor_decl(1);
+sensor_decl(2);
+sensor_decl(3);

BTW, your patch would be somewhat smaller if you weren't renaming these macros for no good reason I can see.

Quoted 12 lines
drivers/hwmon/w83627hf.c +2 -2
 static ssize_t show_name(struct device *dev, struct device_attribute
 			 *devattr, char *buf)
@@ -1098,14 +1103,15 @@ static int __init w83627hf_find(int sioa
 	       superio_inb(WINB_BASE_REG + 1);
 	*addr = val & WINB_ALIGNMENT;
 	if (*addr = 0) {
-		printk(KERN_WARNING DRVNAME ": Base address not set, "
-		       "skipping\n");
+		printk(KERN_WARNING DRVNAME
+		       ": Base address not set, skipping\n");
 		goto exit;

drivers/hwmon/w83627hf.c static int __init w83627hf_find(int sioa · line 1117

Quoted 10 lines
drivers/hwmon/w83627hf.c +2 -1
 	}
 
 	val = superio_inb(WINB_ACT_REG);
 	if (!(val & 0x01)) {
-		printk(KERN_WARNING DRVNAME ": Enabling HWM logical device\n");
+		printk(KERN_WARNING DRVNAME
+		       ": Enabling HWM logical device\n");
 		superio_outb(WINB_ACT_REG, val | 0x01);
 	}
 

Revert.

Quoted 134 lines
drivers/hwmon/w83627hf.c +42 -65
@@ -1118,48 +1124,43 @@ static int __init w83627hf_find(int sioa
 	return err;
 }
 
+#define VIN_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_in##X##_input.dev_attr.attr,	    \
+	&sensor_dev_attr_in##X##_min.dev_attr.attr,	    \
+	&sensor_dev_attr_in##X##_max.dev_attr.attr
+
+#define FAN_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_fan##X##_input.dev_attr.attr,	    \
+	&sensor_dev_attr_fan##X##_min.dev_attr.attr,	    \
+	&sensor_dev_attr_fan##X##_div.dev_attr.attr
+
+#define TEMP_UNIT_ATTRS(X)	\
+	&sensor_dev_attr_temp##X##_input.dev_attr.attr,		\
+	&sensor_dev_attr_temp##X##_max.dev_attr.attr,		\
+	&sensor_dev_attr_temp##X##_max_hyst.dev_attr.attr,	\
+	&sensor_dev_attr_temp##X##_type.dev_attr.attr
+
 static struct attribute *w83627hf_attributes[] = {
 	&dev_attr_in0_input.attr,
 	&dev_attr_in0_min.attr,
 	&dev_attr_in0_max.attr,
-	&dev_attr_in2_input.attr,
-	&dev_attr_in2_min.attr,
-	&dev_attr_in2_max.attr,
-	&dev_attr_in3_input.attr,
-	&dev_attr_in3_min.attr,
-	&dev_attr_in3_max.attr,
-	&dev_attr_in4_input.attr,
-	&dev_attr_in4_min.attr,
-	&dev_attr_in4_max.attr,
-	&dev_attr_in7_input.attr,
-	&dev_attr_in7_min.attr,
-	&dev_attr_in7_max.attr,
-	&dev_attr_in8_input.attr,
-	&dev_attr_in8_min.attr,
-	&dev_attr_in8_max.attr,
-
-	&dev_attr_fan1_input.attr,
-	&dev_attr_fan1_min.attr,
-	&dev_attr_fan1_div.attr,
-	&dev_attr_fan2_input.attr,
-	&dev_attr_fan2_min.attr,
-	&dev_attr_fan2_div.attr,
-
-	&dev_attr_temp1_input.attr,
-	&dev_attr_temp1_max.attr,
-	&dev_attr_temp1_max_hyst.attr,
-	&dev_attr_temp1_type.attr,
-	&dev_attr_temp2_input.attr,
-	&dev_attr_temp2_max.attr,
-	&dev_attr_temp2_max_hyst.attr,
-	&dev_attr_temp2_type.attr,
+
+	VIN_UNIT_ATTRS(2),
+	VIN_UNIT_ATTRS(3),
+	VIN_UNIT_ATTRS(4),
+	VIN_UNIT_ATTRS(7),
+	VIN_UNIT_ATTRS(8),
+	FAN_UNIT_ATTRS(1),
+	FAN_UNIT_ATTRS(2),
+	TEMP_UNIT_ATTRS(1),
+	TEMP_UNIT_ATTRS(2),
 
 	&dev_attr_alarms.attr,
 	&dev_attr_beep_enable.attr,
 	&dev_attr_beep_mask.attr,
 
-	&dev_attr_pwm1.attr,
-	&dev_attr_pwm2.attr,
+	&sensor_dev_attr_pwm1.dev_attr.attr,
+	&sensor_dev_attr_pwm2.dev_attr.attr,
 
 	&dev_attr_name.attr,
 	NULL
@@ -1170,30 +1171,17 @@ static const struct attribute_group w836
 };
 
 static struct attribute *w83627hf_attributes_opt[] = {
-	&dev_attr_in1_input.attr,
-	&dev_attr_in1_min.attr,
-	&dev_attr_in1_max.attr,
-	&dev_attr_in5_input.attr,
-	&dev_attr_in5_min.attr,
-	&dev_attr_in5_max.attr,
-	&dev_attr_in6_input.attr,
-	&dev_attr_in6_min.attr,
-	&dev_attr_in6_max.attr,
-
-	&dev_attr_fan3_input.attr,
-	&dev_attr_fan3_min.attr,
-	&dev_attr_fan3_div.attr,
-
-	&dev_attr_temp3_input.attr,
-	&dev_attr_temp3_max.attr,
-	&dev_attr_temp3_max_hyst.attr,
-	&dev_attr_temp3_type.attr,
-
-	&dev_attr_pwm3.attr,
-
-	&dev_attr_pwm1_freq.attr,
-	&dev_attr_pwm2_freq.attr,
-	&dev_attr_pwm3_freq.attr,
+	VIN_UNIT_ATTRS(1),
+	VIN_UNIT_ATTRS(5),
+	VIN_UNIT_ATTRS(6),
+	FAN_UNIT_ATTRS(3),
+	TEMP_UNIT_ATTRS(3),
+
+	&sensor_dev_attr_pwm3.dev_attr.attr,
+
+	&sensor_dev_attr_pwm1_freq.dev_attr.attr,
+	&sensor_dev_attr_pwm2_freq.dev_attr.attr,
+	&sensor_dev_attr_pwm3_freq.dev_attr.attr,
 	NULL
 };
 
@@ -1251,27 +1239,46 @@ static int __devinit w83627hf_probe(stru
 
 	/* Register chip-specific device attributes */
 	if (data->type = w83627hf || data->type = w83697hf)
-		if ((err = device_create_file(dev, &dev_attr_in5_input))
-		 || (err = device_create_file(dev, &dev_attr_in5_min))
-		 || (err = device_create_file(dev, &dev_attr_in5_max))
-		 || (err = device_create_file(dev, &dev_attr_in6_input))
-		 || (err = device_create_file(dev, &dev_attr_in6_min))
-		 || (err = device_create_file(dev, &dev_attr_in6_max))
-		 || (err = device_create_file(dev, &dev_attr_pwm1_freq))
-		 || (err = device_create_file(dev, &dev_attr_pwm2_freq)))
+		if ((err = device_create_file(dev,
+				&sensor_dev_attr_in5_input.dev_attr))
+		 || (err = device_create_file(dev,

drivers/hwmon/w83627hf.c static int __devinit w83627hf_probe(stru · line 1254

Quoted 10 lines
drivers/hwmon/w83627hf.c +10 -0
+				&sensor_dev_attr_in5_min.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in5_max.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in6_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in6_min.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in6_max.dev_attr))
+		 /* tbd */

What does this comment mean?

Quoted 52 lines
drivers/hwmon/w83627hf.c +26 -13
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_pwm1_freq.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_pwm2_freq.dev_attr)))
 			goto ERROR4;
 
 	if (data->type != w83697hf)
-		if ((err = device_create_file(dev, &dev_attr_in1_input))
-		 || (err = device_create_file(dev, &dev_attr_in1_min))
-		 || (err = device_create_file(dev, &dev_attr_in1_max))
-		 || (err = device_create_file(dev, &dev_attr_fan3_input))
-		 || (err = device_create_file(dev, &dev_attr_fan3_min))
-		 || (err = device_create_file(dev, &dev_attr_fan3_div))
-		 || (err = device_create_file(dev, &dev_attr_temp3_input))
-		 || (err = device_create_file(dev, &dev_attr_temp3_max))
-		 || (err = device_create_file(dev, &dev_attr_temp3_max_hyst))
-		 || (err = device_create_file(dev, &dev_attr_temp3_type)))
+		if ((err = device_create_file(dev,
+				&sensor_dev_attr_in1_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in1_min.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_in1_max.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_fan3_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_fan3_min.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_fan3_div.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_temp3_input.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_temp3_max.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_temp3_max_hyst.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_temp3_type.dev_attr)))
 			goto ERROR4;
 
 	if (data->type != w83697hf && data->vid != 0xff) {
@@ -1285,13 +1292,17 @@ static int __devinit w83627hf_probe(stru
 
 	if (data->type = w83627thf || data->type = w83637hf
 	 || data->type = w83687thf)
-		if ((err = device_create_file(dev, &dev_attr_pwm3)))
+		if ((err = device_create_file(
+				dev, &sensor_dev_attr_pwm3.dev_attr)))
 			goto ERROR4;
 
 	if (data->type = w83637hf || data->type = w83687thf)
-		if ((err = device_create_file(dev, &dev_attr_pwm1_freq))
-		 || (err = device_create_file(dev, &dev_attr_pwm2_freq))

drivers/hwmon/w83627hf.c static int __devinit w83627hf_probe(stru · line 1308

Quoted 10 lines
drivers/hwmon/w83627hf.c +6 -1
-		 || (err = device_create_file(dev, &dev_attr_pwm3_freq)))
+		if ((err = device_create_file(dev,
+				&sensor_dev_attr_pwm1_freq.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_pwm1_freq.dev_attr))
+		 || (err = device_create_file(dev,
+				&sensor_dev_attr_pwm1_freq.dev_attr)))
 			goto ERROR4;
 
 	data->class_dev = hwmon_device_register(dev);

Please provide an updated patch.

Thanks,

Signature · 6 lines
Jean Delvare

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors