diff mbox

[v9,2/2] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC

Message ID 87cc68c82c3c95e82797e0ce3d401a4f1d6daf25.1530776326.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Vaittinen, Matti July 5, 2018, 7:48 a.m. UTC
Document devicetree bindings for ROHM BD71837 PMIC MFD.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt

Comments

Lee Jones July 5, 2018, 9:24 a.m. UTC | #1
On Thu, 05 Jul 2018, Matti Vaittinen wrote:

> Document devicetree bindings for ROHM BD71837 PMIC MFD.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../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/

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

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

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

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

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

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

> +		clock-names = "my-clock";
> +		clocks = <&pmic>;
> +	};

Do you have a real example to give?
Vaittinen, Matti July 5, 2018, 10:39 a.m. UTC | #2
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 <matti.vaittinen@fi.rohmeurope.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../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
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones July 5, 2018, 10:49 a.m. UTC | #3
On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> 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 <matti.vaittinen@fi.rohmeurope.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 67 ++++++++++++++++++++++
> > >  1 file changed, 67 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > > +		clock-names = "my-clock";
> > > +		clocks = <&pmic>;
> > > +	};
> > 
> > Do you have a real example to give?
> 
> For clock consumer? Sorry, no I don't.

Might be better to drop it for the time being then.
Vaittinen, Matti July 5, 2018, 11:51 a.m. UTC | #4
On Thu, Jul 05, 2018 at 11:49:19AM +0100, Lee Jones wrote:
> On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> > 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 <matti.vaittinen@fi.rohmeurope.com>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > ---
> > > >  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 67 ++++++++++++++++++++++
> > > >  1 file changed, 67 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > > > +		clock-names = "my-clock";
> > > > +		clocks = <&pmic>;
> > > > +	};
> > > 
> > > Do you have a real example to give?
> > 
> > For clock consumer? Sorry, no I don't.
> 
> Might be better to drop it for the time being then.

I have tested the clk driver using this dummy consumer. So in a sense it
"works" and can be used as an example on how to write a real clock
consumer node. Thus I see some value in this example node - even if it
does not match to any real world HW. If I had to use the clk from this
PMIC and write HW description I would appreciate this dummy exaple. I
can drop it if you insist - but I would at least like to hear what is
the downside on having it here?

Best regards,
    Matti Vaittinen
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Jones July 5, 2018, 12:44 p.m. UTC | #5
On Thu, 05 Jul 2018, Matti Vaittinen wrote:

> On Thu, Jul 05, 2018 at 11:49:19AM +0100, Lee Jones wrote:
> > On Thu, 05 Jul 2018, Matti Vaittinen wrote:
> > > 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 <matti.vaittinen@fi.rohmeurope.com>
> > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > ---
> > > > >  .../devicetree/bindings/mfd/rohm,bd71837-pmic.txt  | 67 ++++++++++++++++++++++
> > > > >  1 file changed, 67 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd71837-pmic.txt
> > > > > +		clock-names = "my-clock";
> > > > > +		clocks = <&pmic>;
> > > > > +	};
> > > > 
> > > > Do you have a real example to give?
> > > 
> > > For clock consumer? Sorry, no I don't.
> > 
> > Might be better to drop it for the time being then.
> 
> I have tested the clk driver using this dummy consumer. So in a sense it
> "works" and can be used as an example on how to write a real clock
> consumer node. Thus I see some value in this example node - even if it
> does not match to any real world HW. If I had to use the clk from this
> PMIC and write HW description I would appreciate this dummy exaple. I
> can drop it if you insist - but I would at least like to hear what is
> the downside on having it here?

My suggestion then would be to make it look as authentic as possible.

It is only an example, so it doesn't *really* matter, but the current
foo,bar one just looks a bit crumby.
diff mbox

Patch

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
+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
+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
+			  32768 KHz clock is assumed.
+ - #clock-cells		: Should be 0
+ - regulators:		: List of child nodes that specify the regulators
+			  Please see ../regulator/rohm,bd71837-regulator.txt
+
+Optional properties:
+- clock-output-names	: Should contain name for output clock.
+
+Example:
+
+	/* external oscillator node */
+	osc: oscillator {
+		compatible = "fixed-clock";
+		#clock-cells = <1>;
+		clock-frequency  = <32768>;
+		clock-output-names = "osc";
+	};
+
+	/* PMIC node */
+
+	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";
+		/* ... */
+		clock-names = "my-clock";
+		clocks = <&pmic>;
+	};