Re: [PATCH 1/4] mfd: 88pm800: add device tree support

2015-06-16 Thread Vaibhav Hiremath



On Tuesday 16 June 2015 06:32 PM, Rob Herring wrote:

On Tue, Jun 16, 2015 at 2:52 AM, Vaibhav Hiremath
 wrote:



On Monday 01 June 2015 02:08 PM, Lee Jones wrote:


On Sat, 30 May 2015, Vaibhav Hiremath wrote:



Thanks for your review. and sorry for delayed response.


Add DT support to the 88pm800 driver along with below properties
 - marvell,88pm800-irq-write-clear :
   Idicates whether interrupt status is cleared by write

Also, creates the DT binding text file,
Documentation/devicetree/bindings/mfd/88pm800.txt

Signed-off-by: Chao Xie 
Signed-off-by: Vaibhav Hiremath 
---
   Documentation/devicetree/bindings/mfd/88pm800.txt | 57
+++
   drivers/mfd/88pm800.c | 39 



These should be submitted separately.




Ok, will separate it in next version.



   2 files changed, 96 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt
b/Documentation/devicetree/bindings/mfd/88pm800.txt
new file mode 100644
index 000..094951b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -0,0 +1,57 @@
+* Marvell 88PM800 Power Management IC


Might as well name this 88PM8xx from the start.



Good point.
Will include it in next version.

I am ready with V2, but luckily I did not sent next version, as I
wanted to wait for Lee's response. :)


+
+Required parent device properties:
+- compatible : "marvell,88pm800"


You might as well include 88pm805 and 88pm860 here since 805 support
is in the kernel and you will be submitting 860 support. They don't
have to be added with driver support.



Ok.


+- reg : the I2C slave address for the 88pm800 chip
+- interrupts : IRQ line for the 88pm800 chip
+- interrupt-controller: describes the 88pm800 as an interrupt controller
+- #interrupt-cells : should be 1.
+   - The cell is the 88pm800 local IRQ number
+
+Optional parent device properties:
+- marvell,88pm800-irq-write-clr: indicates whether interrupt status is
cleared by write



Drop the device name.  These bindings should be as generic as possible.



OK, how about simply

"mfd-irq_clr_on_write"


No, mfd is a Linux name which doesn't belong in bindings, and
underscores are generally not used.



Oops.

underscore is just typo. did not meant it.


I think Lee meant marvell,irq-write-clr or irq-write-clr.



will pick "marvell,irq-write-clr"



This could also just be dropped altogether and set based on the
compatible strings with match data.



It is configurable option, can be changed. so we still need get an
input.


Also describe what the absence of the property means.



Ok.


+88pm800 consists of a large and varied group of sub-devices:



3?



I have explicitly mentioned in note that more device list will follow.
I just wanted to add entries as and when we add/enable driver support.


+Device  Supply NamesDescription
+--  ---
+88pm80x-onkey  :   : On key
+88pm80x-rtc:   : RTC
+88pm80x:   : Regulators



Surely regulators is 88pm80x-regulator, no?



did not understand what change is expected here.


The node name for regulators node should be 88pm80x-regulator? But
these don't correspond to node names based on the example and the
example is correct IMO. So these are Linux driver names? If so, they
don't belong in the binding doc. What you need is to document the
sub-node compatible strings


Hmmm,
Got it.

rtc and onkey are compatible strings only, I made mistake on regulator.
I will change to "88pm80x-regulator".


Thanks,
Vaibhav



--
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 1/4] mfd: 88pm800: add device tree support

2015-06-16 Thread Rob Herring
On Tue, Jun 16, 2015 at 2:52 AM, Vaibhav Hiremath
 wrote:
>
>
> On Monday 01 June 2015 02:08 PM, Lee Jones wrote:
>>
>> On Sat, 30 May 2015, Vaibhav Hiremath wrote:
>>
>
> Thanks for your review. and sorry for delayed response.
>
>>> Add DT support to the 88pm800 driver along with below properties
>>> - marvell,88pm800-irq-write-clear :
>>>   Idicates whether interrupt status is cleared by write
>>>
>>> Also, creates the DT binding text file,
>>> Documentation/devicetree/bindings/mfd/88pm800.txt
>>>
>>> Signed-off-by: Chao Xie 
>>> Signed-off-by: Vaibhav Hiremath 
>>> ---
>>>   Documentation/devicetree/bindings/mfd/88pm800.txt | 57
>>> +++
>>>   drivers/mfd/88pm800.c | 39 
>>
>>
>> These should be submitted separately.
>>
>
>
> Ok, will separate it in next version.
>
>
>>>   2 files changed, 96 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt
>>> b/Documentation/devicetree/bindings/mfd/88pm800.txt
>>> new file mode 100644
>>> index 000..094951b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
>>> @@ -0,0 +1,57 @@
>>> +* Marvell 88PM800 Power Management IC

Might as well name this 88PM8xx from the start.

>>> +
>>> +Required parent device properties:
>>> +- compatible : "marvell,88pm800"

You might as well include 88pm805 and 88pm860 here since 805 support
is in the kernel and you will be submitting 860 support. They don't
have to be added with driver support.

>>> +- reg : the I2C slave address for the 88pm800 chip
>>> +- interrupts : IRQ line for the 88pm800 chip
>>> +- interrupt-controller: describes the 88pm800 as an interrupt controller
>>> +- #interrupt-cells : should be 1.
>>> +   - The cell is the 88pm800 local IRQ number
>>> +
>>> +Optional parent device properties:
>>> +- marvell,88pm800-irq-write-clr: indicates whether interrupt status is
>>> cleared by write
>>
>>
>> Drop the device name.  These bindings should be as generic as possible.
>>
>
> OK, how about simply
>
> "mfd-irq_clr_on_write"

No, mfd is a Linux name which doesn't belong in bindings, and
underscores are generally not used.

I think Lee meant marvell,irq-write-clr or irq-write-clr.

This could also just be dropped altogether and set based on the
compatible strings with match data.

>> Also describe what the absence of the property means.
>>
>
> Ok.
>
>>> +88pm800 consists of a large and varied group of sub-devices:
>>
>>
>> 3?
>>
>
> I have explicitly mentioned in note that more device list will follow.
> I just wanted to add entries as and when we add/enable driver support.
>
>>> +Device  Supply NamesDescription
>>> +--  ---
>>> +88pm80x-onkey  :   : On key
>>> +88pm80x-rtc:   : RTC
>>> +88pm80x:   : Regulators
>>
>>
>> Surely regulators is 88pm80x-regulator, no?
>>
>
> did not understand what change is expected here.

The node name for regulators node should be 88pm80x-regulator? But
these don't correspond to node names based on the example and the
example is correct IMO. So these are Linux driver names? If so, they
don't belong in the binding doc. What you need is to document the
sub-node compatible strings

Rob
--
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 1/4] mfd: 88pm800: add device tree support

2015-06-16 Thread Vaibhav Hiremath



On Monday 01 June 2015 02:08 PM, Lee Jones wrote:

On Sat, 30 May 2015, Vaibhav Hiremath wrote:



Thanks for your review. and sorry for delayed response.


Add DT support to the 88pm800 driver along with below properties
- marvell,88pm800-irq-write-clear :
  Idicates whether interrupt status is cleared by write

Also, creates the DT binding text file,
Documentation/devicetree/bindings/mfd/88pm800.txt

Signed-off-by: Chao Xie 
Signed-off-by: Vaibhav Hiremath 
---
  Documentation/devicetree/bindings/mfd/88pm800.txt | 57 +++
  drivers/mfd/88pm800.c | 39 


These should be submitted separately.




Ok, will separate it in next version.



  2 files changed, 96 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt 
b/Documentation/devicetree/bindings/mfd/88pm800.txt
new file mode 100644
index 000..094951b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -0,0 +1,57 @@
+* Marvell 88PM800 Power Management IC
+
+Required parent device properties:
+- compatible : "marvell,88pm800"
+- reg : the I2C slave address for the 88pm800 chip
+- interrupts : IRQ line for the 88pm800 chip
+- interrupt-controller: describes the 88pm800 as an interrupt controller
+- #interrupt-cells : should be 1.
+   - The cell is the 88pm800 local IRQ number
+
+Optional parent device properties:
+- marvell,88pm800-irq-write-clr: indicates whether interrupt status is cleared 
by write


Drop the device name.  These bindings should be as generic as possible.



OK, how about simply

"mfd-irq_clr_on_write"


Also describe what the absence of the property means.



Ok.


+88pm800 consists of a large and varied group of sub-devices:


3?



I have explicitly mentioned in note that more device list will follow.
I just wanted to add entries as and when we add/enable driver support.


+Device  Supply NamesDescription
+--  ---
+88pm80x-onkey  :   : On key
+88pm80x-rtc:   : RTC
+88pm80x:   : Regulators


Surely regulators is 88pm80x-regulator, no?



did not understand what change is expected here.


+Note: More device list will follow
+
+Example:
+
+   pmic: 88pm800@30 {
+   compatible = "marvell,88pm800";
+   reg = <0x30>;
+   interrupts = <0 77 0x4>;


Please use the #defines in include/dt-bindings/



Ok.


+   interrupt-parent = <>;
+   interrupt-controller;
+   #interrupt-cells = <1>;
+
+   marvell,88pm800-irq-write-clr;
+
+   regulators {
+   compatible = "marvell,88pm80x-regulator";
+
+   buck1a: BUCK1A {
+   regulator-compatible = "88PM800-BUCK1A";
+   regulator-min-microvolt = <60>;
+   regulator-max-microvolt = <180>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   ldo1: LDO1 {
+   regulator-compatible = "88PM800-LDO1";
+   regulator-min-microvolt = <170>;
+   regulator-max-microvolt = <330>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   };


'\n' here.


+   rtc {
+   compatible = "marvell,88pm80x-rtc";
+   };
+   };
diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 841717a..06ee058 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -27,6 +27,7 @@
  #include 
  #include 
  #include 
+#include 

  /* Interrupt Registers */
  #define PM800_INT_STATUS1 (0x05)
@@ -121,6 +122,11 @@ static const struct i2c_device_id pm80x_id_table[] = {
  };
  MODULE_DEVICE_TABLE(i2c, pm80x_id_table);

+static const struct of_device_id pm80x_of_match_table[] = {
+   { .compatible = "marvell,88pm800", },
+   {},
+};
+
  static struct resource rtc_resources[] = {
{
 .name = "88pm80x-rtc",
@@ -133,6 +139,7 @@ static struct resource rtc_resources[] = {
  static struct mfd_cell rtc_devs[] = {
{
 .name = "88pm80x-rtc",
+.of_compatible = "marvell,88pm80x-rtc",
 .num_resources = ARRAY_SIZE(rtc_resources),
 .resources = _resources[0],
 .id = -1,
@@ -151,6 +158,7 @@ static struct resource onkey_resources[] = {
  static const struct mfd_cell onkey_devs[] = {
{
 .name = "88pm80x-onkey",
+.of_compatible = "marvell,88pm80x-onkey",
 .num_resources = 1,
 .resources = _resources[0],
 .id = -1,
@@ 

Re: [PATCH 1/4] mfd: 88pm800: add device tree support

2015-06-16 Thread Rob Herring
On Tue, Jun 16, 2015 at 2:52 AM, Vaibhav Hiremath
vaibhav.hirem...@linaro.org wrote:


 On Monday 01 June 2015 02:08 PM, Lee Jones wrote:

 On Sat, 30 May 2015, Vaibhav Hiremath wrote:


 Thanks for your review. and sorry for delayed response.

 Add DT support to the 88pm800 driver along with below properties
 - marvell,88pm800-irq-write-clear :
   Idicates whether interrupt status is cleared by write

 Also, creates the DT binding text file,
 Documentation/devicetree/bindings/mfd/88pm800.txt

 Signed-off-by: Chao Xie chao@marvell.com
 Signed-off-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org
 ---
   Documentation/devicetree/bindings/mfd/88pm800.txt | 57
 +++
   drivers/mfd/88pm800.c | 39 


 These should be submitted separately.



 Ok, will separate it in next version.


   2 files changed, 96 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

 diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt
 b/Documentation/devicetree/bindings/mfd/88pm800.txt
 new file mode 100644
 index 000..094951b
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
 @@ -0,0 +1,57 @@
 +* Marvell 88PM800 Power Management IC

Might as well name this 88PM8xx from the start.

 +
 +Required parent device properties:
 +- compatible : marvell,88pm800

You might as well include 88pm805 and 88pm860 here since 805 support
is in the kernel and you will be submitting 860 support. They don't
have to be added with driver support.

 +- reg : the I2C slave address for the 88pm800 chip
 +- interrupts : IRQ line for the 88pm800 chip
 +- interrupt-controller: describes the 88pm800 as an interrupt controller
 +- #interrupt-cells : should be 1.
 +   - The cell is the 88pm800 local IRQ number
 +
 +Optional parent device properties:
 +- marvell,88pm800-irq-write-clr: indicates whether interrupt status is
 cleared by write


 Drop the device name.  These bindings should be as generic as possible.


 OK, how about simply

 mfd-irq_clr_on_write

No, mfd is a Linux name which doesn't belong in bindings, and
underscores are generally not used.

I think Lee meant marvell,irq-write-clr or irq-write-clr.

This could also just be dropped altogether and set based on the
compatible strings with match data.

 Also describe what the absence of the property means.


 Ok.

 +88pm800 consists of a large and varied group of sub-devices:


 3?


 I have explicitly mentioned in note that more device list will follow.
 I just wanted to add entries as and when we add/enable driver support.

 +Device  Supply NamesDescription
 +--  ---
 +88pm80x-onkey  :   : On key
 +88pm80x-rtc:   : RTC
 +88pm80x:   : Regulators


 Surely regulators is 88pm80x-regulator, no?


 did not understand what change is expected here.

The node name for regulators node should be 88pm80x-regulator? But
these don't correspond to node names based on the example and the
example is correct IMO. So these are Linux driver names? If so, they
don't belong in the binding doc. What you need is to document the
sub-node compatible strings

Rob
--
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 1/4] mfd: 88pm800: add device tree support

2015-06-16 Thread Vaibhav Hiremath



On Tuesday 16 June 2015 06:32 PM, Rob Herring wrote:

On Tue, Jun 16, 2015 at 2:52 AM, Vaibhav Hiremath
vaibhav.hirem...@linaro.org wrote:



On Monday 01 June 2015 02:08 PM, Lee Jones wrote:


On Sat, 30 May 2015, Vaibhav Hiremath wrote:



Thanks for your review. and sorry for delayed response.


Add DT support to the 88pm800 driver along with below properties
 - marvell,88pm800-irq-write-clear :
   Idicates whether interrupt status is cleared by write

Also, creates the DT binding text file,
Documentation/devicetree/bindings/mfd/88pm800.txt

Signed-off-by: Chao Xie chao@marvell.com
Signed-off-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org
---
   Documentation/devicetree/bindings/mfd/88pm800.txt | 57
+++
   drivers/mfd/88pm800.c | 39 



These should be submitted separately.




Ok, will separate it in next version.



   2 files changed, 96 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt
b/Documentation/devicetree/bindings/mfd/88pm800.txt
new file mode 100644
index 000..094951b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -0,0 +1,57 @@
+* Marvell 88PM800 Power Management IC


Might as well name this 88PM8xx from the start.



Good point.
Will include it in next version.

I am ready with V2, but luckily I did not sent next version, as I
wanted to wait for Lee's response. :)


+
+Required parent device properties:
+- compatible : marvell,88pm800


You might as well include 88pm805 and 88pm860 here since 805 support
is in the kernel and you will be submitting 860 support. They don't
have to be added with driver support.



Ok.


+- reg : the I2C slave address for the 88pm800 chip
+- interrupts : IRQ line for the 88pm800 chip
+- interrupt-controller: describes the 88pm800 as an interrupt controller
+- #interrupt-cells : should be 1.
+   - The cell is the 88pm800 local IRQ number
+
+Optional parent device properties:
+- marvell,88pm800-irq-write-clr: indicates whether interrupt status is
cleared by write



Drop the device name.  These bindings should be as generic as possible.



OK, how about simply

mfd-irq_clr_on_write


No, mfd is a Linux name which doesn't belong in bindings, and
underscores are generally not used.



Oops.

underscore is just typo. did not meant it.


I think Lee meant marvell,irq-write-clr or irq-write-clr.



will pick marvell,irq-write-clr



This could also just be dropped altogether and set based on the
compatible strings with match data.



It is configurable option, can be changed. so we still need get an
input.


Also describe what the absence of the property means.



Ok.


+88pm800 consists of a large and varied group of sub-devices:



3?



I have explicitly mentioned in note that more device list will follow.
I just wanted to add entries as and when we add/enable driver support.


+Device  Supply NamesDescription
+--  ---
+88pm80x-onkey  :   : On key
+88pm80x-rtc:   : RTC
+88pm80x:   : Regulators



Surely regulators is 88pm80x-regulator, no?



did not understand what change is expected here.


The node name for regulators node should be 88pm80x-regulator? But
these don't correspond to node names based on the example and the
example is correct IMO. So these are Linux driver names? If so, they
don't belong in the binding doc. What you need is to document the
sub-node compatible strings


Hmmm,
Got it.

rtc and onkey are compatible strings only, I made mistake on regulator.
I will change to 88pm80x-regulator.


Thanks,
Vaibhav



--
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 1/4] mfd: 88pm800: add device tree support

2015-06-16 Thread Vaibhav Hiremath



On Monday 01 June 2015 02:08 PM, Lee Jones wrote:

On Sat, 30 May 2015, Vaibhav Hiremath wrote:



Thanks for your review. and sorry for delayed response.


Add DT support to the 88pm800 driver along with below properties
- marvell,88pm800-irq-write-clear :
  Idicates whether interrupt status is cleared by write

Also, creates the DT binding text file,
Documentation/devicetree/bindings/mfd/88pm800.txt

Signed-off-by: Chao Xie chao@marvell.com
Signed-off-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org
---
  Documentation/devicetree/bindings/mfd/88pm800.txt | 57 +++
  drivers/mfd/88pm800.c | 39 


These should be submitted separately.




Ok, will separate it in next version.



  2 files changed, 96 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt 
b/Documentation/devicetree/bindings/mfd/88pm800.txt
new file mode 100644
index 000..094951b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -0,0 +1,57 @@
+* Marvell 88PM800 Power Management IC
+
+Required parent device properties:
+- compatible : marvell,88pm800
+- reg : the I2C slave address for the 88pm800 chip
+- interrupts : IRQ line for the 88pm800 chip
+- interrupt-controller: describes the 88pm800 as an interrupt controller
+- #interrupt-cells : should be 1.
+   - The cell is the 88pm800 local IRQ number
+
+Optional parent device properties:
+- marvell,88pm800-irq-write-clr: indicates whether interrupt status is cleared 
by write


Drop the device name.  These bindings should be as generic as possible.



OK, how about simply

mfd-irq_clr_on_write


Also describe what the absence of the property means.



Ok.


+88pm800 consists of a large and varied group of sub-devices:


3?



I have explicitly mentioned in note that more device list will follow.
I just wanted to add entries as and when we add/enable driver support.


+Device  Supply NamesDescription
+--  ---
+88pm80x-onkey  :   : On key
+88pm80x-rtc:   : RTC
+88pm80x:   : Regulators


Surely regulators is 88pm80x-regulator, no?



did not understand what change is expected here.


+Note: More device list will follow
+
+Example:
+
+   pmic: 88pm800@30 {
+   compatible = marvell,88pm800;
+   reg = 0x30;
+   interrupts = 0 77 0x4;


Please use the #defines in include/dt-bindings/



Ok.


+   interrupt-parent = gic;
+   interrupt-controller;
+   #interrupt-cells = 1;
+
+   marvell,88pm800-irq-write-clr;
+
+   regulators {
+   compatible = marvell,88pm80x-regulator;
+
+   buck1a: BUCK1A {
+   regulator-compatible = 88PM800-BUCK1A;
+   regulator-min-microvolt = 60;
+   regulator-max-microvolt = 180;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   ldo1: LDO1 {
+   regulator-compatible = 88PM800-LDO1;
+   regulator-min-microvolt = 170;
+   regulator-max-microvolt = 330;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   };


'\n' here.


+   rtc {
+   compatible = marvell,88pm80x-rtc;
+   };
+   };
diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 841717a..06ee058 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -27,6 +27,7 @@
  #include linux/mfd/core.h
  #include linux/mfd/88pm80x.h
  #include linux/slab.h
+#include linux/of_device.h

  /* Interrupt Registers */
  #define PM800_INT_STATUS1 (0x05)
@@ -121,6 +122,11 @@ static const struct i2c_device_id pm80x_id_table[] = {
  };
  MODULE_DEVICE_TABLE(i2c, pm80x_id_table);

+static const struct of_device_id pm80x_of_match_table[] = {
+   { .compatible = marvell,88pm800, },
+   {},
+};
+
  static struct resource rtc_resources[] = {
{
 .name = 88pm80x-rtc,
@@ -133,6 +139,7 @@ static struct resource rtc_resources[] = {
  static struct mfd_cell rtc_devs[] = {
{
 .name = 88pm80x-rtc,
+.of_compatible = marvell,88pm80x-rtc,
 .num_resources = ARRAY_SIZE(rtc_resources),
 .resources = rtc_resources[0],
 .id = -1,
@@ -151,6 +158,7 @@ static struct resource onkey_resources[] = {
  static const struct mfd_cell onkey_devs[] = {
{
 .name = 88pm80x-onkey,
+.of_compatible = marvell,88pm80x-onkey,
 

Re: [PATCH 1/4] mfd: 88pm800: add device tree support

2015-06-01 Thread Lee Jones
On Sat, 30 May 2015, Vaibhav Hiremath wrote:

> Add DT support to the 88pm800 driver along with below properties
>   - marvell,88pm800-irq-write-clear :
> Idicates whether interrupt status is cleared by write
> 
> Also, creates the DT binding text file,
> Documentation/devicetree/bindings/mfd/88pm800.txt
> 
> Signed-off-by: Chao Xie 
> Signed-off-by: Vaibhav Hiremath 
> ---
>  Documentation/devicetree/bindings/mfd/88pm800.txt | 57 
> +++
>  drivers/mfd/88pm800.c | 39 

These should be submitted separately.

>  2 files changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt 
> b/Documentation/devicetree/bindings/mfd/88pm800.txt
> new file mode 100644
> index 000..094951b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
> @@ -0,0 +1,57 @@
> +* Marvell 88PM800 Power Management IC
> +
> +Required parent device properties:
> +- compatible : "marvell,88pm800"
> +- reg : the I2C slave address for the 88pm800 chip
> +- interrupts : IRQ line for the 88pm800 chip
> +- interrupt-controller: describes the 88pm800 as an interrupt controller
> +- #interrupt-cells : should be 1.
> + - The cell is the 88pm800 local IRQ number
> +
> +Optional parent device properties:
> +- marvell,88pm800-irq-write-clr: indicates whether interrupt status is 
> cleared by write

Drop the device name.  These bindings should be as generic as possible.

Also describe what the absence of the property means.

> +88pm800 consists of a large and varied group of sub-devices:

3?

> +DeviceSupply NamesDescription
> +-----
> +88pm80x-onkey:   : On key
> +88pm80x-rtc  :   : RTC
> +88pm80x  :   : Regulators

Surely regulators is 88pm80x-regulator, no?

> +Note: More device list will follow
> +
> +Example:
> +
> + pmic: 88pm800@30 {
> + compatible = "marvell,88pm800";
> + reg = <0x30>;
> + interrupts = <0 77 0x4>;

Please use the #defines in include/dt-bindings/

> + interrupt-parent = <>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + marvell,88pm800-irq-write-clr;
> +
> + regulators {
> + compatible = "marvell,88pm80x-regulator";
> +
> + buck1a: BUCK1A {
> + regulator-compatible = "88PM800-BUCK1A";
> + regulator-min-microvolt = <60>;
> + regulator-max-microvolt = <180>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> + ldo1: LDO1 {
> + regulator-compatible = "88PM800-LDO1";
> + regulator-min-microvolt = <170>;
> + regulator-max-microvolt = <330>;
> + regulator-boot-on;
> + regulator-always-on;
> + };
> + };

'\n' here.

> + rtc {
> + compatible = "marvell,88pm80x-rtc";
> + };
> + };
> diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
> index 841717a..06ee058 100644
> --- a/drivers/mfd/88pm800.c
> +++ b/drivers/mfd/88pm800.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /* Interrupt Registers */
>  #define PM800_INT_STATUS1(0x05)
> @@ -121,6 +122,11 @@ static const struct i2c_device_id pm80x_id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
>  
> +static const struct of_device_id pm80x_of_match_table[] = {
> + { .compatible = "marvell,88pm800", },
> + {},
> +};
> +
>  static struct resource rtc_resources[] = {
>   {
>.name = "88pm80x-rtc",
> @@ -133,6 +139,7 @@ static struct resource rtc_resources[] = {
>  static struct mfd_cell rtc_devs[] = {
>   {
>.name = "88pm80x-rtc",
> +  .of_compatible = "marvell,88pm80x-rtc",
>.num_resources = ARRAY_SIZE(rtc_resources),
>.resources = _resources[0],
>.id = -1,
> @@ -151,6 +158,7 @@ static struct resource onkey_resources[] = {
>  static const struct mfd_cell onkey_devs[] = {
>   {
>.name = "88pm80x-onkey",
> +  .of_compatible = "marvell,88pm80x-onkey",
>.num_resources = 1,
>.resources = _resources[0],
>.id = -1,
> @@ -160,6 +168,7 @@ static const struct mfd_cell onkey_devs[] = {
>  static const struct mfd_cell regulator_devs[] = {
>   {
>.name = "88pm80x-regulator",
> +  .of_compatible = "marvell,88pm80x-regulator",
>.id = -1,
>   },
>  };
> @@ -538,14 +547,43 @@ out:
> 

Re: [PATCH 1/4] mfd: 88pm800: add device tree support

2015-06-01 Thread Lee Jones
On Sat, 30 May 2015, Vaibhav Hiremath wrote:

 Add DT support to the 88pm800 driver along with below properties
   - marvell,88pm800-irq-write-clear :
 Idicates whether interrupt status is cleared by write
 
 Also, creates the DT binding text file,
 Documentation/devicetree/bindings/mfd/88pm800.txt
 
 Signed-off-by: Chao Xie chao@marvell.com
 Signed-off-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org
 ---
  Documentation/devicetree/bindings/mfd/88pm800.txt | 57 
 +++
  drivers/mfd/88pm800.c | 39 

These should be submitted separately.

  2 files changed, 96 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt
 
 diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt 
 b/Documentation/devicetree/bindings/mfd/88pm800.txt
 new file mode 100644
 index 000..094951b
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
 @@ -0,0 +1,57 @@
 +* Marvell 88PM800 Power Management IC
 +
 +Required parent device properties:
 +- compatible : marvell,88pm800
 +- reg : the I2C slave address for the 88pm800 chip
 +- interrupts : IRQ line for the 88pm800 chip
 +- interrupt-controller: describes the 88pm800 as an interrupt controller
 +- #interrupt-cells : should be 1.
 + - The cell is the 88pm800 local IRQ number
 +
 +Optional parent device properties:
 +- marvell,88pm800-irq-write-clr: indicates whether interrupt status is 
 cleared by write

Drop the device name.  These bindings should be as generic as possible.

Also describe what the absence of the property means.

 +88pm800 consists of a large and varied group of sub-devices:

3?

 +DeviceSupply NamesDescription
 +-----
 +88pm80x-onkey:   : On key
 +88pm80x-rtc  :   : RTC
 +88pm80x  :   : Regulators

Surely regulators is 88pm80x-regulator, no?

 +Note: More device list will follow
 +
 +Example:
 +
 + pmic: 88pm800@30 {
 + compatible = marvell,88pm800;
 + reg = 0x30;
 + interrupts = 0 77 0x4;

Please use the #defines in include/dt-bindings/

 + interrupt-parent = gic;
 + interrupt-controller;
 + #interrupt-cells = 1;
 +
 + marvell,88pm800-irq-write-clr;
 +
 + regulators {
 + compatible = marvell,88pm80x-regulator;
 +
 + buck1a: BUCK1A {
 + regulator-compatible = 88PM800-BUCK1A;
 + regulator-min-microvolt = 60;
 + regulator-max-microvolt = 180;
 + regulator-boot-on;
 + regulator-always-on;
 + };
 + ldo1: LDO1 {
 + regulator-compatible = 88PM800-LDO1;
 + regulator-min-microvolt = 170;
 + regulator-max-microvolt = 330;
 + regulator-boot-on;
 + regulator-always-on;
 + };
 + };

'\n' here.

 + rtc {
 + compatible = marvell,88pm80x-rtc;
 + };
 + };
 diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
 index 841717a..06ee058 100644
 --- a/drivers/mfd/88pm800.c
 +++ b/drivers/mfd/88pm800.c
 @@ -27,6 +27,7 @@
  #include linux/mfd/core.h
  #include linux/mfd/88pm80x.h
  #include linux/slab.h
 +#include linux/of_device.h
  
  /* Interrupt Registers */
  #define PM800_INT_STATUS1(0x05)
 @@ -121,6 +122,11 @@ static const struct i2c_device_id pm80x_id_table[] = {
  };
  MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
  
 +static const struct of_device_id pm80x_of_match_table[] = {
 + { .compatible = marvell,88pm800, },
 + {},
 +};
 +
  static struct resource rtc_resources[] = {
   {
.name = 88pm80x-rtc,
 @@ -133,6 +139,7 @@ static struct resource rtc_resources[] = {
  static struct mfd_cell rtc_devs[] = {
   {
.name = 88pm80x-rtc,
 +  .of_compatible = marvell,88pm80x-rtc,
.num_resources = ARRAY_SIZE(rtc_resources),
.resources = rtc_resources[0],
.id = -1,
 @@ -151,6 +158,7 @@ static struct resource onkey_resources[] = {
  static const struct mfd_cell onkey_devs[] = {
   {
.name = 88pm80x-onkey,
 +  .of_compatible = marvell,88pm80x-onkey,
.num_resources = 1,
.resources = onkey_resources[0],
.id = -1,
 @@ -160,6 +168,7 @@ static const struct mfd_cell onkey_devs[] = {
  static const struct mfd_cell regulator_devs[] = {
   {
.name = 88pm80x-regulator,
 +  .of_compatible = marvell,88pm80x-regulator,
.id = -1,
   },
  };
 @@ -538,14 +547,43 @@ out:
   return ret;
  }
  
 +static int 

[PATCH 1/4] mfd: 88pm800: add device tree support

2015-05-29 Thread Vaibhav Hiremath
Add DT support to the 88pm800 driver along with below properties
- marvell,88pm800-irq-write-clear :
  Idicates whether interrupt status is cleared by write

Also, creates the DT binding text file,
Documentation/devicetree/bindings/mfd/88pm800.txt

Signed-off-by: Chao Xie 
Signed-off-by: Vaibhav Hiremath 
---
 Documentation/devicetree/bindings/mfd/88pm800.txt | 57 +++
 drivers/mfd/88pm800.c | 39 
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt 
b/Documentation/devicetree/bindings/mfd/88pm800.txt
new file mode 100644
index 000..094951b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -0,0 +1,57 @@
+* Marvell 88PM800 Power Management IC
+
+Required parent device properties:
+- compatible : "marvell,88pm800"
+- reg : the I2C slave address for the 88pm800 chip
+- interrupts : IRQ line for the 88pm800 chip
+- interrupt-controller: describes the 88pm800 as an interrupt controller
+- #interrupt-cells : should be 1.
+   - The cell is the 88pm800 local IRQ number
+
+Optional parent device properties:
+- marvell,88pm800-irq-write-clr: indicates whether interrupt status is cleared 
by write
+
+88pm800 consists of a large and varied group of sub-devices:
+
+Device  Supply NamesDescription
+--  ---
+88pm80x-onkey  :   : On key
+88pm80x-rtc:   : RTC
+88pm80x:   : Regulators
+
+Note: More device list will follow
+
+Example:
+
+   pmic: 88pm800@30 {
+   compatible = "marvell,88pm800";
+   reg = <0x30>;
+   interrupts = <0 77 0x4>;
+   interrupt-parent = <>;
+   interrupt-controller;
+   #interrupt-cells = <1>;
+
+   marvell,88pm800-irq-write-clr;
+
+   regulators {
+   compatible = "marvell,88pm80x-regulator";
+
+   buck1a: BUCK1A {
+   regulator-compatible = "88PM800-BUCK1A";
+   regulator-min-microvolt = <60>;
+   regulator-max-microvolt = <180>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   ldo1: LDO1 {
+   regulator-compatible = "88PM800-LDO1";
+   regulator-min-microvolt = <170>;
+   regulator-max-microvolt = <330>;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   };
+   rtc {
+   compatible = "marvell,88pm80x-rtc";
+   };
+   };
diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 841717a..06ee058 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* Interrupt Registers */
 #define PM800_INT_STATUS1  (0x05)
@@ -121,6 +122,11 @@ static const struct i2c_device_id pm80x_id_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
 
+static const struct of_device_id pm80x_of_match_table[] = {
+   { .compatible = "marvell,88pm800", },
+   {},
+};
+
 static struct resource rtc_resources[] = {
{
 .name = "88pm80x-rtc",
@@ -133,6 +139,7 @@ static struct resource rtc_resources[] = {
 static struct mfd_cell rtc_devs[] = {
{
 .name = "88pm80x-rtc",
+.of_compatible = "marvell,88pm80x-rtc",
 .num_resources = ARRAY_SIZE(rtc_resources),
 .resources = _resources[0],
 .id = -1,
@@ -151,6 +158,7 @@ static struct resource onkey_resources[] = {
 static const struct mfd_cell onkey_devs[] = {
{
 .name = "88pm80x-onkey",
+.of_compatible = "marvell,88pm80x-onkey",
 .num_resources = 1,
 .resources = _resources[0],
 .id = -1,
@@ -160,6 +168,7 @@ static const struct mfd_cell onkey_devs[] = {
 static const struct mfd_cell regulator_devs[] = {
{
 .name = "88pm80x-regulator",
+.of_compatible = "marvell,88pm80x-regulator",
 .id = -1,
},
 };
@@ -538,14 +547,43 @@ out:
return ret;
 }
 
+static int pm800_probe_dt(struct device_node *np,
+struct device *dev,
+struct pm80x_platform_data *pdata)
+{
+   pdata->irq_mode =
+   of_property_read_bool(np, "marvell,88pm800-irq-write-clear");
+
+   return 0;
+}
+
+
 static int pm800_probe(struct i2c_client *client,
 const struct i2c_device_id *id)
 {
int ret = 0;
struct pm80x_chip 

[PATCH 1/4] mfd: 88pm800: add device tree support

2015-05-29 Thread Vaibhav Hiremath
Add DT support to the 88pm800 driver along with below properties
- marvell,88pm800-irq-write-clear :
  Idicates whether interrupt status is cleared by write

Also, creates the DT binding text file,
Documentation/devicetree/bindings/mfd/88pm800.txt

Signed-off-by: Chao Xie chao@marvell.com
Signed-off-by: Vaibhav Hiremath vaibhav.hirem...@linaro.org
---
 Documentation/devicetree/bindings/mfd/88pm800.txt | 57 +++
 drivers/mfd/88pm800.c | 39 
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/88pm800.txt

diff --git a/Documentation/devicetree/bindings/mfd/88pm800.txt 
b/Documentation/devicetree/bindings/mfd/88pm800.txt
new file mode 100644
index 000..094951b
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/88pm800.txt
@@ -0,0 +1,57 @@
+* Marvell 88PM800 Power Management IC
+
+Required parent device properties:
+- compatible : marvell,88pm800
+- reg : the I2C slave address for the 88pm800 chip
+- interrupts : IRQ line for the 88pm800 chip
+- interrupt-controller: describes the 88pm800 as an interrupt controller
+- #interrupt-cells : should be 1.
+   - The cell is the 88pm800 local IRQ number
+
+Optional parent device properties:
+- marvell,88pm800-irq-write-clr: indicates whether interrupt status is cleared 
by write
+
+88pm800 consists of a large and varied group of sub-devices:
+
+Device  Supply NamesDescription
+--  ---
+88pm80x-onkey  :   : On key
+88pm80x-rtc:   : RTC
+88pm80x:   : Regulators
+
+Note: More device list will follow
+
+Example:
+
+   pmic: 88pm800@30 {
+   compatible = marvell,88pm800;
+   reg = 0x30;
+   interrupts = 0 77 0x4;
+   interrupt-parent = gic;
+   interrupt-controller;
+   #interrupt-cells = 1;
+
+   marvell,88pm800-irq-write-clr;
+
+   regulators {
+   compatible = marvell,88pm80x-regulator;
+
+   buck1a: BUCK1A {
+   regulator-compatible = 88PM800-BUCK1A;
+   regulator-min-microvolt = 60;
+   regulator-max-microvolt = 180;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   ldo1: LDO1 {
+   regulator-compatible = 88PM800-LDO1;
+   regulator-min-microvolt = 170;
+   regulator-max-microvolt = 330;
+   regulator-boot-on;
+   regulator-always-on;
+   };
+   };
+   rtc {
+   compatible = marvell,88pm80x-rtc;
+   };
+   };
diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
index 841717a..06ee058 100644
--- a/drivers/mfd/88pm800.c
+++ b/drivers/mfd/88pm800.c
@@ -27,6 +27,7 @@
 #include linux/mfd/core.h
 #include linux/mfd/88pm80x.h
 #include linux/slab.h
+#include linux/of_device.h
 
 /* Interrupt Registers */
 #define PM800_INT_STATUS1  (0x05)
@@ -121,6 +122,11 @@ static const struct i2c_device_id pm80x_id_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
 
+static const struct of_device_id pm80x_of_match_table[] = {
+   { .compatible = marvell,88pm800, },
+   {},
+};
+
 static struct resource rtc_resources[] = {
{
 .name = 88pm80x-rtc,
@@ -133,6 +139,7 @@ static struct resource rtc_resources[] = {
 static struct mfd_cell rtc_devs[] = {
{
 .name = 88pm80x-rtc,
+.of_compatible = marvell,88pm80x-rtc,
 .num_resources = ARRAY_SIZE(rtc_resources),
 .resources = rtc_resources[0],
 .id = -1,
@@ -151,6 +158,7 @@ static struct resource onkey_resources[] = {
 static const struct mfd_cell onkey_devs[] = {
{
 .name = 88pm80x-onkey,
+.of_compatible = marvell,88pm80x-onkey,
 .num_resources = 1,
 .resources = onkey_resources[0],
 .id = -1,
@@ -160,6 +168,7 @@ static const struct mfd_cell onkey_devs[] = {
 static const struct mfd_cell regulator_devs[] = {
{
 .name = 88pm80x-regulator,
+.of_compatible = marvell,88pm80x-regulator,
 .id = -1,
},
 };
@@ -538,14 +547,43 @@ out:
return ret;
 }
 
+static int pm800_probe_dt(struct device_node *np,
+struct device *dev,
+struct pm80x_platform_data *pdata)
+{
+   pdata-irq_mode =
+   of_property_read_bool(np, marvell,88pm800-irq-write-clear);
+
+   return 0;
+}
+
+
 static int pm800_probe(struct i2c_client *client,
 const