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

2013-06-06 Thread Stephen Warren
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 
>>>
>>> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-06-06 Thread Stephen Warren
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 g...@slimlogic.co.uk

 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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-06-05 Thread J, KEERTHY
Hi Grant,

> -Original Message-
> From: Grant Likely [mailto:glik...@secretlab.ca] On Behalf Of Grant
> Likely
> Sent: Thursday, June 06, 2013 5:32 AM
> To: J, KEERTHY; linux-kernel@vger.kernel.org
> Cc: linux-...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org;
> swar...@wwwdotorg.org; broo...@opensource.wolfsonmicro.com;
> rob.herr...@calxeda.com; r...@landley.net; sa...@linux.intel.com;
> w...@iguana.be; lgirdw...@gmail.com; g...@slimlogic.co.uk; Kristo, Tero;
> lee.jo...@linaro.org; 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  wrote:
> > From: Graeme Gregory 
> >
> > 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 
> > Signed-off-by: J Keerthy 
> > Signed-off-by: Ian Lartey 
> 
> 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 000..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 = <>;
> > +   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 000..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

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

2013-06-05 Thread J, KEERTHY
Hi Stephen,

Thanks for the quick review.

> -Original Message-
> From: Stephen Warren [mailto:swar...@wwwdotorg.org]
> Sent: Wednesday, June 05, 2013 10:44 PM
> To: J, KEERTHY
> Cc: linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
> devicetree-disc...@lists.ozlabs.org;
> broo...@opensource.wolfsonmicro.com; rob.herr...@calxeda.com;
> r...@landley.net; sa...@linux.intel.com; w...@iguana.be;
> lgirdw...@gmail.com; g...@slimlogic.co.uk; Kristo, Tero;
> lee.jo...@linaro.org; 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 
> >
> > 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 = <>;
> > +   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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-06-05 Thread Grant Likely
On Tue, 4 Jun 2013 14:11:49 +0530, J Keerthy  wrote:
> From: Graeme Gregory 
> 
> 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 
> Signed-off-by: J Keerthy 
> Signed-off-by: Ian Lartey 

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 000..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 = <>;
> + 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 000..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 
> +
> +pmic {
> + compatible = "ti,twl6035-pmic", "ti,palmas-pmic";
> + interrupt-parent = <>;
> + interrupts = <14 IRQ_TYPE_NONE>;
> + interrupt-name = "short-irq";
> +
> + ti,ldo6-vibrator;
> +
> + regulators {
> + smps12_reg : smps12 {
> + regulator-name = "smps12";
> + regulator-min-microvolt = < 60>;
> + regulator-max-microvolt = <150>;
> + 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";
> + 

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

2013-06-05 Thread Stephen Warren
On 06/04/2013 02:41 AM, J Keerthy wrote:
> From: Graeme Gregory 
> 
> 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 = <>;
> + 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-06-05 Thread Stephen Warren
On 06/04/2013 02:41 AM, J Keerthy wrote:
 From: Graeme Gregory g...@slimlogic.co.uk
 
 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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-06-05 Thread Grant Likely
On Tue, 4 Jun 2013 14:11:49 +0530, J Keerthy j-keer...@ti.com wrote:
 From: Graeme Gregory g...@slimlogic.co.uk
 
 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 g...@slimlogic.co.uk
 Signed-off-by: J Keerthy j-keer...@ti.com
 Signed-off-by: Ian Lartey i...@slimlogic.co.uk

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 000..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 000..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 =  60;
 + regulator-max-microvolt = 150;
 + 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 = 280;
 + 

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

2013-06-05 Thread J, KEERTHY
Hi Stephen,

Thanks for the quick review.

 -Original Message-
 From: Stephen Warren [mailto:swar...@wwwdotorg.org]
 Sent: Wednesday, June 05, 2013 10:44 PM
 To: J, KEERTHY
 Cc: linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
 devicetree-disc...@lists.ozlabs.org;
 broo...@opensource.wolfsonmicro.com; rob.herr...@calxeda.com;
 r...@landley.net; sa...@linux.intel.com; w...@iguana.be;
 lgirdw...@gmail.com; g...@slimlogic.co.uk; Kristo, Tero;
 lee.jo...@linaro.org; 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 g...@slimlogic.co.uk
 
  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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-06-05 Thread J, KEERTHY
Hi Grant,

 -Original Message-
 From: Grant Likely [mailto:glik...@secretlab.ca] On Behalf Of Grant
 Likely
 Sent: Thursday, June 06, 2013 5:32 AM
 To: J, KEERTHY; linux-kernel@vger.kernel.org
 Cc: linux-...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org;
 swar...@wwwdotorg.org; broo...@opensource.wolfsonmicro.com;
 rob.herr...@calxeda.com; r...@landley.net; sa...@linux.intel.com;
 w...@iguana.be; lgirdw...@gmail.com; g...@slimlogic.co.uk; Kristo, Tero;
 lee.jo...@linaro.org; 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 j-keer...@ti.com wrote:
  From: Graeme Gregory g...@slimlogic.co.uk
 
  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 g...@slimlogic.co.uk
  Signed-off-by: J Keerthy j-keer...@ti.com
  Signed-off-by: Ian Lartey i...@slimlogic.co.uk
 
 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 000..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 000..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

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

2013-06-04 Thread J, KEERTHY
Hello Lee Jones,

> -Original Message-
> From: Lee Jones [mailto:lee.jo...@linaro.org]
> Sent: Tuesday, June 04, 2013 5:45 PM
> To: J, KEERTHY
> Cc: linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
> devicetree-disc...@lists.ozlabs.org; swar...@wwwdotorg.org;
> broo...@opensource.wolfsonmicro.com; rob.herr...@calxeda.com;
> r...@landley.net; sa...@linux.intel.com; w...@iguana.be;
> lgirdw...@gmail.com; g...@slimlogic.co.uk; 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 
> >
> > 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 
> > Signed-off-by: J Keerthy 
> > Signed-off-by: Ian Lartey 
> > ---
> > 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�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

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

2013-06-04 Thread Lee Jones
On Tue, 04 Jun 2013, J Keerthy wrote:

> From: Graeme Gregory 
> 
> 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 
> Signed-off-by: J Keerthy 
> Signed-off-by: Ian Lartey 
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-06-04 Thread Lee Jones
On Tue, 04 Jun 2013, J Keerthy wrote:

 From: Graeme Gregory g...@slimlogic.co.uk
 
 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 g...@slimlogic.co.uk
 Signed-off-by: J Keerthy j-keer...@ti.com
 Signed-off-by: Ian Lartey i...@slimlogic.co.uk
 ---
 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2013-06-04 Thread J, KEERTHY
Hello Lee Jones,

 -Original Message-
 From: Lee Jones [mailto:lee.jo...@linaro.org]
 Sent: Tuesday, June 04, 2013 5:45 PM
 To: J, KEERTHY
 Cc: linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
 devicetree-disc...@lists.ozlabs.org; swar...@wwwdotorg.org;
 broo...@opensource.wolfsonmicro.com; rob.herr...@calxeda.com;
 r...@landley.net; sa...@linux.intel.com; w...@iguana.be;
 lgirdw...@gmail.com; g...@slimlogic.co.uk; 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 g...@slimlogic.co.uk
 
  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 g...@slimlogic.co.uk
  Signed-off-by: J Keerthy j-keer...@ti.com
  Signed-off-by: Ian Lartey i...@slimlogic.co.uk
  ---
  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�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i