Re: [RFC PATCH 1/3] dt_bindings: ROHM BD99954 Charger

["Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>]

On Mon, 2020-02-10 at 14:11 +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@fi.rohmeurope.com>
> ---
> I think these properties could probably be generic and not
> vendor specific, right? If so, then I could try adding parser
> function(s) for them in power/supply framework. How do you
> guys see this?

Sorry folks. I wasn't reading existing docs carefully enough. I skipped
battery.txt as I was doing driver for charger... I should've understood
that many of the limits here come from battery properties :/

So right after sending this out I hit the battry.txt binding doc and
following note in power_supply_class.rst:

"Drivers should call power_supply_get_battery_info() to obtain battery
characteristics from a devicetree battery node, defined in think there
must be quite a few examples of that in-tree too. 
Documentation/devicetree/bindings/power/supply/battery.txt"

It seems we already have bunch of common bindings and the helper. I'd
better study the charging properties from battery.txt and re-use those
I can re-use. I still see couple of things we might want to add?

(My new guess for "the correect thing to do" is that the charging
currents and voltages should be given in a battery node. The driver
should then use power_supply_get_battery_info() to extract this
information - makes sense. Input current limits are still different
thing)

> 
>  .../bindings/power/supply/rohm,bdDocumentation/devicetree/bindings/p
> ower/supply/battery.txt.9995x.yaml   | 118 ++++++++++++++++++
>  1 file changed, 118 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..ba9fe1dc3f2f
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/power/supply/rohm,bd9995x.yaml
> @@ -0,0 +1,118 @@
> +# 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
> +
> +maintainers:
> +  - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> +  - Markus Laine <markus.laine@fi.rohmeurope.com>
> +  - Mikko Mutanen <mikko.mutanen@fi.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
> +
> +  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

I think these input current limits (input for charger) can't be in
battery node. These are system/charger limits, right?

> +  rohm,trickle-charging-current-microamp:
I think this should be replaced by precharge-current-microamp:

> +    description:
> +      battery specific value for trickle-charging current.
> +    minimum: 64000
> +    maximum: 1024000
> +    multipleOf: 64000
> +
> +  rohm,pre-charging-current-microamp:
This should be replaced by constant-charge-current-max-microamp:

> +    description:
> +      battery specific value for pre-charging current.
> +    minimum: 64000
> +    maximum: 1024000
> +    multipleOf: 64000
> +
> +  rohm,fast-charging-regulation-microvolt:
This should be replaced by constant-charge-voltage-max-microvolt:

> +    description:
> +      battery/system specific upper limit for battry/system voltage.
> +    minimum: 2560000
> +    maximum: 19200000
> +    multipleOf: 16000
> +
> +  rohm,vsys-regulation-microvolt:
> +    description:
> +      system specific lower limit for system voltage.
> +    minimum: 2560000
> +    maximum: 19200000
> +    multipleOf: 64000
> +
> +  rohm,pre-charge-voltage-threshold-microvolt:
> +    description:
> +      voltage limit for changing from trickle to pre-charging.
Can we use voltage-min-design-microvolt: for this or invent a new one?

> +    minimum: 2048000
> +    maximum: 19200000
> +    multipleOf: 64000
> +
> +  rohm,re-charge-battery-voltage-microvolt:
> +    description:
> +      voltage limit for automatically re-starting the battery 
I don't see a match for this either.

> charging.
> +    minimum: 2560000
> +    maximum: 19200000
> +    multipleOf: 16000
> +
> +  rohm,battery-over-voltage-threshold-microvolt:
> +    description:
> +      voltage limit for detecting battery over voltage.
And this is also something we might want to add?

> +    minimum: 2560000
> +    maximum: 19200000
> +    multipleOf: 16000
> +
> +required:
> +  - compatible
> +  - rohm,vbus-input-current-limit-microamp
> +  - rohm,vcc-input-current-limit-microamp
> +  - rohm,trickle-charging-current-microamp
> +  - rohm,pre-charging-current-microamp
> +  - rohm,fast-charging-regulation-microvolt
> +  - rohm,vsys-regulation-microvolt
> +  - rohm,pre-charge-voltage-threshold-microvolt
> +  - rohm,re-charge-battery-voltage-microvolt
> +  - rohm,battery-over-voltage-threshold-microvolt
> +
> +examples:
> +  - |
> +    i2c {
> +        charger@9 {
> +            compatible = "rohm,bd9995x-charger";
> +            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>;
> +            rohm,trickle-charging-current-microamp = <256000>;
> +            rohm,pre-charging-current-microamp = <256000>;
> +            rohm,fast-charging-regulation-microvolt = <8400000>;
> +            rohm,pre-charge-voltage-threshold-microvolt = <2048000>;
> +            rohm,re-charge-battery-voltage-microvolt = <8112000>;
> +            rohm,battery-over-voltage-threshold-microvolt =
> <8912000>;
> +        };
> +    };
> -- 
> 2.21.0
> 
>