2013-06-04 08:50:16

by Keerthy

[permalink] [raw]
Subject: [PATCH v2] mfd: DT bindings for the palmas family MFD

From: Graeme Gregory <[email protected]>

Add the various binding files for the palmas family of chips. There is a
top level MFD binding then a seperate binding for regulators IP blocks on chips.

Signed-off-by: Graeme Gregory <[email protected]>
Signed-off-by: J Keerthy <[email protected]>
Signed-off-by: Ian Lartey <[email protected]>
---
Changes from v1:
* Corrected ti,ldo6_vibrator property ---> ti,ldo6-vibrator
* Added the irq.h header file inclusion for defining type of interrupt

Documentation/devicetree/bindings/mfd/palmas.txt | 49 +++++++++++++++
.../devicetree/bindings/regulator/palmas-pmic.txt | 65 ++++++++++++++++++++
2 files changed, 114 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/palmas.txt
create mode 100644 Documentation/devicetree/bindings/regulator/palmas-pmic.txt

diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
new file mode 100644
index 0000000..c6c5e78
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/palmas.txt
@@ -0,0 +1,49 @@
+* palmas device tree bindings
+
+The TI palmas family current members :-
+twl6035 (palmas)
+twl6037 (palmas)
+tps65913 (palmas)
+tps65914 (palmas)
+
+Required properties:
+- compatible : Should be from the list
+ ti,twl6035
+ ti,twl6036
+ ti,twl6037
+ ti,tps65913
+ ti,tps65914
+ ti,tps80036
+and also the generic series names
+ ti,palmas
+- interrupt-controller : palmas has its own internal IRQs
+- #interrupt-cells : should be set to 2 for IRQ number and flags
+ The first cell is the IRQ number.
+ The second cell is the flags, encoded as the trigger masks from
+ Documentation/devicetree/bindings/interrupts.txt
+- interrupt-parent : The parent interrupt controller.
+
+Optional properties:
+ ti,mux_padX : set the pad register X (1-2) to the correct muxing for the
+ hardware, if not set will use muxing in OTP.
+
+Example:
+
+palmas {
+ compatible = "ti,twl6035", "ti,palmas";
+ reg = <0x48>
+ interrupt-parent = <&intc>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ ti,mux-pad1 = <0>;
+ ti,mux-pad2 = <0>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pmic {
+ compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
+ ....
+ };
+}
diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
new file mode 100644
index 0000000..a0ccdf2
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
@@ -0,0 +1,65 @@
+* palmas regulator IP block devicetree bindings
+
+Required properties:
+- compatible : Should be from the list
+ ti,twl6035-pmic
+ ti,twl6036-pmic
+ ti,twl6037-pmic
+ ti,tps65913-pmic
+ ti,tps65914-pmic
+and also the generic series names
+ ti,palmas-pmic
+
+Optional properties:
+- ti,ldo6-vibrator : ldo6 is in vibrator mode
+
+Optional nodes:
+- regulators : should contain the constrains and init information for the
+ regulators. It should contain a subnode per regulator from the
+ list.
+ For ti,palmas-pmic - smps12, smps123, smps3 depending on OTP,
+ smps45, smps457, smps7 depending on varient, smps6, smps[8-10],
+ ldo[1-9], ldoln, ldousb
+
+ optional chip specific regulator fields :-
+ ti,warm-reset - maintain voltage during warm reset(boolean)
+ ti,roof-floor - control voltage selection by pin(boolean)
+ ti,sleep-mode - mode to adopt in pmic sleep 0 - off, 1 - auto,
+ 2 - eco, 3 - forced pwm
+ ti,tstep - slope control 0 - Jump, 1 10mV/us, 2 5mV/us, 3 2.5mV/us
+ ti,smps-range - OTP has the wrong range set for the hardware so override
+ 0 - low range, 1 - high range
+
+Example:
+
+#include <include/dt-bindings/interrupt-controller/irq.h>
+
+pmic {
+ compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
+ interrupt-parent = <&palmas>;
+ interrupts = <14 IRQ_TYPE_NONE>;
+ interrupt-name = "short-irq";
+
+ ti,ldo6-vibrator;
+
+ regulators {
+ smps12_reg : smps12 {
+ regulator-name = "smps12";
+ regulator-min-microvolt = < 600000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-always-on;
+ regulator-boot-on;
+ ti,warm-reset;
+ ti,roof-floor;
+ ti,mode-sleep = <0>;
+ ti,tstep = <0>;
+ ti,smps-range = <1>;
+ };
+
+ ldo1_reg: ldo1 {
+ regulator-name = "ldo1";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+ };
+};
--
1.7.5.4


2013-06-04 12:14:53

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v2] mfd: DT bindings for the palmas family MFD

On Tue, 04 Jun 2013, J Keerthy wrote:

> From: Graeme Gregory <[email protected]>
>
> Add the various binding files for the palmas family of chips. There is a
> top level MFD binding then a seperate binding for regulators IP blocks on chips.
>
> Signed-off-by: Graeme Gregory <[email protected]>
> Signed-off-by: J Keerthy <[email protected]>
> Signed-off-by: Ian Lartey <[email protected]>
> ---
> Changes from v1:
> * Corrected ti,ldo6_vibrator property ---> ti,ldo6-vibrator
> * Added the irq.h header file inclusion for defining type of interrupt
>
> Documentation/devicetree/bindings/mfd/palmas.txt | 49 +++++++++++++++
> .../devicetree/bindings/regulator/palmas-pmic.txt | 65 ++++++++++++++++++++
> 2 files changed, 114 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/palmas.txt
> create mode 100644 Documentation/devicetree/bindings/regulator/palmas-pmic.txt

Is there a driver to go with this patch, or was the documentation an
afterthought in this case? If the latter is true I'd be happy to apply
this patch with an Ack from Stephen or one of the other previous
reviewers.

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2013-06-05 04:25:05

by Keerthy

[permalink] [raw]
Subject: RE: [PATCH v2] mfd: DT bindings for the palmas family MFD

Hello Lee Jones,

> -----Original Message-----
> From: Lee Jones [mailto:[email protected]]
> Sent: Tuesday, June 04, 2013 5:45 PM
> To: J, KEERTHY
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Kristo, Tero; Ian Lartey
> Subject: Re: [PATCH v2] mfd: DT bindings for the palmas family MFD
>
> On Tue, 04 Jun 2013, J Keerthy wrote:
>
> > From: Graeme Gregory <[email protected]>
> >
> > Add the various binding files for the palmas family of chips. There
> is
> > a top level MFD binding then a seperate binding for regulators IP
> blocks on chips.
> >
> > Signed-off-by: Graeme Gregory <[email protected]>
> > Signed-off-by: J Keerthy <[email protected]>
> > Signed-off-by: Ian Lartey <[email protected]>
> > ---
> > Changes from v1:
> > * Corrected ti,ldo6_vibrator property ---> ti,ldo6-vibrator
> > * Added the irq.h header file inclusion for defining type of
> interrupt
> >
> > Documentation/devicetree/bindings/mfd/palmas.txt | 49
> +++++++++++++++
> > .../devicetree/bindings/regulator/palmas-pmic.txt | 65
> ++++++++++++++++++++
> > 2 files changed, 114 insertions(+), 0 deletions(-) create mode
> > 100644 Documentation/devicetree/bindings/mfd/palmas.txt
> > create mode 100644
> > Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>
> Is there a driver to go with this patch, or was the documentation an
> afterthought in this case? If the latter is true I'd be happy to apply
> this patch with an Ack from Stephen or one of the other previous
> reviewers.

Well that's the latter case :-).

Stephen,

Can this be taken?

>
> --
> Lee Jones
> Linaro ST-Ericsson Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook
> | Twitter | Blog

Regards,
Keerthy
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-06-05 17:13:52

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v2] mfd: DT bindings for the palmas family MFD

On 06/04/2013 02:41 AM, J Keerthy wrote:
> From: Graeme Gregory <[email protected]>
>
> Add the various binding files for the palmas family of chips. There is a
> top level MFD binding then a seperate binding for regulators IP blocks on chips.

> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt

> +Optional properties:
> + ti,mux_padX : set the pad register X (1-2) to the correct muxing for the
> + hardware, if not set will use muxing in OTP.
> +
> +Example:
...
> + ti,mux-pad1 = <0>;
> + ti,mux-pad2 = <0>;

Use of - vs. _ is inconsistent there. It should be -.

> diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt

> +Optional nodes:
> +- regulators : should contain the constrains and init information for the
> + regulators. It should contain a subnode per regulator from the
> + list.

I would re-phrase that as:

Must contain a sub-node per regulator from the list below. Each sub-node
should contain the constraints and initialization information for that
regulator. See regulator.txt for a description of standard properties
for these sub-nodes. Additional custom properties are listed below.

> + For ti,palmas-pmic - smps12, smps123, smps3 depending on OTP,
> + smps45, smps457, smps7 depending on varient, smps6, smps[8-10],

typo: s/varient/variant/.

> + ldo[1-9], ldoln, ldousb

nit: s/$/./ ?

> +
> + optional chip specific regulator fields :-

Perhaps "Optional sub-node properties:"?

> +pmic {
> + compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
> + interrupt-parent = <&palmas>;
> + interrupts = <14 IRQ_TYPE_NONE>;
> + interrupt-name = "short-irq";

If those are required, shouldn't they be listed in a "Required
properties" section above? In particular, the order of entries in the
interrupts property must be defined, as well as the expected nameds in
the interrupt-name property.

Oh, and it's interrupt-names not interrupt-name.

Oh, one question though: How does the regulator driver determine the
register address of the regulator sub-device within the overall PMIC?
Presumably if these are pluggable independent modules, that could change
depending on which overall chip the PMIC device is plugged into. don't
you need a reg property to specify that?

Aside from those comments, this all looks reasonable to me.

2013-06-06 00:02:30

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v2] mfd: DT bindings for the palmas family MFD

On Tue, 4 Jun 2013 14:11:49 +0530, J Keerthy <[email protected]> wrote:
> From: Graeme Gregory <[email protected]>
>
> Add the various binding files for the palmas family of chips. There is a
> top level MFD binding then a seperate binding for regulators IP blocks on chips.
>
> Signed-off-by: Graeme Gregory <[email protected]>
> Signed-off-by: J Keerthy <[email protected]>
> Signed-off-by: Ian Lartey <[email protected]>

Applied, thanks.

g.

> ---
> Changes from v1:
> * Corrected ti,ldo6_vibrator property ---> ti,ldo6-vibrator
> * Added the irq.h header file inclusion for defining type of interrupt
>
> Documentation/devicetree/bindings/mfd/palmas.txt | 49 +++++++++++++++
> .../devicetree/bindings/regulator/palmas-pmic.txt | 65 ++++++++++++++++++++
> 2 files changed, 114 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/palmas.txt
> create mode 100644 Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt b/Documentation/devicetree/bindings/mfd/palmas.txt
> new file mode 100644
> index 0000000..c6c5e78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/palmas.txt
> @@ -0,0 +1,49 @@
> +* palmas device tree bindings
> +
> +The TI palmas family current members :-
> +twl6035 (palmas)
> +twl6037 (palmas)
> +tps65913 (palmas)
> +tps65914 (palmas)
> +
> +Required properties:
> +- compatible : Should be from the list
> + ti,twl6035
> + ti,twl6036
> + ti,twl6037
> + ti,tps65913
> + ti,tps65914
> + ti,tps80036
> +and also the generic series names
> + ti,palmas
> +- interrupt-controller : palmas has its own internal IRQs
> +- #interrupt-cells : should be set to 2 for IRQ number and flags
> + The first cell is the IRQ number.
> + The second cell is the flags, encoded as the trigger masks from
> + Documentation/devicetree/bindings/interrupts.txt
> +- interrupt-parent : The parent interrupt controller.
> +
> +Optional properties:
> + ti,mux_padX : set the pad register X (1-2) to the correct muxing for the
> + hardware, if not set will use muxing in OTP.
> +
> +Example:
> +
> +palmas {
> + compatible = "ti,twl6035", "ti,palmas";
> + reg = <0x48>
> + interrupt-parent = <&intc>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> +
> + ti,mux-pad1 = <0>;
> + ti,mux-pad2 = <0>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmic {
> + compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
> + ....
> + };
> +}
> diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> new file mode 100644
> index 0000000..a0ccdf2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> @@ -0,0 +1,65 @@
> +* palmas regulator IP block devicetree bindings
> +
> +Required properties:
> +- compatible : Should be from the list
> + ti,twl6035-pmic
> + ti,twl6036-pmic
> + ti,twl6037-pmic
> + ti,tps65913-pmic
> + ti,tps65914-pmic
> +and also the generic series names
> + ti,palmas-pmic
> +
> +Optional properties:
> +- ti,ldo6-vibrator : ldo6 is in vibrator mode
> +
> +Optional nodes:
> +- regulators : should contain the constrains and init information for the
> + regulators. It should contain a subnode per regulator from the
> + list.
> + For ti,palmas-pmic - smps12, smps123, smps3 depending on OTP,
> + smps45, smps457, smps7 depending on varient, smps6, smps[8-10],
> + ldo[1-9], ldoln, ldousb
> +
> + optional chip specific regulator fields :-
> + ti,warm-reset - maintain voltage during warm reset(boolean)
> + ti,roof-floor - control voltage selection by pin(boolean)
> + ti,sleep-mode - mode to adopt in pmic sleep 0 - off, 1 - auto,
> + 2 - eco, 3 - forced pwm
> + ti,tstep - slope control 0 - Jump, 1 10mV/us, 2 5mV/us, 3 2.5mV/us
> + ti,smps-range - OTP has the wrong range set for the hardware so override
> + 0 - low range, 1 - high range
> +
> +Example:
> +
> +#include <include/dt-bindings/interrupt-controller/irq.h>
> +
> +pmic {
> + compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
> + interrupt-parent = <&palmas>;
> + interrupts = <14 IRQ_TYPE_NONE>;
> + interrupt-name = "short-irq";
> +
> + ti,ldo6-vibrator;
> +
> + regulators {
> + smps12_reg : smps12 {
> + regulator-name = "smps12";
> + regulator-min-microvolt = < 600000>;
> + regulator-max-microvolt = <1500000>;
> + regulator-always-on;
> + regulator-boot-on;
> + ti,warm-reset;
> + ti,roof-floor;
> + ti,mode-sleep = <0>;
> + ti,tstep = <0>;
> + ti,smps-range = <1>;
> + };
> +
> + ldo1_reg: ldo1 {
> + regulator-name = "ldo1";
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + };
> + };
> +};
> --
> 1.7.5.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

2013-06-06 03:34:59

by Keerthy

[permalink] [raw]
Subject: RE: [PATCH v2] mfd: DT bindings for the palmas family MFD

Hi Stephen,

Thanks for the quick review.

> -----Original Message-----
> From: Stephen Warren [mailto:[email protected]]
> Sent: Wednesday, June 05, 2013 10:44 PM
> To: J, KEERTHY
> Cc: [email protected]; [email protected];
> [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Kristo, Tero;
> [email protected]; Ian Lartey
> Subject: Re: [PATCH v2] mfd: DT bindings for the palmas family MFD
>
> On 06/04/2013 02:41 AM, J Keerthy wrote:
> > From: Graeme Gregory <[email protected]>
> >
> > Add the various binding files for the palmas family of chips. There
> is
> > a top level MFD binding then a seperate binding for regulators IP
> blocks on chips.
>
> > diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt
> > b/Documentation/devicetree/bindings/mfd/palmas.txt
>
> > +Optional properties:
> > + ti,mux_padX : set the pad register X (1-2) to the correct muxing
> for the
> > + hardware, if not set will use muxing in OTP.
> > +
> > +Example:
> ...
> > + ti,mux-pad1 = <0>;
> > + ti,mux-pad2 = <0>;
>
> Use of - vs. _ is inconsistent there. It should be -.
>

Oops. I will fix this.

> > diff --git
> > a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
>
> > +Optional nodes:
> > +- regulators : should contain the constrains and init information
> for the
> > + regulators. It should contain a subnode per regulator from
> the
> > + list.
>
> I would re-phrase that as:
>
> Must contain a sub-node per regulator from the list below. Each sub-
> node should contain the constraints and initialization information for
> that regulator. See regulator.txt for a description of standard
> properties for these sub-nodes. Additional custom properties are
> listed below.
>
> > + For ti,palmas-pmic - smps12, smps123, smps3 depending on
> OTP,
> > + smps45, smps457, smps7 depending on varient, smps6,
> > +smps[8-10],
>
> typo: s/varient/variant/.

I will fix this.

>
> > + ldo[1-9], ldoln, ldousb
>
> nit: s/$/./ ?
>

Ok.

> > +
> > + optional chip specific regulator fields :-
>
> Perhaps "Optional sub-node properties:"?

Ok.

>
> > +pmic {
> > + compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
> > + interrupt-parent = <&palmas>;
> > + interrupts = <14 IRQ_TYPE_NONE>;
> > + interrupt-name = "short-irq";
>
> If those are required, shouldn't they be listed in a "Required
> properties" section above? In particular, the order of entries in the
> interrupts property must be defined, as well as the expected nameds in
> the interrupt-name property.
>
> Oh, and it's interrupt-names not interrupt-name.

Ok.

>
> Oh, one question though: How does the regulator driver determine the
> register address of the regulator sub-device within the overall PMIC?
> Presumably if these are pluggable independent modules, that could
> change depending on which overall chip the PMIC device is plugged into.
> don't you need a reg property to specify that?

The variants have identical register addresses. These are not pluggable
Independent modules. All the variants come with all the regulators
Listed above in general. The driver today has a statically defined
Array of all the above mentioned regulators with their addresses.

drivers/regulator/palmas-regulator.c

Line 38.

>
> Aside from those comments, this all looks reasonable to me.

Once again thanks for the comprehensive feedback.

Regards,
Keerthy

2013-06-06 03:38:37

by Keerthy

[permalink] [raw]
Subject: RE: [PATCH v2] mfd: DT bindings for the palmas family MFD

Hi Grant,

> -----Original Message-----
> From: Grant Likely [mailto:[email protected]] On Behalf Of Grant
> Likely
> Sent: Thursday, June 06, 2013 5:32 AM
> To: J, KEERTHY; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Kristo, Tero;
> [email protected]; J, KEERTHY; Ian Lartey
> Subject: Re: [PATCH v2] mfd: DT bindings for the palmas family MFD
>
> On Tue, 4 Jun 2013 14:11:49 +0530, J Keerthy <[email protected]> wrote:
> > From: Graeme Gregory <[email protected]>
> >
> > Add the various binding files for the palmas family of chips. There
> is
> > a top level MFD binding then a seperate binding for regulators IP
> blocks on chips.
> >
> > Signed-off-by: Graeme Gregory <[email protected]>
> > Signed-off-by: J Keerthy <[email protected]>
> > Signed-off-by: Ian Lartey <[email protected]>
>
> Applied, thanks.
>

There are couple of minor comments from Stephen. I will post a v3
Of this with an Acked-by: Stephen. Could you please pull that
And drop this?

> g.
>

Regards,
Keerthy

> > ---
> > Changes from v1:
> > * Corrected ti,ldo6_vibrator property ---> ti,ldo6-vibrator
> > * Added the irq.h header file inclusion for defining type of
> interrupt
> >
> > Documentation/devicetree/bindings/mfd/palmas.txt | 49
> +++++++++++++++
> > .../devicetree/bindings/regulator/palmas-pmic.txt | 65
> ++++++++++++++++++++
> > 2 files changed, 114 insertions(+), 0 deletions(-) create mode
> > 100644 Documentation/devicetree/bindings/mfd/palmas.txt
> > create mode 100644
> > Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/palmas.txt
> > b/Documentation/devicetree/bindings/mfd/palmas.txt
> > new file mode 100644
> > index 0000000..c6c5e78
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/palmas.txt
> > @@ -0,0 +1,49 @@
> > +* palmas device tree bindings
> > +
> > +The TI palmas family current members :-
> > +twl6035 (palmas)
> > +twl6037 (palmas)
> > +tps65913 (palmas)
> > +tps65914 (palmas)
> > +
> > +Required properties:
> > +- compatible : Should be from the list
> > + ti,twl6035
> > + ti,twl6036
> > + ti,twl6037
> > + ti,tps65913
> > + ti,tps65914
> > + ti,tps80036
> > +and also the generic series names
> > + ti,palmas
> > +- interrupt-controller : palmas has its own internal IRQs
> > +- #interrupt-cells : should be set to 2 for IRQ number and flags
> > + The first cell is the IRQ number.
> > + The second cell is the flags, encoded as the trigger masks from
> > + Documentation/devicetree/bindings/interrupts.txt
> > +- interrupt-parent : The parent interrupt controller.
> > +
> > +Optional properties:
> > + ti,mux_padX : set the pad register X (1-2) to the correct muxing
> for the
> > + hardware, if not set will use muxing in OTP.
> > +
> > +Example:
> > +
> > +palmas {
> > + compatible = "ti,twl6035", "ti,palmas";
> > + reg = <0x48>
> > + interrupt-parent = <&intc>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > +
> > + ti,mux-pad1 = <0>;
> > + ti,mux-pad2 = <0>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + pmic {
> > + compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
> > + ....
> > + };
> > +}
> > diff --git
> > a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > new file mode 100644
> > index 0000000..a0ccdf2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt
> > @@ -0,0 +1,65 @@
> > +* palmas regulator IP block devicetree bindings
> > +
> > +Required properties:
> > +- compatible : Should be from the list
> > + ti,twl6035-pmic
> > + ti,twl6036-pmic
> > + ti,twl6037-pmic
> > + ti,tps65913-pmic
> > + ti,tps65914-pmic
> > +and also the generic series names
> > + ti,palmas-pmic
> > +
> > +Optional properties:
> > +- ti,ldo6-vibrator : ldo6 is in vibrator mode
> > +
> > +Optional nodes:
> > +- regulators : should contain the constrains and init information
> for the
> > + regulators. It should contain a subnode per regulator from
> the
> > + list.
> > + For ti,palmas-pmic - smps12, smps123, smps3 depending on
> OTP,
> > + smps45, smps457, smps7 depending on varient, smps6,
> smps[8-10],
> > + ldo[1-9], ldoln, ldousb
> > +
> > + optional chip specific regulator fields :-
> > + ti,warm-reset - maintain voltage during warm
> reset(boolean)
> > + ti,roof-floor - control voltage selection by pin(boolean)
> > + ti,sleep-mode - mode to adopt in pmic sleep 0 - off, 1 -
> auto,
> > + 2 - eco, 3 - forced pwm
> > + ti,tstep - slope control 0 - Jump, 1 10mV/us, 2 5mV/us, 3
> 2.5mV/us
> > + ti,smps-range - OTP has the wrong range set for the
> hardware so override
> > + 0 - low range, 1 - high range
> > +
> > +Example:
> > +
> > +#include <include/dt-bindings/interrupt-controller/irq.h>
> > +
> > +pmic {
> > + compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
> > + interrupt-parent = <&palmas>;
> > + interrupts = <14 IRQ_TYPE_NONE>;
> > + interrupt-name = "short-irq";
> > +
> > + ti,ldo6-vibrator;
> > +
> > + regulators {
> > + smps12_reg : smps12 {
> > + regulator-name = "smps12";
> > + regulator-min-microvolt = < 600000>;
> > + regulator-max-microvolt = <1500000>;
> > + regulator-always-on;
> > + regulator-boot-on;
> > + ti,warm-reset;
> > + ti,roof-floor;
> > + ti,mode-sleep = <0>;
> > + ti,tstep = <0>;
> > + ti,smps-range = <1>;
> > + };
> > +
> > + ldo1_reg: ldo1 {
> > + regulator-name = "ldo1";
> > + regulator-min-microvolt = <2800000>;
> > + regulator-max-microvolt = <2800000>;
> > + };
> > + };
> > +};
> > --
> > 1.7.5.4
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-kernel" in the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> --
> Grant Likely, B.Sc, P.Eng.
> Secret Lab Technologies, Ltd.

2013-06-06 15:53:49

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v2] mfd: DT bindings for the palmas family MFD

On 06/05/2013 09:34 PM, J, KEERTHY wrote:
> Hi Stephen,
>
> Thanks for the quick review.
>
> Stephen Warren wrote at Wednesday, June 05, 2013 10:44 PM:
>> On 06/04/2013 02:41 AM, J Keerthy wrote:
>>> From: Graeme Gregory <[email protected]>
>>>
>>> Add the various binding files for the palmas family of chips. There is
>>> a top level MFD binding then a seperate binding for regulators IP
>> blocks on chips.
...
>> Oh, one question though: How does the regulator driver determine the
>> register address of the regulator sub-device within the overall PMIC?
>> Presumably if these are pluggable independent modules, that could
>> change depending on which overall chip the PMIC device is plugged into.
>> don't you need a reg property to specify that?
>
> The variants have identical register addresses. These are not pluggable
> Independent modules. All the variants come with all the regulators
> Listed above in general. The driver today has a statically defined
> Array of all the above mentioned regulators with their addresses.
>
> drivers/regulator/palmas-regulator.c
>
> Line 38.

I meant the I2C address used to communicate with the regulator registers
really, and I suppose the base address of the regulator register block.

In the driver, I see this is handled by the top-level Palmas driver
creating a regmap object which the regulator driver used. This keeps the
regulator driver completely unaware of these issues, only the top-level
chip driver cares about this, which is fine.

While that justification is in terms of OS-specific code, the basic
argument can be applied to the HW itself (the top-level chip implies the
I2C address and any register offset), so this really is a HW-driven
argument, so I guess it's fine not having a reg property in the
top-level regulator node.

One question though: I wonder why if the HW IP blocks aren't completely
independent modules that can be mixed/matched together to form new
chips, there's even a need for a separate regulator node with its own
compatible value. Still, I suppose it's a valid way to construct the DT
either way, so it's fine.