All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH 34/39] w83781d: Fix unchecked return status
@ 2006-09-28 22:48 Greg KH
  0 siblings, 0 replies; only message in thread
From: Greg KH @ 2006-09-28 22:48 UTC (permalink / raw)
  To: lm-sensors

From: Jim Cromie <jim.cromie at gmail.com>

w83781d: Fix unchecked return status

Add 2 attr-file groups (for base and model-specific attrs respectively),
create the base group with single call to sysfs_create_group,
check the return code on individual calls to device_create_file for each
of the model-specific attr-files.

Signed-off-by: Jim Cromie <jim.cromie at gmail.com>
Signed-off-by: Jean Delvare <khali at linux-fr.org>
Signed-off-by: Greg Kroah-Hartman <gregkh at suse.de>
---
 drivers/hwmon/w83781d.c |  277 ++++++++++++++++++++++++-----------------------
 1 files changed, 143 insertions(+), 134 deletions(-)

diff --git a/drivers/hwmon/w83781d.c b/drivers/hwmon/w83781d.c
index 95221b1..a4584ec 100644
--- a/drivers/hwmon/w83781d.c
+++ b/drivers/hwmon/w83781d.c
@@ -41,6 +41,7 @@ #include <linux/i2c.h>
 #include <linux/i2c-isa.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-vid.h>
+#include <linux/sysfs.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
 #include <asm/io.h>
@@ -360,13 +361,6 @@ sysfs_in_offsets(6);
 sysfs_in_offsets(7);
 sysfs_in_offsets(8);
 
-#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) \
 { \
@@ -421,12 +415,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) \
 { \
@@ -497,13 +485,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)
 {
@@ -511,10 +492,8 @@ 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 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)
 {
@@ -535,10 +514,8 @@ 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 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)
 {
@@ -546,10 +523,8 @@ show_alarms_reg(struct device *dev, stru
 	return sprintf(buf, "%u\n", 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);
+static DEVICE_ATTR(alarms, S_IRUGO, show_alarms_reg, NULL);
+
 static ssize_t show_beep_mask (struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct w83781d_data *data = w83781d_update_device(dev);
@@ -615,12 +590,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)
 {
@@ -686,11 +655,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)
 {
@@ -787,16 +751,6 @@ sysfs_pwmenable(2);		/* only PWM2 can be
 sysfs_pwm(3);
 sysfs_pwm(4);
 
-#define device_create_file_pwm(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_pwm##offset); \
-} while (0)
-
-#define device_create_file_pwmenable(client, offset) \
-do { \
-device_create_file(&client->dev, &dev_attr_pwm##offset##_enable); \
-} while (0)
-
 static ssize_t
 show_sensor_reg(struct device *dev, char *buf, int nr)
 {
@@ -865,11 +819,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)
-
 /* This function is called when:
      * w83781d_driver is inserted (when this module is loaded), for each
        available adapter
@@ -994,11 +943,69 @@ ERROR_SC_0:
 	return err;
 }
 
+#define IN_UNIT_ATTRS(X)			\
+	&dev_attr_in##X##_input.attr,		\
+	&dev_attr_in##X##_min.attr,		\
+	&dev_attr_in##X##_max.attr
+
+#define FAN_UNIT_ATTRS(X)			\
+	&dev_attr_fan##X##_input.attr,		\
+	&dev_attr_fan##X##_min.attr,		\
+	&dev_attr_fan##X##_div.attr
+
+#define TEMP_UNIT_ATTRS(X)			\
+	&dev_attr_temp##X##_input.attr,		\
+	&dev_attr_temp##X##_max.attr,		\
+	&dev_attr_temp##X##_max_hyst.attr
+
+static struct attribute* w83781d_attributes[] = {
+	IN_UNIT_ATTRS(0),
+	IN_UNIT_ATTRS(2),
+	IN_UNIT_ATTRS(3),
+	IN_UNIT_ATTRS(4),
+	IN_UNIT_ATTRS(5),
+	IN_UNIT_ATTRS(6),
+	FAN_UNIT_ATTRS(1),
+	FAN_UNIT_ATTRS(2),
+	FAN_UNIT_ATTRS(3),
+	TEMP_UNIT_ATTRS(1),
+	TEMP_UNIT_ATTRS(2),
+	&dev_attr_cpu0_vid.attr,
+	&dev_attr_vrm.attr,
+	&dev_attr_alarms.attr,
+	&dev_attr_beep_mask.attr,
+	&dev_attr_beep_enable.attr,
+	NULL
+};
+static const struct attribute_group w83781d_group = {
+	.attrs = w83781d_attributes,
+};
+
+static struct attribute *w83781d_attributes_opt[] = {
+	IN_UNIT_ATTRS(1),
+	IN_UNIT_ATTRS(7),
+	IN_UNIT_ATTRS(8),
+	TEMP_UNIT_ATTRS(3),
+	&dev_attr_pwm1.attr,
+	&dev_attr_pwm2.attr,
+	&dev_attr_pwm2_enable.attr,
+	&dev_attr_pwm3.attr,
+	&dev_attr_pwm4.attr,
+	&dev_attr_temp1_type.attr,
+	&dev_attr_temp2_type.attr,
+	&dev_attr_temp3_type.attr,
+	NULL
+};
+static const struct attribute_group w83781d_group_opt = {
+	.attrs = w83781d_attributes_opt,
+};
+
 static int
 w83781d_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	int i = 0, val1 = 0, val2;
-	struct i2c_client *new_client;
+	struct i2c_client *client;
+	struct device *dev;
 	struct w83781d_data *data;
 	int err;
 	const char *client_name = "";
@@ -1075,13 +1082,14 @@ #undef REALLY_SLOW_IO
 		goto ERROR1;
 	}
 
-	new_client = &data->client;
-	i2c_set_clientdata(new_client, data);
-	new_client->addr = address;
+	client = &data->client;
+	i2c_set_clientdata(client, data);
+	client->addr = address;
 	mutex_init(&data->lock);
-	new_client->adapter = adapter;
-	new_client->driver = is_isa ? &w83781d_isa_driver : &w83781d_driver;
-	new_client->flags = 0;
+	client->adapter = adapter;
+	client->driver = is_isa ? &w83781d_isa_driver : &w83781d_driver;
+	client->flags = 0;
+	dev = &client->dev;
 
 	/* Now, we do the remaining detection. */
 
@@ -1090,20 +1098,18 @@ #undef REALLY_SLOW_IO
 	   force_*=... parameter, and the Winbond will be reset to the right
 	   bank. */
 	if (kind < 0) {
-		if (w83781d_read_value(new_client, W83781D_REG_CONFIG) & 0x80) {
-			dev_dbg(&new_client->dev, "Detection failed at step "
-				"3\n");
+		if (w83781d_read_value(client, W83781D_REG_CONFIG) & 0x80) {
+			dev_dbg(dev, "Detection failed at step 3\n");
 			err = -ENODEV;
 			goto ERROR2;
 		}
-		val1 = w83781d_read_value(new_client, W83781D_REG_BANK);
-		val2 = w83781d_read_value(new_client, W83781D_REG_CHIPMAN);
+		val1 = w83781d_read_value(client, W83781D_REG_BANK);
+		val2 = w83781d_read_value(client, W83781D_REG_CHIPMAN);
 		/* Check for Winbond or Asus ID if in bank 0 */
 		if ((!(val1 & 0x07)) &&
 		    (((!(val1 & 0x80)) && (val2 != 0xa3) && (val2 != 0xc3))
 		     || ((val1 & 0x80) && (val2 != 0x5c) && (val2 != 0x12)))) {
-			dev_dbg(&new_client->dev, "Detection failed at step "
-				"4\n");
+			dev_dbg(dev, "Detection failed at step 4\n");
 			err = -ENODEV;
 			goto ERROR2;
 		}
@@ -1112,9 +1118,8 @@ #undef REALLY_SLOW_IO
 		if ((!is_isa) && (((!(val1 & 0x80)) && (val2 = 0xa3)) ||
 				  ((val1 & 0x80) && (val2 = 0x5c)))) {
 			if (w83781d_read_value
-			    (new_client, W83781D_REG_I2C_ADDR) != address) {
-				dev_dbg(&new_client->dev, "Detection failed "
-					"at step 5\n");
+			    (client, W83781D_REG_I2C_ADDR) != address) {
+				dev_dbg(dev, "Detection failed at step 5\n");
 				err = -ENODEV;
 				goto ERROR2;
 			}
@@ -1123,27 +1128,26 @@ #undef REALLY_SLOW_IO
 
 	/* We have either had a force parameter, or we have already detected the
 	   Winbond. Put it now into bank 0 and Vendor ID High Byte */
-	w83781d_write_value(new_client, W83781D_REG_BANK,
-			    (w83781d_read_value(new_client,
-						W83781D_REG_BANK) & 0x78) |
-			    0x80);
+	w83781d_write_value(client, W83781D_REG_BANK,
+			    (w83781d_read_value(client, W83781D_REG_BANK)
+			     & 0x78) | 0x80);
 
 	/* Determine the chip type. */
 	if (kind <= 0) {
 		/* get vendor ID */
-		val2 = w83781d_read_value(new_client, W83781D_REG_CHIPMAN);
+		val2 = w83781d_read_value(client, W83781D_REG_CHIPMAN);
 		if (val2 = 0x5c)
 			vendid = winbond;
 		else if (val2 = 0x12)
 			vendid = asus;
 		else {
-			dev_dbg(&new_client->dev, "Chip was made by neither "
+			dev_dbg(dev, "Chip was made by neither "
 				"Winbond nor Asus?\n");
 			err = -ENODEV;
 			goto ERROR2;
 		}
 
-		val1 = w83781d_read_value(new_client, W83781D_REG_WCHIPID);
+		val1 = w83781d_read_value(client, W83781D_REG_WCHIPID);
 		if ((val1 = 0x10 || val1 = 0x11) && vendid = winbond)
 			kind = w83781d;
 		else if (val1 = 0x30 && vendid = winbond)
@@ -1157,7 +1161,7 @@ #undef REALLY_SLOW_IO
 			kind = as99127f;
 		else {
 			if (kind = 0)
-				dev_warn(&new_client->dev, "Ignoring 'force' "
+				dev_warn(dev, "Ignoring 'force' "
 					 "parameter for unknown chip at "
 					 "adapter %d, address 0x%02x\n",
 					 i2c_adapter_id(adapter), address);
@@ -1179,20 +1183,20 @@ #undef REALLY_SLOW_IO
 	}
 
 	/* Fill in the remaining client fields and put into the global list */
-	strlcpy(new_client->name, client_name, I2C_NAME_SIZE);
+	strlcpy(client->name, client_name, I2C_NAME_SIZE);
 	data->type = kind;
 
 	data->valid = 0;
 	mutex_init(&data->update_lock);
 
 	/* Tell the I2C layer a new client has arrived */
-	if ((err = i2c_attach_client(new_client)))
+	if ((err = i2c_attach_client(client)))
 		goto ERROR2;
 
 	/* attach secondary i2c lm75-like clients */
 	if (!is_isa) {
 		if ((err = w83781d_detect_subclients(adapter, address,
-				kind, new_client)))
+				kind, client)))
 			goto ERROR3;
 	} else {
 		data->lm75[0] = NULL;
@@ -1200,11 +1204,11 @@ #undef REALLY_SLOW_IO
 	}
 
 	/* Initialize the chip */
-	w83781d_init_client(new_client);
+	w83781d_init_client(client);
 
 	/* A few vars need to be filled upon startup */
 	for (i = 1; i <= 3; i++) {
-		data->fan_min[i - 1] = w83781d_read_value(new_client,
+		data->fan_min[i - 1] = w83781d_read_value(client,
 					W83781D_REG_FAN_MIN(i));
 	}
 	if (kind != w83781d && kind != as99127f)
@@ -1212,65 +1216,68 @@ #undef REALLY_SLOW_IO
 			data->pwmenable[i] = 1;
 
 	/* Register sysfs hooks */
-	data->class_dev = hwmon_device_register(&new_client->dev);
-	if (IS_ERR(data->class_dev)) {
-		err = PTR_ERR(data->class_dev);
+	if ((err = sysfs_create_group(&dev->kobj, &w83781d_group)))
 		goto ERROR4;
-	}
 
-	device_create_file_in(new_client, 0);
-	if (kind != w83783s)
-		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);
+	if (kind != w83783s) {
+		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)))
+			goto ERROR4;
+	}
 	if (kind != as99127f && kind != w83781d && kind != w83783s) {
-		device_create_file_in(new_client, 7);
-		device_create_file_in(new_client, 8);
+		if ((err = device_create_file(dev, &dev_attr_in7_input))
+		    || (err = device_create_file(dev, &dev_attr_in7_min))
+		    || (err = device_create_file(dev, &dev_attr_in7_max))
+		    || (err = device_create_file(dev, &dev_attr_in8_input))
+		    || (err = device_create_file(dev, &dev_attr_in8_min))
+		    || (err = device_create_file(dev, &dev_attr_in8_max)))
+			goto ERROR4;
+	}
+	if (kind != w83783s) {
+		if ((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)))
+			goto ERROR4;
 	}
-
-	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);
-	if (kind != w83783s)
-		device_create_file_temp(new_client, 3);
-
-	device_create_file_vid(new_client);
-	device_create_file_vrm(new_client);
-
-	device_create_file_fan_div(new_client, 1);
-	device_create_file_fan_div(new_client, 2);
-	device_create_file_fan_div(new_client, 3);
-
-	device_create_file_alarms(new_client);
-
-	device_create_file_beep(new_client);
 
 	if (kind != w83781d && kind != as99127f) {
-		device_create_file_pwm(new_client, 1);
-		device_create_file_pwm(new_client, 2);
-		device_create_file_pwmenable(new_client, 2);
+		if ((err = device_create_file(dev, &dev_attr_pwm1))
+		    || (err = device_create_file(dev, &dev_attr_pwm2))
+		    || (err = device_create_file(dev, &dev_attr_pwm2_enable)))
+			goto ERROR4;
 	}
 	if (kind = w83782d && !is_isa) {
-		device_create_file_pwm(new_client, 3);
-		device_create_file_pwm(new_client, 4);
+		if ((err = device_create_file(dev, &dev_attr_pwm3))
+		    || (err = device_create_file(dev, &dev_attr_pwm4)))
+			goto ERROR4;
 	}
 
 	if (kind != as99127f && kind != w83781d) {
-		device_create_file_sensor(new_client, 1);
-		device_create_file_sensor(new_client, 2);
-		if (kind != w83783s)
-			device_create_file_sensor(new_client, 3);
+		if ((err = device_create_file(dev, &dev_attr_temp1_type))
+		    || (err = device_create_file(dev,
+						 &dev_attr_temp2_type)))
+			goto ERROR4;
+		if (kind != w83783s) {
+			if ((err = device_create_file(dev,
+						      &dev_attr_temp3_type)))
+				goto ERROR4;
+		}
+	}
+
+	data->class_dev = hwmon_device_register(dev);
+	if (IS_ERR(data->class_dev)) {
+		err = PTR_ERR(data->class_dev);
+		goto ERROR4;
 	}
 
 	return 0;
 
 ERROR4:
+	sysfs_remove_group(&dev->kobj, &w83781d_group);
+	sysfs_remove_group(&dev->kobj, &w83781d_group_opt);
+
 	if (data->lm75[1]) {
 		i2c_detach_client(data->lm75[1]);
 		kfree(data->lm75[1]);
@@ -1280,7 +1287,7 @@ ERROR4:
 		kfree(data->lm75[0]);
 	}
 ERROR3:
-	i2c_detach_client(new_client);
+	i2c_detach_client(client);
 ERROR2:
 	kfree(data);
 ERROR1:
@@ -1297,9 +1304,11 @@ w83781d_detach_client(struct i2c_client 
 	int err;
 
 	/* main client */
-	if (data)
+	if (data) {
 		hwmon_device_unregister(data->class_dev);
-
+		sysfs_remove_group(&client->dev.kobj, &w83781d_group);
+		sysfs_remove_group(&client->dev.kobj, &w83781d_group_opt);
+	}
 	if (i2c_is_isa_client(client))
 		release_region(client->addr, W83781D_EXTENT);
 
-- 
1.4.2.1



^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2006-09-28 22:48 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-28 22:48 [lm-sensors] [PATCH 34/39] w83781d: Fix unchecked return status Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.