[PATCHv1 00/14] power: supply: Fix custom sysfs attribute registration

[PATCHv1 00/14] power: supply: Fix custom sysfs attribute registration

[Sebastian Reichel]

Hi,

Udev may not detect sysfs attributes, that have been added
after the device has been created. Code doing that is racy [0].
The power-supply class properly registers its attributes
when the device is created, but some drivers add additional
custom attributes in a racy way.

The first patch from this series adds native support for custom
sysfs attributes to the power-supply subsystem. This feature
allows cleaner code and avoids a race condition preventing udev
from noticing the custom properties.

The following patches replace all instances of sysfs_create_*
in drivers/power/supply to use the new feature and a couple
of small cleanups.

TL;DR: This patchsets replaces all sysfs_create_* in power-supply.

[0] http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/

-- Sebastian

Sebastian Reichel (14):
  power: supply: core: add support for custom sysfs attributes
  power: supply: bq2415x: fix race-condition in sysfs registration
  power: supply: ds2780: fix race-condition in sysfs registration
  power: supply: ds2781: fix race-condition in sysfs registration
  power: supply: lp8788: fix race-condition in sysfs registration
  power: supply: bq24190_charger: fix race-condition in sysfs
    registration
  power: supply: bq24257: fix race-condition in sysfs registration
  power: supply: charger-manager: simplify generation of sysfs attribute
    group name
  power: supply: charger-manager: fix race-condition in sysfs
    registration
  power: supply: pcf50633: fix race-condition in sysfs registration
  power: supply: ds2780: fix race-condition in bin attribute
    registration
  power: supply: ds2781: fix race-condition in bin attribute
    registration
  power: supply: ds2780: switch to devm_power_supply_register
  power: supply: ds2781: switch to devm_power_supply_register

 drivers/power/supply/bq2415x_charger.c   | 119 ++++++++++-------------
 drivers/power/supply/bq24190_charger.c   |  43 ++------
 drivers/power/supply/bq24257_charger.c   |  15 +--
 drivers/power/supply/charger-manager.c   |  62 +++++-------
 drivers/power/supply/ds2780_battery.c    |  87 +++++------------
 drivers/power/supply/ds2781_battery.c    |  82 +++++-----------
 drivers/power/supply/lp8788-charger.c    |  62 +++++-------
 drivers/power/supply/pcf50633-charger.c  |  17 ++--
 drivers/power/supply/power_supply_core.c |   1 +
 include/linux/power/charger-manager.h    |   3 +-
 include/linux/power_supply.h             |   3 +
 11 files changed, 172 insertions(+), 322 deletions(-)

-- 
2.19.0

[PATCHv1 01/14] power: supply: core: add support for custom sysfs attributes

[Sebastian Reichel]

Add functionality to setup device specific sysfs attributes
in a race condition free manner

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/power_supply_core.c | 1 +
 include/linux/power_supply.h             | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index e85361878450..cb7ddced85ed 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -880,6 +880,7 @@ __power_supply_register(struct device *parent,
 	dev_set_drvdata(dev, psy);
 	psy->desc = desc;
 	if (cfg) {
+		dev->groups = cfg->attr_grp;
 		psy->drv_data = cfg->drv_data;
 		psy->of_node =
 			cfg->fwnode ? to_of_node(cfg->fwnode) : cfg->of_node;
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index f80769175c56..410466946890 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -204,6 +204,9 @@ struct power_supply_config {
 	/* Driver private data */
 	void *drv_data;
 
+	/* Device specific sysfs attributes */
+	const struct attribute_group **attr_grp;
+
 	char **supplied_to;
 	size_t num_supplicants;
 };
-- 
2.19.0

[PATCHv1 02/14] power: supply: bq2415x: fix race-condition in sysfs registration

[Sebastian Reichel]

This registers custom sysfs properties using the native functionality
of the power-supply framework, which cleans up the code a bit and
fixes a race-condition. Before this patch the sysfs attributes were
not properly registered to udev.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/bq2415x_charger.c | 119 ++++++++++---------------
 1 file changed, 49 insertions(+), 70 deletions(-)

diff --git a/drivers/power/supply/bq2415x_charger.c b/drivers/power/supply/bq2415x_charger.c
index cbec70f3e73e..6693e7aeead5 100644
--- a/drivers/power/supply/bq2415x_charger.c
+++ b/drivers/power/supply/bq2415x_charger.c
@@ -1032,54 +1032,6 @@ static int bq2415x_power_supply_get_property(struct power_supply *psy,
 	return 0;
 }
 
-static int bq2415x_power_supply_init(struct bq2415x_device *bq)
-{
-	int ret;
-	int chip;
-	char revstr[8];
-	struct power_supply_config psy_cfg = {
-		.drv_data = bq,
-		.of_node = bq->dev->of_node,
-	};
-
-	bq->charger_desc.name = bq->name;
-	bq->charger_desc.type = POWER_SUPPLY_TYPE_USB;
-	bq->charger_desc.properties = bq2415x_power_supply_props;
-	bq->charger_desc.num_properties =
-			ARRAY_SIZE(bq2415x_power_supply_props);
-	bq->charger_desc.get_property = bq2415x_power_supply_get_property;
-
-	ret = bq2415x_detect_chip(bq);
-	if (ret < 0)
-		chip = BQUNKNOWN;
-	else
-		chip = ret;
-
-	ret = bq2415x_detect_revision(bq);
-	if (ret < 0)
-		strcpy(revstr, "unknown");
-	else
-		sprintf(revstr, "1.%d", ret);
-
-	bq->model = kasprintf(GFP_KERNEL,
-				"chip %s, revision %s, vender code %.3d",
-				bq2415x_chip_name[chip], revstr,
-				bq2415x_get_vender_code(bq));
-	if (!bq->model) {
-		dev_err(bq->dev, "failed to allocate model name\n");
-		return -ENOMEM;
-	}
-
-	bq->charger = power_supply_register(bq->dev, &bq->charger_desc,
-					    &psy_cfg);
-	if (IS_ERR(bq->charger)) {
-		kfree(bq->model);
-		return PTR_ERR(bq->charger);
-	}
-
-	return 0;
-}
-
 static void bq2415x_power_supply_exit(struct bq2415x_device *bq)
 {
 	bq->autotimer = 0;
@@ -1496,7 +1448,7 @@ static DEVICE_ATTR(charge_status, S_IRUGO, bq2415x_sysfs_show_status, NULL);
 static DEVICE_ATTR(boost_status, S_IRUGO, bq2415x_sysfs_show_status, NULL);
 static DEVICE_ATTR(fault_status, S_IRUGO, bq2415x_sysfs_show_status, NULL);
 
-static struct attribute *bq2415x_sysfs_attributes[] = {
+static struct attribute *bq2415x_sysfs_attrs[] = {
 	/*
 	 * TODO: some (appropriate) of these attrs should be switched to
 	 * use power supply class props.
@@ -1525,19 +1477,55 @@ static struct attribute *bq2415x_sysfs_attributes[] = {
 	NULL,
 };
 
-static const struct attribute_group bq2415x_sysfs_attr_group = {
-	.attrs = bq2415x_sysfs_attributes,
-};
+ATTRIBUTE_GROUPS(bq2415x_sysfs);
 
-static int bq2415x_sysfs_init(struct bq2415x_device *bq)
+static int bq2415x_power_supply_init(struct bq2415x_device *bq)
 {
-	return sysfs_create_group(&bq->charger->dev.kobj,
-			&bq2415x_sysfs_attr_group);
-}
+	int ret;
+	int chip;
+	char revstr[8];
+	struct power_supply_config psy_cfg = {
+		.drv_data = bq,
+		.of_node = bq->dev->of_node,
+		.attr_grp = bq2415x_sysfs_groups,
+	};
 
-static void bq2415x_sysfs_exit(struct bq2415x_device *bq)
-{
-	sysfs_remove_group(&bq->charger->dev.kobj, &bq2415x_sysfs_attr_group);
+	bq->charger_desc.name = bq->name;
+	bq->charger_desc.type = POWER_SUPPLY_TYPE_USB;
+	bq->charger_desc.properties = bq2415x_power_supply_props;
+	bq->charger_desc.num_properties =
+			ARRAY_SIZE(bq2415x_power_supply_props);
+	bq->charger_desc.get_property = bq2415x_power_supply_get_property;
+
+	ret = bq2415x_detect_chip(bq);
+	if (ret < 0)
+		chip = BQUNKNOWN;
+	else
+		chip = ret;
+
+	ret = bq2415x_detect_revision(bq);
+	if (ret < 0)
+		strcpy(revstr, "unknown");
+	else
+		sprintf(revstr, "1.%d", ret);
+
+	bq->model = kasprintf(GFP_KERNEL,
+				"chip %s, revision %s, vender code %.3d",
+				bq2415x_chip_name[chip], revstr,
+				bq2415x_get_vender_code(bq));
+	if (!bq->model) {
+		dev_err(bq->dev, "failed to allocate model name\n");
+		return -ENOMEM;
+	}
+
+	bq->charger = power_supply_register(bq->dev, &bq->charger_desc,
+					    &psy_cfg);
+	if (IS_ERR(bq->charger)) {
+		kfree(bq->model);
+		return PTR_ERR(bq->charger);
+	}
+
+	return 0;
 }
 
 /* main bq2415x probe function */
@@ -1651,16 +1639,10 @@ static int bq2415x_probe(struct i2c_client *client,
 		goto error_2;
 	}
 
-	ret = bq2415x_sysfs_init(bq);
-	if (ret) {
-		dev_err(bq->dev, "failed to create sysfs entries: %d\n", ret);
-		goto error_3;
-	}
-
 	ret = bq2415x_set_defaults(bq);
 	if (ret) {
 		dev_err(bq->dev, "failed to set default values: %d\n", ret);
-		goto error_4;
+		goto error_3;
 	}
 
 	if (bq->notify_node || bq->init_data.notify_device) {
@@ -1668,7 +1650,7 @@ static int bq2415x_probe(struct i2c_client *client,
 		ret = power_supply_reg_notifier(&bq->nb);
 		if (ret) {
 			dev_err(bq->dev, "failed to reg notifier: %d\n", ret);
-			goto error_4;
+			goto error_3;
 		}
 
 		bq->automode = 1;
@@ -1707,8 +1689,6 @@ static int bq2415x_probe(struct i2c_client *client,
 	dev_info(bq->dev, "driver registered\n");
 	return 0;
 
-error_4:
-	bq2415x_sysfs_exit(bq);
 error_3:
 	bq2415x_power_supply_exit(bq);
 error_2:
@@ -1733,7 +1713,6 @@ static int bq2415x_remove(struct i2c_client *client)
 		power_supply_unreg_notifier(&bq->nb);
 
 	of_node_put(bq->notify_node);
-	bq2415x_sysfs_exit(bq);
 	bq2415x_power_supply_exit(bq);
 
 	bq2415x_reset_chip(bq);
-- 
2.19.0

[PATCHv1 03/14] power: supply: ds2780: fix race-condition in sysfs registration

[Sebastian Reichel]

This registers custom sysfs properties using the native functionality
of the power-supply framework, which cleans up the code a bit and
fixes a race-condition. Before this patch the sysfs attributes were
not properly registered to udev.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/ds2780_battery.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/power/supply/ds2780_battery.c b/drivers/power/supply/ds2780_battery.c
index 370e9109342b..fa015787d58d 100644
--- a/drivers/power/supply/ds2780_battery.c
+++ b/drivers/power/supply/ds2780_battery.c
@@ -723,7 +723,7 @@ static DEVICE_ATTR(pio_pin, S_IRUGO | S_IWUSR, ds2780_get_pio_pin,
 	ds2780_set_pio_pin);
 
 
-static struct attribute *ds2780_attributes[] = {
+static struct attribute *ds2780_sysfs_attrs[] = {
 	&dev_attr_pmod_enabled.attr,
 	&dev_attr_sense_resistor_value.attr,
 	&dev_attr_rsgain_setting.attr,
@@ -731,9 +731,7 @@ static struct attribute *ds2780_attributes[] = {
 	NULL
 };
 
-static const struct attribute_group ds2780_attr_group = {
-	.attrs = ds2780_attributes,
-};
+ATTRIBUTE_GROUPS(ds2780_sysfs);
 
 static int ds2780_battery_probe(struct platform_device *pdev)
 {
@@ -758,6 +756,7 @@ static int ds2780_battery_probe(struct platform_device *pdev)
 	dev_info->bat_desc.get_property	= ds2780_battery_get_property;
 
 	psy_cfg.drv_data		= dev_info;
+	psy_cfg.attr_grp		= ds2780_sysfs_groups;
 
 	dev_info->bat = power_supply_register(&pdev->dev, &dev_info->bat_desc,
 					      &psy_cfg);
@@ -767,18 +766,12 @@ static int ds2780_battery_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	ret = sysfs_create_group(&dev_info->bat->dev.kobj, &ds2780_attr_group);
-	if (ret) {
-		dev_err(dev_info->dev, "failed to create sysfs group\n");
-		goto fail_unregister;
-	}
-
 	ret = sysfs_create_bin_file(&dev_info->bat->dev.kobj,
 					&ds2780_param_eeprom_bin_attr);
 	if (ret) {
 		dev_err(dev_info->dev,
 				"failed to create param eeprom bin file");
-		goto fail_remove_group;
+		goto fail_unregister;
 	}
 
 	ret = sysfs_create_bin_file(&dev_info->bat->dev.kobj,
@@ -794,8 +787,6 @@ static int ds2780_battery_probe(struct platform_device *pdev)
 fail_remove_bin_file:
 	sysfs_remove_bin_file(&dev_info->bat->dev.kobj,
 				&ds2780_param_eeprom_bin_attr);
-fail_remove_group:
-	sysfs_remove_group(&dev_info->bat->dev.kobj, &ds2780_attr_group);
 fail_unregister:
 	power_supply_unregister(dev_info->bat);
 fail:
@@ -806,12 +797,6 @@ static int ds2780_battery_remove(struct platform_device *pdev)
 {
 	struct ds2780_device_info *dev_info = platform_get_drvdata(pdev);
 
-	/*
-	 * Remove attributes before unregistering power supply
-	 * because 'bat' will be freed on power_supply_unregister() call.
-	 */
-	sysfs_remove_group(&dev_info->bat->dev.kobj, &ds2780_attr_group);
-
 	power_supply_unregister(dev_info->bat);
 
 	return 0;
-- 
2.19.0

[PATCHv1 04/14] power: supply: ds2781: fix race-condition in sysfs registration

[Sebastian Reichel]

This registers custom sysfs properties using the native functionality
of the power-supply framework, which cleans up the code a bit and
fixes a race-condition. Before this patch the sysfs attributes were
not properly registered to udev.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/ds2781_battery.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/power/supply/ds2781_battery.c b/drivers/power/supply/ds2781_battery.c
index d1b5a19aae7c..5bfd4c77eb89 100644
--- a/drivers/power/supply/ds2781_battery.c
+++ b/drivers/power/supply/ds2781_battery.c
@@ -726,7 +726,7 @@ static DEVICE_ATTR(pio_pin, S_IRUGO | S_IWUSR, ds2781_get_pio_pin,
 	ds2781_set_pio_pin);
 
 
-static struct attribute *ds2781_attributes[] = {
+static struct attribute *ds2781_sysfs_attrs[] = {
 	&dev_attr_pmod_enabled.attr,
 	&dev_attr_sense_resistor_value.attr,
 	&dev_attr_rsgain_setting.attr,
@@ -734,9 +734,7 @@ static struct attribute *ds2781_attributes[] = {
 	NULL
 };
 
-static const struct attribute_group ds2781_attr_group = {
-	.attrs = ds2781_attributes,
-};
+ATTRIBUTE_GROUPS(ds2781_sysfs);
 
 static int ds2781_battery_probe(struct platform_device *pdev)
 {
@@ -759,6 +757,7 @@ static int ds2781_battery_probe(struct platform_device *pdev)
 	dev_info->bat_desc.get_property	= ds2781_battery_get_property;
 
 	psy_cfg.drv_data		= dev_info;
+	psy_cfg.attr_grp		= ds2781_sysfs_groups;
 
 	dev_info->bat = power_supply_register(&pdev->dev, &dev_info->bat_desc,
 						&psy_cfg);
@@ -768,18 +767,12 @@ static int ds2781_battery_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	ret = sysfs_create_group(&dev_info->bat->dev.kobj, &ds2781_attr_group);
-	if (ret) {
-		dev_err(dev_info->dev, "failed to create sysfs group\n");
-		goto fail_unregister;
-	}
-
 	ret = sysfs_create_bin_file(&dev_info->bat->dev.kobj,
 					&ds2781_param_eeprom_bin_attr);
 	if (ret) {
 		dev_err(dev_info->dev,
 				"failed to create param eeprom bin file");
-		goto fail_remove_group;
+		goto fail_unregister;
 	}
 
 	ret = sysfs_create_bin_file(&dev_info->bat->dev.kobj,
@@ -795,8 +788,6 @@ static int ds2781_battery_probe(struct platform_device *pdev)
 fail_remove_bin_file:
 	sysfs_remove_bin_file(&dev_info->bat->dev.kobj,
 				&ds2781_param_eeprom_bin_attr);
-fail_remove_group:
-	sysfs_remove_group(&dev_info->bat->dev.kobj, &ds2781_attr_group);
 fail_unregister:
 	power_supply_unregister(dev_info->bat);
 fail:
@@ -807,12 +798,6 @@ static int ds2781_battery_remove(struct platform_device *pdev)
 {
 	struct ds2781_device_info *dev_info = platform_get_drvdata(pdev);
 
-	/*
-	 * Remove attributes before unregistering power supply
-	 * because 'bat' will be freed on power_supply_unregister() call.
-	 */
-	sysfs_remove_group(&dev_info->bat->dev.kobj, &ds2781_attr_group);
-
 	power_supply_unregister(dev_info->bat);
 
 	return 0;
-- 
2.19.0

[PATCHv1 05/14] power: supply: lp8788: fix race-condition in sysfs registration

[Sebastian Reichel]

This registers custom sysfs properties using the native functionality
of the power-supply framework, which cleans up the code a bit and
fixes a race-condition. Before this patch the sysfs attributes were
not properly registered to udev.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/lp8788-charger.c | 62 ++++++++++++---------------
 1 file changed, 27 insertions(+), 35 deletions(-)

diff --git a/drivers/power/supply/lp8788-charger.c b/drivers/power/supply/lp8788-charger.c
index 0f3432795f3c..309e6efbb8ef 100644
--- a/drivers/power/supply/lp8788-charger.c
+++ b/drivers/power/supply/lp8788-charger.c
@@ -410,30 +410,6 @@ static const struct power_supply_desc lp8788_psy_battery_desc = {
 	.get_property	= lp8788_battery_get_property,
 };
 
-static int lp8788_psy_register(struct platform_device *pdev,
-				struct lp8788_charger *pchg)
-{
-	struct power_supply_config charger_cfg = {};
-
-	charger_cfg.supplied_to = battery_supplied_to;
-	charger_cfg.num_supplicants = ARRAY_SIZE(battery_supplied_to);
-
-	pchg->charger = power_supply_register(&pdev->dev,
-					      &lp8788_psy_charger_desc,
-					      &charger_cfg);
-	if (IS_ERR(pchg->charger))
-		return -EPERM;
-
-	pchg->battery = power_supply_register(&pdev->dev,
-					      &lp8788_psy_battery_desc, NULL);
-	if (IS_ERR(pchg->battery)) {
-		power_supply_unregister(pchg->charger);
-		return -EPERM;
-	}
-
-	return 0;
-}
-
 static void lp8788_psy_unregister(struct lp8788_charger *pchg)
 {
 	power_supply_unregister(pchg->battery);
@@ -690,16 +666,39 @@ static DEVICE_ATTR(charger_status, S_IRUSR, lp8788_show_charger_status, NULL);
 static DEVICE_ATTR(eoc_time, S_IRUSR, lp8788_show_eoc_time, NULL);
 static DEVICE_ATTR(eoc_level, S_IRUSR, lp8788_show_eoc_level, NULL);
 
-static struct attribute *lp8788_charger_attr[] = {
+static struct attribute *lp8788_charger_sysfs_attrs[] = {
 	&dev_attr_charger_status.attr,
 	&dev_attr_eoc_time.attr,
 	&dev_attr_eoc_level.attr,
 	NULL,
 };
 
-static const struct attribute_group lp8788_attr_group = {
-	.attrs = lp8788_charger_attr,
-};
+ATTRIBUTE_GROUPS(lp8788_charger_sysfs);
+
+static int lp8788_psy_register(struct platform_device *pdev,
+				struct lp8788_charger *pchg)
+{
+	struct power_supply_config charger_cfg = {};
+
+	charger_cfg.attr_grp = lp8788_charger_sysfs_groups;
+	charger_cfg.supplied_to = battery_supplied_to;
+	charger_cfg.num_supplicants = ARRAY_SIZE(battery_supplied_to);
+
+	pchg->charger = power_supply_register(&pdev->dev,
+					      &lp8788_psy_charger_desc,
+					      &charger_cfg);
+	if (IS_ERR(pchg->charger))
+		return -EPERM;
+
+	pchg->battery = power_supply_register(&pdev->dev,
+					      &lp8788_psy_battery_desc, NULL);
+	if (IS_ERR(pchg->battery)) {
+		power_supply_unregister(pchg->charger);
+		return -EPERM;
+	}
+
+	return 0;
+}
 
 static int lp8788_charger_probe(struct platform_device *pdev)
 {
@@ -726,12 +725,6 @@ static int lp8788_charger_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = sysfs_create_group(&pdev->dev.kobj, &lp8788_attr_group);
-	if (ret) {
-		lp8788_psy_unregister(pchg);
-		return ret;
-	}
-
 	ret = lp8788_irq_register(pdev, pchg);
 	if (ret)
 		dev_warn(dev, "failed to register charger irq: %d\n", ret);
@@ -745,7 +738,6 @@ static int lp8788_charger_remove(struct platform_device *pdev)
 
 	flush_work(&pchg->charger_work);
 	lp8788_irq_unregister(pdev, pchg);
-	sysfs_remove_group(&pdev->dev.kobj, &lp8788_attr_group);
 	lp8788_psy_unregister(pchg);
 	lp8788_release_adc_channel(pchg);
 
-- 
2.19.0

[PATCHv1 06/14] power: supply: bq24190_charger: fix race-condition in sysfs registration

[Sebastian Reichel]

This registers custom sysfs properties using the native functionality
of the power-supply framework, which cleans up the code a bit and
fixes a race-condition. Before this patch the sysfs attributes were
not properly registered to udev.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/bq24190_charger.c | 43 +++++---------------------
 1 file changed, 8 insertions(+), 35 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index b58df04d03b3..9d63d4e601cc 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -402,9 +402,7 @@ static struct bq24190_sysfs_field_info bq24190_sysfs_field_tbl[] = {
 static struct attribute *
 	bq24190_sysfs_attrs[ARRAY_SIZE(bq24190_sysfs_field_tbl) + 1];
 
-static const struct attribute_group bq24190_sysfs_attr_group = {
-	.attrs = bq24190_sysfs_attrs,
-};
+ATTRIBUTE_GROUPS(bq24190_sysfs);
 
 static void bq24190_sysfs_init_attrs(void)
 {
@@ -491,26 +489,6 @@ static ssize_t bq24190_sysfs_store(struct device *dev,
 
 	return count;
 }
-
-static int bq24190_sysfs_create_group(struct bq24190_dev_info *bdi)
-{
-	bq24190_sysfs_init_attrs();
-
-	return sysfs_create_group(&bdi->charger->dev.kobj,
-			&bq24190_sysfs_attr_group);
-}
-
-static void bq24190_sysfs_remove_group(struct bq24190_dev_info *bdi)
-{
-	sysfs_remove_group(&bdi->charger->dev.kobj, &bq24190_sysfs_attr_group);
-}
-#else
-static int bq24190_sysfs_create_group(struct bq24190_dev_info *bdi)
-{
-	return 0;
-}
-
-static inline void bq24190_sysfs_remove_group(struct bq24190_dev_info *bdi) {}
 #endif
 
 #ifdef CONFIG_REGULATOR
@@ -1736,6 +1714,11 @@ static int bq24190_probe(struct i2c_client *client,
 		goto out_pmrt;
 	}
 
+#ifdef CONFIG_SYSFS
+	bq24190_sysfs_init_attrs();
+	charger_cfg.attr_grp = bq24190_sysfs_groups;
+#endif
+
 	charger_cfg.drv_data = bdi;
 	charger_cfg.of_node = dev->of_node;
 	charger_cfg.supplied_to = bq24190_charger_supplied_to;
@@ -1773,12 +1756,6 @@ static int bq24190_probe(struct i2c_client *client,
 		goto out_charger;
 	}
 
-	ret = bq24190_sysfs_create_group(bdi);
-	if (ret < 0) {
-		dev_err(dev, "Can't create sysfs entries\n");
-		goto out_charger;
-	}
-
 	bdi->initialized = true;
 
 	ret = devm_request_threaded_irq(dev, client->irq, NULL,
@@ -1787,12 +1764,12 @@ static int bq24190_probe(struct i2c_client *client,
 			"bq24190-charger", bdi);
 	if (ret < 0) {
 		dev_err(dev, "Can't set up irq handler\n");
-		goto out_sysfs;
+		goto out_charger;
 	}
 
 	ret = bq24190_register_vbus_regulator(bdi);
 	if (ret < 0)
-		goto out_sysfs;
+		goto out_charger;
 
 	enable_irq_wake(client->irq);
 
@@ -1801,9 +1778,6 @@ static int bq24190_probe(struct i2c_client *client,
 
 	return 0;
 
-out_sysfs:
-	bq24190_sysfs_remove_group(bdi);
-
 out_charger:
 	if (!IS_ERR_OR_NULL(bdi->battery))
 		power_supply_unregister(bdi->battery);
@@ -1828,7 +1802,6 @@ static int bq24190_remove(struct i2c_client *client)
 	}
 
 	bq24190_register_reset(bdi);
-	bq24190_sysfs_remove_group(bdi);
 	if (bdi->battery)
 		power_supply_unregister(bdi->battery);
 	power_supply_unregister(bdi->charger);
-- 
2.19.0

[PATCHv1 07/14] power: supply: bq24257: fix race-condition in sysfs registration

[Sebastian Reichel]

This registers custom sysfs properties using the native functionality
of the power-supply framework, which cleans up the code a bit and
fixes a race-condition. Before this patch the sysfs attributes were
not properly registered to udev.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/bq24257_charger.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/power/supply/bq24257_charger.c b/drivers/power/supply/bq24257_charger.c
index 6fc31bdc639b..673c7d6ff1a7 100644
--- a/drivers/power/supply/bq24257_charger.c
+++ b/drivers/power/supply/bq24257_charger.c
@@ -845,7 +845,7 @@ static DEVICE_ATTR(high_impedance_enable, S_IWUSR | S_IRUGO,
 static DEVICE_ATTR(sysoff_enable, S_IWUSR | S_IRUGO,
 		   bq24257_sysfs_show_enable, bq24257_sysfs_set_enable);
 
-static struct attribute *bq24257_charger_attr[] = {
+static struct attribute *bq24257_charger_sysfs_attrs[] = {
 	&dev_attr_ovp_voltage.attr,
 	&dev_attr_in_dpm_voltage.attr,
 	&dev_attr_high_impedance_enable.attr,
@@ -853,14 +853,13 @@ static struct attribute *bq24257_charger_attr[] = {
 	NULL,
 };
 
-static const struct attribute_group bq24257_attr_group = {
-	.attrs = bq24257_charger_attr,
-};
+ATTRIBUTE_GROUPS(bq24257_charger_sysfs);
 
 static int bq24257_power_supply_init(struct bq24257_device *bq)
 {
 	struct power_supply_config psy_cfg = { .drv_data = bq, };
 
+	psy_cfg.attr_grp = bq24257_charger_sysfs_groups;
 	psy_cfg.supplied_to = bq24257_charger_supplied_to;
 	psy_cfg.num_supplicants = ARRAY_SIZE(bq24257_charger_supplied_to);
 
@@ -1084,12 +1083,6 @@ static int bq24257_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	ret = sysfs_create_group(&bq->charger->dev.kobj, &bq24257_attr_group);
-	if (ret < 0) {
-		dev_err(dev, "Can't create sysfs entries\n");
-		return ret;
-	}
-
 	return 0;
 }
 
@@ -1100,8 +1093,6 @@ static int bq24257_remove(struct i2c_client *client)
 	if (bq->iilimit_autoset_enable)
 		cancel_delayed_work_sync(&bq->iilimit_setup_work);
 
-	sysfs_remove_group(&bq->charger->dev.kobj, &bq24257_attr_group);
-
 	bq24257_field_write(bq, F_RESET, 1); /* reset to defaults */
 
 	return 0;
-- 
2.19.0

[PATCHv1 08/14] power: supply: charger-manager: simplify generation of sysfs attribute group name

[Sebastian Reichel]

This is a simple cleanup and there should be no functional changes.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/charger-manager.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index faa1a67cf3d2..7ce0a4a2645c 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -1369,8 +1369,7 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
 	struct charger_desc *desc = cm->desc;
 	struct charger_regulator *charger;
 	int chargers_externally_control = 1;
-	char buf[11];
-	char *str;
+	char *name;
 	int ret;
 	int i;
 
@@ -1378,19 +1377,15 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
 	for (i = 0; i < desc->num_charger_regulators; i++) {
 		charger = &desc->charger_regulators[i];
 
-		snprintf(buf, 10, "charger.%d", i);
-		str = devm_kzalloc(cm->dev,
-				strlen(buf) + 1, GFP_KERNEL);
-		if (!str)
+		name = devm_kasprintf(cm->dev, GFP_KERNEL, "charger.%d", i);
+		if (!name)
 			return -ENOMEM;
 
-		strcpy(str, buf);
-
 		charger->attrs[0] = &charger->attr_name.attr;
 		charger->attrs[1] = &charger->attr_state.attr;
 		charger->attrs[2] = &charger->attr_externally_control.attr;
 		charger->attrs[3] = NULL;
-		charger->attr_g.name = str;
+		charger->attr_g.name = name;
 		charger->attr_g.attrs = charger->attrs;
 
 		sysfs_attr_init(&charger->attr_name.attr);
-- 
2.19.0

[PATCHv1 09/14] power: supply: charger-manager: fix race-condition in sysfs registration

[Sebastian Reichel]

This registers custom sysfs properties using the native functionality
of the power-supply framework, which cleans up the code a bit and
fixes a race-condition. Before this patch the sysfs attributes were
not properly registered to udev.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/charger-manager.c | 51 +++++++++++---------------
 include/linux/power/charger-manager.h  |  3 +-
 2 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/drivers/power/supply/charger-manager.c b/drivers/power/supply/charger-manager.c
index 7ce0a4a2645c..dab68c6af77a 100644
--- a/drivers/power/supply/charger-manager.c
+++ b/drivers/power/supply/charger-manager.c
@@ -1352,7 +1352,7 @@ static ssize_t charger_externally_control_store(struct device *dev,
 }
 
 /**
- * charger_manager_register_sysfs - Register sysfs entry for each charger
+ * charger_manager_prepare_sysfs - Prepare sysfs entry for each charger
  * @cm: the Charger Manager representing the battery.
  *
  * This function add sysfs entry for charger(regulator) to control charger from
@@ -1364,13 +1364,12 @@ static ssize_t charger_externally_control_store(struct device *dev,
  * externally_control, this charger isn't controlled from charger-manager and
  * always stay off state of regulator.
  */
-static int charger_manager_register_sysfs(struct charger_manager *cm)
+static int charger_manager_prepare_sysfs(struct charger_manager *cm)
 {
 	struct charger_desc *desc = cm->desc;
 	struct charger_regulator *charger;
 	int chargers_externally_control = 1;
 	char *name;
-	int ret;
 	int i;
 
 	/* Create sysfs entry to control charger(regulator) */
@@ -1385,8 +1384,10 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
 		charger->attrs[1] = &charger->attr_state.attr;
 		charger->attrs[2] = &charger->attr_externally_control.attr;
 		charger->attrs[3] = NULL;
-		charger->attr_g.name = name;
-		charger->attr_g.attrs = charger->attrs;
+
+		charger->attr_grp.name = name;
+		charger->attr_grp.attrs = charger->attrs;
+		desc->sysfs_groups[i] = &charger->attr_grp;
 
 		sysfs_attr_init(&charger->attr_name.attr);
 		charger->attr_name.attr.name = "name";
@@ -1413,14 +1414,6 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
 
 		dev_info(cm->dev, "'%s' regulator's externally_control is %d\n",
 			 charger->regulator_name, charger->externally_control);
-
-		ret = sysfs_create_group(&cm->charger_psy->dev.kobj,
-					&charger->attr_g);
-		if (ret < 0) {
-			dev_err(cm->dev, "Cannot create sysfs entry of %s regulator\n",
-				charger->regulator_name);
-			return ret;
-		}
 	}
 
 	if (chargers_externally_control) {
@@ -1561,6 +1554,13 @@ static struct charger_desc *of_cm_parse_desc(struct device *dev)
 
 		desc->charger_regulators = chg_regs;
 
+		desc->sysfs_groups = devm_kcalloc(dev,
+					desc->num_charger_regulators + 1,
+					sizeof(*desc->sysfs_groups),
+					GFP_KERNEL);
+		if (!desc->sysfs_groups)
+			return ERR_PTR(-ENOMEM);
+
 		for_each_child_of_node(np, child) {
 			struct charger_cable *cables;
 			struct device_node *_child;
@@ -1767,6 +1767,15 @@ static int charger_manager_probe(struct platform_device *pdev)
 
 	INIT_DELAYED_WORK(&cm->fullbatt_vchk_work, fullbatt_vchk);
 
+	/* Register sysfs entry for charger(regulator) */
+	ret = charger_manager_prepare_sysfs(cm);
+	if (ret < 0) {
+		dev_err(&pdev->dev,
+			"Cannot prepare sysfs entry of regulators\n");
+		return ret;
+	}
+	psy_cfg.attr_grp = desc->sysfs_groups;
+
 	cm->charger_psy = power_supply_register(&pdev->dev,
 						&cm->charger_psy_desc,
 						&psy_cfg);
@@ -1783,14 +1792,6 @@ static int charger_manager_probe(struct platform_device *pdev)
 		goto err_reg_extcon;
 	}
 
-	/* Register sysfs entry for charger(regulator) */
-	ret = charger_manager_register_sysfs(cm);
-	if (ret < 0) {
-		dev_err(&pdev->dev,
-			"Cannot initialize sysfs entry of regulator\n");
-		goto err_reg_sysfs;
-	}
-
 	/* Add to the list */
 	mutex_lock(&cm_list_mtx);
 	list_add(&cm->entry, &cm_list);
@@ -1814,14 +1815,6 @@ static int charger_manager_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_reg_sysfs:
-	for (i = 0; i < desc->num_charger_regulators; i++) {
-		struct charger_regulator *charger;
-
-		charger = &desc->charger_regulators[i];
-		sysfs_remove_group(&cm->charger_psy->dev.kobj,
-				&charger->attr_g);
-	}
 err_reg_extcon:
 	for (i = 0; i < desc->num_charger_regulators; i++) {
 		struct charger_regulator *charger;
diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h
index c4fa907c8f14..2ce8d00c20de 100644
--- a/include/linux/power/charger-manager.h
+++ b/include/linux/power/charger-manager.h
@@ -119,7 +119,7 @@ struct charger_regulator {
 	struct charger_cable *cables;
 	int num_cables;
 
-	struct attribute_group attr_g;
+	struct attribute_group attr_grp;
 	struct device_attribute attr_name;
 	struct device_attribute attr_state;
 	struct device_attribute attr_externally_control;
@@ -186,6 +186,7 @@ struct charger_desc {
 
 	int num_charger_regulators;
 	struct charger_regulator *charger_regulators;
+	const struct attribute_group **sysfs_groups;
 
 	const char *psy_fuel_gauge;
 
-- 
2.19.0

[PATCHv1 10/14] power: supply: pcf50633: fix race-condition in sysfs registration

[Sebastian Reichel]

This registers custom sysfs properties using the native functionality
of the power-supply framework, which cleans up the code a bit and
fixes a race-condition. Before this patch the sysfs attributes were
not properly registered to udev.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/pcf50633-charger.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/power/supply/pcf50633-charger.c b/drivers/power/supply/pcf50633-charger.c
index 1aba14046a83..28ed463c866d 100644
--- a/drivers/power/supply/pcf50633-charger.c
+++ b/drivers/power/supply/pcf50633-charger.c
@@ -245,17 +245,14 @@ static ssize_t set_chglim(struct device *dev,
  */
 static DEVICE_ATTR(chg_curlim, S_IRUGO | S_IWUSR, show_chglim, set_chglim);
 
-static struct attribute *pcf50633_mbc_sysfs_entries[] = {
+static struct attribute *pcf50633_mbc_sysfs_attrs[] = {
 	&dev_attr_chgmode.attr,
 	&dev_attr_usb_curlim.attr,
 	&dev_attr_chg_curlim.attr,
 	NULL,
 };
 
-static const struct attribute_group mbc_attr_group = {
-	.name	= NULL,			/* put in device directory */
-	.attrs	= pcf50633_mbc_sysfs_entries,
-};
+ATTRIBUTE_GROUPS(pcf50633_mbc_sysfs);
 
 static void
 pcf50633_mbc_irq_handler(int irq, void *data)
@@ -390,6 +387,7 @@ static const struct power_supply_desc pcf50633_mbc_ac_desc = {
 static int pcf50633_mbc_probe(struct platform_device *pdev)
 {
 	struct power_supply_config psy_cfg = {};
+	struct power_supply_config usb_psy_cfg;
 	struct pcf50633_mbc *mbc;
 	int i;
 	u8 mbcs1;
@@ -419,8 +417,11 @@ static int pcf50633_mbc_probe(struct platform_device *pdev)
 		return PTR_ERR(mbc->adapter);
 	}
 
+	usb_psy_cfg = psy_cfg;
+	usb_psy_cfg.attr_grp = pcf50633_mbc_sysfs_groups;
+
 	mbc->usb = power_supply_register(&pdev->dev, &pcf50633_mbc_usb_desc,
-					 &psy_cfg);
+					 &usb_psy_cfg);
 	if (IS_ERR(mbc->usb)) {
 		dev_err(mbc->pcf->dev, "failed to register usb\n");
 		power_supply_unregister(mbc->adapter);
@@ -436,9 +437,6 @@ static int pcf50633_mbc_probe(struct platform_device *pdev)
 		return PTR_ERR(mbc->ac);
 	}
 
-	if (sysfs_create_group(&pdev->dev.kobj, &mbc_attr_group))
-		dev_err(mbc->pcf->dev, "failed to create sysfs entries\n");
-
 	mbcs1 = pcf50633_reg_read(mbc->pcf, PCF50633_REG_MBCS1);
 	if (mbcs1 & PCF50633_MBCS1_USBPRES)
 		pcf50633_mbc_irq_handler(PCF50633_IRQ_USBINS, mbc);
@@ -457,7 +455,6 @@ static int pcf50633_mbc_remove(struct platform_device *pdev)
 	for (i = 0; i < ARRAY_SIZE(mbc_irq_handlers); i++)
 		pcf50633_free_irq(mbc->pcf, mbc_irq_handlers[i]);
 
-	sysfs_remove_group(&pdev->dev.kobj, &mbc_attr_group);
 	power_supply_unregister(mbc->usb);
 	power_supply_unregister(mbc->adapter);
 	power_supply_unregister(mbc->ac);
-- 
2.19.0

[PATCHv1 11/14] power: supply: ds2780: fix race-condition in bin attribute registration

[Sebastian Reichel]

This is a follow-up patch to the previous one, which fixed a
race-condition during registration of the attribute group.
This fixes the same issue for the binary attributes by adding
them to the properly registered group. As a side effect the
code is further cleaned up.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/ds2780_battery.c | 55 ++++++++++-----------------
 1 file changed, 20 insertions(+), 35 deletions(-)

diff --git a/drivers/power/supply/ds2780_battery.c b/drivers/power/supply/ds2780_battery.c
index fa015787d58d..d8be8c236029 100644
--- a/drivers/power/supply/ds2780_battery.c
+++ b/drivers/power/supply/ds2780_battery.c
@@ -658,7 +658,7 @@ static ssize_t ds2780_write_param_eeprom_bin(struct file *filp,
 	return count;
 }
 
-static const struct bin_attribute ds2780_param_eeprom_bin_attr = {
+static struct bin_attribute ds2780_param_eeprom_bin_attr = {
 	.attr = {
 		.name = "param_eeprom",
 		.mode = S_IRUGO | S_IWUSR,
@@ -703,7 +703,7 @@ static ssize_t ds2780_write_user_eeprom_bin(struct file *filp,
 	return count;
 }
 
-static const struct bin_attribute ds2780_user_eeprom_bin_attr = {
+static struct bin_attribute ds2780_user_eeprom_bin_attr = {
 	.attr = {
 		.name = "user_eeprom",
 		.mode = S_IRUGO | S_IWUSR,
@@ -722,7 +722,6 @@ static DEVICE_ATTR(rsgain_setting, S_IRUGO | S_IWUSR, ds2780_get_rsgain_setting,
 static DEVICE_ATTR(pio_pin, S_IRUGO | S_IWUSR, ds2780_get_pio_pin,
 	ds2780_set_pio_pin);
 
-
 static struct attribute *ds2780_sysfs_attrs[] = {
 	&dev_attr_pmod_enabled.attr,
 	&dev_attr_sense_resistor_value.attr,
@@ -731,19 +730,30 @@ static struct attribute *ds2780_sysfs_attrs[] = {
 	NULL
 };
 
-ATTRIBUTE_GROUPS(ds2780_sysfs);
+static struct bin_attribute *ds2780_sysfs_bin_attrs[] = {
+	&ds2780_param_eeprom_bin_attr,
+	&ds2780_user_eeprom_bin_attr,
+	NULL
+};
+
+static const struct attribute_group ds2780_sysfs_group = {
+	.attrs = ds2780_sysfs_attrs,
+	.bin_attrs = ds2780_sysfs_bin_attrs,
+};
+
+static const struct attribute_group *ds2780_sysfs_groups[] = {
+	&ds2780_sysfs_group,
+	NULL,
+};
 
 static int ds2780_battery_probe(struct platform_device *pdev)
 {
 	struct power_supply_config psy_cfg = {};
-	int ret = 0;
 	struct ds2780_device_info *dev_info;
 
 	dev_info = devm_kzalloc(&pdev->dev, sizeof(*dev_info), GFP_KERNEL);
-	if (!dev_info) {
-		ret = -ENOMEM;
-		goto fail;
-	}
+	if (!dev_info)
+		return -ENOMEM;
 
 	platform_set_drvdata(pdev, dev_info);
 
@@ -762,35 +772,10 @@ static int ds2780_battery_probe(struct platform_device *pdev)
 					      &psy_cfg);
 	if (IS_ERR(dev_info->bat)) {
 		dev_err(dev_info->dev, "failed to register battery\n");
-		ret = PTR_ERR(dev_info->bat);
-		goto fail;
-	}
-
-	ret = sysfs_create_bin_file(&dev_info->bat->dev.kobj,
-					&ds2780_param_eeprom_bin_attr);
-	if (ret) {
-		dev_err(dev_info->dev,
-				"failed to create param eeprom bin file");
-		goto fail_unregister;
-	}
-
-	ret = sysfs_create_bin_file(&dev_info->bat->dev.kobj,
-					&ds2780_user_eeprom_bin_attr);
-	if (ret) {
-		dev_err(dev_info->dev,
-				"failed to create user eeprom bin file");
-		goto fail_remove_bin_file;
+		return PTR_ERR(dev_info->bat);
 	}
 
 	return 0;
-
-fail_remove_bin_file:
-	sysfs_remove_bin_file(&dev_info->bat->dev.kobj,
-				&ds2780_param_eeprom_bin_attr);
-fail_unregister:
-	power_supply_unregister(dev_info->bat);
-fail:
-	return ret;
 }
 
 static int ds2780_battery_remove(struct platform_device *pdev)
-- 
2.19.0

[PATCHv1 12/14] power: supply: ds2781: fix race-condition in bin attribute registration

[Sebastian Reichel]

This is a follow-up patch to the previous one, which fixed a
race-condition during registration of the attribute group.
This fixes the same issue for the binary attributes by adding
them to the properly registered group. As a side effect the
code is further cleaned up.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/ds2781_battery.c | 50 ++++++++++-----------------
 1 file changed, 19 insertions(+), 31 deletions(-)

diff --git a/drivers/power/supply/ds2781_battery.c b/drivers/power/supply/ds2781_battery.c
index 5bfd4c77eb89..ea8c66abf602 100644
--- a/drivers/power/supply/ds2781_battery.c
+++ b/drivers/power/supply/ds2781_battery.c
@@ -660,7 +660,7 @@ static ssize_t ds2781_write_param_eeprom_bin(struct file *filp,
 	return count;
 }
 
-static const struct bin_attribute ds2781_param_eeprom_bin_attr = {
+static struct bin_attribute ds2781_param_eeprom_bin_attr = {
 	.attr = {
 		.name = "param_eeprom",
 		.mode = S_IRUGO | S_IWUSR,
@@ -706,7 +706,7 @@ static ssize_t ds2781_write_user_eeprom_bin(struct file *filp,
 	return count;
 }
 
-static const struct bin_attribute ds2781_user_eeprom_bin_attr = {
+static struct bin_attribute ds2781_user_eeprom_bin_attr = {
 	.attr = {
 		.name = "user_eeprom",
 		.mode = S_IRUGO | S_IWUSR,
@@ -725,7 +725,6 @@ static DEVICE_ATTR(rsgain_setting, S_IRUGO | S_IWUSR, ds2781_get_rsgain_setting,
 static DEVICE_ATTR(pio_pin, S_IRUGO | S_IWUSR, ds2781_get_pio_pin,
 	ds2781_set_pio_pin);
 
-
 static struct attribute *ds2781_sysfs_attrs[] = {
 	&dev_attr_pmod_enabled.attr,
 	&dev_attr_sense_resistor_value.attr,
@@ -734,12 +733,26 @@ static struct attribute *ds2781_sysfs_attrs[] = {
 	NULL
 };
 
-ATTRIBUTE_GROUPS(ds2781_sysfs);
+static struct bin_attribute *ds2781_sysfs_bin_attrs[] = {
+	&ds2781_param_eeprom_bin_attr,
+	&ds2781_user_eeprom_bin_attr,
+	NULL,
+};
+
+static const struct attribute_group ds2781_sysfs_group = {
+	.attrs = ds2781_sysfs_attrs,
+	.bin_attrs = ds2781_sysfs_bin_attrs,
+
+};
+
+static const struct attribute_group *ds2781_sysfs_groups[] = {
+	&ds2781_sysfs_group,
+	NULL,
+};
 
 static int ds2781_battery_probe(struct platform_device *pdev)
 {
 	struct power_supply_config psy_cfg = {};
-	int ret = 0;
 	struct ds2781_device_info *dev_info;
 
 	dev_info = devm_kzalloc(&pdev->dev, sizeof(*dev_info), GFP_KERNEL);
@@ -763,35 +776,10 @@ static int ds2781_battery_probe(struct platform_device *pdev)
 						&psy_cfg);
 	if (IS_ERR(dev_info->bat)) {
 		dev_err(dev_info->dev, "failed to register battery\n");
-		ret = PTR_ERR(dev_info->bat);
-		goto fail;
-	}
-
-	ret = sysfs_create_bin_file(&dev_info->bat->dev.kobj,
-					&ds2781_param_eeprom_bin_attr);
-	if (ret) {
-		dev_err(dev_info->dev,
-				"failed to create param eeprom bin file");
-		goto fail_unregister;
-	}
-
-	ret = sysfs_create_bin_file(&dev_info->bat->dev.kobj,
-					&ds2781_user_eeprom_bin_attr);
-	if (ret) {
-		dev_err(dev_info->dev,
-				"failed to create user eeprom bin file");
-		goto fail_remove_bin_file;
+		return PTR_ERR(dev_info->bat);
 	}
 
 	return 0;
-
-fail_remove_bin_file:
-	sysfs_remove_bin_file(&dev_info->bat->dev.kobj,
-				&ds2781_param_eeprom_bin_attr);
-fail_unregister:
-	power_supply_unregister(dev_info->bat);
-fail:
-	return ret;
 }
 
 static int ds2781_battery_remove(struct platform_device *pdev)
-- 
2.19.0

[PATCHv1 13/14] power: supply: ds2780: switch to devm_power_supply_register

[Sebastian Reichel]

Simplify/Cleanup the driver by switching to devm_power_supply_register
and dropping the driver's remove function.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/ds2780_battery.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/power/supply/ds2780_battery.c b/drivers/power/supply/ds2780_battery.c
index d8be8c236029..9502f163ea0b 100644
--- a/drivers/power/supply/ds2780_battery.c
+++ b/drivers/power/supply/ds2780_battery.c
@@ -768,8 +768,9 @@ static int ds2780_battery_probe(struct platform_device *pdev)
 	psy_cfg.drv_data		= dev_info;
 	psy_cfg.attr_grp		= ds2780_sysfs_groups;
 
-	dev_info->bat = power_supply_register(&pdev->dev, &dev_info->bat_desc,
-					      &psy_cfg);
+	dev_info->bat = devm_power_supply_register(&pdev->dev,
+						   &dev_info->bat_desc,
+						   &psy_cfg);
 	if (IS_ERR(dev_info->bat)) {
 		dev_err(dev_info->dev, "failed to register battery\n");
 		return PTR_ERR(dev_info->bat);
@@ -778,21 +779,11 @@ static int ds2780_battery_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int ds2780_battery_remove(struct platform_device *pdev)
-{
-	struct ds2780_device_info *dev_info = platform_get_drvdata(pdev);
-
-	power_supply_unregister(dev_info->bat);
-
-	return 0;
-}
-
 static struct platform_driver ds2780_battery_driver = {
 	.driver = {
 		.name = "ds2780-battery",
 	},
 	.probe	  = ds2780_battery_probe,
-	.remove   = ds2780_battery_remove,
 };
 
 module_platform_driver(ds2780_battery_driver);
-- 
2.19.0

[PATCHv1 14/14] power: supply: ds2781: switch to devm_power_supply_register

[Sebastian Reichel]

Simplify/Cleanup the driver by switching to devm_power_supply_register
and dropping the driver's remove function.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/power/supply/ds2781_battery.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/power/supply/ds2781_battery.c b/drivers/power/supply/ds2781_battery.c
index ea8c66abf602..b1a4669e1f9b 100644
--- a/drivers/power/supply/ds2781_battery.c
+++ b/drivers/power/supply/ds2781_battery.c
@@ -772,8 +772,9 @@ static int ds2781_battery_probe(struct platform_device *pdev)
 	psy_cfg.drv_data		= dev_info;
 	psy_cfg.attr_grp		= ds2781_sysfs_groups;
 
-	dev_info->bat = power_supply_register(&pdev->dev, &dev_info->bat_desc,
-						&psy_cfg);
+	dev_info->bat = devm_power_supply_register(&pdev->dev,
+						   &dev_info->bat_desc,
+						   &psy_cfg);
 	if (IS_ERR(dev_info->bat)) {
 		dev_err(dev_info->dev, "failed to register battery\n");
 		return PTR_ERR(dev_info->bat);
@@ -782,21 +783,11 @@ static int ds2781_battery_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int ds2781_battery_remove(struct platform_device *pdev)
-{
-	struct ds2781_device_info *dev_info = platform_get_drvdata(pdev);
-
-	power_supply_unregister(dev_info->bat);
-
-	return 0;
-}
-
 static struct platform_driver ds2781_battery_driver = {
 	.driver = {
 		.name = "ds2781-battery",
 	},
 	.probe	  = ds2781_battery_probe,
-	.remove   = ds2781_battery_remove,
 };
 module_platform_driver(ds2781_battery_driver);
 
-- 
2.19.0

Re: [PATCHv1 00/14] power: supply: Fix custom sysfs attribute registration

[Greg Kroah-Hartman]

On Mon, Oct 01, 2018 at 12:03:35AM +0200, Sebastian Reichel wrote:
> Hi,
> 
> Udev may not detect sysfs attributes, that have been added
> after the device has been created. Code doing that is racy [0].
> The power-supply class properly registers its attributes
> when the device is created, but some drivers add additional
> custom attributes in a racy way.
> 
> The first patch from this series adds native support for custom
> sysfs attributes to the power-supply subsystem. This feature
> allows cleaner code and avoids a race condition preventing udev
> from noticing the custom properties.
> 
> The following patches replace all instances of sysfs_create_*
> in drivers/power/supply to use the new feature and a couple
> of small cleanups.
> 
> TL;DR: This patchsets replaces all sysfs_create_* in power-supply.
> 
> [0] http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
> 
> -- Sebastian
> 
> Sebastian Reichel (14):
>   power: supply: core: add support for custom sysfs attributes
>   power: supply: bq2415x: fix race-condition in sysfs registration
>   power: supply: ds2780: fix race-condition in sysfs registration
>   power: supply: ds2781: fix race-condition in sysfs registration
>   power: supply: lp8788: fix race-condition in sysfs registration
>   power: supply: bq24190_charger: fix race-condition in sysfs
>     registration
>   power: supply: bq24257: fix race-condition in sysfs registration
>   power: supply: charger-manager: simplify generation of sysfs attribute
>     group name
>   power: supply: charger-manager: fix race-condition in sysfs
>     registration
>   power: supply: pcf50633: fix race-condition in sysfs registration
>   power: supply: ds2780: fix race-condition in bin attribute
>     registration
>   power: supply: ds2781: fix race-condition in bin attribute
>     registration
>   power: supply: ds2780: switch to devm_power_supply_register
>   power: supply: ds2781: switch to devm_power_supply_register
> 
>  drivers/power/supply/bq2415x_charger.c   | 119 ++++++++++-------------
>  drivers/power/supply/bq24190_charger.c   |  43 ++------
>  drivers/power/supply/bq24257_charger.c   |  15 +--
>  drivers/power/supply/charger-manager.c   |  62 +++++-------
>  drivers/power/supply/ds2780_battery.c    |  87 +++++------------
>  drivers/power/supply/ds2781_battery.c    |  82 +++++-----------
>  drivers/power/supply/lp8788-charger.c    |  62 +++++-------
>  drivers/power/supply/pcf50633-charger.c  |  17 ++--
>  drivers/power/supply/power_supply_core.c |   1 +
>  include/linux/power/charger-manager.h    |   3 +-
>  include/linux/power_supply.h             |   3 +
>  11 files changed, 172 insertions(+), 322 deletions(-)

Nice work, and it removes more code than it adds, always a win.

greg k-h