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