Re: [PATCH] dt-bindings: bd718x7: Yamlify and add BD71850
From: Vaittinen, Matti
Date: Fri Dec 27 2019 - 04:01:55 EST
Thanks for taking a look on this Rob,
On Thu, 2019-12-26 at 15:16 -0700, Rob Herring wrote:
> On Tue, Dec 17, 2019 at 10:48:24AM +0200, Matti Vaittinen wrote:
> > Convert ROHM bd71837 and bd71847 PMIC binding text docs to yaml.
> > Split
> > the binding document to two separate documents (own for BD71837 and
> > BD71847)
> > as they have different amount of regulators. This way we can better
> > enforce
> > the node name check for regulators. ROHM is also providing BD71850
> > - which
> > is almost identical to BD71847 - main difference is some initial
> > regulator
> > states. The BD71850 can be driven by same driver and it has same
> > buck/LDO
> > setup as BD71847 - add it to BD71847 binding document and introduce
> > compatible for it.
> >
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
> > ---
> >
> > Oh dear how bad I am with yaml...
>
> Looks pretty good overall.
Thanks. But it was not easy for me. It took bunch of trials to fix all
the errors...
> I hope 'yamlify' doesn't catch on. :)
I can almost see that in OED already xD
> > Lee, I think the support series for BD71828 included some changes
> > to drivers/mfd/rohm-bd718x7.c - I will add BD71850 compatible to
> > next
> > version of that series in order to avoid conflicts. Does that work
> > for you?
> >
> > .../bindings/mfd/rohm,bd71837-pmic.txt | 90 --------
> > .../bindings/mfd/rohm,bd71837-pmic.yaml | 198
> > ++++++++++++++++++
> > .../bindings/mfd/rohm,bd71847-pmic.yaml | 181
> > ++++++++++++++++
> > .../regulator/rohm,bd71837-regulator.txt | 162 --------------
> > .../regulator/rohm,bd71837-regulator.yaml | 103 +++++++++
> > .../regulator/rohm,bd71847-regulator.yaml | 97 +++++++++
> > 6 files changed, 579 insertions(+), 252 deletions(-)
> > delete mode 100644
> > Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > create mode 100644
> > Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.yaml
> > create mode 100644
> > Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml
> > delete mode 100644
> > Documentation/devicetree/bindings/regulator/rohm,bd71837-
> > regulator.txt
> > create mode 100644
> > Documentation/devicetree/bindings/regulator/rohm,bd71837-
> > regulator.yaml
> > create mode 100644
> > Documentation/devicetree/bindings/regulator/rohm,bd71847-
> > regulator.yaml
> > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-
> > pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71837-
> > pmic.yaml
> > new file mode 100644
> > index 000000000000..3a6d408aebbd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.yaml
> > @@ -0,0 +1,198 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/rohm,bd71837-pmic.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ROHM BD71837 Power Management Integrated Circuit bindings
> > +
> > +maintainers:
> > + - Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx>
> > +
> > +description: |
> > + BD71837MWV is programmable Power Management ICs for powering
> > single-core,
> > + dual-core, and quad-core SoCs such as NXP-i.MX 8M. It is
> > optimized for low
> > + BOM cost and compact solution footprint. BD71837MWV integrates
> > 8 Buck
> > + regulators and 7 LDOs.
> > + Datasheet for BD71837 is available at
> > +
> > https://www.rohm.com/products/power-management/power-management-ic-for-system/industrial-consumer-applications/nxp-imx/bd71837amwv-product
> > +
> > +properties:
> > + compatible:
> > + const: rohm,bd71837
> > +
> > + reg:
> > + description:
> > + I2C slave address.
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + "#clock-cells":
> > + const: 0
> > +
> > +# The BD718x7 supports two different HW states as reset target
> > states. States
> > +# are called as SNVS and READY. At READY state all the PMIC power
> > outputs go
> > +# down and OTP is reload. At the SNVS state all other logic and
> > external
> > +# devices apart from the SNVS power domain are shut off. Please
> > refer to NXP
> > +# i.MX8 documentation for further information regarding SNVS
> > state. When a
> > +# reset is done via SNVS state the PMIC OTP data is not reload.
> > This causes
> > +# power outputs that have been under SW control to stay down when
> > reset has
> > +# switched power state to SNVS. If reset is done via READY state
> > the power
> > +# outputs will be returned to HW control by OTP loading. Thus the
> > reset
> > +# target state is set to READY by default. If SNVS state is used
> > the boot
> > +# crucial regulators must have the regulator-always-on and
> > regulator-boot-on
> > +# properties set in regulator node.
> > +
> > + rohm,reset-snvs-powered:
> > + description: |
> > + Transfer PMIC to SNVS state at reset
> > + type: boolean
> > +
> > +# Configure the "short press" and "long press" timers for the
> > power button.
> > +# Values are rounded to what hardware supports (500ms multiple for
> > short and
> > +# 1000ms multiple for long). If these properties are not present
> > the existing
> > +# configuration (from bootloader or OTP) is not touched.
>
> You can use 'multipleOf' keyword below for some constraints.
Umm.. Actually, now that I checked the datasheet for proper
constraints... Minimum value for both short and long press is 10ms.
After that the values are multiples of 500ms or 1000ms - but first
value is 10ms. I'll add minimum and maximum and update the comment
accordingly. Thanks for catching this :)
>
> Same for the other file.
>
> > +
> > + rohm,short-press-ms:
> > + description:
> > + Short press duration in milliseconds
> > +
> > + rohm,long-press-ms:
> > + description:
> > + Long press duration in milliseconds
> > +
> > + regulators:
> > + $ref: ../regulator/rohm,bd71837-regulator.yaml
> > + description:
> > + List of child nodes that specify the regulators.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - "#clock-cells"
> > + - regulators
>
> additionalProperties: false
Ok.
>
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/leds/common.h>
> > + #
> > +
> > + i2c {
> > + pmic: pmic@4b {
> > + compatible = "rohm,bd71837";
> > + reg = <0x4b>;
> > + interrupt-parent = <&gpio1>;
> > + interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> > + #clock-cells = <0>;
> > + clocks = <&osc 0>;
> > + clock-output-names = "bd71837-32k-out";
>
> Not documented.
I'll just drop this from the example for now. We can add ref to common
clock bindings later - if they are needed at some point.
> > + rohm,reset-snvs-powered;
> > +
> > + regulators {
> > + buck1: BUCK1 {
> > + regulator-name = "buck1";
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <1300000>;
> > + regulator-boot-on;
> > + regulator-always-on;
> > + regulator-ramp-delay = <1250>;
> > + rohm,dvs-run-voltage = <900000>;
> > + rohm,dvs-idle-voltage = <850000>;
> > + rohm,dvs-suspend-voltage = <800000>;
> > + };
> > + buck2: BUCK2 {
> > + regulator-name = "buck2";
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <1300000>;
> > + regulator-boot-on;
> > + regulator-always-on;
> > + regulator-ramp-delay = <1250>;
> > + rohm,dvs-run-voltage = <1000000>;
> > + rohm,dvs-idle-voltage = <900000>;
> > + };
> > + buck3: BUCK3 {
> > + regulator-name = "buck3";
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <1300000>;
> > + regulator-boot-on;
> > + rohm,dvs-run-voltage = <1000000>;
> > + };
> > + buck4: BUCK4 {
> > + regulator-name = "buck4";
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <1300000>;
> > + regulator-boot-on;
> > + rohm,dvs-run-voltage = <1000000>;
> > + };
> > + buck5: BUCK5 {
> > + regulator-name = "buck5";
> > + regulator-min-microvolt = <700000>;
> > + regulator-max-microvolt = <1350000>;
> > + regulator-boot-on;
> > + };
> > + buck6: BUCK6 {
> > + regulator-name = "buck6";
> > + regulator-min-microvolt = <3000000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-boot-on;
> > + };
> > + buck7: BUCK7 {
> > + regulator-name = "buck7";
> > + regulator-min-microvolt = <1605000>;
> > + regulator-max-microvolt = <1995000>;
> > + regulator-boot-on;
> > + };
> > + buck8: BUCK8 {
> > + regulator-name = "buck8";
> > + regulator-min-microvolt = <800000>;
> > + regulator-max-microvolt = <1400000>;
> > + };
> > +
> > + ldo1: LDO1 {
> > + regulator-name = "ldo1";
> > + regulator-min-microvolt = <3000000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-boot-on;
> > + };
> > + ldo2: LDO2 {
> > + regulator-name = "ldo2";
> > + regulator-min-microvolt = <900000>;
> > + regulator-max-microvolt = <900000>;
> > + regulator-boot-on;
> > + };
> > + ldo3: LDO3 {
> > + regulator-name = "ldo3";
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <3300000>;
> > + };
> > + ldo4: LDO4 {
> > + regulator-name = "ldo4";
> > + regulator-min-microvolt = <900000>;
> > + regulator-max-microvolt = <1800000>;
> > + };
> > + ldo5: LDO5 {
> > + regulator-name = "ldo5";
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <3300000>;
> > + };
> > + ldo6: LDO6 {
> > + regulator-name = "ldo6";
> > + regulator-min-microvolt = <900000>;
> > + regulator-max-microvolt = <1800000>;
> > + };
> > + ldo7_reg: LDO7 {
> > + regulator-name = "ldo7";
> > + regulator-min-microvolt = <1800000>;
> > + regulator-max-microvolt = <3300000>;
> > + };
> > + };
> > + };
> > + };