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

[Rob Herring]

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.

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

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

> +# - 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 =]