On Mon, Mar 20, 2017 at 9:45 AM, Andrew F. Davis <afd@xxxxxx> wrote:
> On 03/17/2017 04:43 PM, Liam Breck wrote:
>> On Fri, Mar 17, 2017 at 8:21 AM, Andrew F. Davis <afd@xxxxxx> wrote:
>>> 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.
>>
>> I know little about regulators, but wikipedia mentions fixed and
>> variable/adjustable kinds. We're not contemplating an "adjustable
>> battery" type, so the fixed/adjustable dichotomy isn't a natural fit
>> here.
>>
>
> I think we are though, you are saying this is a "simple" battery because
> it has fixed design parameters. Any battery that does not have fixed
> design parameters is some kinda futuristic adjustable battery, what else
> would you call a battery with non-fixed design parameters?
fantasy-battery :-)
>>>>> 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?
>>
>> Simple is not an integral term, either :-)
>>
>> simple-battery, gauged-battery, coin-battery, bare-battery (no
>> protection circuit)...
>>
>
> Fixed/simple/coin/bare batteries are all the same to DT and so do not
> need new node/compatible names. "Gauged-battery" would be described by a
> node representing the fuel-gauge and that node would then point to the
> battery that it gauges, no still no need for a new term.
>
> In our world we still have only three things:
> Charger -
> | |-> Battery
> \/ |
> Gauge - -
>
> Sometimes we will have a battery, sometimes we will have a charger or a
> gauge with an attached battery, and sometimes we will have charger with
> embedded gauging functions. All these relations can be described in DT
> without adding extra compatibles.
Here's the dichotomy you want: clever-battery, simple-battery :-)
But seriously... acpi-battery could be relevant.
>>>>>>>>> +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
