[RFC PATCH 2.6.18-rc4-mm1] hwmon: unchecked return status fixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all:

Added w83627hf, which should demonstrate how to do all the remaining
hwmon drivers.  There is more (ab)use of sysfs_remove_group() here to
make life easier.

This patch was lightly tested against w83627thf.  Testing on the other
variants supported by that driver would be nice.

I'll add lm75, lm78, and smsc47b397 to this patch as I have time.
Jean: don't apply yet.

---

This patch fixes up some hwmon drivers so that they no longer ignore
return status from device_create_file().

Signed-off-by: Mark M. Hoffman <mhoffman at lightlink.com>

---
 drivers/hwmon/asb100.c   |  122 +++++++++++++----------
 drivers/hwmon/w83627hf.c |  245 +++++++++++++++++++++++++++++------------------
 2 files changed, 223 insertions(+), 144 deletions(-)

--- linux-2.6.18-rc4-mm1.orig/drivers/hwmon/asb100.c
+++ linux-2.6.18-rc4-mm1/drivers/hwmon/asb100.c
@@ -298,12 +298,6 @@ sysfs_in(4);
 sysfs_in(5);
 sysfs_in(6);
 
-#define device_create_file_in(client, offset) do { \
-	device_create_file(&client->dev, &dev_attr_in##offset##_input); \
-	device_create_file(&client->dev, &dev_attr_in##offset##_min); \
-	device_create_file(&client->dev, &dev_attr_in##offset##_max); \
-} while (0)
-
 /* 3 Fans */
 static ssize_t show_fan(struct device *dev, char *buf, int nr)
 {
@@ -421,12 +415,6 @@ sysfs_fan(1);
 sysfs_fan(2);
 sysfs_fan(3);
 
-#define device_create_file_fan(client, offset) do { \
-	device_create_file(&client->dev, &dev_attr_fan##offset##_input); \
-	device_create_file(&client->dev, &dev_attr_fan##offset##_min); \
-	device_create_file(&client->dev, &dev_attr_fan##offset##_div); \
-} while (0)
-
 /* 4 Temp. Sensors */
 static int sprintf_temp_from_reg(u16 reg, char *buf, int nr)
 {
@@ -515,12 +503,6 @@ sysfs_temp(3);
 sysfs_temp(4);
 
 /* VID */
-#define device_create_file_temp(client, num) do { \
-	device_create_file(&client->dev, &dev_attr_temp##num##_input); \
-	device_create_file(&client->dev, &dev_attr_temp##num##_max); \
-	device_create_file(&client->dev, &dev_attr_temp##num##_max_hyst); \
-} while (0)
-
 static ssize_t show_vid(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct asb100_data *data = asb100_update_device(dev);
@@ -528,8 +510,6 @@ static ssize_t show_vid(struct device *d
 }
 
 static DEVICE_ATTR(cpu0_vid, S_IRUGO, show_vid, NULL);
-#define device_create_file_vid(client) \
-device_create_file(&client->dev, &dev_attr_cpu0_vid)
 
 /* VRM */
 static ssize_t show_vrm(struct device *dev, struct device_attribute *attr, char *buf)
@@ -549,8 +529,6 @@ static ssize_t set_vrm(struct device *de
 
 /* Alarms */
 static DEVICE_ATTR(vrm, S_IRUGO | S_IWUSR, show_vrm, set_vrm);
-#define device_create_file_vrm(client) \
-device_create_file(&client->dev, &dev_attr_vrm);
 
 static ssize_t show_alarms(struct device *dev, struct device_attribute *attr, char *buf)
 {
@@ -559,8 +537,6 @@ static ssize_t show_alarms(struct device
 }
 
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
-#define device_create_file_alarms(client) \
-device_create_file(&client->dev, &dev_attr_alarms)
 
 /* 1 PWM */
 static ssize_t show_pwm1(struct device *dev, struct device_attribute *attr, char *buf)
@@ -607,10 +583,65 @@ static ssize_t set_pwm_enable1(struct de
 static DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm1, set_pwm1);
 static DEVICE_ATTR(pwm1_enable, S_IRUGO | S_IWUSR,
 		show_pwm_enable1, set_pwm_enable1);
-#define device_create_file_pwm1(client) do { \
-	device_create_file(&new_client->dev, &dev_attr_pwm1); \
-	device_create_file(&new_client->dev, &dev_attr_pwm1_enable); \
-} while (0)
+
+static struct attribute *asb100_attributes[] = {
+	&dev_attr_in0_input.attr,
+	&dev_attr_in0_min.attr,
+	&dev_attr_in0_max.attr,
+	&dev_attr_in1_input.attr,
+	&dev_attr_in1_min.attr,
+	&dev_attr_in1_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_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_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_fan3_input.attr,
+	&dev_attr_fan3_min.attr,
+	&dev_attr_fan3_div.attr,
+
+	&dev_attr_temp1_input.attr,
+	&dev_attr_temp1_max.attr,
+	&dev_attr_temp1_max_hyst.attr,
+	&dev_attr_temp2_input.attr,
+	&dev_attr_temp2_max.attr,
+	&dev_attr_temp2_max_hyst.attr,
+	&dev_attr_temp3_input.attr,
+	&dev_attr_temp3_max.attr,
+	&dev_attr_temp3_max_hyst.attr,
+	&dev_attr_temp4_input.attr,
+	&dev_attr_temp4_max.attr,
+	&dev_attr_temp4_max_hyst.attr,
+
+	&dev_attr_cpu0_vid.attr,
+	&dev_attr_vrm.attr,
+	&dev_attr_alarms.attr,
+	&dev_attr_pwm1.attr,
+	&dev_attr_pwm1_enable.attr,
+
+	NULL,
+};
+
+static struct attribute_group asb100_group = {
+	.attrs = asb100_attributes,
+};
 
 /* This function is called when:
 	asb100_driver is inserted (when this module is loaded), for each
@@ -810,38 +841,19 @@ static int asb100_detect(struct i2c_adap
 	data->fan_min[2] = asb100_read_value(new_client, ASB100_REG_FAN_MIN(2));
 
 	/* Register sysfs hooks */
+	if ((err = sysfs_create_group(&new_client->dev.kobj, &asb100_group)))
+		goto ERROR3;
+
 	data->class_dev = hwmon_device_register(&new_client->dev);
 	if (IS_ERR(data->class_dev)) {
 		err = PTR_ERR(data->class_dev);
-		goto ERROR3;
+		goto ERROR4;
 	}
 
-	device_create_file_in(new_client, 0);
-	device_create_file_in(new_client, 1);
-	device_create_file_in(new_client, 2);
-	device_create_file_in(new_client, 3);
-	device_create_file_in(new_client, 4);
-	device_create_file_in(new_client, 5);
-	device_create_file_in(new_client, 6);
-
-	device_create_file_fan(new_client, 1);
-	device_create_file_fan(new_client, 2);
-	device_create_file_fan(new_client, 3);
-
-	device_create_file_temp(new_client, 1);
-	device_create_file_temp(new_client, 2);
-	device_create_file_temp(new_client, 3);
-	device_create_file_temp(new_client, 4);
-
-	device_create_file_vid(new_client);
-	device_create_file_vrm(new_client);
-
-	device_create_file_alarms(new_client);
-
-	device_create_file_pwm1(new_client);
-
 	return 0;
 
+ERROR4:
+	sysfs_remove_group(&new_client->dev.kobj, &asb100_group);
 ERROR3:
 	i2c_detach_client(data->lm75[1]);
 	i2c_detach_client(data->lm75[0]);
@@ -861,8 +873,10 @@ static int asb100_detach_client(struct i
 	int err;
 
 	/* main client */
-	if (data)
+	if (data) {
 		hwmon_device_unregister(data->class_dev);
+		sysfs_remove_group(&client->dev.kobj, &asb100_group);
+	}
 
 	if ((err = i2c_detach_client(client)))
 		return err;
--- linux-2.6.18-rc4-mm1.orig/drivers/hwmon/w83627hf.c
+++ linux-2.6.18-rc4-mm1/drivers/hwmon/w83627hf.c
@@ -511,13 +511,6 @@ static DEVICE_ATTR(in0_min, S_IRUGO | S_
 static DEVICE_ATTR(in0_max, S_IRUGO | S_IWUSR,
 	show_regs_in_max0, store_regs_in_max0);
 
-#define device_create_file_in(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_in##offset##_input); \
-device_create_file(&client->dev, &dev_attr_in##offset##_min); \
-device_create_file(&client->dev, &dev_attr_in##offset##_max); \
-} while (0)
-
 #define show_fan_reg(reg) \
 static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
 { \
@@ -575,12 +568,6 @@ sysfs_fan_min_offset(2);
 sysfs_fan_offset(3);
 sysfs_fan_min_offset(3);
 
-#define device_create_file_fan(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_fan##offset##_input); \
-device_create_file(&client->dev, &dev_attr_fan##offset##_min); \
-} while (0)
-
 #define show_temp_reg(reg) \
 static ssize_t show_##reg (struct device *dev, char *buf, int nr) \
 { \
@@ -655,13 +642,6 @@ sysfs_temp_offsets(1);
 sysfs_temp_offsets(2);
 sysfs_temp_offsets(3);
 
-#define device_create_file_temp(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_temp##offset##_input); \
-device_create_file(&client->dev, &dev_attr_temp##offset##_max); \
-device_create_file(&client->dev, &dev_attr_temp##offset##_max_hyst); \
-} while (0)
-
 static ssize_t
 show_vid_reg(struct device *dev, struct device_attribute *attr, char *buf)
 {
@@ -669,8 +649,6 @@ show_vid_reg(struct device *dev, struct 
 	return sprintf(buf, "%ld\n", (long) vid_from_reg(data->vid, data->vrm));
 }
 static DEVICE_ATTR(cpu0_vid, S_IRUGO, show_vid_reg, NULL);
-#define device_create_file_vid(client) \
-device_create_file(&client->dev, &dev_attr_cpu0_vid)
 
 static ssize_t
 show_vrm_reg(struct device *dev, struct device_attribute *attr, char *buf)
@@ -691,8 +669,6 @@ store_vrm_reg(struct device *dev, struct
 	return count;
 }
 static DEVICE_ATTR(vrm, S_IRUGO | S_IWUSR, show_vrm_reg, store_vrm_reg);
-#define device_create_file_vrm(client) \
-device_create_file(&client->dev, &dev_attr_vrm)
 
 static ssize_t
 show_alarms_reg(struct device *dev, struct device_attribute *attr, char *buf)
@@ -701,8 +677,6 @@ show_alarms_reg(struct device *dev, stru
 	return sprintf(buf, "%ld\n", (long) data->alarms);
 }
 static DEVICE_ATTR(alarms, S_IRUGO, show_alarms_reg, NULL);
-#define device_create_file_alarms(client) \
-device_create_file(&client->dev, &dev_attr_alarms)
 
 #define show_beep_reg(REG, reg) \
 static ssize_t show_beep_##reg (struct device *dev, struct device_attribute *attr, char *buf) \
@@ -765,12 +739,6 @@ static DEVICE_ATTR(beep_##reg, S_IRUGO |
 sysfs_beep(ENABLE, enable);
 sysfs_beep(MASK, mask);
 
-#define device_create_file_beep(client) \
-do { \
-device_create_file(&client->dev, &dev_attr_beep_enable); \
-device_create_file(&client->dev, &dev_attr_beep_mask); \
-} while (0)
-
 static ssize_t
 show_fan_div_reg(struct device *dev, char *buf, int nr)
 {
@@ -836,11 +804,6 @@ sysfs_fan_div(1);
 sysfs_fan_div(2);
 sysfs_fan_div(3);
 
-#define device_create_file_fan_div(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_fan##offset##_div); \
-} while (0)
-
 static ssize_t
 show_pwm_reg(struct device *dev, char *buf, int nr)
 {
@@ -895,11 +858,6 @@ sysfs_pwm(1);
 sysfs_pwm(2);
 sysfs_pwm(3);
 
-#define device_create_file_pwm(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_pwm##offset); \
-} while (0)
-
 static ssize_t
 show_sensor_reg(struct device *dev, char *buf, int nr)
 {
@@ -971,12 +929,6 @@ sysfs_sensor(1);
 sysfs_sensor(2);
 sysfs_sensor(3);
 
-#define device_create_file_sensor(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_temp##offset##_type); \
-} while (0)
-
-
 static int __init w83627hf_find(int sioaddr, unsigned short *addr)
 {
 	u16 val;
@@ -1008,6 +960,85 @@ static int __init w83627hf_find(int sioa
 	return 0;
 }
 
+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,
+
+	&dev_attr_alarms.attr,
+	&dev_attr_beep_enable.attr,
+	&dev_attr_beep_mask.attr,
+
+	&dev_attr_pwm1.attr,
+	&dev_attr_pwm2.attr,
+
+	NULL
+};
+
+static struct attribute_group w83627hf_group = {
+	.attrs = w83627hf_attributes,
+};
+
+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,
+
+	NULL
+};
+
+static struct attribute_group w83627hf_group_opt = {
+	.attrs = w83627hf_attributes_opt,
+};
+
 static int w83627hf_detect(struct i2c_adapter *adapter)
 {
 	int val, kind;
@@ -1107,62 +1138,93 @@ static int w83627hf_detect(struct i2c_ad
 	data->fan_min[1] = w83627hf_read_value(new_client, W83781D_REG_FAN_MIN(2));
 	data->fan_min[2] = w83627hf_read_value(new_client, W83781D_REG_FAN_MIN(3));
 
-	/* Register sysfs hooks */
-	data->class_dev = hwmon_device_register(&new_client->dev);
-	if (IS_ERR(data->class_dev)) {
-		err = PTR_ERR(data->class_dev);
+	/* Register common device attributes */
+	if ((err = sysfs_create_group(&new_client->dev.kobj, &w83627hf_group)))
 		goto ERROR3;
+
+	/* Register chip-specific device attributes */
+	if (kind != w83697hf) {
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in1_input)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in1_min)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in1_max)))
+			goto ERROR4;
 	}
 
-	device_create_file_in(new_client, 0);
-	if (kind != w83697hf)
-		device_create_file_in(new_client, 1);
-	device_create_file_in(new_client, 2);
-	device_create_file_in(new_client, 3);
-	device_create_file_in(new_client, 4);
 	if (kind == w83627hf || kind == w83697hf) {
-		device_create_file_in(new_client, 5);
-		device_create_file_in(new_client, 6);
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in5_input)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in5_min)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in5_max)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in6_input)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in6_min)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in6_max)))
+			goto ERROR4;
+	}
+
+	if (kind != w83697hf) {
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_fan3_input)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_fan3_min)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_fan3_div)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_temp3_input)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_temp3_max)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_temp3_max_hyst)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_temp3_type)))
+			goto ERROR4;
 	}
-	device_create_file_in(new_client, 7);
-	device_create_file_in(new_client, 8);
-
-	device_create_file_fan(new_client, 1);
-	device_create_file_fan(new_client, 2);
-	if (kind != w83697hf)
-		device_create_file_fan(new_client, 3);
-
-	device_create_file_temp(new_client, 1);
-	device_create_file_temp(new_client, 2);
-	if (kind != w83697hf)
-		device_create_file_temp(new_client, 3);
 
 	if (kind != w83697hf && data->vid != 0xff) {
-		device_create_file_vid(new_client);
-		device_create_file_vrm(new_client);
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_cpu0_vid)))
+			goto ERROR4;
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_vrm)))
+			goto ERROR4;
 	}
 
-	device_create_file_fan_div(new_client, 1);
-	device_create_file_fan_div(new_client, 2);
-	if (kind != w83697hf)
-		device_create_file_fan_div(new_client, 3);
-
-	device_create_file_alarms(new_client);
-
-	device_create_file_beep(new_client);
-
-	device_create_file_pwm(new_client, 1);
-	device_create_file_pwm(new_client, 2);
 	if (kind == w83627thf || kind == w83637hf || kind == w83687thf)
-		device_create_file_pwm(new_client, 3);
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_pwm3)))
+			goto ERROR4;
 
-	device_create_file_sensor(new_client, 1);
-	device_create_file_sensor(new_client, 2);
-	if (kind != w83697hf)
-		device_create_file_sensor(new_client, 3);
+	data->class_dev = hwmon_device_register(&new_client->dev);
+	if (IS_ERR(data->class_dev)) {
+		err = PTR_ERR(data->class_dev);
+		goto ERROR4;
+	}
 
 	return 0;
 
+      ERROR4:
+	sysfs_remove_group(&new_client->dev.kobj, &w83627hf_group);
+	sysfs_remove_group(&new_client->dev.kobj, &w83627hf_group_opt);
       ERROR3:
 	i2c_detach_client(new_client);
       ERROR2:
@@ -1180,6 +1242,9 @@ static int w83627hf_detach_client(struct
 
 	hwmon_device_unregister(data->class_dev);
 
+	sysfs_remove_group(&client->dev.kobj, &w83627hf_group);
+	sysfs_remove_group(&client->dev.kobj, &w83627hf_group_opt);
+
 	if ((err = i2c_detach_client(client)))
 		return err;
 
-- 
Mark M. Hoffman
mhoffman at lightlink.com





[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux