[RFC v2 0/3] Add DT Binding for Power-Supply power-supply property

[RFC v2 0/3] Add DT Binding for Power-Supply power-supply 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 supplies and define a common function which can retrieve them
automatically.

Changes:
 - v2: Inverted the logic so that supplies (batteries) contain a list
       of the supplies (chargers) which supply them.



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         |   23 ++++++
 drivers/power/power_supply_core.c                  |   81 +++++++++++++++++---
 include/linux/power_supply.h                       |    9 +++
 3 files changed, 101 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power_supply/power_supply.txt

-- 
1.7.9.5

[RFC v2 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2:
  - Changed property to "power-supply" which should be contained in the
    battery rather than the charger. Also updated example to match.

 .../bindings/power_supply/power_supply.txt         |   23 ++++++++++++++++++++
 1 file changed, 23 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..7d8951c
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/power_supply.txt
@@ -0,0 +1,23 @@
+Power Supply Core Support
+
+Optional Properties:
+ - power-supply : This property is added to a supply in order to list the
+   devices which supply it power, referenced by their phandles.
+
+Example:
+
+	usb-charger: power@e {
+		compatible = "some,usb-charger";
+		...
+	};
+
+	ac-charger: power@e {
+		compatible = "some,ac-charger";
+		...
+	};
+
+	battery@b {
+		compatible = "some,battery";
+		...
+		power-supply = <&usb-charger>, <&ac-charger>;
+	};
-- 
1.7.9.5

[RFC v2 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 use of
dt features to make the general code cleaner. This patch is an
attempt to commonize how chargers and their supplies are linked.

Following common dt convention, the "supplied-to" char** list is
replaced with phandle lists defined in the supplies which contain
phandles of their suppliers.

This has the effect however of introducing an inversion in the internal
mechanics of how this information is stored. In the case of non-dt,
the char** list of supplies is stored in the charger. In the dt case,
a device_node * list is stored in the supplies of their chargers,
however this seems to be the only way to support this.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
v2:
  - Changed from struct device_node* contained in suppliers to a list stored
    in the supplies.
  - changed logic for the is_supplied_by check to handle the entire loop
    as the array structure is different between the 2 paths.

 drivers/power/power_supply_core.c |   46 +++++++++++++++++++++++++++----------
 include/linux/power_supply.h      |    9 ++++++++
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 8a7cfb3..a87c5b8 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -26,17 +26,40 @@ EXPORT_SYMBOL_GPL(power_supply_class);
 
 static struct device_type power_supply_dev_type;
 
+static int __power_supply_is_supplied_by(struct power_supply *supplier,
+					 struct power_supply *supply)
+{
+	int i;
+#ifdef CONFIG_OF
+	struct power_supply_supplies *pss;
+#endif
+
+#ifdef CONFIG_OF
+	list_for_each_entry(pss, &supply->supplies.list, list) {
+		if (supplier->supplies.node == pss->node)
+			return 0;
+	}
+#endif
+
+	for (i = 0; i < supplier->num_supplicants; i++) {
+		if (!supply->name || !supplier->supplied_to)
+			continue;
+		if (!strcmp(supplier->supplied_to[i], supply->name))
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
 static int __power_supply_changed_work(struct device *dev, void *data)
 {
 	struct power_supply *psy = (struct power_supply *)data;
 	struct power_supply *pst = dev_get_drvdata(dev);
-	int i;
 
-	for (i = 0; i < psy->num_supplicants; i++)
-		if (!strcmp(psy->supplied_to[i], pst->name)) {
-			if (pst->external_power_changed)
-				pst->external_power_changed(pst);
-		}
+	if (__power_supply_is_supplied_by(psy, pst)) {
+		if (pst->external_power_changed)
+			pst->external_power_changed(pst);
+	}
 	return 0;
 }
 
@@ -68,13 +91,9 @@ static int __power_supply_am_i_supplied(struct device *dev, void *data)
 	union power_supply_propval ret = {0,};
 	struct power_supply *psy = (struct power_supply *)data;
 	struct power_supply *epsy = dev_get_drvdata(dev);
-	int i;
 
-	for (i = 0; i < epsy->num_supplicants; i++) {
-		if (!strcmp(epsy->supplied_to[i], psy->name)) {
-			if (epsy->get_property(epsy,
-				  POWER_SUPPLY_PROP_ONLINE, &ret))
-				continue;
+	if (__power_supply_is_supplied_by(epsy, psy)) {
+		if (!epsy->get_property(epsy, POWER_SUPPLY_PROP_ONLINE, &ret)) {
 			if (ret.intval)
 				return ret.intval;
 		}
@@ -334,6 +353,9 @@ int power_supply_register(struct device *parent, struct power_supply *psy)
 	dev_set_drvdata(dev, psy);
 	psy->dev = dev;
 
+#ifdef CONFIG_OF
+	INIT_LIST_HEAD(&psy->supplies.list);
+#endif
 	INIT_WORK(&psy->changed_work, power_supply_changed_work);
 
 	rc = kobject_set_name(&dev->kobj, "%s", psy->name);
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 1f0ab90..d16f6ab 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -15,6 +15,7 @@
 
 #include <linux/workqueue.h>
 #include <linux/leds.h>
+#include <linux/list.h>
 
 struct device;
 
@@ -160,12 +161,20 @@ union power_supply_propval {
 	const char *strval;
 };
 
+struct power_supply_supplies {
+	struct device_node *node;
+	struct list_head list;
+};
+
 struct power_supply {
 	const char *name;
 	enum power_supply_type type;
 	enum power_supply_property *properties;
 	size_t num_properties;
 
+#ifdef CONFIG_OF
+	struct power_supply_supplies supplies;
+#endif
 	char **supplied_to;
 	size_t num_supplicants;
 
-- 
1.7.9.5

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

[Rhyland Klein]

With the addition of the device_nodes to use to link suppliers and
supplicants, its useful to add logic to handle the creation of
the list in a common place so as to be common for all drivers.

As of now, as long as the supply's device_node is supplied, the core
will attempt to parse the list of supplies and store it in the
power_supply structure. In this way, it shouldn't break existing
drivers, but allow for transition cleanly.

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
---
v2:
  - Simplified and renamed the logic to parse dt for the charger list.
  - Tied the dt parsing directly to power_supply_register to make fewer
    changes required for converting existing chargers/supplies.

 drivers/power/power_supply_core.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index a87c5b8..05e2031 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -86,6 +86,39 @@ void power_supply_changed(struct power_supply *psy)
 }
 EXPORT_SYMBOL_GPL(power_supply_changed);
 
+#ifdef CONFIG_OF
+#include <linux/of.h>
+
+int power_supply_register_supplicant(struct power_supply *psy)
+{
+	struct device_node *np;
+	int i = 0;
+
+	if (!psy->supplies.node)
+		return -EINVAL;
+
+	do {
+		struct power_supply_supplies *lst;
+
+		np = of_parse_phandle(psy->supplies.node, "power-supply", i++);
+		if (!np)
+			continue;
+
+		lst = devm_kzalloc(psy->dev->parent, sizeof(*lst), GFP_KERNEL);
+		if (!lst) {
+			dev_warn(psy->dev->parent,
+				 "Failed to alloc mem for supplies list\n");
+			return -ENOMEM;
+		}
+
+		lst->node = np;
+		list_add(&(lst->list), &(psy->supplies.list));
+	} while (np);
+
+	return 0;
+}
+#endif
+
 static int __power_supply_am_i_supplied(struct device *dev, void *data)
 {
 	union power_supply_propval ret = {0,};
@@ -355,6 +388,8 @@ int power_supply_register(struct device *parent, struct power_supply *psy)
 
 #ifdef CONFIG_OF
 	INIT_LIST_HEAD(&psy->supplies.list);
+
+	power_supply_register_supplicant(psy);
 #endif
 	INIT_WORK(&psy->changed_work, power_supply_changed_work);
 
-- 
1.7.9.5

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

[Stephen Warren]

On 02/21/2013 04:11 PM, 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.

> diff --git a/Documentation/devicetree/bindings/power_supply/power_supply.txt b/Documentation/devicetree/bindings/power_supply/power_supply.txt

> +Optional Properties:
> + - power-supply : This property is added to a supply in order to list the
> +   devices which supply it power, referenced by their phandles.

DT properties that reference resources are usually named in the plural,
so "power-supplies" would be more appropriate here.

It seems plausible that a single DT node could represent/instantiate
multiple separate supply objects. I think we want to employ the standard
pattern of <phandle args*> rather than just <phandle>.

That way, each supply that can supply others would have something like a
#supply-cells = <n>, where n is the number of cells that the supply uses
to name the multiple supplies provided by that node. 0 would be a common
value here. 1 might be used for a node that represents many supplies.

When a client supply uses a providing supply as the supply(!), do you
need any flags to parameterize the connection? If so, that might be
cause for a supplier to have a larger #supply-cells, so the flags could
be represented.

That all said, regulators assume 1 node == 1 regulator, so an
alternative would be for a multi-supply node to include a child node per
supply, e.g.:

power@xxx {
    ...
    supply1 {
        ...
    };
    supply2 {
        ...
    };
};

client {
    supplies = <&supply1> <&supply2>;
};

I don't recall why regulators went for the style above rather than the
#supply-cells style. Cc Mark Brown for any comment here.

Also, do supplies and regulators need to inter-operate in any way (e.g.
reference each-other in DT)?

> +Example:
> +
> +	usb-charger: power@e {
> +		compatible = "some,usb-charger";
> +		...
> +	};
> +
> +	ac-charger: power@e {
> +		compatible = "some,ac-charger";
> +		...
> +	};
> +
> +	battery@b {
> +		compatible = "some,battery";
> +		...
> +		power-supply = <&usb-charger>, <&ac-charger>;
> +	};

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

[Stephen Warren]

On 02/21/2013 04:11 PM, Rhyland Klein wrote:
> With the growing support for dt, it make sense to try to make use of
> dt features to make the general code cleaner. This patch is an
> attempt to commonize how chargers and their supplies are linked.
> 
> Following common dt convention, the "supplied-to" char** list is
> replaced with phandle lists defined in the supplies which contain
> phandles of their suppliers.
> 
> This has the effect however of introducing an inversion in the internal
> mechanics of how this information is stored. In the case of non-dt,
> the char** list of supplies is stored in the charger. In the dt case,
> a device_node * list is stored in the supplies of their chargers,
> however this seems to be the only way to support this.

When parsing the DT, you can convert from phandle (or struct device_node
*) to the name of the referenced supply by simple lookup. So, you could
store supply names rather than device_node *. Can't you then also fill
in the referenced supply's existing char** list of supplies?

Of course, making this interact-with/use -EPROBE_DEFERRED might be
challenging, since this would be operating in the inverse order to other
producer/consumer relationships, which might cause loops.

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

[Stephen Warren]

On 02/21/2013 04:11 PM, Rhyland Klein wrote:
> With the growing support for dt, it make sense to try to make use of
> dt features to make the general code cleaner. This patch is an
> attempt to commonize how chargers and their supplies are linked.
> 
> Following common dt convention, the "supplied-to" char** list is
> replaced with phandle lists defined in the supplies which contain
> phandles of their suppliers.
> 
> This has the effect however of introducing an inversion in the internal
> mechanics of how this information is stored. In the case of non-dt,
> the char** list of supplies is stored in the charger. In the dt case,
> a device_node * list is stored in the supplies of their chargers,
> however this seems to be the only way to support this.

grep over the whole kernel tree for supplied_to doesn't yield /too/ many
hits, although I didn't look at the complexity of most of them. Would it
be possible to invert all the current in-kernel uses to represent a
supplied_from/by model instead? That would mean the proposed DT binding
would then represent the same relationship ordering as the kernel code,
which would be easier to handle.

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

[Rhyland Klein]

On 2/22/2013 2:49 PM, Stephen Warren wrote:
> On 02/21/2013 04:11 PM, Rhyland Klein wrote:
>> With the growing support for dt, it make sense to try to make use of
>> dt features to make the general code cleaner. This patch is an
>> attempt to commonize how chargers and their supplies are linked.
>>
>> Following common dt convention, the "supplied-to" char** list is
>> replaced with phandle lists defined in the supplies which contain
>> phandles of their suppliers.
>>
>> This has the effect however of introducing an inversion in the internal
>> mechanics of how this information is stored. In the case of non-dt,
>> the char** list of supplies is stored in the charger. In the dt case,
>> a device_node * list is stored in the supplies of their chargers,
>> however this seems to be the only way to support this.
> When parsing the DT, you can convert from phandle (or struct device_node
> *) to the name of the referenced supply by simple lookup. So, you could
> store supply names rather than device_node *. Can't you then also fill
> in the referenced supply's existing char** list of supplies?
>
> Of course, making this interact-with/use -EPROBE_DEFERRED might be
> challenging, since this would be operating in the inverse order to other
> producer/consumer relationships, which might cause loops.
The main problem I ran into when I was essentially trying to do this, 
was that the list of names that
are used to match the power_supplies are the strings set as "name" in 
the power_supply structs. This
doesn't get set automatically based on their nodes, and it is currently 
up to each driver to define their
own name.

For example, the sbs-battery driver uses the name "sbs-XXX" where XX is 
its dev_name. Other drivers
use "%s-$%d" as i2c_device_id->name, + instance number. Then the only 
solution I see is to require a new
property that defines the power-supply's name in the devicetree.

This solution with device_nodes, while not ideal, seems the be the best 
bet from what I see. Maybe
someone else has a better idea.

-rhyland

-- 
nvpublic

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

[Rhyland Klein]

On 2/22/2013 3:09 PM, Stephen Warren wrote:
> On 02/21/2013 04:11 PM, Rhyland Klein wrote:
>> With the growing support for dt, it make sense to try to make use of
>> dt features to make the general code cleaner. This patch is an
>> attempt to commonize how chargers and their supplies are linked.
>>
>> Following common dt convention, the "supplied-to" char** list is
>> replaced with phandle lists defined in the supplies which contain
>> phandles of their suppliers.
>>
>> This has the effect however of introducing an inversion in the internal
>> mechanics of how this information is stored. In the case of non-dt,
>> the char** list of supplies is stored in the charger. In the dt case,
>> a device_node * list is stored in the supplies of their chargers,
>> however this seems to be the only way to support this.
> grep over the whole kernel tree for supplied_to doesn't yield /too/ many
> hits, although I didn't look at the complexity of most of them. Would it
> be possible to invert all the current in-kernel uses to represent a
> supplied_from/by model instead? That would mean the proposed DT binding
> would then represent the same relationship ordering as the kernel code,
> which would be easier to handle.
I think it is surely possible to change all the existing drivers to the 
inverse logic as
you suggested. That might make a good follow patchset.

-rhyland

-- 
nvpublic

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

[Rhyland Klein]

On 2/22/2013 2:46 PM, Stephen Warren wrote:
> On 02/21/2013 04:11 PM, 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.
>> diff --git a/Documentation/devicetree/bindings/power_supply/power_supply.txt b/Documentation/devicetree/bindings/power_supply/power_supply.txt
>> +Optional Properties:
>> + - power-supply : This property is added to a supply in order to list the
>> +   devices which supply it power, referenced by their phandles.
> DT properties that reference resources are usually named in the plural,
> so "power-supplies" would be more appropriate here.
>
> It seems plausible that a single DT node could represent/instantiate
> multiple separate supply objects. I think we want to employ the standard
> pattern of <phandle args*> rather than just <phandle>.
>
> That way, each supply that can supply others would have something like a
> #supply-cells = <n>, where n is the number of cells that the supply uses
> to name the multiple supplies provided by that node. 0 would be a common
> value here. 1 might be used for a node that represents many supplies.
>
> When a client supply uses a providing supply as the supply(!), do you
> need any flags to parameterize the connection? If so, that might be
> cause for a supplier to have a larger #supply-cells, so the flags could
> be represented.
>
> That all said, regulators assume 1 node == 1 regulator, so an
> alternative would be for a multi-supply node to include a child node per
> supply, e.g.:
>
> power@xxx {
>      ...
>      supply1 {
>          ...
>      };
>      supply2 {
>          ...
>      };
> };
>
> client {
>      supplies = <&supply1> <&supply2>;
> };
>
> I don't recall why regulators went for the style above rather than the
> #supply-cells style. Cc Mark Brown for any comment here.
>
> Also, do supplies and regulators need to inter-operate in any way (e.g.
> reference each-other in DT)?
>
>> +Example:
>> +
>> +	usb-charger: power@e {
>> +		compatible = "some,usb-charger";
>> +		...
>> +	};
>> +
>> +	ac-charger: power@e {
>> +		compatible = "some,ac-charger";
>> +		...
>> +	};
>> +
>> +	battery@b {
>> +		compatible = "some,battery";
>> +		...
>> +		power-supply = <&usb-charger>, <&ac-charger>;
>> +	};

The "connection" between supplier and supplies isn't really a hard 
connection.
Essentially, the core code uses the names/nodes in the list and iterates 
over
all the power_supplies that are registered and does matching.

I don't have any experience working with a single node that would spawn 
multiple
supplies, though the situation I am sure is possible. I am interested to 
see what
the consensus is around this design for multiple supplies, as I don't have a
preference either way.

-rhyland

-- 
nvpublic

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

[Stephen Warren]

On 02/22/2013 02:55 PM, Rhyland Klein wrote:
> On 2/22/2013 2:49 PM, Stephen Warren wrote:
>> On 02/21/2013 04:11 PM, Rhyland Klein wrote:
>>> With the growing support for dt, it make sense to try to make use of
>>> dt features to make the general code cleaner. This patch is an
>>> attempt to commonize how chargers and their supplies are linked.
>>>
>>> Following common dt convention, the "supplied-to" char** list is
>>> replaced with phandle lists defined in the supplies which contain
>>> phandles of their suppliers.
>>>
>>> This has the effect however of introducing an inversion in the internal
>>> mechanics of how this information is stored. In the case of non-dt,
>>> the char** list of supplies is stored in the charger. In the dt case,
>>> a device_node * list is stored in the supplies of their chargers,
>>> however this seems to be the only way to support this.
>> When parsing the DT, you can convert from phandle (or struct device_node
>> *) to the name of the referenced supply by simple lookup. So, you could
>> store supply names rather than device_node *. Can't you then also fill
>> in the referenced supply's existing char** list of supplies?
>>
>> Of course, making this interact-with/use -EPROBE_DEFERRED might be
>> challenging, since this would be operating in the inverse order to other
>> producer/consumer relationships, which might cause loops.
> The main problem I ran into when I was essentially trying to do this,
> was that the list of names that
> are used to match the power_supplies are the strings set as "name" in
> the power_supply structs. This
> doesn't get set automatically based on their nodes, and it is currently
> up to each driver to define their
> own name.
> 
> For example, the sbs-battery driver uses the name "sbs-XXX" where XX is
> its dev_name. Other drivers
> use "%s-$%d" as i2c_device_id->name, + instance number. Then the only
> solution I see is to require a new
> property that defines the power-supply's name in the devicetree.
> 
> This solution with device_nodes, while not ideal, seems the be the best
> bet from what I see. Maybe
> someone else has a better idea.

For other resource types, this is handled by the (phandle -> whatever)
conversion process actually being a function call on the referenced
object, so that the driver code for it can look up the data in the
actual device/... object etc. See the various .of_xlate functions that
exist in the kernel.

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

[Rhyland Klein]

On 2/22/2013 4:58 PM, Rhyland Klein wrote:
> On 2/22/2013 3:09 PM, Stephen Warren wrote:
>> On 02/21/2013 04:11 PM, Rhyland Klein wrote:
>>> With the growing support for dt, it make sense to try to make use of
>>> dt features to make the general code cleaner. This patch is an
>>> attempt to commonize how chargers and their supplies are linked.
>>>
>>> Following common dt convention, the "supplied-to" char** list is
>>> replaced with phandle lists defined in the supplies which contain
>>> phandles of their suppliers.
>>>
>>> This has the effect however of introducing an inversion in the internal
>>> mechanics of how this information is stored. In the case of non-dt,
>>> the char** list of supplies is stored in the charger. In the dt case,
>>> a device_node * list is stored in the supplies of their chargers,
>>> however this seems to be the only way to support this.
>> grep over the whole kernel tree for supplied_to doesn't yield /too/ many
>> hits, although I didn't look at the complexity of most of them. Would it
>> be possible to invert all the current in-kernel uses to represent a
>> supplied_from/by model instead? That would mean the proposed DT binding
>> would then represent the same relationship ordering as the kernel code,
>> which would be easier to handle.
> I think it is surely possible to change all the existing drivers to the
> inverse logic as
> you suggested. That might make a good follow patchset.
>
> -rhyland
>

Anton, David, would you be adverse to the changing of supplied_to from 
being a
list of batteries stored in a charger to being a list of chargers stored 
in batteries?

(I only use terms charger and batteries as it is much clearer for me to 
read in place
of terms like supplier and supply which are more accurate but much more 
confusing
when used together).

-rhyland

-- 
nvpublic

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

[Rhyland Klein]

On 2/22/2013 6:01 PM, Stephen Warren wrote:
> On 02/22/2013 02:55 PM, Rhyland Klein wrote:
>> On 2/22/2013 2:49 PM, Stephen Warren wrote:
>>> On 02/21/2013 04:11 PM, Rhyland Klein wrote:
>>>> With the growing support for dt, it make sense to try to make use of
>>>> dt features to make the general code cleaner. This patch is an
>>>> attempt to commonize how chargers and their supplies are linked.
>>>>
>>>> Following common dt convention, the "supplied-to" char** list is
>>>> replaced with phandle lists defined in the supplies which contain
>>>> phandles of their suppliers.
>>>>
>>>> This has the effect however of introducing an inversion in the internal
>>>> mechanics of how this information is stored. In the case of non-dt,
>>>> the char** list of supplies is stored in the charger. In the dt case,
>>>> a device_node * list is stored in the supplies of their chargers,
>>>> however this seems to be the only way to support this.
>>> When parsing the DT, you can convert from phandle (or struct device_node
>>> *) to the name of the referenced supply by simple lookup. So, you could
>>> store supply names rather than device_node *. Can't you then also fill
>>> in the referenced supply's existing char** list of supplies?
>>>
>>> Of course, making this interact-with/use -EPROBE_DEFERRED might be
>>> challenging, since this would be operating in the inverse order to other
>>> producer/consumer relationships, which might cause loops.
>> The main problem I ran into when I was essentially trying to do this,
>> was that the list of names that
>> are used to match the power_supplies are the strings set as "name" in
>> the power_supply structs. This
>> doesn't get set automatically based on their nodes, and it is currently
>> up to each driver to define their
>> own name.
>>
>> For example, the sbs-battery driver uses the name "sbs-XXX" where XX is
>> its dev_name. Other drivers
>> use "%s-$%d" as i2c_device_id->name, + instance number. Then the only
>> solution I see is to require a new
>> property that defines the power-supply's name in the devicetree.
>>
>> This solution with device_nodes, while not ideal, seems the be the best
>> bet from what I see. Maybe
>> someone else has a better idea.
> For other resource types, this is handled by the (phandle -> whatever)
> conversion process actually being a function call on the referenced
> object, so that the driver code for it can look up the data in the
> actual device/... object etc. See the various .of_xlate functions that
> exist in the kernel.

I think this makes sense assuming we can change the existing supplies_to 
property
to match this style so that there isn't an inversion in the 2 paths.

Then I think the idea of an of_xlate function would work fine given that 
there are no
circular dependencies (causes issues with -EPROBE_DEFER). If that is the 
case, then
we could add a step to power_supply registration where it would generate the
char * list of supplies and store it in the power_supply being 
registered. In that way,
from then on, it wouldn't matter how the power_supply was registered, 
and the list
of supplies would be the same in either case.

Anton, David, have you seen it, or can you potentially see a case where 
circular
dependencies could arise? I can't but maybe you know of a situation I don't.

I will start working on patches to support this, including changing the way
supplied_to currently works, while I await answers whether or not it 
would be
acceptable.

-rhyland

-- 
nvpublic

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

[Anton Vorontsov]

On Thu, Feb 28, 2013 at 02:48:03PM -0500, Rhyland Klein wrote:
[...]
> Anton, David, would you be adverse to the changing of supplied_to
> from being a
> list of batteries stored in a charger to being a list of chargers
> stored in batteries?

I wonder if we can support both ways?..

Thanks,
Anton

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

[Rhyland Klein]

On 3/2/2013 5:48 PM, Anton Vorontsov wrote:
> On Thu, Feb 28, 2013 at 02:48:03PM -0500, Rhyland Klein wrote:
> [...]
>> Anton, David, would you be adverse to the changing of supplied_to
>> from being a
>> list of batteries stored in a charger to being a list of chargers
>> stored in batteries?
> I wonder if we can support both ways?..
>
> Thanks,
> Anton
Well, the interesting factor becomes either we end up with 2 arrays
(supplied_to, supplied_from) or we make 1 array (horray, save space!)
but need to then either have a flag or use the power_supply type to
know how to interpret the single array (as supplied_to or supplied_from).

Adding in the second array and adds a char ** and an int, which doesn't seem
like as much overhead as trying to figure out how to interpret the 
single array
so I am inclined to stick with 2 arrays.

-rhyland

-- 
nvpublic

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

[Anton Vorontsov]

On Mon, Mar 04, 2013 at 12:32:35PM -0500, Rhyland Klein wrote:
[...]
> >>Anton, David, would you be adverse to the changing of supplied_to
> >>from being a
> >>list of batteries stored in a charger to being a list of chargers
> >>stored in batteries?
> >>
> >I wonder if we can support both ways?..
>
> Well, the interesting factor becomes either we end up with 2 arrays
> (supplied_to, supplied_from) or we make 1 array (horray, save space!)
> but need to then either have a flag or use the power_supply type to
> know how to interpret the single array (as supplied_to or supplied_from).
> 
> Adding in the second array and adds a char ** and an int, which doesn't seem
> like as much overhead as trying to figure out how to interpret the
> single array
> so I am inclined to stick with 2 arrays.

Yup, two arrays seems very reasonable. Plus that way we don't need to
convert existing drivers, so we avoid churn.

Thanks!

Anton