Re: [PATCH v10 1/8] devicetree: power: Add battery.txt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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?

>>>> 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.


>>>>>>>> +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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux