On 03/16/2017 05:58 PM, Liam Breck wrote:
> On Thu, Mar 16, 2017 at 6:21 AM, Andrew F. Davis <afd@xxxxxx> wrote:
>> 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.
>
> A "fixed" battery sounds like a non-removable one to me. I'll switch
> to "simple-battery" in v11 unless a better idea bubbles up first.
>
Are fixed-regulators non-removable, or do most people understand. Also
this is for non-removable batteries, if a battery is changeable then
this DT is not valid as it would be describing the current configuration
of the device. I guess it could be done with overlays for the current
battery, not sure, but something to think about.
>> All the properties described here are "fixed", if we say "simple" then a
>> battery with more properties would be a "complex" battery..
>
> Simple is not a boolean term; it really doesn't mandate a
> "complex-battery" type.
>
Yes, but the spectrum would imply levels of simplicity, what would *you*
call a "non-simple" 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
>
--
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
