Skip to contentSkip to conversation

power: supply: wm97xx: Use devm-variants in probe

PATCH Applied Waqar Hameed 2 participants 4 messages Jan 23 – Jan 30 Power supply

lore
Share & tools
lore
#0 lore raw

v2 cover power: supply: wm97xx: Use devm-variants in probe

This patch series contains some clean-ups that makes life a little easier. It was originally triggered by a NULL pointer fix in this driver that has already been applied from the previous version [1]. Therefore note that this series is based on for-next branch in Sebastian Reichel's tree [2].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/commit/?h=for-next&id=39fe0eac6d755ef215026518985fcf8de9360e9e
[2] https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/?h=for-next

Changes in v2:

* Fix clean-up order by applying devm_kcalloc()-patch before the devm_power_supply_register()-patch.

Link to v1: https://lore.kernel.org/lkml/[email protected]/

Waqar Hameed (2):
power: supply: wm97xx: Use devm_kcalloc()
power: supply: wm97xx: Use devm_power_supply_register()

 drivers/power/supply/wm97xx_battery.c | 32 ++++++++-------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

base-commit: 432b119ab8d8bf1f550c247f7fbe960e87ba6c92

Signature · 1 line
2.39.5
#1 lore raw

PATCH v2 1/2 power: supply: wm97xx: Use devm_kcalloc()

Instead of handling the memory allocation manually, use the automatic devres variant devm_kcalloc(). This is less error prone and eliminates the goto-path.

---
 drivers/power/supply/wm97xx_battery.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)
drivers/power/supply/wm97xx_battery.c +5 -11
index f00722c88c6fe..c30c347b48f99 100644
@@ -192,7 +192,7 @@ static int wm97xx_bat_probe(struct platform_device *dev)
 	if (pdata->min_voltage >= 0)
 		props++;	/* POWER_SUPPLY_PROP_VOLTAGE_MIN */
 
-	prop = kcalloc(props, sizeof(*prop), GFP_KERNEL);
+	prop = devm_kcalloc(&dev->dev, props, sizeof(*prop), GFP_KERNEL);
 	if (!prop)
 		return -ENOMEM;
 
@@ -224,12 +224,10 @@ static int wm97xx_bat_probe(struct platform_device *dev)
 	bat_psy_desc.num_properties = props;
 
 	bat_psy = power_supply_register(&dev->dev, &bat_psy_desc, &cfg);
-	if (!IS_ERR(bat_psy)) {
-		schedule_work(&bat_work);
-	} else {
-		ret = PTR_ERR(bat_psy);
-		goto free;
-	}
+	if (IS_ERR(bat_psy))
+		return PTR_ERR(bat_psy);
+
+	schedule_work(&bat_work);
 
 	if (charge_gpiod) {
 		ret = request_irq(gpiod_to_irq(charge_gpiod), wm97xx_chrg_irq,
@@ -246,9 +244,6 @@ static int wm97xx_bat_probe(struct platform_device *dev)
 unregister:
 	power_supply_unregister(bat_psy);
 
-free:
-	kfree(prop);
-
 	return ret;
 }
 
@@ -258,7 +253,6 @@ static void wm97xx_bat_remove(struct platform_device *dev)
 		free_irq(gpiod_to_irq(charge_gpiod), dev);
 	cancel_work_sync(&bat_work);
 	power_supply_unregister(bat_psy);
-	kfree(prop);
 }
 
 static struct platform_driver wm97xx_bat_driver = {
Signature · 1 line
2.39.5
#2 lore raw

PATCH v2 2/2 power: supply: wm97xx: Use devm_power_supply_register()

In reply to Waqar Hameed

Instead of handling the registration manually, use the automatic devres variant devm_power_supply_register(). This is less error prone and cleaner.

---
 drivers/power/supply/wm97xx_battery.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)
drivers/power/supply/wm97xx_battery.c +4 -12
index c30c347b48f99..a6c55b1e02863 100644
@@ -223,7 +223,7 @@ static int wm97xx_bat_probe(struct platform_device *dev)
 	bat_psy_desc.properties = prop;
 	bat_psy_desc.num_properties = props;
 
-	bat_psy = power_supply_register(&dev->dev, &bat_psy_desc, &cfg);
+	bat_psy = devm_power_supply_register(&dev->dev, &bat_psy_desc, &cfg);
 	if (IS_ERR(bat_psy))
 		return PTR_ERR(bat_psy);
 
@@ -232,19 +232,12 @@ static int wm97xx_bat_probe(struct platform_device *dev)
 	if (charge_gpiod) {
 		ret = request_irq(gpiod_to_irq(charge_gpiod), wm97xx_chrg_irq,
 				  0, "AC Detect", dev);
-		if (ret) {
-			dev_err_probe(&dev->dev, ret,
-				      "failed to request GPIO irq\n");
-			goto unregister;
-		}
+		if (ret)
+			return dev_err_probe(&dev->dev, ret,
+					     "failed to request GPIO irq\n");
 	}
 
 	return 0;
-
-unregister:
-	power_supply_unregister(bat_psy);
-
-	return ret;
 }
 
 static void wm97xx_bat_remove(struct platform_device *dev)
@@ -252,7 +245,6 @@ static void wm97xx_bat_remove(struct platform_device *dev)
 	if (charge_gpiod)
 		free_irq(gpiod_to_irq(charge_gpiod), dev);
 	cancel_work_sync(&bat_work);
-	power_supply_unregister(bat_psy);
 }
 
 static struct platform_driver wm97xx_bat_driver = {
Signature · 1 line
2.39.5
#3 lore raw
In reply to Waqar Hameed
Quoted 10 lines · Waqar Hameed

This patch series contains some clean-ups that makes life a little easier. It was originally triggered by a NULL pointer fix in this driver that has already been applied from the previous version [1]. Therefore note that this series is based on for-next branch in Sebastian Reichel's tree [2].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/commit/?h=for-next&id=39fe0eac6d755ef215026518985fcf8de9360e9e
[2] https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git/?h=for-next

[...]

Applied, thanks!

[1/2] power: supply: wm97xx: Use devm_kcalloc()

      commit: 72db889394d89ffde61e7438f0cdfc0135c9da48
[2/2] power: supply: wm97xx: Use devm_power_supply_register()
      commit: 3db37475d78ed0c2857f289a535dd44b045a904a

Best regards,

Signature · 1 line
Sebastian Reichel <[email protected]>