[RFC 0/3] Add DT Binding for Power-Supply supplied-nodes property

[RFC 0/3] Add DT Binding for Power-Supply supplied-nodes property

[Rhyland Klein]

This series is an attempt to define a common way for devicetree
initialized power_supplies to define their list of supplicants
in a common manner.

Instead of relying on custom properties which contain is list of
strings, use the much more direct method of phandles to reference
the supplicants and define a common function which can retrieve them
automatically.

Rhyland Klein (3):
  power_supply: Define Binding for supplied-nodes
  power: power_supply: Add core support for supplied_nodes
  power: power_supply: add support for getting supplied-nodes from dt

 .../bindings/power_supply/power_supply.txt         |   17 ++++
 drivers/power/power_supply_core.c                  |   82 +++++++++++++++++++-
 include/linux/power_supply.h                       |   10 +++
 3 files changed, 107 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power_supply/power_supply.txt

-- 
1.7.9.5

[RFC 1/3] power_supply: Define Binding for supplied-nodes

[Rhyland Klein]

This property is meant to be used in device nodes which represent
power_supply devices that wish to provide a list of supplies to
which they provide power. A common case is a AC Charger with
the batteries it powers.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 .../bindings/power_supply/power_supply.txt         |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power_supply/power_supply.txt

diff --git a/Documentation/devicetree/bindings/power_supply/power_supply.txt b/Documentation/devicetree/bindings/power_supply/power_supply.txt
new file mode 100644
index 0000000..1c58d4ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/power_supply.txt
@@ -0,0 +1,17 @@
+Power Supply Core Support
+
+Optional Properties:
+ - power-supply,supplied-nodes : This property is added to a supply
+   in order to specify the list of supplicant devices directly by their
+   phandles.
+
+Example:
+
+	charger@e {
+		compatible = "some,charger";
+		...
+
+		power-supply,supplied-nodes = <&some_battery>,
+					      <&another_battery>;
+		...
+	};
-- 
1.7.9.5

Re: [RFC 1/3] power_supply: Define Binding for supplied-nodes

[Anton Vorontsov]

On Fri, Feb 15, 2013 at 06:36:54PM -0500, Rhyland Klein wrote:
> This property is meant to be used in device nodes which represent
> power_supply devices that wish to provide a list of supplies to
> which they provide power. A common case is a AC Charger with
> the batteries it powers.
> 
> Signed-off-by: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  .../bindings/power_supply/power_supply.txt         |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power_supply/power_supply.txt
> 
> diff --git a/Documentation/devicetree/bindings/power_supply/power_supply.txt b/Documentation/devicetree/bindings/power_supply/power_supply.txt
> new file mode 100644
> index 0000000..1c58d4ff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power_supply/power_supply.txt
> @@ -0,0 +1,17 @@
> +Power Supply Core Support
> +
> +Optional Properties:
> + - power-supply,supplied-nodes : This property is added to a supply
> +   in order to specify the list of supplicant devices directly by their
> +   phandles.

"supplied nodes" sounds confusing (doesn't reflect direction), IMO. I'd
rather call it power-supply,supplied-to = <&some_battery>;

But... I'm recalling there was a similar discussion not that long ago, and
Arnd came up with the idea that supplied-to is not fully in spirit of DT,
and proposed his view of proper bindings. Please find the discussion here:

  http://lkml.org/lkml/2012/9/14/104

Thanks,

Anton

> +
> +Example:
> +
> +	charger@e {
> +		compatible = "some,charger";
> +		...
> +
> +		power-supply,supplied-nodes = <&some_battery>,
> +					      <&another_battery>;
> +		...
> +	};
> -- 
> 1.7.9.5

Re: [RFC 1/3] power_supply: Define Binding for supplied-nodes

[Rajanikanth HV]

On Sunday 17 February 2013 04:08 AM, Anton Vorontsov wrote:
[...]
> 
> "supplied nodes" sounds confusing (doesn't reflect direction), IMO. I'd
> rather call it power-supply,supplied-to = <&some_battery>;
> 
> But... I'm recalling there was a similar discussion not that long ago, and
> Arnd came up with the idea that supplied-to is not fully in spirit of DT,
> and proposed his view of proper bindings. Please find the discussion here:
> 
>   http://lkml.org/lkml/2012/9/14/104
'supplied-to or supplied nodes' is specific to "power-supply core and
battery managed devices/drivers" and not platform specific.
Dependent battery drivers interacts/shares power supply events with the
help of 'supplied_to' 'num_supplicants' and external_power_changed(...)
identifiers which are defined and fixed during driver design, it will be
more meaningful to have it internal to drivers instead DT.
Ref: ab8500_[fg,btemp,charger].c, abx500_chargalg.c and
arch/arm/boot/dts/dbx5x0.dtsi


Thanks,
Rajanikanth
[...]

Re: [RFC 1/3] power_supply: Define Binding for supplied-nodes

[Rhyland Klein]

On 2/16/2013 5:38 PM, Anton Vorontsov wrote:
> On Fri, Feb 15, 2013 at 06:36:54PM -0500, Rhyland Klein wrote:
>> This property is meant to be used in device nodes which represent
>> power_supply devices that wish to provide a list of supplies to
>> which they provide power. A common case is a AC Charger with
>> the batteries it powers.
>>
>> Signed-off-by: Rhyland Klein <rklein-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>   .../bindings/power_supply/power_supply.txt         |   17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/power_supply/power_supply.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power_supply/power_supply.txt b/Documentation/devicetree/bindings/power_supply/power_supply.txt
>> new file mode 100644
>> index 0000000..1c58d4ff
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power_supply/power_supply.txt
>> @@ -0,0 +1,17 @@
>> +Power Supply Core Support
>> +
>> +Optional Properties:
>> + - power-supply,supplied-nodes : This property is added to a supply
>> +   in order to specify the list of supplicant devices directly by their
>> +   phandles.
> "supplied nodes" sounds confusing (doesn't reflect direction), IMO. I'd
> rather call it power-supply,supplied-to = <&some_battery>;
>
> But... I'm recalling there was a similar discussion not that long ago, and
> Arnd came up with the idea that supplied-to is not fully in spirit of DT,
> and proposed his view of proper bindings. Please find the discussion here:
>
>    http://lkml.org/lkml/2012/9/14/104
>
> Thanks,
>
> Anton
Ahhh, I missed that when I glanced around. I think this does make more 
sense. This sounds like it would entail adding some amount of complexity 
to the power_supply subsystem, namely, being able to register 
"supplied-to" devices with "suppliers". Is that something that has been 
looked into yet? If not I might see how it plays out.

Thanks for the pointer!

-rhyland

-- 
nvpublic

[RFC 2/3] power: power_supply: Add core support for supplied_nodes

[Rhyland Klein]

With the growing support for dt, it make sense to try to make sure of
dt features as to make the general code cleaner. This patch is an
attempt to commonize how chargers receive their "supplied_to" list
of supplicants. This was previously done via passing an array of
strings.

Currently, charger drivers that make use of the supplicant list, do
so using custom private dt mechanisms, this will should make a common
implementation that all can use.

With device tree, it is much cleaner to instead use a list of phandles
in the device tree to point directly to the supplicants thus ensuring
a proper reference without having to hard code in names.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/power/power_supply_core.c |   25 +++++++++++++++++++++++--
 include/linux/power_supply.h      |    4 ++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 8a7cfb3..9e42702 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -26,6 +26,27 @@ EXPORT_SYMBOL_GPL(power_supply_class);
 
 static struct device_type power_supply_dev_type;
 
+static int __power_supply_is_supplied_by(struct power_supply *psy1,
+					 struct power_supply *psy2,
+					 int supply_idx)
+{
+
+	if (supply_idx < 0)
+		return -EINVAL;
+
+#ifdef CONFIG_OF
+	if (psy2->node && psy1->supplied_nodes)
+		if (psy1->supplied_nodes[supply_idx] == psy2->node)
+			return 0;
+#endif
+
+	if (psy2->name && psy1->supplied_to)
+		if (!strcmp(psy1->supplied_to[supply_idx], psy2->name))
+			return 0;
+
+	return -EINVAL;
+}
+
 static int __power_supply_changed_work(struct device *dev, void *data)
 {
 	struct power_supply *psy = (struct power_supply *)data;
@@ -33,7 +54,7 @@ static int __power_supply_changed_work(struct device *dev, void *data)
 	int i;
 
 	for (i = 0; i < psy->num_supplicants; i++)
-		if (!strcmp(psy->supplied_to[i], pst->name)) {
+		if (__power_supply_is_supplied_by(psy, pst, i)) {
 			if (pst->external_power_changed)
 				pst->external_power_changed(pst);
 		}
@@ -71,7 +92,7 @@ static int __power_supply_am_i_supplied(struct device *dev, void *data)
 	int i;
 
 	for (i = 0; i < epsy->num_supplicants; i++) {
-		if (!strcmp(epsy->supplied_to[i], psy->name)) {
+		if (__power_supply_is_supplied_by(epsy, psy, i)) {
 			if (epsy->get_property(epsy,
 				  POWER_SUPPLY_PROP_ONLINE, &ret))
 				continue;
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 1f0ab90..8c8693b 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -166,6 +166,10 @@ struct power_supply {
 	enum power_supply_property *properties;
 	size_t num_properties;
 
+#ifdef CONFIG_OF
+	struct device_node *node;
+	struct device_node **supplied_nodes;
+#endif
 	char **supplied_to;
 	size_t num_supplicants;
 
-- 
1.7.9.5

[RFC 3/3] power: power_supply: add support for getting supplied-nodes from dt

[Rhyland Klein]

With the addition of the device_nodes to use similar to how
supplied_to works, add a helper function which will parse out the
phandles for the supplied-nodes directly from the node. This
implmentation requires the property name to be "supplied-nodes".

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
 drivers/power/power_supply_core.c |   57 +++++++++++++++++++++++++++++++++++++
 include/linux/power_supply.h      |    6 ++++
 2 files changed, 63 insertions(+)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 9e42702..4bc682b 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -84,6 +84,63 @@ void power_supply_changed(struct power_supply *psy)
 }
 EXPORT_SYMBOL_GPL(power_supply_changed);
 
+#ifdef CONFIG_OF
+#include <linux/of.h>
+
+static int __power_supply_get_supplied_nodes(struct device *dev,
+					     struct device_node *np,
+					     struct device_node **supplicants,
+					     int *count)
+{
+	int i = 0;
+	int num_nodes = 0;
+	struct device_node *node;
+
+	do {
+		node = of_parse_phandle(np, "power-supply,supplied-nodes", i++);
+		if (node)
+			num_nodes++;
+	} while (node);
+
+	if (!num_nodes) {
+		dev_warn(dev, "No supplicant phandles found.\n");
+		return -EINVAL;
+	}
+
+	*supplicants = devm_kzalloc(dev, sizeof(struct device_node *) *
+				    num_nodes, GFP_KERNEL);
+
+	if (!*supplicants) {
+		dev_err(dev, "Failed allocation for memory for supplicants\n");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < num_nodes; i++)
+		supplicants[i] = of_parse_phandle(np,
+					"power-supply,supplied-nodes", i);
+
+	*count = num_nodes;
+
+	return 0;
+}
+
+int power_supply_get_supplied_nodes(struct device *dev,
+				    struct device_node *np,
+				    struct device_node **supplicants,
+				    int *count)
+{
+	int error;
+
+	if (!np || !supplicants)
+		return -EINVAL;
+
+	error = __power_supply_get_supplied_nodes(dev, np, supplicants, count);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(power_supply_get_supplied_nodes);
+#endif
+
 static int __power_supply_am_i_supplied(struct device *dev, void *data)
 {
 	union power_supply_propval ret = {0,};
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 8c8693b..359335a 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -230,6 +230,12 @@ struct power_supply_info {
 
 extern struct power_supply *power_supply_get_by_name(char *name);
 extern void power_supply_changed(struct power_supply *psy);
+#ifdef CONFIG_OF
+extern int power_supply_get_supplied_nodes(struct device *dev,
+					   struct device_node *np,
+					   struct device_node **supplicants,
+					   int *count);
+#endif
 extern int power_supply_am_i_supplied(struct power_supply *psy);
 extern int power_supply_set_battery_charged(struct power_supply *psy);
 
-- 
1.7.9.5