Skip to contentSkip to conversation

Fix and start using supplied_from logic

PATCH Applied Rhyland Klein 3 participants 10 messages Jun 10, 2013 – Jul 1, 2013

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover Fix and start using supplied_from logic

This patch series first fixes a refcount issue with the dt nodes that was introduced with the supplied_from patch series. We add a call to of_node_put to decrement the refcount for each node we get from calling of_parse_phandle.

After that, this series enables using the supplied_from automatic population from dt by passing in the dt nodes for the sbs battery driver and the tps65090-charger driver.

Finally, the last patch creates this link for the tegra114-dalmore board.

Rhyland Klein (4):
power_supply: Add of_node_put to fix refcount
power: sbs-battery: Add dt to power_supply struct
power: tps65090-charger: Add dt node to power_supply
arm: tegra: Add power-supplies link between battery and charger

 arch/arm/boot/dts/tegra114-dalmore.dts |    3 ++-
 drivers/power/power_supply_core.c      |    4 ++++
 drivers/power/sbs-battery.c            |    1 +
 drivers/power/tps65090-charger.c       |    1 +
 4 files changed, 8 insertions(+), 1 deletion(-)
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/4 power_supply: Add of_node_put to fix refcount

of_parse_phandle increments the refcount for a dt node before returning it. Add of_node_put where needed to properly decrement the refcount when we are done using a given node.

---
 drivers/power/power_supply_core.c |    4 ++++
 1 file changed, 4 insertions(+)
drivers/power/power_supply_core.c +4 -0
index 1c517c3..3b2d5df 100644
@@ -109,8 +109,10 @@ static int __power_supply_populate_supplied_from(struct device *dev,
 				psy->name, epsy->name);
 			psy->supplied_from[i-1] = (char *)epsy->name;
 			psy->num_supplies++;
+			of_node_put(np);
 			break;
 		}
+		of_node_put(np);
 	} while (np);
 
 	return 0;
@@ -193,8 +195,10 @@ static int power_supply_check_supplies(struct power_supply *psy)
 		ret = power_supply_find_supply_from_node(np);
 		if (ret) {
 			dev_dbg(psy->dev, "Failed to find supply, defer!\n");
+			of_node_put(np);
 			return -EPROBE_DEFER;
 		}
+		of_node_put(np);
 	} while (np);
 
 	/* All supplies found, allocate char ** array for filling */
Signature · 1 line
1.7.9.5
#2 lore raw

Re: [PATCH 1/4] power_supply: Add of_node_put to fix refcount

Quoted 6 lines · Rhyland Klein

of_parse_phandle increments the refcount for a dt node before returning it. Add of_node_put where needed to properly decrement the refcount when we are done using a given node.

---

With CONFIG_OF=n I got this:

CC drivers/power/sbs-battery.o
drivers/power/sbs-battery.c: In function ?sbs_probe?:
drivers/power/sbs-battery.c:707:20: error: ?struct power_supply? has no member named ?of_node?
make[1]: *** [drivers/power/sbs-battery.o] Error 1

I fixed this by the patch below and applied your 1-3 series.

Thanks!

Anton

commit b50df95c8f0703c95625181d2eaf53855c5ebee5

Author: Anton Vorontsov <[email protected]>
Date:   Fri Jun 28 18:17:22 2013 -0700

power_supply: Move of_node out of the #ifdef CONFIG_OF

Similar to linux/device.h, move of_node struct member out of the #ifdef CONFIG_OF so that the drivers won't have to mess with #ifdefs in .c files.

include/linux/power_supply.h +2 -2
index 3828cef..804b906 100644
@@ -162,6 +162,8 @@ union power_supply_propval {
 	const char *strval;
 };
 
+struct device_node;
+
 struct power_supply {
 	const char *name;
 	enum power_supply_type type;
@@ -173,9 +175,7 @@ struct power_supply {
 
 	char **supplied_from;
 	size_t num_supplies;
-#ifdef CONFIG_OF
 	struct device_node *of_node;
-#endif
 
 	int (*get_property)(struct power_supply *psy,
 			    enum power_supply_property psp,
#3 lore raw

Re: [PATCH 1/4] power_supply: Add of_node_put to fix refcount

Quoted 48 lines · Anton Vorontsov
Quoted 6 lines · Rhyland Klein

of_parse_phandle increments the refcount for a dt node before returning it. Add of_node_put where needed to properly decrement the refcount when we are done using a given node.

---

With CONFIG_OF=n I got this:

CC drivers/power/sbs-battery.o
drivers/power/sbs-battery.c: In function ?sbs_probe?:
drivers/power/sbs-battery.c:707:20: error: ?struct power_supply? has no member named ?of_node?
make[1]: *** [drivers/power/sbs-battery.o] Error 1

I fixed this by the patch below and applied your 1-3 series.

Thanks!

Anton

commit b50df95c8f0703c95625181d2eaf53855c5ebee5

Author: Anton Vorontsov <[email protected]>
Date:   Fri Jun 28 18:17:22 2013 -0700

power_supply: Move of_node out of the #ifdef CONFIG_OF

Similar to linux/device.h, move of_node struct member out of the #ifdef CONFIG_OF so that the drivers won't have to mess with #ifdefs in .c files.

include/linux/power_supply.h +2 -0
index 3828cef..804b906 100644
@@ -162,6 +162,8 @@ union power_supply_propval {
 	const char *strval;
 };
 
+struct device_node;
+
 struct power_supply {
 	const char *name;
 	enum power_supply_type type;

include/linux/power_supply.h struct power_supply { · line 181

Quoted 9 lines
include/linux/power_supply.h +0 -2
@@ -173,9 +175,7 @@ struct power_supply {
 
 	char **supplied_from;
 	size_t num_supplies;
-#ifdef CONFIG_OF
 	struct device_node *of_node;
-#endif
 
 	int (*get_property)(struct power_supply *psy,
 			    enum power_supply_property psp,

Thanks!

-rhyland

Signature · 1 line
nvpublic
#4 lore raw

PATCH 2/4 power: sbs-battery: Add dt to power_supply struct

In reply to Rhyland Klein

By passing in the dt node of this device, we enable the logic for linking power_supplies together from dt. This is specified by adding a "power-supplies" property with a phandle to the charger for a given supply.

Enable this logic now for the sbs-battery driver.

---
 drivers/power/sbs-battery.c |    1 +
 1 file changed, 1 insertion(+)
drivers/power/sbs-battery.c +1 -0
index c8c78a7..b5f2a76 100644
@@ -704,6 +704,7 @@ static int sbs_probe(struct i2c_client *client,
 	chip->power_supply.properties = sbs_properties;
 	chip->power_supply.num_properties = ARRAY_SIZE(sbs_properties);
 	chip->power_supply.get_property = sbs_get_property;
+	chip->power_supply.of_node = client->dev.of_node;
 	/* ignore first notification of external change, it is generated
 	 * from the power_supply_register call back
 	 */
Signature · 1 line
1.7.9.5
#5 lore raw

PATCH 3/4 power: tps65090-charger: Add dt node to power_supply

In reply to Rhyland Klein

Passing in the dt node for this charger enables the logic in the core to lookup this device, to see if it is supplying another power_supply, through dt.

---
 drivers/power/tps65090-charger.c |    1 +
 1 file changed, 1 insertion(+)
drivers/power/tps65090-charger.c +1 -0
index 77ab856..3f4b9cc 100644
@@ -230,6 +230,7 @@ static int tps65090_charger_probe(struct platform_device *pdev)
 	cdata->ac.num_properties	= ARRAY_SIZE(tps65090_ac_props);
 	cdata->ac.supplied_to		= pdata->supplied_to;
 	cdata->ac.num_supplicants	= pdata->num_supplicants;
+	cdata->ac.of_node		= pdev->dev.of_node;
 
 	ret = power_supply_register(&pdev->dev, &cdata->ac);
 	if (ret) {
Signature · 1 line
1.7.9.5
#6 lore raw

PATCH 4/4 arm: tegra: Add power-supplies link between battery and charger

In reply to Rhyland Klein

The power supply core now supports detecting linkages between batteries and chargers through the use of the power-supplies property. Adding this to the battery, the core will use the phandle list to find the associated charger and pair them up. This facilitates notifications from the charger to the battery when ac power is dissconnected or connected for instance.

---
 arch/arm/boot/dts/tegra114-dalmore.dts |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm/boot/dts/tegra114-dalmore.dts +2 -1
index cfeb0f5..b92922f 100644
@@ -727,6 +727,7 @@
 			battery-name = "battery";
 			sbs,i2c-retry-count = <2>;
 			sbs,poll-retry-count = <100>;
+			power-supplies = <&charger>;
 		};
 	};
 
@@ -763,7 +764,7 @@
 			vsys-l1-supply = <&vdd_ac_bat_reg>;
 			vsys-l2-supply = <&vdd_ac_bat_reg>;
 
-			charger {
+			charger: charger {
 				compatible = "ti,tps65090-charger";
 				ti,enable-low-current-chrg;
 			};
Signature · 1 line
1.7.9.5
#7 lore raw

Re: [PATCH 4/4] arm: tegra: Add power-supplies link between battery and charger

Quoted 6 lines · Rhyland Klein

The power supply core now supports detecting linkages between batteries and chargers through the use of the power-supplies property. Adding this to the battery, the core will use the phandle list to find the associated charger and pair them up. This facilitates notifications from the charger to the battery when ac power is dissconnected or connected for instance.

Rhyland, if I apply this patch without patches 1-3 being in the same branch, will it cause any bugs? If so, I guess 1-3 should go through the drivers/power maintainer for 3.11, and I should defer this patch to 3.12?

#8 lore raw

Re: [PATCH 4/4] arm: tegra: Add power-supplies link between battery and charger

Quoted 8 lines · Stephen Warren
Quoted 6 lines · Rhyland Klein

The power supply core now supports detecting linkages between batteries and chargers through the use of the power-supplies property. Adding this to the battery, the core will use the phandle list to find the associated charger and pair them up. This facilitates notifications from the charger to the battery when ac power is dissconnected or connected for instance.

Quoted 4 lines

Rhyland, if I apply this patch without patches 1-3 being in the same branch, will it cause any bugs? If so, I guess 1-3 should go through the drivers/power maintainer for 3.11, and I should defer this patch to 3.12?

No, this alone shouldn't cause a problem. Patches 2 & 3 enable the support for using the power-supplies property in the charger and battery drivers. Without them, this property will just be ignored for now.

--rhyland

Signature · 1 line
nvpublic
#9 lore raw

Re: [PATCH 4/4] arm: tegra: Add power-supplies link between battery and charger

In reply to Rhyland Klein
Quoted 6 lines · Rhyland Klein

The power supply core now supports detecting linkages between batteries and chargers through the use of the power-supplies property. Adding this to the battery, the core will use the phandle list to find the associated charger and pair them up. This facilitates notifications from the charger to the battery when ac power is dissconnected or connected for instance.

Applied to Tegra's for-3.11/dt branch.