And thanks again Lee!

On Thu, Jul 05, 2018 at 10:24:44AM +0100, Lee Jones wrote:
> On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> 
> > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> > 
> > Signed-off-by: Matti Vaittinen <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > ---
> >  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 67 
> > ++++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt 
> > b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > new file mode 100644
> > index 000000000000..67f2616288d9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > @@ -0,0 +1,67 @@
> > +* ROHM BD71837 Power Management Integrated Circuit bindings
> > +
> > +BD71837MWV is a programmable Power Management IC for powering single-core,
> > +dual-core, and quad-core SoC’s such as NXP-i.MX 8M. It is optimized for
> 
> Nit: s/SOC's/SOCs/

I'll change this.

> > +low BOM cost and compact solution footprint. It integrates 8 Buck
> > +egulators and 7 LDO’s to provide all the power rails required by the SoC 
> > and
> 
> Nit: s/LDO's/LDOs/

I'll change this.

> > +the commonly used peripherals.
> > +
> > +Datasheet for PMIC is available at:
> > +https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
> > +
> > +Required properties:
> > + - compatible              : Should be "rohm,bd71837".
> > + - reg                     : I2C slave address.
> > + - interrupt-parent        : Phandle to the parent interrupt controller.
> > + - interrupts              : The interrupt line the device is connected to.
> > + - clocks          : The parent clock connected to PMIC. If this is missng
> 
> s/missng/missing/

I'll change this.

> > +                     32768 KHz clock is assumed.
> > + - #clock-cells            : Should be 0
> > + - regulators:             : List of child nodes that specify the 
> > regulators
> 
> By your convention, the bottom two entries are missing '.'s.

I'll change this.

> > +                     Please see ../regulator/rohm,bd71837-regulator.txt
> > +
> > +Optional properties:
> > +- clock-output-names       : Should contain name for output clock.
> > +
> > +Example:
> > +
> > +   /* external oscillator node */
> 
> Uppercase character to start a sentence.

I'll change this.

> > +   osc: oscillator {
> > +           compatible = "fixed-clock";
> > +           #clock-cells = <1>;
> > +           clock-frequency  = <32768>;
> > +           clock-output-names = "osc";
> > +   };
> > +
> > +   /* PMIC node */
> 
> Is this comment really necessary?  The node is called "pmic".

I'll remove this.

> > +   pmic: pmic@4b {
> > +           compatible = "rohm,bd71837";
> > +           reg = <0x4b>;
> > +           interrupt-parent = <&gpio1>;
> > +           interrupts = <29 GPIO_ACTIVE_LOW>;
> > +           interrupt-names = "irq";
> > +           #clock-cells = <0>;
> > +           clocks = <&osc 0>;
> > +           clock-output-names = "bd71837-32k-out";
> > +
> > +           regulators {
> > +                   buck1: BUCK1 {
> > +                           regulator-name = "buck1";
> > +                           regulator-min-microvolt = <700000>;
> > +                           regulator-max-microvolt = <1300000>;
> > +                           regulator-boot-on;
> > +                           regulator-ramp-delay = <1250>;
> > +                   };
> > +                   /* ... */
> > +           };
> > +   };
> > +
> > +   /* Clock consumer node */
> > +
> > +   foo@0 {
> > +           compatible = "bar,foo";
> > +           /* ... */
> 
> No need for this line.  Additional properties are always expected.

I'll remove this.

> > +           clock-names = "my-clock";
> > +           clocks = <&pmic>;
> > +   };
> 
> Do you have a real example to give?

For clock consumer? Sorry, no I don't.

Br,
        Matti Vaittinen

> -- 
> Lee Jones [李琼斯]
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Reply via email to