Re: [PATCH v4 2/9] dt_bindings: ROHM BD99954 Charger

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

 



Hello Rob,

Thanks for the review!

On Mon, 2020-03-02 at 16:07 -0600, Rob Herring wrote:
> On Tue, Feb 25, 2020 at 10:52:32AM +0200, Matti Vaittinen wrote:
> > The ROHM BD99954 is a Battery Management LSI for 1-4 cell Lithium-
> > Ion
> > secondary battery. Intended to be used in space-constraint
> > equipment such
> > as Low profile Notebook PC, Tablets and other applications. BD99954
> > provides a Dual-source Battery Charger, two port BC1.2 detection
> > and a
> > Battery Monitor.
> > 
> > Document the DT bindings for BD99954
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
> > ---
> > 
> > Changes from rfc-v3:
> >   - uncomment multipleOf
> >   - add address and size cells properties to example I2C node
> > 
> >  .../bindings/power/supply/rohm,bd9995x.yaml   | 155
> > ++++++++++++++++++
> >  1 file changed, 155 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml
> > b/Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml
> > new file mode 100644
> > index 000000000000..547403773ec5
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml
> > @@ -0,0 +1,155 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/power/supply/rohm,bd9995x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ROHM BD99954 Battery charger driver
> 
> Bindings are for h/w devices, not drivers.
Right. I'll change this.

> 
> > +
> > +maintainers:
> > +  - Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
> > +  - Markus Laine <markus.laine@xxxxxxxxxxxxxxxxx>
> > +  - Mikko Mutanen <mikko.mutanen@xxxxxxxxxxxxxxxxx>
> > +
> > +description: |
> > +  The ROHM BD99954 is a Battery Management LSI for 1-4 cell
> > Lithium-Ion
> > +  secondary battery intended to be used in space-constraint
> > equipment such
> > +  as Low profile Notebook PC, Tablets and other applications.
> > BD99954
> > +  provides a Dual-source Battery Charger, two port BC1.2 detection
> > and a
> > +  Battery Monitor.
> > +
> > +
> > +properties:
> > +  compatible:
> > +    const: rohm,bd9995x-charger
> 
> You can drop '-charger' if the whole chip is a charger IC. If it is
> not, 
> then your example is wrong.
> 
> You use BD99954 elsewhere, use that here too. We don't do wildcards
> in 
> compatible strings.

Makes sense. And this chip is a charger.

> 
> > +#
> > +#    The battery charging profile of BD99954.
> > +#
> > +#    Curve (1) represents charging current.
> > +#    Curve (2) represents battery voltage.
> > +#
> > +#    The BD99954 data sheet divides charging to three phases.
> > +#    a) Trickle-charge with constant current (8).
> > +#    b) pre-charge with constant current (6)
> > +#    c) fast-charge with:
> > +#       First a constant current (5) phase (CC)
> > +#       Then constant voltage (CV) phase (after the battery
> > voltage has reached
> > +#       target level - until charging current has dropped to
> > termination
> > +#       level (7)
> > +#
> > +#     V ^                                                        ^
> > I
> > +#       .                                                        .
> > +#       .                                                        .
> > +# (4)- -.- - - - - - - - - - - - - -  +++++++++++++++++++++++++++.
> > +#       .                            /                           .
> > +#       .                     ++++++/++ - - - - - - - - - - - -
> > -.- - (5)
> > +#       .                     +    /  +                          .
> > +#       .                     +   -   --                         .
> > +#       .                     +  -     +                         .
> > +#       .                     +.-      -:                        .
> > +#       .                    .+         +`                       .
> > +#       .                  .- +       | `/                       .
> > +#       .               .."   +          .:                      .
> > +#       .             -"      +           --                     .
> > +#       .    (2)  ..."        +       |    :-                    .
> > +#       .    ...""            +             -:                   .
> > +# (3)- -.-.""- - - - -+++++++++ - - - - - - -.:- - - - - - - - -
> > .- - (6)
> > +#       .             +                       `:.                .
> > +#       .             +               |         -:               .
> > +#       .             +                           -:             .
> > +#       .             +                             ..           .
> > +#       .   (1)       +               |               "+++- - -
> > -.- - (7)
> > +#       -++++++++++++++- - - - - - - - - - - - - - - - - + - - -
> > .- - (8)
> > +#       .                                                +       -
> > +#       -------------------------------------------------
> > +++++++++-->
> > +#       |             |       |   CC   |      CV         |
> > +#       | --trickle-- | -pre- | ---------fast----------- |
> > +#
> > +#   The charger uses the following battery properties
> > +# - trickle-charge-current-microamp:
> > +#     Current used at trickle-charge phase (8 in above chart)
> > +#     minimum: 64000
> > +#     maximum: 1024000
> > +#     multipleOf: 64000
> 
> Why is all of this commented out still?

Because these will not live in charger node or describe the charger.
These are the battery properties. Used in battery node if static
battery node is used instead of fetching the information from smart
battery. Eg, these are documented in battery.txt So this is not
relevant regarding describing the charger - but these are very relevant
for one who composes DT for battery and charger. So I just wanted to
somehow document the battery bindings which this charger obeys.

Eg. These commented out properties are used from battery node which is
referenced by monitored-battery property. (I almost hear you telling me
that whether these properties are used by charger driver or not is
software behaviour and should not be in HW description - which is true
- but as I said, this is very relevant for one who composes DT - and
this one will hopefully be reading the binding docs. Hence comment
block not "proper" description).

> 
> > +# - precharge-current-microamp:
> > +#     Current used at pre-charge phase (6 in above chart)
> > +#     minimum: 64000
> > +#     maximum: 1024000
> > +#     multipleOf: 64000
> > +# - constant-charge-current-max-microamp
> > +#     Current used at fast charge constant current phase (5 in
> > above chart)
> > +#     minimum: 64000
> > +#     maximum: 1024000
> > +#     multipleOf: 64000
> > +# - constant-charge-voltage-max-microvolt
> > +#     The constant voltage used in fast charging phase (4 in above
> > chart)
> > +#     minimum: 2560000
> > +#     maximum: 19200000
> > +#     multipleOf: 16000
> > +# - precharge-upper-limit-microvolt
> > +#     charging mode is changed from trickle charging to pre-
> > charging
> > +#     when battery voltage exceeds this limit voltage (3 in above
> > chart)
> > +#     minimum: 2048000
> > +#     maximum: 19200000
> > +#     multipleOf: 64000
> > +# - re-charge-voltage-microvolt
> > +#     minimum: 2560000
> > +#     maximum: 19200000
> > +#     multipleOf: 16000
> > +#     re-charging is automatically started when battry has been
> > discharging
> > +#     to the point where the battery voltage drops below this
> > limit
> > +# - over-voltage-threshold-microvolt
> > +#     battery is expected to be faulty if battery voltage exceeds
> > this limit.
> > +#     Charger will then enter to a "battery faulty" -state
> > +#     minimum: 2560000
> > +#     maximum: 19200000
> > +#     multipleOf: 16000
> > +# - charge-term-current-microamp
> > +#     minimum: 0
> > +#     maximum: 1024000
> > +#     multipleOf: 64000
> > +#     a charge cycle terminates when the battery voltage is above
> > recharge
> > +#     threshold, and the current is below this setting (7 in above
> > chart)
> > +#   See also
> > Documentation/devicetree/bindings/power/supply/battery.txt
> > +
> > +  monitored-battery:
> > +    description:
> > +      phandle of battery characteristics devicetree node
> > +
> > +  rohm,vsys-regulation-microvolt:
> > +    description: system specific lower limit for system voltage.
> > +    minimum: 2560000
> > +    maximum: 19200000
> > +    multipleOf: 64000
> > +
> > +  rohm,vbus-input-current-limit-microamp:
> > +    description: system specific VBUS input current limit (in
> > microamps).
> > +    minimum: 32000
> > +    maximum: 16352000
> > +    multipleOf: 32000
> > +
> > +  rohm,vcc-input-current-limit-microamp:
> > +    description: system specific VCC/VACP input current limit (in
> > microamps).
> > +    minimum: 32000
> > +    maximum: 16352000
> > +    multipleOf: 32000
> > +
> > +required:
> > +  - compatible
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        charger@9 {
> > +            compatible = "rohm,bd9995x-charger";
> > +            monitored-battery = <&battery>;
> > +            reg = <0x9>;
> > +            interrupt-parent = <&gpio1>;
> > +            interrupts = <29 8>;
> > +            rohm,vsys-regulation-microvolt = <8960000>;
> > +            rohm,vbus-input-current-limit-microamp = <1472000>;
> > +            rohm,vcc-input-current-limit-microamp = <1472000>;
> > +        };
> > +    };
> > -- 
> > 2.21.0
> > 
> > 
> > -- 
> > Matti Vaittinen, Linux device drivers
> > ROHM Semiconductors, Finland SWDC
> > Kiviharjunlenkki 1E
> > 90220 OULU
> > FINLAND
> > 
> > ~~~ "I don't think so," said Rene Descartes. Just then he vanished
> > ~~~
> > Simon says - in Latin please.
> > ~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
> > Thanks to Simon Glass for the translation =] 





[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