[RFC PATCH 0/8] hwmon: Add devres support

[RFC PATCH 0/8] hwmon: Add devres support

[Guenter Roeck]

Provide devres functions for hwmon_register_device.

Besides providing the devres function, the new API automates sysfs attribute
file creation and attaches attribute files to the hwmon device instead of
the hardware device.

Some driver conversions are provided as examples. Only the patches for
max16065, max6697, and ina209 have been tested so far, the others are
informational only.

An open question is if the new API function names should reflect that the sysfs
attribute groups are provided as parameter, and, if so, if a matching non-devres
API function should be provided as well.

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

Guenter Roeck (8):
      hwmon: Add devres support
      hwmon: (ina209) Convert to devm_hwmon_device_register API
      hwmon: (ltc4215) Convert to devm_hwmon_device_register API
      hwmon: (ltc4261) convert to devm_hwmon_device_register API
      hwmon: (max6697) Convert to devm_hwmon_device_register API
      hwmon: (lm90) Convert to devm_hwmon_device_register API
      hwmon: (max16065) Convert to devm_hwmon_device_register API
      hwmon: (tmp401) Convert to devm_hwmon_device_register API

 drivers/hwmon/hwmon.c    |  123 +++++++++++++++++++++++++++++---
 drivers/hwmon/ina209.c   |   29 ++++----
 drivers/hwmon/lm90.c     |  106 ++++++++++++---------------
 drivers/hwmon/ltc4215.c  |   38 +++------
 drivers/hwmon/ltc4261.c  |   39 +++------
 drivers/hwmon/max16065.c |  107 ++++++++++++---------------
 drivers/hwmon/max6697.c  |  157 +++++++++++++++++++++------------------
 drivers/hwmon/tmp401.c   |  184 +++++++++++++++++++++++-----------------------
 include/linux/hwmon.h    |    5 ++
 9 files changed, 424 insertions(+), 364 deletions(-)

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

Object file size comparisons (x86_64) against 3.9-rc3. Mileage varies
from substantial object file size reductions (lm90) to substantial
increases (max6697). This largely depends on the number of is_visible()
calls which have to be added to the code. Also, if a driver did not use
attribute groups, savings are consumed by the need to declare those;
essentially, code is replaced with data. The tmp401 driver is a good
example for this phenomen.

size old:
   text    data     bss     dec     hex filename
   1297     776     352    2425     979 drivers/hwmon/hwmon.o
   4133    4033    1040    9206    23f6 drivers/hwmon/ina209.o
  11623    5504    3184   20311    4f57 drivers/hwmon/lm90.o
   1923    1833     496    4252    109c drivers/hwmon/ltc4215.o
   1868    1657     496    4021     fb5 drivers/hwmon/ltc4261.o
   4172    6816     872   11860    2e54 drivers/hwmon/max16065.o
   4788    4769    1088   10645    2995 drivers/hwmon/max6697.o
   5240    3232    1376    9848    2678 drivers/hwmon/tmp401.o

size new:
   text    data     bss     dec     hex filename
   2327    1432    1440    5199    144f drivers/hwmon/hwmon.o
   4084    4049    1024    9157    23c5 drivers/hwmon/ina209.o
  11280    5472    2960   19712    4d00 drivers/hwmon/lm90.o
   1781    1785     448    4014     fae drivers/hwmon/ltc4215.o
   1720    1625     432    3777     ec1 drivers/hwmon/ltc4261.o
   3957    6816     784   11557    2d25 drivers/hwmon/max16065.o
   4847    5233    1088   11168    2ba0 drivers/hwmon/max6697.o
   4955    3520    1240    9715    25f3 drivers/hwmon/tmp401.o

[RFC PATCH 1/8] hwmon: Add devres support

[Guenter Roeck]

This patch provices new API functions devm_hwmon_device_register() and
devm_hwmon_device_unregister().

devm_hwmon_device_register() has a new parameter, a pointer to an array of type
attribute_group. When using this API, callers to not have to create or remove
sysfs attribute groups. Instead, attributes are created by the infrastructure.

devm_hwmon_device_unregister() does not have to be called except for error
handling.

The new API attaches the provided attributes to the hwmon device.
It automatically creates a 'name' attribute as required by the hwmon sysfs ABI.
The name is set to either the name provided with devm_hwmon_device_register
or the name of the hardware device driver.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/hwmon.c |  123 +++++++++++++++++++++++++++++++++++++++++++++----
 include/linux/hwmon.h |    5 ++
 2 files changed, 119 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 646314f..4681b98 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -29,6 +29,51 @@ static struct class *hwmon_class;
 
 static DEFINE_IDA(hwmon_ida);
 
+static void hwmon_device_register_release(struct device *dev)
+{
+	kfree(dev);
+}
+
+static struct device *_hwmon_device_register(struct device *dev,
+					const struct attribute_group **groups,
+					const char *name)
+{
+	struct device *hwdev;
+	int id, err;
+
+	hwdev = kzalloc(sizeof(*hwdev), GFP_KERNEL);
+	if (!hwdev)
+		return ERR_PTR(-ENOMEM);
+
+	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
+	if (id < 0) {
+		err = id;
+		goto err_free;
+	}
+
+	hwdev->groups = groups;
+	hwdev->class = hwmon_class;
+	hwdev->parent = dev;
+	hwdev->release = hwmon_device_register_release;
+	dev_set_drvdata(hwdev, (void *)name);
+
+	err = kobject_set_name(&hwdev->kobj, HWMON_ID_FORMAT, id);
+	if (err)
+		goto err_remove;
+
+	err = device_register(hwdev);
+	if (err)
+		goto err_remove;
+
+	return hwdev;
+
+err_remove:
+	ida_simple_remove(&hwmon_ida, id);
+err_free:
+	kfree(hwdev);
+	return ERR_PTR(err);
+}
+
 /**
  * hwmon_device_register - register w/ hwmon
  * @dev: the device to register
@@ -40,22 +85,52 @@ static DEFINE_IDA(hwmon_ida);
  */
 struct device *hwmon_device_register(struct device *dev)
 {
+	return _hwmon_device_register(dev, NULL, NULL);
+}
+EXPORT_SYMBOL_GPL(hwmon_device_register);
+
+struct hwmon_devres {
+	struct list_head node;
 	struct device *hwdev;
-	int id;
+};
 
-	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
-	if (id < 0)
-		return ERR_PTR(id);
+static void devm_hwmon_release(struct device *dev, void *res)
+{
+	struct hwmon_devres *devres = res;
 
-	hwdev = device_create(hwmon_class, dev, MKDEV(0, 0), NULL,
-			      HWMON_ID_FORMAT, id);
+	hwmon_device_unregister(devres->hwdev);
+}
 
-	if (IS_ERR(hwdev))
-		ida_simple_remove(&hwmon_ida, id);
+struct device *devm_hwmon_device_register(struct device *dev,
+					  const struct attribute_group **groups,
+					  const char *name)
+{
+	struct hwmon_devres *devres;
+	struct device *hwdev;
+	int err;
+
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
+	devres = devres_alloc(devm_hwmon_release, sizeof(*devres),
+			      GFP_KERNEL);
+	if (!devres)
+		return ERR_PTR(-ENOMEM);
 
+	hwdev = _hwmon_device_register(dev, groups, name);
+	if (IS_ERR(hwdev)) {
+		err = PTR_ERR(hwdev);
+		goto err_free;
+	}
+	devres->hwdev = hwdev;
+	devres_add(dev, devres);
 	return hwdev;
+
+err_free:
+	devres_free(devres);
+	return ERR_PTR(err);
 }
-EXPORT_SYMBOL_GPL(hwmon_device_register);
+EXPORT_SYMBOL_GPL(devm_hwmon_device_register);
 
 /**
  * hwmon_device_unregister - removes the previously registered class device
@@ -75,6 +150,22 @@ void hwmon_device_unregister(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(hwmon_device_unregister);
 
+static int devm_hwmon_match(struct device *dev, void *res, void *data)
+{
+	struct hwmon_devres *devres = res;
+
+	return devres->hwdev == data;
+}
+
+void devm_hwmon_device_unregister(struct device *hwdev)
+{
+	WARN_ON(devres_destroy(hwdev->parent, devm_hwmon_release,
+			       devm_hwmon_match, hwdev));
+
+	hwmon_device_unregister(hwdev);
+}
+EXPORT_SYMBOL_GPL(devm_hwmon_device_unregister);
+
 static void __init hwmon_pci_quirks(void)
 {
 #if defined CONFIG_X86 && defined CONFIG_PCI
@@ -103,6 +194,19 @@ static void __init hwmon_pci_quirks(void)
 #endif
 }
 
+static ssize_t hwmon_name_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	const char *name = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", name ? : dev->parent->driver->name);
+}
+
+struct device_attribute hwmon_dev_attrs[] = {
+	__ATTR(name, S_IRUGO, hwmon_name_show, NULL),
+	__ATTR_NULL,
+};
+
 static int __init hwmon_init(void)
 {
 	hwmon_pci_quirks();
@@ -112,6 +216,7 @@ static int __init hwmon_init(void)
 		pr_err("couldn't create sysfs class\n");
 		return PTR_ERR(hwmon_class);
 	}
+	hwmon_class->dev_attrs = hwmon_dev_attrs;
 	return 0;
 }
 
diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
index b2514f7..67c9c08 100644
--- a/include/linux/hwmon.h
+++ b/include/linux/hwmon.h
@@ -15,9 +15,14 @@
 #define _HWMON_H_
 
 struct device;
+struct attribute_group;
 
 struct device *hwmon_device_register(struct device *dev);
+struct device *devm_hwmon_device_register(struct device *dev,
+					const struct attribute_group **groups,
+					const char *name);
 
 void hwmon_device_unregister(struct device *dev);
+void devm_hwmon_device_unregister(struct device *dev);
 
 #endif
-- 
1.7.9.7

[RFC PATCH 2/8] hwmon: (ina209) Convert to devm_hwmon_device_register API

[Guenter Roeck]

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ina209.c |   29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/hwmon/ina209.c b/drivers/hwmon/ina209.c
index c6fdd5b..547935f 100644
--- a/drivers/hwmon/ina209.c
+++ b/drivers/hwmon/ina209.c
@@ -78,7 +78,7 @@ struct ina209_data {
 
 static struct ina209_data *ina209_update_device(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct ina209_data *data = i2c_get_clientdata(client);
 	struct ina209_data *ret = data;
 	s32 val;
@@ -234,7 +234,7 @@ static ssize_t ina209_set_interval(struct device *dev,
 				   struct device_attribute *da,
 				   const char *buf, size_t count)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct ina209_data *data = ina209_update_device(dev);
 	long val;
 	u16 regval;
@@ -260,7 +260,7 @@ static ssize_t ina209_set_interval(struct device *dev,
 static ssize_t ina209_show_interval(struct device *dev,
 				    struct device_attribute *da, char *buf)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct ina209_data *data = i2c_get_clientdata(client);
 
 	return snprintf(buf, PAGE_SIZE, "%d\n", data->update_interval);
@@ -285,7 +285,7 @@ static ssize_t ina209_reset_history(struct device *dev,
 				    const char *buf,
 				    size_t count)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct ina209_data *data = i2c_get_clientdata(client);
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
 	u32 mask = attr->index;
@@ -312,7 +312,7 @@ static ssize_t ina209_set_value(struct device *dev,
 				const char *buf,
 				size_t count)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct ina209_data *data = ina209_update_device(dev);
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
 	int reg = attr->index;
@@ -503,6 +503,11 @@ static const struct attribute_group ina209_group = {
 	.attrs = ina209_attributes,
 };
 
+static const struct attribute_group *ina209_groups[] = {
+	&ina209_group,
+	NULL
+};
+
 static void ina209_restore_conf(struct i2c_client *client,
 				struct ina209_data *data)
 {
@@ -581,21 +586,15 @@ static int ina209_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	/* Register sysfs hooks */
-	ret = sysfs_create_group(&client->dev.kobj, &ina209_group);
-	if (ret)
-		goto out_restore_conf;
-
-	data->hwmon_dev = hwmon_device_register(&client->dev);
+	data->hwmon_dev = devm_hwmon_device_register(&client->dev,
+						     ina209_groups, id->name);
 	if (IS_ERR(data->hwmon_dev)) {
 		ret = PTR_ERR(data->hwmon_dev);
-		goto out_hwmon_device_register;
+		goto out_restore_conf;
 	}
 
 	return 0;
 
-out_hwmon_device_register:
-	sysfs_remove_group(&client->dev.kobj, &ina209_group);
 out_restore_conf:
 	ina209_restore_conf(client, data);
 	return ret;
@@ -605,8 +604,6 @@ static int ina209_remove(struct i2c_client *client)
 {
 	struct ina209_data *data = i2c_get_clientdata(client);
 
-	hwmon_device_unregister(data->hwmon_dev);
-	sysfs_remove_group(&client->dev.kobj, &ina209_group);
 	ina209_restore_conf(client, data);
 
 	return 0;
-- 
1.7.9.7

[RFC PATCH 3/8] hwmon: (ltc4215) Convert to devm_hwmon_device_register API

[Guenter Roeck]

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ltc4215.c |   32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/hwmon/ltc4215.c b/drivers/hwmon/ltc4215.c
index e887610..83bc0ba 100644
--- a/drivers/hwmon/ltc4215.c
+++ b/drivers/hwmon/ltc4215.c
@@ -45,7 +45,7 @@ struct ltc4215_data {
 
 static struct ltc4215_data *ltc4215_update_device(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct ltc4215_data *data = i2c_get_clientdata(client);
 	s32 val;
 	int i;
@@ -244,12 +244,16 @@ static const struct attribute_group ltc4215_group = {
 	.attrs = ltc4215_attributes,
 };
 
+static const struct attribute_group *ltc4215_groups[] = {
+	&ltc4215_group,
+	NULL
+};
+
 static int ltc4215_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adapter = client->adapter;
 	struct ltc4215_data *data;
-	int ret;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
@@ -264,31 +268,17 @@ static int ltc4215_probe(struct i2c_client *client,
 	/* Initialize the LTC4215 chip */
 	i2c_smbus_write_byte_data(client, LTC4215_FAULT, 0x00);
 
-	/* Register sysfs hooks */
-	ret = sysfs_create_group(&client->dev.kobj, &ltc4215_group);
-	if (ret)
-		return ret;
-
-	data->hwmon_dev = hwmon_device_register(&client->dev);
-	if (IS_ERR(data->hwmon_dev)) {
-		ret = PTR_ERR(data->hwmon_dev);
-		goto out_hwmon_device_register;
-	}
+	data->hwmon_dev = devm_hwmon_device_register(&client->dev,
+						     ltc4215_groups,
+						     id->name);
+	if (IS_ERR(data->hwmon_dev))
+		return PTR_ERR(data->hwmon_dev);
 
 	return 0;
-
-out_hwmon_device_register:
-	sysfs_remove_group(&client->dev.kobj, &ltc4215_group);
-	return ret;
 }
 
 static int ltc4215_remove(struct i2c_client *client)
 {
-	struct ltc4215_data *data = i2c_get_clientdata(client);
-
-	hwmon_device_unregister(data->hwmon_dev);
-	sysfs_remove_group(&client->dev.kobj, &ltc4215_group);
-
 	return 0;
 }
 
-- 
1.7.9.7

[RFC PATCH 4/8] hwmon: (ltc4261) convert to devm_hwmon_device_register API

[Guenter Roeck]

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/ltc4261.c |   33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/ltc4261.c b/drivers/hwmon/ltc4261.c
index 84a2d28..a6da54c 100644
--- a/drivers/hwmon/ltc4261.c
+++ b/drivers/hwmon/ltc4261.c
@@ -67,7 +67,7 @@ struct ltc4261_data {
 
 static struct ltc4261_data *ltc4261_update_device(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct ltc4261_data *data = i2c_get_clientdata(client);
 	struct ltc4261_data *ret = data;
 
@@ -150,7 +150,7 @@ static ssize_t ltc4261_show_bool(struct device *dev,
 				 struct device_attribute *da, char *buf)
 {
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct ltc4261_data *data = ltc4261_update_device(dev);
 	u8 fault;
 
@@ -221,12 +221,16 @@ static const struct attribute_group ltc4261_group = {
 	.attrs = ltc4261_attributes,
 };
 
+static const struct attribute_group *ltc4261_groups[] = {
+	&ltc4261_group,
+	NULL
+};
+
 static int ltc4261_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adapter = client->adapter;
 	struct ltc4261_data *data;
-	int ret;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
@@ -246,31 +250,16 @@ static int ltc4261_probe(struct i2c_client *client,
 	/* Clear faults */
 	i2c_smbus_write_byte_data(client, LTC4261_FAULT, 0x00);
 
-	/* Register sysfs hooks */
-	ret = sysfs_create_group(&client->dev.kobj, &ltc4261_group);
-	if (ret)
-		return ret;
-
-	data->hwmon_dev = hwmon_device_register(&client->dev);
-	if (IS_ERR(data->hwmon_dev)) {
-		ret = PTR_ERR(data->hwmon_dev);
-		goto out_hwmon_device_register;
-	}
+	data->hwmon_dev = devm_hwmon_device_register(&client->dev,
+						     ltc4261_groups, id->name);
+	if (IS_ERR(data->hwmon_dev))
+		return PTR_ERR(data->hwmon_dev);
 
 	return 0;
-
-out_hwmon_device_register:
-	sysfs_remove_group(&client->dev.kobj, &ltc4261_group);
-	return ret;
 }
 
 static int ltc4261_remove(struct i2c_client *client)
 {
-	struct ltc4261_data *data = i2c_get_clientdata(client);
-
-	hwmon_device_unregister(data->hwmon_dev);
-	sysfs_remove_group(&client->dev.kobj, &ltc4261_group);
-
 	return 0;
 }
 
-- 
1.7.9.7

[RFC PATCH 5/8] hwmon: (max6697) Convert to devm_hwmon_device_register API

[Guenter Roeck]

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/max6697.c |  157 ++++++++++++++++++++++++++---------------------
 1 file changed, 86 insertions(+), 71 deletions(-)

diff --git a/drivers/hwmon/max6697.c b/drivers/hwmon/max6697.c
index bf4aa37..068425b 100644
--- a/drivers/hwmon/max6697.c
+++ b/drivers/hwmon/max6697.c
@@ -181,7 +181,7 @@ static const struct max6697_chip_data max6697_chip_data[] = {
 
 static struct max6697_data *max6697_update_device(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct max6697_data *data = i2c_get_clientdata(client);
 	struct max6697_data *ret = data;
 	int val;
@@ -303,7 +303,7 @@ static ssize_t set_temp(struct device *dev,
 {
 	int nr = to_sensor_dev_attr_2(devattr)->nr;
 	int index = to_sensor_dev_attr_2(devattr)->index;
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct max6697_data *data = i2c_get_clientdata(client);
 	long temp;
 	int ret;
@@ -466,15 +466,86 @@ static struct attribute *max6697_attributes[8][7] = {
 	}
 };
 
+static umode_t max6697_is_visible(struct kobject *kobj, struct attribute *attr,
+				  int index, int channel)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct i2c_client *client = to_i2c_client(dev->parent);
+	struct max6697_data *data = i2c_get_clientdata(client);
+	const struct max6697_chip_data *chip = data->chip;
+
+	if (channel >= chip->channels)
+		return 0;
+
+	if ((index == 3 || index == 4) && !(chip->have_crit & (1 << channel)))
+		return 0;
+	if (index == 5 && !(chip->have_fault & (1 << channel)))
+		return 0;
+
+	return attr->mode;
+}
+
+static umode_t max6697_visible_0(struct kobject *k, struct attribute *a, int i)
+{
+	return max6697_is_visible(k, a, i, 0);
+}
+
+static umode_t max6697_visible_1(struct kobject *k, struct attribute *a, int i)
+{
+	return max6697_is_visible(k, a, i, 1);
+}
+
+static umode_t max6697_visible_2(struct kobject *k, struct attribute *a, int i)
+{
+	return max6697_is_visible(k, a, i, 2);
+}
+
+static umode_t max6697_visible_3(struct kobject *k, struct attribute *a, int i)
+{
+	return max6697_is_visible(k, a, i, 3);
+}
+
+static umode_t max6697_visible_4(struct kobject *k, struct attribute *a, int i)
+{
+	return max6697_is_visible(k, a, i, 4);
+}
+
+static umode_t max6697_visible_5(struct kobject *k, struct attribute *a, int i)
+{
+	return max6697_is_visible(k, a, i, 5);
+}
+
+static umode_t max6697_visible_6(struct kobject *k, struct attribute *a, int i)
+{
+	return max6697_is_visible(k, a, i, 6);
+}
+
+static umode_t max6697_visible_7(struct kobject *k, struct attribute *a, int i)
+{
+	return max6697_is_visible(k, a, i, 7);
+}
+
 static const struct attribute_group max6697_group[8] = {
-	{ .attrs = max6697_attributes[0] },
-	{ .attrs = max6697_attributes[1] },
-	{ .attrs = max6697_attributes[2] },
-	{ .attrs = max6697_attributes[3] },
-	{ .attrs = max6697_attributes[4] },
-	{ .attrs = max6697_attributes[5] },
-	{ .attrs = max6697_attributes[6] },
-	{ .attrs = max6697_attributes[7] },
+	{ .attrs = max6697_attributes[0], .is_visible = max6697_visible_0 },
+	{ .attrs = max6697_attributes[1], .is_visible = max6697_visible_1 },
+	{ .attrs = max6697_attributes[2], .is_visible = max6697_visible_2 },
+	{ .attrs = max6697_attributes[3], .is_visible = max6697_visible_3 },
+	{ .attrs = max6697_attributes[4], .is_visible = max6697_visible_4 },
+	{ .attrs = max6697_attributes[5], .is_visible = max6697_visible_5 },
+	{ .attrs = max6697_attributes[6], .is_visible = max6697_visible_6 },
+	{ .attrs = max6697_attributes[7], .is_visible = max6697_visible_7 },
+};
+
+static const struct attribute_group *max6697_groups[] = {
+	&max6697_group[0],
+	&max6697_group[1],
+	&max6697_group[2],
+	&max6697_group[3],
+	&max6697_group[4],
+	&max6697_group[5],
+	&max6697_group[6],
+	&max6697_group[7],
+	NULL
 };
 
 static void max6697_get_config_of(struct device_node *node,
@@ -606,21 +677,13 @@ done:
 	return 0;
 }
 
-static void max6697_remove_files(struct i2c_client *client)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(max6697_group); i++)
-		sysfs_remove_group(&client->dev.kobj, &max6697_group[i]);
-}
-
 static int max6697_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adapter = client->adapter;
 	struct device *dev = &client->dev;
 	struct max6697_data *data;
-	int i, err;
+	int err;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
@@ -639,57 +702,10 @@ static int max6697_probe(struct i2c_client *client,
 	if (err)
 		return err;
 
-	for (i = 0; i < data->chip->channels; i++) {
-		err = sysfs_create_file(&dev->kobj,
-					max6697_attributes[i][0]);
-		if (err)
-			goto error;
-		err = sysfs_create_file(&dev->kobj,
-					max6697_attributes[i][1]);
-		if (err)
-			goto error;
-		err = sysfs_create_file(&dev->kobj,
-					max6697_attributes[i][2]);
-		if (err)
-			goto error;
-
-		if (data->chip->have_crit & (1 << i)) {
-			err = sysfs_create_file(&dev->kobj,
-						max6697_attributes[i][3]);
-			if (err)
-				goto error;
-			err = sysfs_create_file(&dev->kobj,
-						max6697_attributes[i][4]);
-			if (err)
-				goto error;
-		}
-		if (data->chip->have_fault & (1 << i)) {
-			err = sysfs_create_file(&dev->kobj,
-						max6697_attributes[i][5]);
-			if (err)
-				goto error;
-		}
-	}
-
-	data->hwmon_dev = hwmon_device_register(dev);
-	if (IS_ERR(data->hwmon_dev)) {
-		err = PTR_ERR(data->hwmon_dev);
-		goto error;
-	}
-
-	return 0;
-
-error:
-	max6697_remove_files(client);
-	return err;
-}
-
-static int max6697_remove(struct i2c_client *client)
-{
-	struct max6697_data *data = i2c_get_clientdata(client);
-
-	hwmon_device_unregister(data->hwmon_dev);
-	max6697_remove_files(client);
+	data->hwmon_dev = devm_hwmon_device_register(dev, max6697_groups,
+						     id->name);
+	if (IS_ERR(data->hwmon_dev))
+		return PTR_ERR(data->hwmon_dev);
 
 	return 0;
 }
@@ -715,7 +731,6 @@ static struct i2c_driver max6697_driver = {
 		.name	= "max6697",
 	},
 	.probe = max6697_probe,
-	.remove	= max6697_remove,
 	.id_table = max6697_id,
 };
 
-- 
1.7.9.7

[RFC PATCH 6/8] hwmon: (lm90) Convert to devm_hwmon_device_register API

[Guenter Roeck]

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c |  106 ++++++++++++++++++++++----------------------------
 1 file changed, 46 insertions(+), 60 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index 8eeb141..a35fa47 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -306,6 +306,7 @@ struct lm90_data {
 	unsigned long last_updated; /* in jiffies */
 	int kind;
 	u32 flags;
+	const struct attribute_group *groups[7];
 
 	int update_interval;	/* in milliseconds */
 
@@ -464,7 +465,7 @@ static void lm90_set_convrate(struct i2c_client *client, struct lm90_data *data,
 
 static struct lm90_data *lm90_update_device(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct lm90_data *data = i2c_get_clientdata(client);
 	unsigned long next_update;
 
@@ -738,7 +739,7 @@ static ssize_t set_temp8(struct device *dev, struct device_attribute *devattr,
 	};
 
 	struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct lm90_data *data = i2c_get_clientdata(client);
 	int nr = attr->index;
 	long val;
@@ -805,7 +806,7 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr,
 	};
 
 	struct sensor_device_attribute_2 *attr = to_sensor_dev_attr_2(devattr);
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct lm90_data *data = i2c_get_clientdata(client);
 	int nr = attr->nr;
 	int index = attr->index;
@@ -867,7 +868,7 @@ static ssize_t show_temphyst(struct device *dev,
 static ssize_t set_temphyst(struct device *dev, struct device_attribute *dummy,
 			    const char *buf, size_t count)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct lm90_data *data = i2c_get_clientdata(client);
 	long val;
 	int err;
@@ -921,7 +922,7 @@ static ssize_t set_update_interval(struct device *dev,
 				   struct device_attribute *attr,
 				   const char *buf, size_t count)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct lm90_data *data = i2c_get_clientdata(client);
 	unsigned long val;
 	int err;
@@ -971,6 +972,15 @@ static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
 static DEVICE_ATTR(update_interval, S_IRUGO | S_IWUSR, show_update_interval,
 		   set_update_interval);
 
+static struct attribute *temp2_attributes[] = {
+	&sensor_dev_attr_temp2_offset.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group temp2_group = {
+	.attrs = temp2_attributes,
+};
+
 static struct attribute *lm90_attributes[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
@@ -1083,14 +1093,14 @@ static const struct attribute_group lm90_temp3_group = {
 static ssize_t show_pec(struct device *dev, struct device_attribute *dummy,
 			char *buf)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	return sprintf(buf, "%d\n", !!(client->flags & I2C_CLIENT_PEC));
 }
 
 static ssize_t set_pec(struct device *dev, struct device_attribute *dummy,
 		       const char *buf, size_t count)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	long val;
 	int err;
 
@@ -1114,6 +1124,15 @@ static ssize_t set_pec(struct device *dev, struct device_attribute *dummy,
 
 static DEVICE_ATTR(pec, S_IWUSR | S_IRUGO, show_pec, set_pec);
 
+static struct attribute *pec_attributes[] = {
+	&dev_attr_pec.attr,
+	NULL
+};
+
+static const struct attribute_group pec_group = {
+	.attrs = pec_attributes,
+};
+
 /*
  * Real code
  */
@@ -1320,22 +1339,6 @@ static int lm90_detect(struct i2c_client *client,
 	return 0;
 }
 
-static void lm90_remove_files(struct i2c_client *client, struct lm90_data *data)
-{
-	struct device *dev = &client->dev;
-
-	if (data->flags & LM90_HAVE_TEMP3)
-		sysfs_remove_group(&dev->kobj, &lm90_temp3_group);
-	if (data->flags & LM90_HAVE_EMERGENCY_ALARM)
-		sysfs_remove_group(&dev->kobj, &lm90_emergency_alarm_group);
-	if (data->flags & LM90_HAVE_EMERGENCY)
-		sysfs_remove_group(&dev->kobj, &lm90_emergency_group);
-	if (data->flags & LM90_HAVE_OFFSET)
-		device_remove_file(dev, &sensor_dev_attr_temp2_offset.dev_attr);
-	device_remove_file(dev, &dev_attr_pec);
-	sysfs_remove_group(&dev->kobj, &lm90_group);
-}
-
 static void lm90_restore_conf(struct i2c_client *client, struct lm90_data *data)
 {
 	/* Restore initial configuration */
@@ -1397,7 +1400,7 @@ static int lm90_probe(struct i2c_client *client,
 	struct device *dev = &client->dev;
 	struct i2c_adapter *adapter = to_i2c_adapter(dev->parent);
 	struct lm90_data *data;
-	int err;
+	int err, group = 0;
 
 	data = devm_kzalloc(&client->dev, sizeof(struct lm90_data), GFP_KERNEL);
 	if (!data)
@@ -1430,47 +1433,32 @@ static int lm90_probe(struct i2c_client *client,
 	lm90_init_client(client);
 
 	/* Register sysfs hooks */
-	err = sysfs_create_group(&dev->kobj, &lm90_group);
-	if (err)
-		goto exit_restore;
-	if (client->flags & I2C_CLIENT_PEC) {
-		err = device_create_file(dev, &dev_attr_pec);
-		if (err)
-			goto exit_remove_files;
-	}
-	if (data->flags & LM90_HAVE_OFFSET) {
-		err = device_create_file(dev,
-					&sensor_dev_attr_temp2_offset.dev_attr);
-		if (err)
-			goto exit_remove_files;
-	}
-	if (data->flags & LM90_HAVE_EMERGENCY) {
-		err = sysfs_create_group(&dev->kobj, &lm90_emergency_group);
-		if (err)
-			goto exit_remove_files;
-	}
-	if (data->flags & LM90_HAVE_EMERGENCY_ALARM) {
-		err = sysfs_create_group(&dev->kobj,
-					 &lm90_emergency_alarm_group);
-		if (err)
-			goto exit_remove_files;
-	}
-	if (data->flags & LM90_HAVE_TEMP3) {
-		err = sysfs_create_group(&dev->kobj, &lm90_temp3_group);
-		if (err)
-			goto exit_remove_files;
-	}
+	data->groups[group++] = &lm90_group;
 
-	data->hwmon_dev = hwmon_device_register(dev);
+	if (client->flags & I2C_CLIENT_PEC)
+		data->groups[group++] = &pec_group;
+
+	if (data->flags & LM90_HAVE_OFFSET)
+		data->groups[group++] = &temp2_group;
+
+	if (data->flags & LM90_HAVE_EMERGENCY)
+		data->groups[group++] = &lm90_emergency_group;
+
+	if (data->flags & LM90_HAVE_EMERGENCY_ALARM)
+		data->groups[group++] = &lm90_emergency_alarm_group;
+
+	if (data->flags & LM90_HAVE_TEMP3)
+		data->groups[group++] = &lm90_temp3_group;
+
+	data->hwmon_dev = devm_hwmon_device_register(dev, data->groups,
+						     id->name);
 	if (IS_ERR(data->hwmon_dev)) {
 		err = PTR_ERR(data->hwmon_dev);
-		goto exit_remove_files;
+		goto exit_restore;
 	}
 
 	return 0;
 
-exit_remove_files:
-	lm90_remove_files(client, data);
 exit_restore:
 	lm90_restore_conf(client, data);
 	return err;
@@ -1480,8 +1468,6 @@ static int lm90_remove(struct i2c_client *client)
 {
 	struct lm90_data *data = i2c_get_clientdata(client);
 
-	hwmon_device_unregister(data->hwmon_dev);
-	lm90_remove_files(client, data);
 	lm90_restore_conf(client, data);
 
 	return 0;
-- 
1.7.9.7

[RFC PATCH 7/8] hwmon: (max16065) Convert to devm_hwmon_device_register API

[Guenter Roeck]

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/max16065.c |  103 +++++++++++++++++++++-------------------------
 1 file changed, 47 insertions(+), 56 deletions(-)

diff --git a/drivers/hwmon/max16065.c b/drivers/hwmon/max16065.c
index 2fa2c02..cb05219 100644
--- a/drivers/hwmon/max16065.c
+++ b/drivers/hwmon/max16065.c
@@ -84,6 +84,7 @@ static const bool max16065_have_current[] = {
 struct max16065_data {
 	enum chips type;
 	struct device *hwmon_dev;
+	const struct attribute_group *groups[4];
 	struct mutex update_lock;
 	bool valid;
 	unsigned long last_updated; /* in jiffies */
@@ -144,7 +145,7 @@ static int max16065_read_adc(struct i2c_client *client, int reg)
 
 static struct max16065_data *max16065_update_device(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct max16065_data *data = i2c_get_clientdata(client);
 
 	mutex_lock(&data->update_lock);
@@ -186,7 +187,7 @@ static ssize_t max16065_show_alarm(struct device *dev,
 
 	val &= (1 << attr2->index);
 	if (val)
-		i2c_smbus_write_byte_data(to_i2c_client(dev),
+		i2c_smbus_write_byte_data(to_i2c_client(dev->parent),
 					  MAX16065_FAULT(attr2->nr), val);
 
 	return snprintf(buf, PAGE_SIZE, "%d\n", !!val);
@@ -223,7 +224,7 @@ static ssize_t max16065_set_limit(struct device *dev,
 				  const char *buf, size_t count)
 {
 	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(da);
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct max16065_data *data = i2c_get_clientdata(client);
 	unsigned long val;
 	int err;
@@ -250,7 +251,7 @@ static ssize_t max16065_show_limit(struct device *dev,
 				   struct device_attribute *da, char *buf)
 {
 	struct sensor_device_attribute_2 *attr2 = to_sensor_dev_attr_2(da);
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct max16065_data *data = i2c_get_clientdata(client);
 
 	return snprintf(buf, PAGE_SIZE, "%d\n",
@@ -516,36 +517,58 @@ static struct attribute *max16065_max_attributes[] = {
 	NULL
 };
 
+static umode_t max16065_basic_is_visible(struct kobject *kobj,
+					 struct attribute *attr, int index)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct i2c_client *client = to_i2c_client(dev->parent);
+	struct max16065_data *data = i2c_get_clientdata(client);
+	int channel = index / 4;
+
+	if (channel >= data->num_adc || !data->range[channel])
+		return 0;
+
+	return attr->mode;
+}
+
 static const struct attribute_group max16065_basic_group = {
 	.attrs = max16065_basic_attributes,
+	.is_visible = max16065_basic_is_visible,
 };
 
 static const struct attribute_group max16065_current_group = {
 	.attrs = max16065_current_attributes,
 };
 
+static umode_t max16065_limit_is_visible(struct kobject *kobj,
+					 struct attribute *attr, int index)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct i2c_client *client = to_i2c_client(dev->parent);
+	struct max16065_data *data = i2c_get_clientdata(client);
+
+	if (index >= data->num_adc || !data->range[index])
+		return 0;
+
+	return attr->mode;
+}
+
 static const struct attribute_group max16065_min_group = {
 	.attrs = max16065_min_attributes,
+	.is_visible = max16065_limit_is_visible,
 };
 
 static const struct attribute_group max16065_max_group = {
 	.attrs = max16065_max_attributes,
+	.is_visible = max16065_limit_is_visible,
 };
 
-static void max16065_cleanup(struct i2c_client *client)
-{
-	sysfs_remove_group(&client->dev.kobj, &max16065_max_group);
-	sysfs_remove_group(&client->dev.kobj, &max16065_min_group);
-	sysfs_remove_group(&client->dev.kobj, &max16065_current_group);
-	sysfs_remove_group(&client->dev.kobj, &max16065_basic_group);
-}
-
 static int max16065_probe(struct i2c_client *client,
 			  const struct i2c_device_id *id)
 {
 	struct i2c_adapter *adapter = client->adapter;
 	struct max16065_data *data;
-	int i, j, val, ret;
+	int i, j, val, group = 0;
 	bool have_secondary;		/* true if chip has secondary limits */
 	bool secondary_is_max = false;	/* secondary limits reflect max */
 
@@ -597,37 +620,17 @@ static int max16065_probe(struct i2c_client *client,
 	}
 
 	/* Register sysfs hooks */
-	for (i = 0; i < data->num_adc * 4; i++) {
-		/* Do not create sysfs entry if channel is disabled */
-		if (!data->range[i / 4])
-			continue;
-
-		ret = sysfs_create_file(&client->dev.kobj,
-					max16065_basic_attributes[i]);
-		if (unlikely(ret))
-			goto out;
-	}
+	data->groups[group++] = &max16065_basic_group;
 
 	if (have_secondary) {
-		struct attribute **attr = secondary_is_max ?
-		  max16065_max_attributes : max16065_min_attributes;
-
-		for (i = 0; i < data->num_adc; i++) {
-			if (!data->range[i])
-				continue;
-
-			ret = sysfs_create_file(&client->dev.kobj, attr[i]);
-			if (unlikely(ret))
-				goto out;
-		}
+		data->groups[group++] =
+		  secondary_is_max ? &max16065_max_group : &max16065_min_group;
 	}
 
 	if (data->have_current) {
 		val = i2c_smbus_read_byte_data(client, MAX16065_CURR_CONTROL);
-		if (unlikely(val < 0)) {
-			ret = val;
-			goto out;
-		}
+		if (unlikely(val < 0))
+			return val;
 		if (val & MAX16065_CURR_ENABLE) {
 			/*
 			 * Current gain is 6, 12, 24, 48 based on values in
@@ -636,34 +639,22 @@ static int max16065_probe(struct i2c_client *client,
 			data->curr_gain = 6 << ((val >> 2) & 0x03);
 			data->range[MAX16065_NUM_ADC]
 			  = max16065_csp_adc_range[(val >> 1) & 0x01];
-			ret = sysfs_create_group(&client->dev.kobj,
-						 &max16065_current_group);
-			if (unlikely(ret))
-				goto out;
+			data->groups[group++] = &max16065_current_group;
 		} else {
 			data->have_current = false;
 		}
 	}
 
-	data->hwmon_dev = hwmon_device_register(&client->dev);
-	if (unlikely(IS_ERR(data->hwmon_dev))) {
-		ret = PTR_ERR(data->hwmon_dev);
-		goto out;
-	}
-	return 0;
+	data->hwmon_dev = devm_hwmon_device_register(&client->dev,
+						     data->groups, id->name);
+	if (unlikely(IS_ERR(data->hwmon_dev)))
+		return PTR_ERR(data->hwmon_dev);
 
-out:
-	max16065_cleanup(client);
-	return ret;
+	return 0;
 }
 
 static int max16065_remove(struct i2c_client *client)
 {
-	struct max16065_data *data = i2c_get_clientdata(client);
-
-	hwmon_device_unregister(data->hwmon_dev);
-	max16065_cleanup(client);
-
 	return 0;
 }
 
-- 
1.7.9.7

[RFC PATCH 8/8] hwmon: (tmp401) Convert to devm_hwmon_device_register API

[Guenter Roeck]

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/tmp401.c |  184 ++++++++++++++++++++++++------------------------
 1 file changed, 92 insertions(+), 92 deletions(-)

diff --git a/drivers/hwmon/tmp401.c b/drivers/hwmon/tmp401.c
index c85f696..69b5293 100644
--- a/drivers/hwmon/tmp401.c
+++ b/drivers/hwmon/tmp401.c
@@ -108,6 +108,7 @@ MODULE_DEVICE_TABLE(i2c, tmp401_id);
 
 struct tmp401_data {
 	struct device *hwmon_dev;
+	const struct attribute_group *groups[3];
 	struct mutex update_lock;
 	char valid; /* zero until following fields are valid */
 	unsigned long last_updated; /* in jiffies */
@@ -213,7 +214,7 @@ static struct tmp401_data *tmp401_update_device_reg16(
 
 static struct tmp401_data *tmp401_update_device(struct device *dev)
 {
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct tmp401_data *data = i2c_get_clientdata(client);
 
 	mutex_lock(&data->update_lock);
@@ -329,6 +330,7 @@ static ssize_t show_status(struct device *dev,
 static ssize_t store_temp_min(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	int index = to_sensor_dev_attr(devattr)->index;
 	struct tmp401_data *data = tmp401_update_device(dev);
 	long val;
@@ -341,9 +343,9 @@ static ssize_t store_temp_min(struct device *dev, struct device_attribute
 
 	mutex_lock(&data->update_lock);
 
-	i2c_smbus_write_byte_data(to_i2c_client(dev),
+	i2c_smbus_write_byte_data(client,
 		TMP401_TEMP_LOW_LIMIT_MSB_WRITE[index], reg >> 8);
-	i2c_smbus_write_byte_data(to_i2c_client(dev),
+	i2c_smbus_write_byte_data(client,
 		TMP401_TEMP_LOW_LIMIT_LSB[index], reg & 0xFF);
 
 	data->temp_low[index] = reg;
@@ -356,6 +358,7 @@ static ssize_t store_temp_min(struct device *dev, struct device_attribute
 static ssize_t store_temp_max(struct device *dev, struct device_attribute
 	*devattr, const char *buf, size_t count)
 {
+	struct i2c_client *client = to_i2c_client(dev->parent);
 	int index = to_sensor_dev_attr(devattr)->index;
 	struct tmp401_data *data = tmp401_update_device(dev);
 	long val;
@@ -368,9 +371,9 @@ static ssize_t store_temp_max(struct device *dev, struct device_attribute
 
 	mutex_lock(&data->update_lock);
 
-	i2c_smbus_write_byte_data(to_i2c_client(dev),
+	i2c_smbus_write_byte_data(client,
 		TMP401_TEMP_HIGH_LIMIT_MSB_WRITE[index], reg >> 8);
-	i2c_smbus_write_byte_data(to_i2c_client(dev),
+	i2c_smbus_write_byte_data(client,
 		TMP401_TEMP_HIGH_LIMIT_LSB[index], reg & 0xFF);
 
 	data->temp_high[index] = reg;
@@ -395,7 +398,7 @@ static ssize_t store_temp_crit(struct device *dev, struct device_attribute
 
 	mutex_lock(&data->update_lock);
 
-	i2c_smbus_write_byte_data(to_i2c_client(dev),
+	i2c_smbus_write_byte_data(to_i2c_client(dev->parent),
 		TMP401_TEMP_CRIT_LIMIT[index], reg);
 
 	data->temp_crit[index] = reg;
@@ -427,7 +430,7 @@ static ssize_t store_temp_crit_hyst(struct device *dev, struct device_attribute
 	val = clamp_val(val, temp - 255000, temp);
 	reg = ((temp - val) + 500) / 1000;
 
-	i2c_smbus_write_byte_data(to_i2c_client(dev),
+	i2c_smbus_write_byte_data(to_i2c_client(dev->parent),
 		TMP401_TEMP_CRIT_HYST, reg);
 
 	data->temp_crit_hyst = reg;
@@ -455,44 +458,68 @@ static ssize_t reset_temp_history(struct device *dev,
 			" supported. Use 1 to reset the history!\n", val);
 		return -EINVAL;
 	}
-	i2c_smbus_write_byte_data(to_i2c_client(dev),
+	i2c_smbus_write_byte_data(to_i2c_client(dev->parent),
 		TMP411_TEMP_LOWEST_MSB[0], val);
 
 	return count;
 }
 
-static struct sensor_device_attribute tmp401_attr[] = {
-	SENSOR_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0),
-	SENSOR_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp_min,
-		    store_temp_min, 0),
-	SENSOR_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
-		    store_temp_max, 0),
-	SENSOR_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp_crit,
-		    store_temp_crit, 0),
-	SENSOR_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_temp_crit_hyst,
-		    store_temp_crit_hyst, 0),
-	SENSOR_ATTR(temp1_min_alarm, S_IRUGO, show_status, NULL,
-		    TMP401_STATUS_LOCAL_LOW),
-	SENSOR_ATTR(temp1_max_alarm, S_IRUGO, show_status, NULL,
-		    TMP401_STATUS_LOCAL_HIGH),
-	SENSOR_ATTR(temp1_crit_alarm, S_IRUGO, show_status, NULL,
-		    TMP401_STATUS_LOCAL_CRIT),
-	SENSOR_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1),
-	SENSOR_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp_min,
-		    store_temp_min, 1),
-	SENSOR_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp_max,
-		    store_temp_max, 1),
-	SENSOR_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_temp_crit,
-		    store_temp_crit, 1),
-	SENSOR_ATTR(temp2_crit_hyst, S_IRUGO, show_temp_crit_hyst, NULL, 1),
-	SENSOR_ATTR(temp2_fault, S_IRUGO, show_status, NULL,
-		    TMP401_STATUS_REMOTE_OPEN),
-	SENSOR_ATTR(temp2_min_alarm, S_IRUGO, show_status, NULL,
-		    TMP401_STATUS_REMOTE_LOW),
-	SENSOR_ATTR(temp2_max_alarm, S_IRUGO, show_status, NULL,
-		    TMP401_STATUS_REMOTE_HIGH),
-	SENSOR_ATTR(temp2_crit_alarm, S_IRUGO, show_status, NULL,
-		    TMP401_STATUS_REMOTE_CRIT),
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_value, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO, show_temp_min,
+			  store_temp_min, 0);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
+			  store_temp_max, 0);
+static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp_crit,
+			  store_temp_crit, 0);
+static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO,
+			  show_temp_crit_hyst, store_temp_crit_hyst, 0);
+static SENSOR_DEVICE_ATTR(temp1_min_alarm, S_IRUGO, show_status, NULL,
+			  TMP401_STATUS_LOCAL_LOW);
+static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, show_status, NULL,
+			  TMP401_STATUS_LOCAL_HIGH);
+static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_status, NULL,
+			  TMP401_STATUS_LOCAL_CRIT);
+static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_value, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp_min,
+			  store_temp_min, 1);
+static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp_max,
+			  store_temp_max, 1);
+static SENSOR_DEVICE_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_temp_crit,
+			  store_temp_crit, 1);
+static SENSOR_DEVICE_ATTR(temp2_crit_hyst, S_IRUGO, show_temp_crit_hyst,
+			  NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_status, NULL,
+			  TMP401_STATUS_REMOTE_OPEN);
+static SENSOR_DEVICE_ATTR(temp2_min_alarm, S_IRUGO, show_status, NULL,
+			  TMP401_STATUS_REMOTE_LOW);
+static SENSOR_DEVICE_ATTR(temp2_max_alarm, S_IRUGO, show_status, NULL,
+			  TMP401_STATUS_REMOTE_HIGH);
+static SENSOR_DEVICE_ATTR(temp2_crit_alarm, S_IRUGO, show_status, NULL,
+			  TMP401_STATUS_REMOTE_CRIT);
+
+static struct attribute *tmp401_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_temp1_min.dev_attr.attr,
+	&sensor_dev_attr_temp1_max.dev_attr.attr,
+	&sensor_dev_attr_temp1_crit.dev_attr.attr,
+	&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
+	&sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
+	&sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
+	&sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
+	&sensor_dev_attr_temp2_input.dev_attr.attr,
+	&sensor_dev_attr_temp2_min.dev_attr.attr,
+	&sensor_dev_attr_temp2_max.dev_attr.attr,
+	&sensor_dev_attr_temp2_crit.dev_attr.attr,
+	&sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+	&sensor_dev_attr_temp2_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_min_alarm.dev_attr.attr,
+	&sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
+	&sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group tmp401_group = {
+	.attrs = tmp401_attributes,
 };
 
 /*
@@ -502,12 +529,24 @@ static struct sensor_device_attribute tmp401_attr[] = {
  * minimum and maximum register reset for both the local
  * and remote channels.
  */
-static struct sensor_device_attribute tmp411_attr[] = {
-	SENSOR_ATTR(temp1_highest, S_IRUGO, show_temp_highest, NULL, 0),
-	SENSOR_ATTR(temp1_lowest, S_IRUGO, show_temp_lowest, NULL, 0),
-	SENSOR_ATTR(temp2_highest, S_IRUGO, show_temp_highest, NULL, 1),
-	SENSOR_ATTR(temp2_lowest, S_IRUGO, show_temp_lowest, NULL, 1),
-	SENSOR_ATTR(temp_reset_history, S_IWUSR, NULL, reset_temp_history, 0),
+static SENSOR_DEVICE_ATTR(temp1_highest, S_IRUGO, show_temp_highest, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_lowest, S_IRUGO, show_temp_lowest, NULL, 0);
+static SENSOR_DEVICE_ATTR(temp2_highest, S_IRUGO, show_temp_highest, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp2_lowest, S_IRUGO, show_temp_lowest, NULL, 1);
+static SENSOR_DEVICE_ATTR(temp_reset_history, S_IWUSR, NULL, reset_temp_history,
+			  0);
+
+static struct attribute *tmp411_attributes[] = {
+	&sensor_dev_attr_temp1_highest.dev_attr.attr,
+	&sensor_dev_attr_temp1_lowest.dev_attr.attr,
+	&sensor_dev_attr_temp2_highest.dev_attr.attr,
+	&sensor_dev_attr_temp2_lowest.dev_attr.attr,
+	&sensor_dev_attr_temp_reset_history.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group tmp411_group = {
+	.attrs = tmp411_attributes,
 };
 
 /*
@@ -577,30 +616,9 @@ static int tmp401_detect(struct i2c_client *client,
 	return 0;
 }
 
-static int tmp401_remove(struct i2c_client *client)
-{
-	struct tmp401_data *data = i2c_get_clientdata(client);
-	int i;
-
-	if (data->hwmon_dev)
-		hwmon_device_unregister(data->hwmon_dev);
-
-	for (i = 0; i < ARRAY_SIZE(tmp401_attr); i++)
-		device_remove_file(&client->dev, &tmp401_attr[i].dev_attr);
-
-	if (data->kind == tmp411) {
-		for (i = 0; i < ARRAY_SIZE(tmp411_attr); i++)
-			device_remove_file(&client->dev,
-					   &tmp411_attr[i].dev_attr);
-	}
-
-	return 0;
-}
-
 static int tmp401_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
-	int i, err = 0;
 	struct tmp401_data *data;
 	const char *names[] = { "TMP401", "TMP411" };
 
@@ -617,37 +635,20 @@ static int tmp401_probe(struct i2c_client *client,
 	tmp401_init_client(client);
 
 	/* Register sysfs hooks */
-	for (i = 0; i < ARRAY_SIZE(tmp401_attr); i++) {
-		err = device_create_file(&client->dev,
-					 &tmp401_attr[i].dev_attr);
-		if (err)
-			goto exit_remove;
-	}
+	data->groups[0] = &tmp401_group;
 
 	/* Register additional tmp411 sysfs hooks */
-	if (data->kind == tmp411) {
-		for (i = 0; i < ARRAY_SIZE(tmp411_attr); i++) {
-			err = device_create_file(&client->dev,
-						 &tmp411_attr[i].dev_attr);
-			if (err)
-				goto exit_remove;
-		}
-	}
+	if (data->kind == tmp411)
+		data->groups[1] = &tmp411_group;
 
-	data->hwmon_dev = hwmon_device_register(&client->dev);
-	if (IS_ERR(data->hwmon_dev)) {
-		err = PTR_ERR(data->hwmon_dev);
-		data->hwmon_dev = NULL;
-		goto exit_remove;
-	}
+	data->hwmon_dev = devm_hwmon_device_register(&client->dev,
+						     data->groups, id->name);
+	if (IS_ERR(data->hwmon_dev))
+		return PTR_ERR(data->hwmon_dev);
 
 	dev_info(&client->dev, "Detected TI %s chip\n", names[data->kind]);
 
 	return 0;
-
-exit_remove:
-	tmp401_remove(client);
-	return err;
 }
 
 static struct i2c_driver tmp401_driver = {
@@ -656,7 +657,6 @@ static struct i2c_driver tmp401_driver = {
 		.name	= "tmp401",
 	},
 	.probe		= tmp401_probe,
-	.remove		= tmp401_remove,
 	.id_table	= tmp401_id,
 	.detect		= tmp401_detect,
 	.address_list	= normal_i2c,
-- 
1.7.9.7