On 03/15/2017 05:57 PM, Sebastian Reichel wrote:
> Hi,
>
> On Wed, Mar 15, 2017 at 03:18:17PM -0700, Liam Breck wrote:
>> Hey Sebastian, welcome back :-)
>>
>> I've taken over work on this patchset from Matt.
>
> Ok.
>
>> On Wed, Mar 15, 2017 at 3:04 PM, Sebastian Reichel <sre@xxxxxxxxxx> wrote:
>>> Hi Liam & Matt,
>>>
>>> Sorry for my absence in the discussion. I skipped the previous
>>> iterations for now and start with this version.
>>>
>>> On Wed, Mar 15, 2017 at 12:26:46PM -0700, Liam Breck wrote:
>>>> From: Liam Breck <kernel@xxxxxxxxxxxxxxxxx>
>>>>
>>>> Documentation of static battery characteristics that can be defined
>>>> for batteries which cannot self-identify. This information is required
>>>> by fuel-gauge and charger chips for proper handling of the battery.
>>>>
>>>> Cc: Rob Herring <robh@xxxxxxxxxx>
>>>> Cc: devicetree@xxxxxxxxxxxxxxx
>>>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>>>> Signed-off-by: Liam Breck <kernel@xxxxxxxxxxxxxxxxx>
>>>> ---
>>>> .../devicetree/bindings/power/supply/battery.txt | 45 ++++++++++++++++++++++
>>>> 1 file changed, 45 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/power/supply/battery.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
>>>> new file mode 100644
>>>> index 0000000..0278617
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
>>>> @@ -0,0 +1,45 @@
>>>> +Battery Characteristics
>>>
>>> Maybe add something like
>>>
>>> This device provides static battery information, that is usually
>>> available in the EEPROM of a smart battery. It's supposed to be
>>> used for batteries, which do not have their own EEPROM (or if its
>>> unusable).
>>
>> OK.
>>
>>>> +Required Properties:
>>>> + - compatible: Must be "fixed-battery"
>>
>> Rob questioned the term "fixed". Shall we consider other terms --
>> simple-battery, plain-battery...?
>
> I don't like the term either, but it was the best I came up with.
> They are known as "dumb" batteries, but that term looks too
> colloquial for DT usage. While I think "simple-battery" is not
> perfect either, it's better than "fixed-battery", so please switch
> to that in the next revision.
>
I rather like "fixed-battery", it matches the regulator and other power
source DTs better. To answer Rob's question on what a non-fixed battery
would look like we can think of a future battery that can change its
physical properties (chemical or physical perhaps) in response to
software. Although I've not seen such a battery I don't think it's too
far-fetched.
All the properties described here are "fixed", if we say "simple" then a
battery with more properties would be a "complex" battery..
>>>> +Optional Properties:
>>>> + - voltage-min-design-microvolt: drained battery voltage
>>>> + - energy-full-design-microwatt-hours: battery design energy
>>>> + - charge-full-design-microamp-hours: battery design capacity
>>>
>>> Looks fine to me.
>>>
>>>> +Because drivers surface properties in sysfs using names derived
>>>> +from enum power_supply_property, e.g.
>>>> +/sys/class/power_supply/<device>/charge_full_design, our
>>>> +battery properties must be named for the corresponding elements in
>>>> +enum power_supply_property, defined in include/linux/power_supply.h.
>>>
>>> This is Linux/implementation specific and does not belong
>>> into a DT binding document.
>>
>> I just wrote on a thread for an earlier version of this patch:
>>
>> "Sebastian proposed DT:battery specifically to be consumed by
>> power_supply_core. Allowing names in DT:battery and
>> power_supply_property to diverge would cause confusion and wasted
>> time, for no particular benefit. As there is no rationale to
>> reconsider the names of these fields for DT:battery, let's write that
>> into the docs."
>
> DT bindings are not "Linux hardware information bindings". Of course
> there is no need to diverge without a good reason, but that kind of
> Documentation just does not belong into the DT bindings.
>
>>>> +Batteries must be referenced by chargers and/or fuel-gauges
>>>> +using a phandle. The phandle's property should be named
>>>> +"monitored-battery".
>>>
>>> This looks fine.
>>>
>>>> +Driver code should call power_supply_get_battery_info() to obtain
>>>> +battery properties via monitored-battery. For details see:
>>>> + drivers/power/supply/power_supply_core.c
>>>> + drivers/power/supply/bq27xxx_battery.c
>>>
>>> This is also Linux/implementation specific and should be dropped.
>>
>> We ought to mention how drivers are expected to consume DT:battery.
>
> That kind of documentation does not belong into
> Documentation/devicetree/bindings. We can add something to
> Documentation/power/power_supply_class.txt instead.
>
>>>> +Example:
>>>> +
>>>> + bat: battery {
>>>> + compatible = "fixed-battery";
>>>> + voltage-min-design-microvolt = <3200000>;
>>>> + energy-full-design-microwatt-hours = <5290000>;
>>>> + charge-full-design-microamp-hours = <1430000>;
>>>> + };
>>>> +
>>>> + charger: charger@11 {
>>>> + ....
>>>> + monitored-battery = <&bat>;
>>>> + ...
>>>> + };
>>>> +
>>>> + fuel_gauge: fuel-gauge@22 {
>>>> + ....
>>>> + monitored-battery = <&bat>;
>>>> + ...
>>>> + };
>>>> --
>>>> 2.9.3
>>>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
