Skip to contentSkip to conversation

hwmon: unchecked return status

PATCH New Mark M. Hoffman 2 participants 5 messages Aug 30, 2006 – Sep 3, 2006

Subsystems (2)
Hardware monitoring ADM1025 hardware monitor driver
lore
Share & tools
lore

Mark M. Hoffman

author
#0 lore raw

PATCH 1/1 hwmon: unchecked return status

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

Signed-off-by Mark M. Hoffman <mhoffman at lightlink.com>
---
 drivers/hwmon/lm77.c |   33 ++++++--
 drivers/hwmon/lm80.c |   85 +++++++++++++---------
 drivers/hwmon/lm85.c |  173 +++++++++++++++++++++++++++-------------------
 drivers/hwmon/lm87.c |  191 +++++++++++++++++++++++++++++++++++++--------------
 drivers/hwmon/lm92.c |   34 ++++++---
 5 files changed, 338 insertions(+), 178 deletions(-)
drivers/hwmon/lm77.c +24 -9
@@ -212,6 +212,23 @@ static int lm77_attach_adapter(struct i2
 	return i2c_probe(adapter, &addr_data, lm77_detect);
 }
 
+static struct attribute *lm77_attributes[] = {
+	&dev_attr_temp1_input.attr,
+	&dev_attr_temp1_crit.attr,
+	&dev_attr_temp1_min.attr,
+	&dev_attr_temp1_max.attr,
+	&dev_attr_temp1_crit_hyst.attr,
+	&dev_attr_temp1_min_hyst.attr,
+	&dev_attr_temp1_max_hyst.attr,
+	&dev_attr_alarms.attr,
+
+	NULL
+};
+
+static const struct attribute_group lm77_group = {
+	.attrs = lm77_attributes,
+};
+
 /* This function is called by i2c_probe */
 static int lm77_detect(struct i2c_adapter *adapter, int address, int kind)
 {

@@ -317,22 +334,19 @@ static int lm77_detect(struct i2c_adapte
 	lm77_init_client(new_client);
 
 	/* Register sysfs hooks */
+	if ((err = sysfs_create_group(&new_client->dev.kobj, &lm77_group)))
+		goto exit_detach;
+
 	data->class_dev = hwmon_device_register(&new_client->dev);
 	if (IS_ERR(data->class_dev)) {
 		err = PTR_ERR(data->class_dev);
-		goto exit_detach;
+		goto exit_remove;
 	}
 
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp1_min);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit_hyst);
-	device_create_file(&new_client->dev, &dev_attr_temp1_min_hyst);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max_hyst);
-	device_create_file(&new_client->dev, &dev_attr_alarms);
 	return 0;
 
+exit_remove:
+	sysfs_remove_group(&new_client->dev.kobj, &lm77_group);
 exit_detach:
 	i2c_detach_client(new_client);
 exit_free:

@@ -345,6 +359,7 @@ static int lm77_detach_client(struct i2c
 {
 	struct lm77_data *data = i2c_get_clientdata(client);
 	hwmon_device_unregister(data->class_dev);
+	sysfs_remove_group(&client->dev.kobj, &lm77_group);
 	i2c_detach_client(client);
 	kfree(data);
 	return 0;
drivers/hwmon/lm80.c +49 -36

110 lines not on this page · raw message

drivers/hwmon/lm85.c +102 -71

201 lines not on this page · raw message

drivers/hwmon/lm87.c +139 -52

228 lines not on this page · raw message

drivers/hwmon/lm92.c +24 -10
@@ -288,6 +288,23 @@ static int max6635_check(struct i2c_clie
 	return 1;
 }
 
+static struct attribute *lm92_attributes[] = {
+	&dev_attr_temp1_input.attr,
+	&dev_attr_temp1_crit.attr,
+	&dev_attr_temp1_crit_hyst.attr,
+	&dev_attr_temp1_min.attr,
+	&dev_attr_temp1_min_hyst.attr,
+	&dev_attr_temp1_max.attr,
+	&dev_attr_temp1_max_hyst.attr,
+	&dev_attr_alarms.attr,
+
+	NULL
+};
+
+static const struct attribute_group lm92_group = {
+	.attrs = lm92_attributes,
+};
+
 /* The following function does more than just detection. If detection
    succeeds, it also registers the new chip. */
 static int lm92_detect(struct i2c_adapter *adapter, int address, int kind)

@@ -359,23 +376,19 @@ static int lm92_detect(struct i2c_adapte
 	lm92_init_client(new_client);
 
 	/* Register sysfs hooks */
+	if ((err = sysfs_create_group(&new_client->dev.kobj, &lm92_group)))
+		goto exit_detach;
+
 	data->class_dev = hwmon_device_register(&new_client->dev);
 	if (IS_ERR(data->class_dev)) {
 		err = PTR_ERR(data->class_dev);
-		goto exit_detach;
+		goto exit_remove;
 	}
 
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit_hyst);
-	device_create_file(&new_client->dev, &dev_attr_temp1_min);
-	device_create_file(&new_client->dev, &dev_attr_temp1_min_hyst);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max_hyst);
-	device_create_file(&new_client->dev, &dev_attr_alarms);
-
 	return 0;
 
+exit_remove:
+	sysfs_remove_group(&new_client->dev.kobj, &lm92_group);
 exit_detach:
 	i2c_detach_client(new_client);
 exit_free:

@@ -397,6 +410,7 @@ static int lm92_detach_client(struct i2c
 	int err;
 
 	hwmon_device_unregister(data->class_dev);
+	sysfs_remove_group(&client->dev.kobj, &lm92_group);
 
 	if ((err = i2c_detach_client(client)))
 		return err;
Signature · 2 lines
Mark M. Hoffman
mhoffman at lightlink.com
#1 lore raw

PATCH 1/1 hwmon: unchecked return

Hi Mark,

Quoted 2 lines

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

Looks good to me, I'd only have two minor comments (I always do, it seems...) No need to resend this patch (unless you want to), but you may want to take them into account for later patches.

Quoted 32 lines

+++ linux-2.6.18-rc4-mm3/drivers/hwmon/lm80.c

drivers/hwmon/lm80.c +27 -0
@@ -394,6 +394,48 @@ static int lm80_attach_adapter(struct i2
 	return i2c_probe(adapter, &addr_data, lm80_detect);
 }
 
+static struct attribute *lm80_attributes[] = {
+	&dev_attr_in0_min.attr,
+	&dev_attr_in1_min.attr,
+	&dev_attr_in2_min.attr,
+	&dev_attr_in3_min.attr,
+	&dev_attr_in4_min.attr,
+	&dev_attr_in5_min.attr,
+	&dev_attr_in6_min.attr,
+	&dev_attr_in0_max.attr,
+	&dev_attr_in1_max.attr,
+	&dev_attr_in2_max.attr,
+	&dev_attr_in3_max.attr,
+	&dev_attr_in4_max.attr,
+	&dev_attr_in5_max.attr,
+	&dev_attr_in6_max.attr,
+	&dev_attr_in0_input.attr,
+	&dev_attr_in1_input.attr,
+	&dev_attr_in2_input.attr,
+	&dev_attr_in3_input.attr,
+	&dev_attr_in4_input.attr,
+	&dev_attr_in5_input.attr,
+	&dev_attr_in6_input.attr,
+	&dev_attr_fan1_min.attr,
+	&dev_attr_fan2_min.attr,
+	&dev_attr_fan1_input.attr,
+	&dev_attr_fan2_input.attr,
+	&dev_attr_fan1_div.attr,

drivers/hwmon/lm80.c static int lm80_attach_adapter(struct i2 · line 433

Quoted 10 lines
drivers/hwmon/lm80.c +10 -0
+	&dev_attr_fan2_div.attr,
+	&dev_attr_temp1_input.attr,
+	&dev_attr_temp1_max.attr,
+	&dev_attr_temp1_max_hyst.attr,
+	&dev_attr_temp1_crit.attr,
+	&dev_attr_temp1_crit_hyst.attr,
+	&dev_attr_alarms.attr,
+
+	NULL
+};

If you were grouping the attributes visually as you did in some other drivers, I agree it would make (some) sense to have a blank line before the NULL terminator. However, with a single block of attributes, it's a bit harder to justify this last blank line.

Quoted 5 lines
drivers/hwmon/lm85.c +0 -2
 	/* The ADT7463 has an optional VRM 10 mode where pin 21 is used
 	   as a sixth digital VID input rather than an analog input. */
 	data->vid = lm85_read_value(new_client, LM85_REG_VID);
-	if (!(kind = adt7463 && (data->vid & 0x80))) {
-		device_create_file(&new_client->dev, &dev_attr_in4_input);

drivers/hwmon/lm85.c static int lm85_detect(struct i2c_adapte · line 1262

Quoted 10 lines
drivers/hwmon/lm85.c +8 -2
-		device_create_file(&new_client->dev, &dev_attr_in4_min);
-		device_create_file(&new_client->dev, &dev_attr_in4_max);
+	if (!(kind = adt7463 && (data->vid & 0x80)))
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in4_input))
+		 || (err = device_create_file(&new_client->dev,
+					&dev_attr_in4_min))
+		 || (err = device_create_file(&new_client->dev,
+					&dev_attr_in4_max)))
+			goto ERROR3;

As a maintainer, I don't much like nested "if"s without curly braces, especially when the conditions span over multiple lines. It's very easy to mess up when later trying to add an "else" clause to the outer-most "if". Thus I'd recommend to keep the curly braces for the outer-most "if" in this case. If nothing else, it also makes the patch a couple lines smaller ;)

Thanks,

Signature · 1 line
Jean Delvare
author
#2 lore raw

PATCH 1/1 hwmon: unchecked return status

In reply to Mark M. Hoffman

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

Signed-off-by Mark M. Hoffman <mhoffman at lightlink.com>
---
 drivers/hwmon/adm1021.c |   31 +++--
 drivers/hwmon/adm1025.c |   94 ++++++++++-----
 drivers/hwmon/adm1026.c |  286 ++++++++++++++++++++++++------------------------
 drivers/hwmon/adm1031.c |  114 +++++++++++--------
 drivers/hwmon/adm9240.c |  105 +++++++----------
 5 files changed, 346 insertions(+), 284 deletions(-)
drivers/hwmon/adm1021.c +22 -9
@@ -190,6 +190,21 @@ static int adm1021_attach_adapter(struct
 	return i2c_probe(adapter, &addr_data, adm1021_detect);
 }
 
+static struct attribute *adm1021_attributes[] = {
+	&dev_attr_temp1_max.attr,
+	&dev_attr_temp1_min.attr,
+	&dev_attr_temp1_input.attr,
+	&dev_attr_temp2_max.attr,
+	&dev_attr_temp2_min.attr,
+	&dev_attr_temp2_input.attr,
+	&dev_attr_alarms.attr,
+	NULL
+};
+
+static const struct attribute_group adm1021_group = {
+	.attrs = adm1021_attributes,
+};
+
 static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind)
 {
 	int i;

@@ -287,22 +302,19 @@ static int adm1021_detect(struct i2c_ada
 		adm1021_init_client(new_client);
 
 	/* Register sysfs hooks */
+	if ((err = sysfs_create_group(&new_client->dev.kobj, &adm1021_group)))
+		goto error2;
+
 	data->class_dev = hwmon_device_register(&new_client->dev);
 	if (IS_ERR(data->class_dev)) {
 		err = PTR_ERR(data->class_dev);
-		goto error2;
+		goto error3;
 	}
 
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp1_min);
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_min);
-	device_create_file(&new_client->dev, &dev_attr_temp2_input);
-	device_create_file(&new_client->dev, &dev_attr_alarms);
-
 	return 0;
 
+error3:
+	sysfs_remove_group(&new_client->dev.kobj, &adm1021_group);
 error2:
 	i2c_detach_client(new_client);
 error1:

@@ -326,6 +338,7 @@ static int adm1021_detach_client(struct 
 	int err;
 
 	hwmon_device_unregister(data->class_dev);
+	sysfs_remove_group(&client->dev.kobj, &adm1021_group);
 
 	if ((err = i2c_detach_client(client)))
 		return err;
drivers/hwmon/adm1025.c +62 -32

120 lines not on this page · raw message

drivers/hwmon/adm1026.c +146 -140

318 lines not on this page · raw message

drivers/hwmon/adm1031.c +70 -44

139 lines not on this page · raw message

drivers/hwmon/adm9240.c +46 -59

130 lines not on this page · raw message

Signature · 2 lines
Mark M. Hoffman
mhoffman at lightlink.com
#3 lore raw

PATCH 1/1 hwmon: unchecked return

In reply to Mark M. Hoffman

Hi Mark,

Quoted 2 lines

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

I have an ADM1025 eval board somewhere, I'll try to resurect it tomorrow and I'll test adm1025. I can't see why it wouldn't work though.

Just one thing...

Quoted 91 lines
drivers/hwmon/adm1025.c +44 -30
@@ -315,6 +315,49 @@ static int adm1025_attach_adapter(struct
 	return i2c_probe(adapter, &addr_data, adm1025_detect);
 }
 
+static struct attribute *adm1025_attributes[] = {
+	&dev_attr_in0_input.attr,
+	&dev_attr_in1_input.attr,
+	&dev_attr_in2_input.attr,
+	&dev_attr_in3_input.attr,
+	&dev_attr_in5_input.attr,
+	&dev_attr_in0_min.attr,
+	&dev_attr_in1_min.attr,
+	&dev_attr_in2_min.attr,
+	&dev_attr_in3_min.attr,
+	&dev_attr_in5_min.attr,
+	&dev_attr_in0_max.attr,
+	&dev_attr_in1_max.attr,
+	&dev_attr_in2_max.attr,
+	&dev_attr_in3_max.attr,
+	&dev_attr_in5_max.attr,
+	&dev_attr_temp1_input.attr,
+	&dev_attr_temp2_input.attr,
+	&dev_attr_temp1_min.attr,
+	&dev_attr_temp2_min.attr,
+	&dev_attr_temp1_max.attr,
+	&dev_attr_temp2_max.attr,
+	&dev_attr_alarms.attr,
+	&dev_attr_cpu0_vid.attr,
+	&dev_attr_vrm.attr,
+	NULL
+};
+
+static const struct attribute_group adm1025_group = {
+	.attrs = adm1025_attributes,
+};
+
+static struct attribute *adm1025_attributes_opt[] = {
+	&dev_attr_in4_input.attr,
+	&dev_attr_in4_min.attr,
+	&dev_attr_in4_max.attr,
+	NULL
+};
+
+static const struct attribute_group adm1025_group_opt = {
+	.attrs = adm1025_attributes_opt,
+};
+
 /*
  * The following function does more than just detection. If detection
  * succeeds, it also registers the new chip.

@@ -415,46 +458,31 @@ static int adm1025_detect(struct i2c_ada
 	adm1025_init_client(new_client);
 
 	/* 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(&new_client->dev.kobj, &adm1025_group)))
 		goto exit_detach;
-	}
-
-	device_create_file(&new_client->dev, &dev_attr_in0_input);
-	device_create_file(&new_client->dev, &dev_attr_in1_input);
-	device_create_file(&new_client->dev, &dev_attr_in2_input);
-	device_create_file(&new_client->dev, &dev_attr_in3_input);
-	device_create_file(&new_client->dev, &dev_attr_in5_input);
-	device_create_file(&new_client->dev, &dev_attr_in0_min);
-	device_create_file(&new_client->dev, &dev_attr_in1_min);
-	device_create_file(&new_client->dev, &dev_attr_in2_min);
-	device_create_file(&new_client->dev, &dev_attr_in3_min);
-	device_create_file(&new_client->dev, &dev_attr_in5_min);
-	device_create_file(&new_client->dev, &dev_attr_in0_max);
-	device_create_file(&new_client->dev, &dev_attr_in1_max);
-	device_create_file(&new_client->dev, &dev_attr_in2_max);
-	device_create_file(&new_client->dev, &dev_attr_in3_max);
-	device_create_file(&new_client->dev, &dev_attr_in5_max);
-	device_create_file(&new_client->dev, &dev_attr_temp1_input);
-	device_create_file(&new_client->dev, &dev_attr_temp2_input);
-	device_create_file(&new_client->dev, &dev_attr_temp1_min);
-	device_create_file(&new_client->dev, &dev_attr_temp2_min);
-	device_create_file(&new_client->dev, &dev_attr_temp1_max);
-	device_create_file(&new_client->dev, &dev_attr_temp2_max);
-	device_create_file(&new_client->dev, &dev_attr_alarms);
-	device_create_file(&new_client->dev, &dev_attr_cpu0_vid);
-	device_create_file(&new_client->dev, &dev_attr_vrm);
 
 	/* Pin 11 is either in4 (+12V) or VID4 */
 	if (!(config & 0x20)) {
-		device_create_file(&new_client->dev, &dev_attr_in4_input);

drivers/hwmon/adm1025.c static int adm1025_detect(struct i2c_ada · line 473

Quoted 10 lines
drivers/hwmon/adm1025.c +8 -2
-		device_create_file(&new_client->dev, &dev_attr_in4_min);
-		device_create_file(&new_client->dev, &dev_attr_in4_max);
+		if ((err = device_create_file(&new_client->dev,
+					&dev_attr_in4_input))
+		 || (err = device_create_file(&new_client->dev,
+					&dev_attr_in4_min))
+		 || (err = device_create_file(&new_client->dev,
+					&dev_attr_in4_max)))
+			goto exit_remove;
+	}

This happens to be exactly adm1025_group_opt, so you could use sysfs_create_group to spare a few lines of code. I see you've been doing that for the adm1031 later, and I did the same for one of my drivers (lm63). I even went as far as using an explicit name for that group (lm63_group_fan1) instead of the generic ${driver}_group_opt.

Quoted 313 lines
drivers/hwmon/adm1025.c +10 -0
+
+	data->class_dev = hwmon_device_register(&new_client->dev);
+	if (IS_ERR(data->class_dev)) {
+		err = PTR_ERR(data->class_dev);
+		goto exit_remove;
 	}
 
 	return 0;
 
+exit_remove:
+	sysfs_remove_group(&new_client->dev.kobj, &adm1025_group);
+	sysfs_remove_group(&new_client->dev.kobj, &adm1025_group_opt);
 exit_detach:
 	i2c_detach_client(new_client);
 exit_free:

@@ -511,6 +539,8 @@ static int adm1025_detach_client(struct 
 	int err;
 
 	hwmon_device_unregister(data->class_dev);
+	sysfs_remove_group(&client->dev.kobj, &adm1025_group);
+	sysfs_remove_group(&client->dev.kobj, &adm1025_group_opt);
 
 	if ((err = i2c_detach_client(client)))
 		return err;
drivers/hwmon/adm1026.c +133 -131
@@ -323,15 +323,6 @@ static int adm1026_attach_adapter(struct
 	return i2c_probe(adapter, &addr_data, adm1026_detect);
 }
 
-static int adm1026_detach_client(struct i2c_client *client)
-{
-	struct adm1026_data *data = i2c_get_clientdata(client);
-	hwmon_device_unregister(data->class_dev);
-	i2c_detach_client(client);
-	kfree(data);
-	return 0;
-}
-
 static int adm1026_read_value(struct i2c_client *client, u8 reg)
 {
 	int res;

@@ -1450,6 +1441,135 @@ static DEVICE_ATTR(temp1_auto_point2_pwm
 static DEVICE_ATTR(temp2_auto_point2_pwm, S_IRUGO, show_auto_pwm_max, NULL);
 static DEVICE_ATTR(temp3_auto_point2_pwm, S_IRUGO, show_auto_pwm_max, NULL);
 
+static struct attribute *adm1026_attributes[] = {
+	&sensor_dev_attr_in0_input.dev_attr.attr,
+	&sensor_dev_attr_in0_max.dev_attr.attr,
+	&sensor_dev_attr_in0_min.dev_attr.attr,
+	&sensor_dev_attr_in1_input.dev_attr.attr,
+	&sensor_dev_attr_in1_max.dev_attr.attr,
+	&sensor_dev_attr_in1_min.dev_attr.attr,
+	&sensor_dev_attr_in2_input.dev_attr.attr,
+	&sensor_dev_attr_in2_max.dev_attr.attr,
+	&sensor_dev_attr_in2_min.dev_attr.attr,
+	&sensor_dev_attr_in3_input.dev_attr.attr,
+	&sensor_dev_attr_in3_max.dev_attr.attr,
+	&sensor_dev_attr_in3_min.dev_attr.attr,
+	&sensor_dev_attr_in4_input.dev_attr.attr,
+	&sensor_dev_attr_in4_max.dev_attr.attr,
+	&sensor_dev_attr_in4_min.dev_attr.attr,
+	&sensor_dev_attr_in5_input.dev_attr.attr,
+	&sensor_dev_attr_in5_max.dev_attr.attr,
+	&sensor_dev_attr_in5_min.dev_attr.attr,
+	&sensor_dev_attr_in6_input.dev_attr.attr,
+	&sensor_dev_attr_in6_max.dev_attr.attr,
+	&sensor_dev_attr_in6_min.dev_attr.attr,
+	&sensor_dev_attr_in7_input.dev_attr.attr,
+	&sensor_dev_attr_in7_max.dev_attr.attr,
+	&sensor_dev_attr_in7_min.dev_attr.attr,
+	&sensor_dev_attr_in8_input.dev_attr.attr,
+	&sensor_dev_attr_in8_max.dev_attr.attr,
+	&sensor_dev_attr_in8_min.dev_attr.attr,
+	&sensor_dev_attr_in9_input.dev_attr.attr,
+	&sensor_dev_attr_in9_max.dev_attr.attr,
+	&sensor_dev_attr_in9_min.dev_attr.attr,
+	&sensor_dev_attr_in10_input.dev_attr.attr,
+	&sensor_dev_attr_in10_max.dev_attr.attr,
+	&sensor_dev_attr_in10_min.dev_attr.attr,
+	&sensor_dev_attr_in11_input.dev_attr.attr,
+	&sensor_dev_attr_in11_max.dev_attr.attr,
+	&sensor_dev_attr_in11_min.dev_attr.attr,
+	&sensor_dev_attr_in12_input.dev_attr.attr,
+	&sensor_dev_attr_in12_max.dev_attr.attr,
+	&sensor_dev_attr_in12_min.dev_attr.attr,
+	&sensor_dev_attr_in13_input.dev_attr.attr,
+	&sensor_dev_attr_in13_max.dev_attr.attr,
+	&sensor_dev_attr_in13_min.dev_attr.attr,
+	&sensor_dev_attr_in14_input.dev_attr.attr,
+	&sensor_dev_attr_in14_max.dev_attr.attr,
+	&sensor_dev_attr_in14_min.dev_attr.attr,
+	&sensor_dev_attr_in15_input.dev_attr.attr,
+	&sensor_dev_attr_in15_max.dev_attr.attr,
+	&sensor_dev_attr_in15_min.dev_attr.attr,
+	&sensor_dev_attr_in16_input.dev_attr.attr,
+	&sensor_dev_attr_in16_max.dev_attr.attr,
+	&sensor_dev_attr_in16_min.dev_attr.attr,
+	&sensor_dev_attr_fan1_input.dev_attr.attr,
+	&sensor_dev_attr_fan1_div.dev_attr.attr,
+	&sensor_dev_attr_fan1_min.dev_attr.attr,
+	&sensor_dev_attr_fan2_input.dev_attr.attr,
+	&sensor_dev_attr_fan2_div.dev_attr.attr,
+	&sensor_dev_attr_fan2_min.dev_attr.attr,
+	&sensor_dev_attr_fan3_input.dev_attr.attr,
+	&sensor_dev_attr_fan3_div.dev_attr.attr,
+	&sensor_dev_attr_fan3_min.dev_attr.attr,
+	&sensor_dev_attr_fan4_input.dev_attr.attr,
+	&sensor_dev_attr_fan4_div.dev_attr.attr,
+	&sensor_dev_attr_fan4_min.dev_attr.attr,
+	&sensor_dev_attr_fan5_input.dev_attr.attr,
+	&sensor_dev_attr_fan5_div.dev_attr.attr,
+	&sensor_dev_attr_fan5_min.dev_attr.attr,
+	&sensor_dev_attr_fan6_input.dev_attr.attr,
+	&sensor_dev_attr_fan6_div.dev_attr.attr,
+	&sensor_dev_attr_fan6_min.dev_attr.attr,
+	&sensor_dev_attr_fan7_input.dev_attr.attr,
+	&sensor_dev_attr_fan7_div.dev_attr.attr,
+	&sensor_dev_attr_fan7_min.dev_attr.attr,
+	&sensor_dev_attr_fan8_input.dev_attr.attr,
+	&sensor_dev_attr_fan8_div.dev_attr.attr,
+	&sensor_dev_attr_fan8_min.dev_attr.attr,
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_temp1_max.dev_attr.attr,
+	&sensor_dev_attr_temp1_min.dev_attr.attr,
+	&sensor_dev_attr_temp2_input.dev_attr.attr,
+	&sensor_dev_attr_temp2_max.dev_attr.attr,
+	&sensor_dev_attr_temp2_min.dev_attr.attr,
+	&sensor_dev_attr_temp3_input.dev_attr.attr,
+	&sensor_dev_attr_temp3_max.dev_attr.attr,
+	&sensor_dev_attr_temp3_min.dev_attr.attr,
+	&sensor_dev_attr_temp1_offset.dev_attr.attr,
+	&sensor_dev_attr_temp2_offset.dev_attr.attr,
+	&sensor_dev_attr_temp3_offset.dev_attr.attr,
+	&sensor_dev_attr_temp1_auto_point1_temp.dev_attr.attr,
+	&sensor_dev_attr_temp2_auto_point1_temp.dev_attr.attr,
+	&sensor_dev_attr_temp3_auto_point1_temp.dev_attr.attr,
+	&sensor_dev_attr_temp1_auto_point1_temp_hyst.dev_attr.attr,
+	&sensor_dev_attr_temp2_auto_point1_temp_hyst.dev_attr.attr,
+	&sensor_dev_attr_temp3_auto_point1_temp_hyst.dev_attr.attr,
+	&sensor_dev_attr_temp1_auto_point2_temp.dev_attr.attr,
+	&sensor_dev_attr_temp2_auto_point2_temp.dev_attr.attr,
+	&sensor_dev_attr_temp3_auto_point2_temp.dev_attr.attr,
+	&sensor_dev_attr_temp1_crit.dev_attr.attr,
+	&sensor_dev_attr_temp2_crit.dev_attr.attr,
+	&sensor_dev_attr_temp3_crit.dev_attr.attr,
+	&dev_attr_temp1_crit_enable.attr,
+	&dev_attr_temp2_crit_enable.attr,
+	&dev_attr_temp3_crit_enable.attr,
+	&dev_attr_cpu0_vid.attr,
+	&dev_attr_vrm.attr,
+	&dev_attr_alarms.attr,
+	&dev_attr_alarm_mask.attr,
+	&dev_attr_gpio.attr,
+	&dev_attr_gpio_mask.attr,
+	&dev_attr_pwm1.attr,
+	&dev_attr_pwm2.attr,
+	&dev_attr_pwm3.attr,
+	&dev_attr_pwm1_enable.attr,
+	&dev_attr_pwm2_enable.attr,
+	&dev_attr_pwm3_enable.attr,
+	&dev_attr_temp1_auto_point1_pwm.attr,
+	&dev_attr_temp2_auto_point1_pwm.attr,
+	&dev_attr_temp3_auto_point1_pwm.attr,
+	&dev_attr_temp1_auto_point2_pwm.attr,
+	&dev_attr_temp2_auto_point2_pwm.attr,
+	&dev_attr_temp3_auto_point2_pwm.attr,
+	&dev_attr_analog_out.attr,
+	NULL
+};
+
+static const struct attribute_group adm1026_group = {
+	.attrs = adm1026_attributes,
+};
+
 static int adm1026_detect(struct i2c_adapter *adapter, int address,
 			  int kind)
 {

@@ -1554,145 +1674,20 @@ static int adm1026_detect(struct i2c_ada
 	adm1026_init_client(new_client);
 
 	/* Register sysfs hooks */
+	if ((err = sysfs_create_group(&new_client->dev.kobj, &adm1026_group)))
+		goto exitdetach;
+
 	data->class_dev = hwmon_device_register(&new_client->dev);
 	if (IS_ERR(data->class_dev)) {
 		err = PTR_ERR(data->class_dev);
-		goto exitdetach;
+		goto exitremove;
 	}
 
-	device_create_file(&new_client->dev, &sensor_dev_attr_in0_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in0_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in0_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in1_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in1_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in1_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in2_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in2_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in2_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in3_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in3_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in3_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in4_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in4_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in4_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in5_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in5_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in5_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in6_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in6_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in6_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in7_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in7_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in7_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in8_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in8_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in8_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in9_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in9_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in9_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in10_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in10_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in10_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in11_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in11_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in11_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in12_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in12_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in12_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in13_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in13_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in13_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in14_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in14_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in14_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in15_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in15_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in15_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in16_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in16_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_in16_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan1_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan1_div.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan1_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan2_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan2_div.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan2_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan3_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan3_div.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan3_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan4_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan4_div.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan4_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan5_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan5_div.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan5_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan6_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan6_div.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan6_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan7_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan7_div.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan7_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan8_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan8_div.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_fan8_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp1_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp1_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp1_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp2_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp2_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp2_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp3_input.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp3_max.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp3_min.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp1_offset.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp2_offset.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp3_offset.dev_attr);
-	device_create_file(&new_client->dev, 
-		&sensor_dev_attr_temp1_auto_point1_temp.dev_attr);
-	device_create_file(&new_client->dev, 
-		&sensor_dev_attr_temp2_auto_point1_temp.dev_attr);
-	device_create_file(&new_client->dev, 
-		&sensor_dev_attr_temp3_auto_point1_temp.dev_attr);
-	device_create_file(&new_client->dev,
-		&sensor_dev_attr_temp1_auto_point1_temp_hyst.dev_attr);
-	device_create_file(&new_client->dev,
-		&sensor_dev_attr_temp2_auto_point1_temp_hyst.dev_attr);
-	device_create_file(&new_client->dev,
-		&sensor_dev_attr_temp3_auto_point1_temp_hyst.dev_attr);
-	device_create_file(&new_client->dev, 
-		&sensor_dev_attr_temp1_auto_point2_temp.dev_attr);
-	device_create_file(&new_client->dev, 
-		&sensor_dev_attr_temp2_auto_point2_temp.dev_attr);
-	device_create_file(&new_client->dev, 
-		&sensor_dev_attr_temp3_auto_point2_temp.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp1_crit.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp2_crit.dev_attr);
-	device_create_file(&new_client->dev, &sensor_dev_attr_temp3_crit.dev_attr);
-	device_create_file(&new_client->dev, &dev_attr_temp1_crit_enable);
-	device_create_file(&new_client->dev, &dev_attr_temp2_crit_enable);
-	device_create_file(&new_client->dev, &dev_attr_temp3_crit_enable);
-	device_create_file(&new_client->dev, &dev_attr_cpu0_vid);
-	device_create_file(&new_client->dev, &dev_attr_vrm);
-	device_create_file(&new_client->dev, &dev_attr_alarms);
-	device_create_file(&new_client->dev, &dev_attr_alarm_mask);
-	device_create_file(&new_client->dev, &dev_attr_gpio);
-	device_create_file(&new_client->dev, &dev_attr_gpio_mask);
-	device_create_file(&new_client->dev, &dev_attr_pwm1);
-	device_create_file(&new_client->dev, &dev_attr_pwm2);
-	device_create_file(&new_client->dev, &dev_attr_pwm3);
-	device_create_file(&new_client->dev, &dev_attr_pwm1_enable);

drivers/hwmon/adm1026.c static int adm1026_detect(struct i2c_ada · line 1686

Quoted 10 lines
drivers/hwmon/adm1026.c +0 -9
-	device_create_file(&new_client->dev, &dev_attr_pwm2_enable);
-	device_create_file(&new_client->dev, &dev_attr_pwm3_enable);
-	device_create_file(&new_client->dev, &dev_attr_temp1_auto_point1_pwm);
-	device_create_file(&new_client->dev, &dev_attr_temp2_auto_point1_pwm);
-	device_create_file(&new_client->dev, &dev_attr_temp3_auto_point1_pwm);
-	device_create_file(&new_client->dev, &dev_attr_temp1_auto_point2_pwm);
-	device_create_file(&new_client->dev, &dev_attr_temp2_auto_point2_pwm);
-	device_create_file(&new_client->dev, &dev_attr_temp3_auto_point2_pwm);
-	device_create_file(&new_client->dev, &dev_attr_analog_out);
 	return 0;

Mmmmm.... :))

Quoted 15 lines
drivers/hwmon/adm1026.c +6 -0
 	/* Error out and cleanup code */
+exitremove:
+	sysfs_remove_group(&new_client->dev.kobj, &adm1026_group);
 exitdetach:
 	i2c_detach_client(new_client);
 exitfree:

@@ -1700,6 +1695,17 @@ exitfree:
 exit:
 	return err;
 }
+
+static int adm1026_detach_client(struct i2c_client *client)
+{
+	struct adm1026_data *data = i2c_get_clientdata(client);

drivers/hwmon/adm1026.c exitfree: · line 1711

Quoted 10 lines
drivers/hwmon/adm1026.c +7 -0
+	hwmon_device_unregister(data->class_dev);
+	sysfs_remove_group(&client->dev.kobj, &adm1026_group);
+	i2c_detach_client(client);
+	kfree(data);
+	return 0;
+}
+
 static int __init sm_adm1026_init(void)
 {
 	return i2c_add_driver(&adm1026_driver);

You may send an updated patch, else I'll just pick this one which is good enough.

Thanks,

Signature · 1 line
Jean Delvare
#4 lore raw

PATCH 1/1 hwmon: unchecked return

In reply to Mark M. Hoffman
Quoted 6 lines
Quoted 2 lines

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

I have an ADM1025 eval board somewhere, I'll try to resurect it tomorrow and I'll test adm1025. I can't see why it wouldn't work though.

Just tested, and it works all OK.

Signature · 1 line
Jean Delvare