diff mbox

[v3,6/7] devicetree: power: bq27xxx: add monitored battery documentation

Message ID 20170111062003.10110-7-matt@ranostay.consulting (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Matt Ranostay Jan. 11, 2017, 6:20 a.m. UTC
Depends-On: http://marc.info/?l=linux-pm&m=148392292830015&w=2
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
---
 Documentation/devicetree/bindings/power/supply/bq27xxx.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Rob Herring Jan. 13, 2017, 5:28 p.m. UTC | #1
On Tue, Jan 10, 2017 at 10:20:02PM -0800, Matt Ranostay wrote:
> Depends-On: http://marc.info/?l=linux-pm&m=148392292830015&w=2
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> ---
>  Documentation/devicetree/bindings/power/supply/bq27xxx.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
> index b0c95ef63e68..0472a2db0f13 100644
> --- a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
> +++ b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
> @@ -28,9 +28,17 @@ Required properties:
>   * "ti,bq27621" - BQ27621
>  - reg: integer, i2c address of the device.
>  
> +Optional properties:
> +- monitored-battery: phandle of battery information devicetree node

We need a common way to describe charger/monitor to battery connections, 
not yet another way. The battery and power supply related bindings are a 
bit of a mess from what I've looked at.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Ranostay Jan. 14, 2017, 6:07 a.m. UTC | #2
On Fri, Jan 13, 2017 at 9:28 AM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Jan 10, 2017 at 10:20:02PM -0800, Matt Ranostay wrote:
>> Depends-On: http://marc.info/?l=linux-pm&m=148392292830015&w=2
>> Cc: Rob Herring <robh@kernel.org>
>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>> ---
>>  Documentation/devicetree/bindings/power/supply/bq27xxx.txt | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
>> index b0c95ef63e68..0472a2db0f13 100644
>> --- a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
>> +++ b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
>> @@ -28,9 +28,17 @@ Required properties:
>>   * "ti,bq27621" - BQ27621
>>  - reg: integer, i2c address of the device.
>>
>> +Optional properties:
>> +- monitored-battery: phandle of battery information devicetree node
>
> We need a common way to describe charger/monitor to battery connections,
> not yet another way. The battery and power supply related bindings are a
> bit of a mess from what I've looked at.

Sebastian, your thoughts here?

Thanks,

Matt

>
> Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sebastian Reichel Jan. 17, 2017, 12:59 a.m. UTC | #3
Hi Matt,

On Fri, Jan 13, 2017 at 10:07:32PM -0800, Matt Ranostay wrote:
> On Fri, Jan 13, 2017 at 9:28 AM, Rob Herring <robh@kernel.org> wrote:
> > On Tue, Jan 10, 2017 at 10:20:02PM -0800, Matt Ranostay wrote:
> >> Depends-On: http://marc.info/?l=linux-pm&m=148392292830015&w=2
> >> Cc: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
> >> ---
> >>  Documentation/devicetree/bindings/power/supply/bq27xxx.txt | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
> >> index b0c95ef63e68..0472a2db0f13 100644
> >> --- a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
> >> +++ b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
> >> @@ -28,9 +28,17 @@ Required properties:
> >>   * "ti,bq27621" - BQ27621
> >>  - reg: integer, i2c address of the device.
> >>
> >> +Optional properties:
> >> +- monitored-battery: phandle of battery information devicetree node
> >
> > We need a common way to describe charger/monitor to battery connections,
> > not yet another way. The battery and power supply related bindings are a
> > bit of a mess from what I've looked at.
> 
> Sebastian, your thoughts here?

On what part? Did you receive my comment on patch 1 from this
patchset? There I wrote basically the same:

> I think we should mandate the property name of the phandle in the
> generic binding instead of each potential fuel-gauge/charger.
> Maybe something like the following paragraph:
> 
> Batteries are supposed to be referenced by chargers and/or
> fuel-gauges using a phandle. The phandle's property should
> be named "monitored-battery".

If you mean the second sentence: Yes, DT bindings for the
power-supply subsystem are a mess unfortunately. It's partially
my fault, but the (IMHO) really bad ones were already there when
I took over the power-supply subsystem (e.g. charger-manager,
which does not even describe real HW).

One of the problems is, that the power-supply subsystem does not
know about the difference between a fuel-gauge and a battery, since
smart batteries contain a fuel-gauge.

Regarding the phandles: We do have a standard property for the
battery <-> charger connection, which is described in

Documentation/devicetree/bindings/power/supply/power_supply.txt

Like most of the power-supply subsystem it expects, that battery
is a smart-battery with fuel-gauge. It does not really fit for
the battery <-> fuel-gauge connection, though.

TLDR: I suggest you implement the changes suggested by me and Rob
in patch 1 and resend the patch series. Then let's see if Rob is
ok with it.

-- Sebastian
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
index b0c95ef63e68..0472a2db0f13 100644
--- a/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
+++ b/Documentation/devicetree/bindings/power/supply/bq27xxx.txt
@@ -28,9 +28,17 @@  Required properties:
  * "ti,bq27621" - BQ27621
 - reg: integer, i2c address of the device.
 
+Optional properties:
+- monitored-battery: phandle of battery information devicetree node
+
+  See Documentation/devicetree/bindings/power/supply/battery.txt
+  If any of the referenced battery properties are set, then all must be.
+
 Example:
 
 bq27510g3 {
     compatible = "ti,bq27510g3";
     reg = <0x55>;
+
+    monitored-battery = <&bat>;
 };