lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 3 Mar 2020 06:30:36 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "robh@...nel.org" <robh@...nel.org>
CC:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "mazziesaccount@...il.com" <mazziesaccount@...il.com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "sre@...nel.org" <sre@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "Laine, Markus" <Markus.Laine@...rohmeurope.com>,
        "Mutanen, Mikko" <Mikko.Mutanen@...rohmeurope.com>
Subject: Re: [PATCH v4 2/9] dt_bindings: ROHM BD99954 Charger

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@...rohmeurope.com>
> > ---
> > 
> > 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@...rohmeurope.com>
> > +  - Markus Laine <markus.laine@...rohmeurope.com>
> > +  - Mikko Mutanen <mikko.mutanen@...rohmeurope.com>
> > +
> > +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 =] 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ