Re: [PATCH v6 4/6] gpio: davinci: add OF support

2013-11-29 Thread Sekhar Nori
On Friday 29 November 2013 01:16 PM, Linus Walleij wrote:
 On Tue, Nov 26, 2013 at 6:12 PM, Sekhar Nori nsek...@ti.com wrote:
 On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote:
 
 Actually, the same was proposed by Linus, but we've tried avoid such huge 
 rework -
 by switching to one irq_domain per all banks for example.

 I didn't really read that proposal from Linus so if two people
 independently suggested the same thing, there must be something worth
 considering there :)
 
 From a GPIO POV it's not such a big deal really, this approach is fine
 and the important thing is that we progress toward a more standard
 driver... it's more a question for the DT people IMO. I really like the
 current patch set.

Rob has acked v5 of this patch. So no objection from DT people too. My
concern was a bit futuristic. Since everyone is happy I think we can go
with the current patch itself.

Thanks,
Sekhar
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v6 4/6] gpio: davinci: add OF support

2013-11-28 Thread Linus Walleij
On Tue, Nov 26, 2013 at 6:12 PM, Sekhar Nori nsek...@ti.com wrote:
 On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote:

 Actually, the same was proposed by Linus, but we've tried avoid such huge 
 rework -
 by switching to one irq_domain per all banks for example.

 I didn't really read that proposal from Linus so if two people
 independently suggested the same thing, there must be something worth
 considering there :)

From a GPIO POV it's not such a big deal really, this approach is fine
and the important thing is that we progress toward a more standard
driver... it's more a question for the DT people IMO. I really like the
current patch set.

Yours,
Linus Walleij
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v6 4/6] gpio: davinci: add OF support

2013-11-26 Thread Prabhakar Lad
Hi Sekhar,

Thanks for the review.

On Mon, Nov 25, 2013 at 4:30 PM, Sekhar Nori nsek...@ti.com wrote:
 On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
 From: KV Sujith sujit...@ti.com

 This patch adds OF parser support for davinci gpio
 driver and also appropriate documentation in gpio-davinci.txt
 located at Documentation/devicetree/bindings/gpio/.

 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Rob Herring rob.herr...@calxeda.com
 Signed-off-by: KV Sujith sujit...@ti.com
 Signed-off-by: Philip Avinash avinashphi...@ti.com
 [prabhakar.cse...@gmail.com: simplified the OF code, removed
   unnecessary DT property and also simplified
   the commit message]
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  .../devicetree/bindings/gpio/gpio-davinci.txt  |   41 ++
  drivers/gpio/gpio-davinci.c|   57 
 ++--
  2 files changed, 95 insertions(+), 3 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt

 diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt 
 b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 new file mode 100644
 index 000..a2e839d
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 @@ -0,0 +1,41 @@
 +Davinci GPIO controller bindings
 +
 +Required Properties:
 +- compatible: should be ti,dm6441-gpio
 +
 +- reg: Physical base address of the controller and the size of memory mapped
 +   registers.
 +
 +- gpio-controller : Marks the device node as a gpio controller.
 +
 +- interrupt-parent: phandle of the parent interrupt controller.
 +
 +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs 
 are
 +   supported at a time.

 If this is true..

 +
 +- ti,ngpio: The number of GPIO pins supported.
 +
 +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual 
 interrupt
 +  line to processor.

 .. then why do you need to maintain this separately? Number of elements
 in interrupts property should give you this answer, no?

 There can certainly be devices (past and future) which use a mixture of
 banked and unbanked IRQs. So a binding which does not take care of this
 is likely to change in future and that is a problem since it brings in
 backward compatibility of the binding into picture.

 The right thing would be to define the DT node per-bank similar to what
 is done on OMAP rather than for all banks together. That way there can
 be a separate property which determines whether that bank supports
 direct-mapped or banked IRQs (or that could be inferred if the number of
 tuples in the interrupts property is more than one).

Can you point me to the OMAP implementation.

Regards,
--Prabhakar Lad
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v6 4/6] gpio: davinci: add OF support

2013-11-26 Thread Grygorii Strashko
On 11/25/2013 01:00 PM, Sekhar Nori wrote:
 On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
 From: KV Sujith sujit...@ti.com

 This patch adds OF parser support for davinci gpio
 driver and also appropriate documentation in gpio-davinci.txt
 located at Documentation/devicetree/bindings/gpio/.

 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Rob Herring rob.herr...@calxeda.com
 Signed-off-by: KV Sujith sujit...@ti.com
 Signed-off-by: Philip Avinash avinashphi...@ti.com
 [prabhakar.cse...@gmail.com: simplified the OF code, removed
  unnecessary DT property and also simplified
  the commit message]
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
   .../devicetree/bindings/gpio/gpio-davinci.txt  |   41 ++
   drivers/gpio/gpio-davinci.c|   57 
 ++--
   2 files changed, 95 insertions(+), 3 deletions(-)
   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt

 diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt 
 b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 new file mode 100644
 index 000..a2e839d
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 @@ -0,0 +1,41 @@
 +Davinci GPIO controller bindings
 +
 +Required Properties:
 +- compatible: should be ti,dm6441-gpio
 +
 +- reg: Physical base address of the controller and the size of memory mapped
 +   registers.
 +
 +- gpio-controller : Marks the device node as a gpio controller.
 +
 +- interrupt-parent: phandle of the parent interrupt controller.
 +
 +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs 
 are
 +  supported at a time.
 
 If this is true..
 
 +
 +- ti,ngpio: The number of GPIO pins supported.
 +
 +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual 
 interrupt
 + line to processor.
 
 .. then why do you need to maintain this separately? Number of elements
 in interrupts property should give you this answer, no?
 
 There can certainly be devices (past and future) which use a mixture of
 banked and unbanked IRQs. So a binding which does not take care of this
 is likely to change in future and that is a problem since it brings in
 backward compatibility of the binding into picture.
 
 The right thing would be to define the DT node per-bank similar to what
 is done on OMAP rather than for all banks together. That way there can
 be a separate property which determines whether that bank supports
 direct-mapped or banked IRQs (or that could be inferred if the number of
 tuples in the interrupts property is more than one).

Number of IRQ can't be simply used to determine type of IRQ - need to handle 
IRQ names,
because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO).

Few things here:
- The mixed banked/unbanked functionality has never been supported before. 
- The Davinci GPIO IP is different from OMAP and has common
  control registers for all banks.
- The proposed approach is more less easy to implement for DT case, but for 
not-DT
  case - the platform data will need to be changed significantly (.
  So, from this point of view, that would be a big change (actually the total 
driver rewriting).

Do you have any thoughts about how it can be done in a simpler way?

Actually, the same was proposed by Linus, but we've tried avoid such huge 
rework -
by switching to one irq_domain per all banks for example.

Regards,
- grygorii
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


Re: [PATCH v6 4/6] gpio: davinci: add OF support

2013-11-26 Thread Sekhar Nori
On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote:
 On 11/25/2013 01:00 PM, Sekhar Nori wrote:
 On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
 From: KV Sujith sujit...@ti.com

 This patch adds OF parser support for davinci gpio
 driver and also appropriate documentation in gpio-davinci.txt
 located at Documentation/devicetree/bindings/gpio/.

 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Rob Herring rob.herr...@calxeda.com
 Signed-off-by: KV Sujith sujit...@ti.com
 Signed-off-by: Philip Avinash avinashphi...@ti.com
 [prabhakar.cse...@gmail.com: simplified the OF code, removed
 unnecessary DT property and also simplified
 the commit message]
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
   .../devicetree/bindings/gpio/gpio-davinci.txt  |   41 ++
   drivers/gpio/gpio-davinci.c|   57 
 ++--
   2 files changed, 95 insertions(+), 3 deletions(-)
   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt

 diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt 
 b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 new file mode 100644
 index 000..a2e839d
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 @@ -0,0 +1,41 @@
 +Davinci GPIO controller bindings
 +
 +Required Properties:
 +- compatible: should be ti,dm6441-gpio
 +
 +- reg: Physical base address of the controller and the size of memory 
 mapped
 +   registers.
 +
 +- gpio-controller : Marks the device node as a gpio controller.
 +
 +- interrupt-parent: phandle of the parent interrupt controller.
 +
 +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs 
 are
 + supported at a time.

 If this is true..

 +
 +- ti,ngpio: The number of GPIO pins supported.
 +
 +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual 
 interrupt
 +line to processor.

 .. then why do you need to maintain this separately? Number of elements
 in interrupts property should give you this answer, no?

 There can certainly be devices (past and future) which use a mixture of
 banked and unbanked IRQs. So a binding which does not take care of this
 is likely to change in future and that is a problem since it brings in
 backward compatibility of the binding into picture.

 The right thing would be to define the DT node per-bank similar to what
 is done on OMAP rather than for all banks together. That way there can
 be a separate property which determines whether that bank supports
 direct-mapped or banked IRQs (or that could be inferred if the number of
 tuples in the interrupts property is more than one).
 
 Number of IRQ can't be simply used to determine type of IRQ - need to handle 
 IRQ names,
 because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO).

Okay. That's why I inserted that comment in parenthesis :)

 
 Few things here:
 - The mixed banked/unbanked functionality has never been supported before. 

True. I actually misread the driver before.

 - The Davinci GPIO IP is different from OMAP and has common
   control registers for all banks.

Well the only common register I can see is BINTEN - bank interrupt
enable. This register can simply be initialized to enable interrupts
from all banks possible as until the rising and falling edge triggers
are programmed, there wont be any actual interrupts generated.

 - The proposed approach is more less easy to implement for DT case, but for 
 not-DT
   case - the platform data will need to be changed significantly (.
   So, from this point of view, that would be a big change (actually the total 
 driver rewriting).

Well, I certainly don't think its a complete driver re-write. It will
take a bit of effort agreed, but I think the driver will also come out a
lot cleaner.

Honestly, I am not so much worried about the kernel code here. Its the
bindings I am worried about. Once the bindings go in assuming there will
never be banked and unbanked GPIO IRQs on the same SoC, changing them to
do something else will be very painful with the need to keep backward
compatibility and support for both semantics.

That said, because there is no present hardware which needs both banked
and unbanked at the same time, I wont press for this to be done endlessly.

 Do you have any thoughts about how it can be done in a simpler way?

I don't know if there is a simpler way, but I don't think there is too
much effort too. I leave it to those implementing it though.

 
 Actually, the same was proposed by Linus, but we've tried avoid such huge 
 rework -
 by switching to one irq_domain per all banks for example.

I didn't really read that proposal from Linus so if two people
independently suggested the same thing, there must be something worth
considering there :)

Thanks,
Sekhar
___
Davinci-linux-open-source mailing 

Re: [PATCH v6 4/6] gpio: davinci: add OF support

2013-11-26 Thread Grygorii Strashko

On 11/26/2013 07:12 PM, Sekhar Nori wrote:

On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote:

On 11/25/2013 01:00 PM, Sekhar Nori wrote:

On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:

From: KV Sujith sujit...@ti.com

This patch adds OF parser support for davinci gpio
driver and also appropriate documentation in gpio-davinci.txt
located at Documentation/devicetree/bindings/gpio/.

Acked-by: Linus Walleij linus.wall...@linaro.org
Acked-by: Rob Herring rob.herr...@calxeda.com
Signed-off-by: KV Sujith sujit...@ti.com
Signed-off-by: Philip Avinash avinashphi...@ti.com
[prabhakar.cse...@gmail.com: simplified the OF code, removed
unnecessary DT property and also simplified
the commit message]
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
   .../devicetree/bindings/gpio/gpio-davinci.txt  |   41 ++
   drivers/gpio/gpio-davinci.c|   57 
++--
   2 files changed, 95 insertions(+), 3 deletions(-)
   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt 
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
new file mode 100644
index 000..a2e839d
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -0,0 +1,41 @@
+Davinci GPIO controller bindings
+
+Required Properties:
+- compatible: should be ti,dm6441-gpio
+
+- reg: Physical base address of the controller and the size of memory mapped
+   registers.
+
+- gpio-controller : Marks the device node as a gpio controller.
+
+- interrupt-parent: phandle of the parent interrupt controller.
+
+- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are
+ supported at a time.


If this is true..


+
+- ti,ngpio: The number of GPIO pins supported.
+
+- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual 
interrupt
+line to processor.


.. then why do you need to maintain this separately? Number of elements
in interrupts property should give you this answer, no?

There can certainly be devices (past and future) which use a mixture of
banked and unbanked IRQs. So a binding which does not take care of this
is likely to change in future and that is a problem since it brings in
backward compatibility of the binding into picture.

The right thing would be to define the DT node per-bank similar to what
is done on OMAP rather than for all banks together. That way there can
be a separate property which determines whether that bank supports
direct-mapped or banked IRQs (or that could be inferred if the number of
tuples in the interrupts property is more than one).


Number of IRQ can't be simply used to determine type of IRQ - need to handle 
IRQ names,
because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO).


Okay. That's why I inserted that comment in parenthesis :)



Few things here:
- The mixed banked/unbanked functionality has never been supported before.


True. I actually misread the driver before.


- The Davinci GPIO IP is different from OMAP and has common
   control registers for all banks.


Well the only common register I can see is BINTEN - bank interrupt
enable. This register can simply be initialized to enable interrupts
from all banks possible as until the rising and falling edge triggers
are programmed, there wont be any actual interrupts generated.


- The proposed approach is more less easy to implement for DT case, but for 
not-DT
   case - the platform data will need to be changed significantly (.
   So, from this point of view, that would be a big change (actually the total 
driver rewriting).


Well, I certainly don't think its a complete driver re-write. It will
take a bit of effort agreed, but I think the driver will also come out a
lot cleaner.

Honestly, I am not so much worried about the kernel code here. Its the
bindings I am worried about. Once the bindings go in assuming there will
never be banked and unbanked GPIO IRQs on the same SoC, changing them to
do something else will be very painful with the need to keep backward
compatibility and support for both semantics.

That said, because there is no present hardware which needs both banked
and unbanked at the same time, I wont press for this to be done endlessly.


Do you have any thoughts about how it can be done in a simpler way?


I don't know if there is a simpler way, but I don't think there is too
much effort too. I leave it to those implementing it though.


Oh. I see no problem to implement it for DT, but this change require to 
convert one device to the tree of devices:

 GPIO controller
 |- GPIO bank1
...
 |- GPIO bankX

And that's will need to be handled somehow from platform code (which is 
non-DT and which I can't verify and which I don't want to touch actually ;).






Actually, the same was proposed by Linus, but we've tried avoid such 

Re: [PATCH v6 4/6] gpio: davinci: add OF support

2013-11-26 Thread Sekhar Nori
On Wednesday 27 November 2013 01:11 AM, Grygorii Strashko wrote:
 On 11/26/2013 07:12 PM, Sekhar Nori wrote:
 On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote:
 On 11/25/2013 01:00 PM, Sekhar Nori wrote:
 On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
 From: KV Sujith sujit...@ti.com

 This patch adds OF parser support for davinci gpio
 driver and also appropriate documentation in gpio-davinci.txt
 located at Documentation/devicetree/bindings/gpio/.

 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Rob Herring rob.herr...@calxeda.com
 Signed-off-by: KV Sujith sujit...@ti.com
 Signed-off-by: Philip Avinash avinashphi...@ti.com
 [prabhakar.cse...@gmail.com: simplified the OF code, removed
 unnecessary DT property and also simplified
 the commit message]
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
.../devicetree/bindings/gpio/gpio-davinci.txt  |   41
 ++
drivers/gpio/gpio-davinci.c|   57
 ++--
2 files changed, 95 insertions(+), 3 deletions(-)
create mode 100644
 Documentation/devicetree/bindings/gpio/gpio-davinci.txt

 diff --git
 a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 new file mode 100644
 index 000..a2e839d
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 @@ -0,0 +1,41 @@
 +Davinci GPIO controller bindings
 +
 +Required Properties:
 +- compatible: should be ti,dm6441-gpio
 +
 +- reg: Physical base address of the controller and the size of
 memory mapped
 +   registers.
 +
 +- gpio-controller : Marks the device node as a gpio controller.
 +
 +- interrupt-parent: phandle of the parent interrupt controller.
 +
 +- interrupts: Array of GPIO interrupt number. Only banked or
 unbanked IRQs are
 +  supported at a time.

 If this is true..

 +
 +- ti,ngpio: The number of GPIO pins supported.
 +
 +- ti,davinci-gpio-unbanked: The number of GPIOs that have an
 individual interrupt
 + line to processor.

 .. then why do you need to maintain this separately? Number of elements
 in interrupts property should give you this answer, no?

 There can certainly be devices (past and future) which use a mixture of
 banked and unbanked IRQs. So a binding which does not take care of this
 is likely to change in future and that is a problem since it brings in
 backward compatibility of the binding into picture.

 The right thing would be to define the DT node per-bank similar to what
 is done on OMAP rather than for all banks together. That way there can
 be a separate property which determines whether that bank supports
 direct-mapped or banked IRQs (or that could be inferred if the
 number of
 tuples in the interrupts property is more than one).

 Number of IRQ can't be simply used to determine type of IRQ - need to
 handle IRQ names,
 because each bank(32 gpios) may have up to 2 banked IRQs (one per 16
 GPIO).

 Okay. That's why I inserted that comment in parenthesis :)


 Few things here:
 - The mixed banked/unbanked functionality has never been supported
 before.

 True. I actually misread the driver before.

 - The Davinci GPIO IP is different from OMAP and has common
control registers for all banks.

 Well the only common register I can see is BINTEN - bank interrupt
 enable. This register can simply be initialized to enable interrupts
 from all banks possible as until the rising and falling edge triggers
 are programmed, there wont be any actual interrupts generated.

 - The proposed approach is more less easy to implement for DT case,
 but for not-DT
case - the platform data will need to be changed significantly (.
So, from this point of view, that would be a big change (actually
 the total driver rewriting).

 Well, I certainly don't think its a complete driver re-write. It will
 take a bit of effort agreed, but I think the driver will also come out a
 lot cleaner.

 Honestly, I am not so much worried about the kernel code here. Its the
 bindings I am worried about. Once the bindings go in assuming there will
 never be banked and unbanked GPIO IRQs on the same SoC, changing them to
 do something else will be very painful with the need to keep backward
 compatibility and support for both semantics.

 That said, because there is no present hardware which needs both banked
 and unbanked at the same time, I wont press for this to be done
 endlessly.

 Do you have any thoughts about how it can be done in a simpler way?

 I don't know if there is a simpler way, but I don't think there is too
 much effort too. I leave it to those implementing it though.
 
 Oh. I see no problem to implement it for DT, but this change require to
 convert one device to the tree of devices:
  GPIO controller
  |- GPIO bank1
 ...
  |- GPIO bankX
 
 And that's will need to be handled somehow from platform code (which is
 non-DT and which I can't verify 

Re: [PATCH v6 4/6] gpio: davinci: add OF support

2013-11-25 Thread Sekhar Nori
On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
 From: KV Sujith sujit...@ti.com
 
 This patch adds OF parser support for davinci gpio
 driver and also appropriate documentation in gpio-davinci.txt
 located at Documentation/devicetree/bindings/gpio/.
 
 Acked-by: Linus Walleij linus.wall...@linaro.org
 Acked-by: Rob Herring rob.herr...@calxeda.com
 Signed-off-by: KV Sujith sujit...@ti.com
 Signed-off-by: Philip Avinash avinashphi...@ti.com
 [prabhakar.cse...@gmail.com: simplified the OF code, removed
   unnecessary DT property and also simplified
   the commit message]
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  .../devicetree/bindings/gpio/gpio-davinci.txt  |   41 ++
  drivers/gpio/gpio-davinci.c|   57 
 ++--
  2 files changed, 95 insertions(+), 3 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 
 diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt 
 b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 new file mode 100644
 index 000..a2e839d
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 @@ -0,0 +1,41 @@
 +Davinci GPIO controller bindings
 +
 +Required Properties:
 +- compatible: should be ti,dm6441-gpio
 +
 +- reg: Physical base address of the controller and the size of memory mapped
 +   registers.
 +
 +- gpio-controller : Marks the device node as a gpio controller.
 +
 +- interrupt-parent: phandle of the parent interrupt controller.
 +
 +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs 
 are
 +   supported at a time.

If this is true..

 +
 +- ti,ngpio: The number of GPIO pins supported.
 +
 +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual 
 interrupt
 +  line to processor.

.. then why do you need to maintain this separately? Number of elements
in interrupts property should give you this answer, no?

There can certainly be devices (past and future) which use a mixture of
banked and unbanked IRQs. So a binding which does not take care of this
is likely to change in future and that is a problem since it brings in
backward compatibility of the binding into picture.

The right thing would be to define the DT node per-bank similar to what
is done on OMAP rather than for all banks together. That way there can
be a separate property which determines whether that bank supports
direct-mapped or banked IRQs (or that could be inferred if the number of
tuples in the interrupts property is more than one).

Thanks,
Sekhar

___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


[PATCH v6 4/6] gpio: davinci: add OF support

2013-11-21 Thread Prabhakar Lad
From: KV Sujith sujit...@ti.com

This patch adds OF parser support for davinci gpio
driver and also appropriate documentation in gpio-davinci.txt
located at Documentation/devicetree/bindings/gpio/.

Acked-by: Linus Walleij linus.wall...@linaro.org
Acked-by: Rob Herring rob.herr...@calxeda.com
Signed-off-by: KV Sujith sujit...@ti.com
Signed-off-by: Philip Avinash avinashphi...@ti.com
[prabhakar.cse...@gmail.com: simplified the OF code, removed
unnecessary DT property and also simplified
the commit message]
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 .../devicetree/bindings/gpio/gpio-davinci.txt  |   41 ++
 drivers/gpio/gpio-davinci.c|   57 ++--
 2 files changed, 95 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt 
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
new file mode 100644
index 000..a2e839d
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -0,0 +1,41 @@
+Davinci GPIO controller bindings
+
+Required Properties:
+- compatible: should be ti,dm6441-gpio
+
+- reg: Physical base address of the controller and the size of memory mapped
+   registers.
+
+- gpio-controller : Marks the device node as a gpio controller.
+
+- interrupt-parent: phandle of the parent interrupt controller.
+
+- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are
+ supported at a time.
+
+- ti,ngpio: The number of GPIO pins supported.
+
+- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual 
interrupt
+line to processor.
+
+The GPIO controller also acts as an interrupt controller. It uses the default
+two cells specifier as described in Documentation/devicetree/bindings/
+interrupt-controller/interrupts.txt.
+
+Example:
+
+gpio: gpio@1e26000 {
+   compatible = ti,dm6441-gpio;
+   gpio-controller;
+   reg = 0x226000 0x1000;
+   interrupt-parent = intc;
+   interrupts = 42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
+   44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
+   46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
+   48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
+   50 IRQ_TYPE_EDGE_BOTH;
+   ti,ngpio = 144;
+   ti,davinci-gpio-unbanked = 0;
+   interrupt-controller;
+   #interrupt-cells = 2;
+};
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 4d6adbc..5bdd9f8 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -17,6 +17,9 @@
 #include linux/io.h
 #include linux/irq.h
 #include linux/irqdomain.h
+#include linux/module.h
+#include linux/of.h
+#include linux/of_device.h
 #include linux/platform_device.h
 #include linux/platform_data/gpio-davinci.h
 
@@ -134,6 +137,40 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset, 
int value)
writel((1  offset), value ? g-set_data : g-clr_data);
 }
 
+static struct davinci_gpio_platform_data *
+davinci_gpio_get_pdata(struct platform_device *pdev)
+{
+   struct device_node *dn = pdev-dev.of_node;
+   struct davinci_gpio_platform_data *pdata;
+   int ret;
+   u32 val;
+
+   if (!IS_ENABLED(CONFIG_OF) || !pdev-dev.of_node)
+   return pdev-dev.platform_data;
+
+   pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL);
+   if (!pdata)
+   return NULL;
+
+   ret = of_property_read_u32(dn, ti,ngpio, val);
+   if (ret)
+   goto of_err;
+
+   pdata-ngpio = val;
+
+   ret = of_property_read_u32(dn, ti,davinci-gpio-unbanked, val);
+   if (ret)
+   goto of_err;
+
+   pdata-gpio_unbanked = val;
+
+   return pdata;
+
+of_err:
+   dev_err(pdev-dev, Populating pdata from DT failed: err %d\n, ret);
+   return NULL;
+}
+
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
int i, base;
@@ -144,12 +181,14 @@ static int davinci_gpio_probe(struct platform_device 
*pdev)
struct device *dev = pdev-dev;
struct resource *res;
 
-   pdata = dev-platform_data;
+   pdata = davinci_gpio_get_pdata(pdev);
if (!pdata) {
dev_err(dev, No platform data found\n);
return -EINVAL;
}
 
+   dev-platform_data = pdata;
+
/*
 * The gpio banks conceptually expose a segmented bitmap,
 * and ngpio is one more than the largest zero-based
@@ -195,6 +234,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
if (chips[i].chip.ngpio  32)
chips[i].chip.ngpio = 32;
 
+#ifdef CONFIG_OF_GPIO
+   chips[i].chip.of_node = dev-of_node;
+#endif
spin_lock_init(chips[i].lock);
 
regs = gpio2regs(base);
@@ -506,11 +548,20 @@ done: