[PATCH v4 0/6] gpio: daVinci: Fixes and feature enhancement

[PATCH v4 0/6] gpio: daVinci: Fixes and feature enhancement

[Lad, Prabhakar]

From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>

Patches 1, 2 and 3 are newly added. This patch series does the following
a> Fixes check for offset for unbanked gpios.
b> Ports the driver to use irqdomain.
c> Adds dt binding support for gpio-davinci.
d> Adds DA850 dt support goio.

Changes for v4:
1: Added irqdomain support for the driver.
2: Dropped the ACK's.
3: Removed unnecessary DT property.

This patch series has been tested on DA850 for GPIO7_4
(ie switch 8 on SW7.). Patches can be found at [1].

[1] http://git.linuxtv.org/mhadli/v4l-dvb-davinci_devices.git/shortlog/refs/heads/davinci_gpio

KV Sujith (3):
  gpio: davinci: add OF support
  ARM: davinci: da850: add GPIO DT node
  ARM: davinci: da850 evm: add GPIO pinumux entries DT node

Lad, Prabhakar (3):
  gpio: davinci: Fixed a check for unbanked gpio
  gpio: davinci: remove unnecessary printk
  gpio: davinci: use irqdomain

 .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 ++++++
 arch/arm/boot/dts/da850-evm.dts                    |   20 ++++
 arch/arm/boot/dts/da850.dtsi                       |   15 +++
 arch/arm/mach-davinci/da830.c                      |    1 -
 arch/arm/mach-davinci/da850.c                      |    1 -
 arch/arm/mach-davinci/dm355.c                      |    1 -
 arch/arm/mach-davinci/dm365.c                      |    1 -
 arch/arm/mach-davinci/dm644x.c                     |    1 -
 arch/arm/mach-davinci/dm646x.c                     |    1 -
 drivers/gpio/gpio-davinci.c                        |  111 +++++++++++++++-----
 include/linux/platform_data/gpio-davinci.h         |    3 +-
 11 files changed, 153 insertions(+), 34 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt

-- 
1.7.9.5

[PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Lad, Prabhakar]

From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>

This patch fixes the check for the offset in
gpio_to_irq_unbanked() function.

Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
 drivers/gpio/gpio-davinci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 8847adf..6c90cfb 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -327,7 +327,7 @@ static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
 	 * NOTE:  we assume for now that only irqs in the first gpio_chip
 	 * can provide direct-mapped IRQs to AINTC (up to 32 GPIOs).
 	 */
-	if (offset < d->irq_base)
+	if (offset < d->gpio_unbanked)
 		return d->gpio_irq + offset;
 	else
 		return -ENODEV;
-- 
1.7.9.5

Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Linus Walleij]

On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:

> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
> This patch fixes the check for the offset in
> gpio_to_irq_unbanked() function.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>

Is this a regression that should go in right now?

Yours,
Linus Walleij

Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Prabhakar Lad]

Hi Linus,

On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
>
>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>
>> This patch fixes the check for the offset in
>> gpio_to_irq_unbanked() function.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>
> Is this a regression that should go in right now?
>
Yes it needs too.

Regards,
--Prabhakar Lad

Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Linus Walleij]

On Wed, Nov 6, 2013 at 10:33 AM, Prabhakar Lad
<prabhakar.csengg@gmail.com> wrote:
> On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar
>> <prabhakar.csengg@gmail.com> wrote:
>>
>>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>>
>>> This patch fixes the check for the offset in
>>> gpio_to_irq_unbanked() function.
>>>
>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>
>> Is this a regression that should go in right now?
>>
> Yes it needs too.

But on top of *what* exactly?

This does not apply to my gpio tree devel branch and
not even on the mainline kernel.

Is this something that should go on top of the davinci
GPIO patch set that is still being elaborated on?

Yours,
Linus Walleij

Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Prabhakar Lad]

Hi Linus,

On Wed, Nov 6, 2013 at 3:26 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Nov 6, 2013 at 10:33 AM, Prabhakar Lad
> <prabhakar.csengg@gmail.com> wrote:
>> On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar
>>> <prabhakar.csengg@gmail.com> wrote:
>>>
>>>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>>>
>>>> This patch fixes the check for the offset in
>>>> gpio_to_irq_unbanked() function.
>>>>
>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>
>>> Is this a regression that should go in right now?
>>>
>> Yes it needs too.
>
> But on top of *what* exactly?
>
> This does not apply to my gpio tree devel branch and
> not even on the mainline kernel.
>
Looks like this needs to go via ARM tree as the earlier
patches have  gone via ARM tree itself [1].
If you can ACK it Sekhar can get it in via ARM tree.

> Is this something that should go on top of the davinci
> GPIO patch set that is still being elaborated on?
>
Nope.

[1]  http://www.spinics.net/lists/arm-kernel/msg275267.html

Regards,
--Prabhakar Lad

Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Sekhar Nori]

On Wednesday 06 November 2013 03:45 PM, Prabhakar Lad wrote:
> Hi Linus,
> 
> On Wed, Nov 6, 2013 at 3:26 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Wed, Nov 6, 2013 at 10:33 AM, Prabhakar Lad
>> <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>> On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar
>>>> <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>
>>>>> From: "Lad, Prabhakar" <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>
>>>>> This patch fixes the check for the offset in
>>>>> gpio_to_irq_unbanked() function.
>>>>>
>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>
>>>> Is this a regression that should go in right now?
>>>>
>>> Yes it needs too.
>>
>> But on top of *what* exactly?
>>
>> This does not apply to my gpio tree devel branch and
>> not even on the mainline kernel.
>>
> Looks like this needs to go via ARM tree as the earlier
> patches have  gone via ARM tree itself [1].
> If you can ACK it Sekhar can get it in via ARM tree.

The dependent patches are all in linux-next through ARM SoC queued for
v3.13 merge. This fix can either be sent late in merge cycle once Linus
has pulled ARM SoC or after v3.13-rc1 comes out.

Thanks,
Sekhar

Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Grygorii Strashko]

Hi Sekhar, Linus,

On 11/06/2013 12:49 PM, Sekhar Nori wrote:
> On Wednesday 06 November 2013 03:45 PM, Prabhakar Lad wrote:
>> Hi Linus,
>>
>> On Wed, Nov 6, 2013 at 3:26 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On Wed, Nov 6, 2013 at 10:33 AM, Prabhakar Lad
>>> <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>> On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar
>>>>> <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>
>>>>>> From: "Lad, Prabhakar" <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>>
>>>>>> This patch fixes the check for the offset in
>>>>>> gpio_to_irq_unbanked() function.
>>>>>>
>>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>
>>>>> Is this a regression that should go in right now?
>>>>>
>>>> Yes it needs too.
>>>
>>> But on top of *what* exactly?
>>>
>>> This does not apply to my gpio tree devel branch and
>>> not even on the mainline kernel.
>>>
>> Looks like this needs to go via ARM tree as the earlier
>> patches have  gone via ARM tree itself [1].
>> If you can ACK it Sekhar can get it in via ARM tree.
>
> The dependent patches are all in linux-next through ARM SoC queued for
> v3.13 merge. This fix can either be sent late in merge cycle once Linus
> has pulled ARM SoC or after v3.13-rc1 comes out.

Pls, wait a bit - this fix is incomplete :( The below changes have to be 
done too:

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 6c90cfb..05dbadb 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -328,7 +328,7 @@ static int gpio_to_irq_unbanked(struct gpio_chip 
*chip, unsigned offset)
          * can provide direct-mapped IRQs to AINTC (up to 32 GPIOs).
          */
         if (offset < d->gpio_unbanked)
-               return d->gpio_irq + offset;
+               return d->irq_base + offset;
         else
                 return -ENODEV;
  }
@@ -341,7 +341,7 @@ static int gpio_irq_type_unbanked(struct irq_data 
*data, unsigned trigger)

         d = (struct davinci_gpio_controller *)data->handler_data;
         g = (struct davinci_gpio_regs __iomem *)d->regs;
-       mask = __gpio_mask(data->irq - d->gpio_irq);
+       mask = __gpio_mask(data->irq - d->irq_base);

         if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
                 return -EINVAL;
@@ -419,6 +419,8 @@ static int davinci_gpio_irq_setup(struct 
platform_device *pdev)

                 /* pass "bank 0" GPIO IRQs to AINTC */
                 chips[0].chip.to_irq = gpio_to_irq_unbanked;
+               chips[0].irq_base = bank_irq;
+               chips[0].gpio_unbanked = pdata->gpio_unbanked;
                 binten = BIT(0);

                 /* AINTC handles mask/unmask; GPIO handles triggering */
diff --git a/include/linux/platform_data/gpio-davinci.h 
b/include/linux/platform_data/gpio-davinci.h
index 6efd202..711a002 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -42,7 +42,6 @@ struct davinci_gpio_controller {
         void __iomem            *clr_data;
         void __iomem            *in_data;
         int                     gpio_unbanked;
-       unsigned                gpio_irq;
  };

==================================================================
Also all places where davinci_gpio_register() is called need to be 
updated to use instead of "sizeof". Like:

diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index ef9ff1f..10334a4 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -906,8 +906,8 @@ static struct davinci_gpio_platform_data 
dm355_gpio_platform_data = {
  int __init dm355_gpio_register(void)
  {
         return davinci_gpio_register(dm355_gpio_resources,
-                                    sizeof(dm355_gpio_resources),
-                                    &dm355_gpio_platform_data);
+                                     ARRAY_SIZE(dm355_gpio_resources),
+                                     &dm355_gpio_platform_data);
  }


Regards,
- grygorii

Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Sekhar Nori]

Hi Grygorii,

On Wednesday 06 November 2013 05:03 PM, Grygorii Strashko wrote:
> Hi Sekhar, Linus,
> 
> On 11/06/2013 12:49 PM, Sekhar Nori wrote:
>> On Wednesday 06 November 2013 03:45 PM, Prabhakar Lad wrote:
>>> Hi Linus,
>>>
>>> On Wed, Nov 6, 2013 at 3:26 PM, Linus Walleij
>>> <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>> On Wed, Nov 6, 2013 at 10:33 AM, Prabhakar Lad
>>>> <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>> On Tue, Nov 5, 2013 at 6:09 PM, Linus Walleij
>>>>> <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>>>> On Sat, Nov 2, 2013 at 4:39 PM, Lad, Prabhakar
>>>>>> <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>>
>>>>>>> From: "Lad, Prabhakar" <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>>>
>>>>>>> This patch fixes the check for the offset in
>>>>>>> gpio_to_irq_unbanked() function.
>>>>>>>
>>>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>>
>>>>>> Is this a regression that should go in right now?
>>>>>>
>>>>> Yes it needs too.
>>>>
>>>> But on top of *what* exactly?
>>>>
>>>> This does not apply to my gpio tree devel branch and
>>>> not even on the mainline kernel.
>>>>
>>> Looks like this needs to go via ARM tree as the earlier
>>> patches have  gone via ARM tree itself [1].
>>> If you can ACK it Sekhar can get it in via ARM tree.
>>
>> The dependent patches are all in linux-next through ARM SoC queued for
>> v3.13 merge. This fix can either be sent late in merge cycle once Linus
>> has pulled ARM SoC or after v3.13-rc1 comes out.
> 
> Pls, wait a bit - this fix is incomplete :( The below changes have to be
> done too:

Can either you or Prabhakar send a separate patch (series) with only the
fixes?

Thanks,
Sekhar

Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio

[Linus Walleij]

On Wed, Nov 6, 2013 at 11:15 AM, Prabhakar Lad
<prabhakar.csengg@gmail.com> wrote:
> On Wed, Nov 6, 2013 at 3:26 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> This does not apply to my gpio tree devel branch and
>> not even on the mainline kernel.
>>
> Looks like this needs to go via ARM tree as the earlier
> patches have  gone via ARM tree itself [1].
> If you can ACK it Sekhar can get it in via ARM tree.

Aha OK.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v4 2/6] gpio: davinci: remove unnecessary printk

[Lad, Prabhakar]

From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>

the devm_*() helper prints error messages in case of
errors no need to do the same in the driver.

Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
 drivers/gpio/gpio-davinci.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 6c90cfb..95c6df1 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -389,11 +389,9 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	}
 
 	clk = devm_clk_get(dev, "gpio");
-	if (IS_ERR(clk)) {
-		printk(KERN_ERR "Error %ld getting gpio clock?\n",
-		       PTR_ERR(clk));
+	if (IS_ERR(clk))
 		return PTR_ERR(clk);
-	}
+
 	clk_prepare_enable(clk);
 
 	/*
-- 
1.7.9.5

[PATCH v4 3/6] gpio: davinci: use irqdomain

[Lad, Prabhakar]

From: "Lad, Prabhakar" <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch converts the davinci gpio driver to use irqdomain
support.

Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/mach-davinci/da830.c              |    1 -
 arch/arm/mach-davinci/da850.c              |    1 -
 arch/arm/mach-davinci/dm355.c              |    1 -
 arch/arm/mach-davinci/dm365.c              |    1 -
 arch/arm/mach-davinci/dm644x.c             |    1 -
 arch/arm/mach-davinci/dm646x.c             |    1 -
 drivers/gpio/gpio-davinci.c                |   49 ++++++++++++++++++----------
 include/linux/platform_data/gpio-davinci.h |    3 +-
 8 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index 0813b51..fb72035 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -1153,7 +1153,6 @@ static struct davinci_id da830_ids[] = {
 
 static struct davinci_gpio_platform_data da830_gpio_platform_data = {
 	.ngpio = 128,
-	.intc_irq_num = DA830_N_CP_INTC_IRQ,
 };
 
 int __init da830_register_gpio(void)
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 352984e..4379317 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -1283,7 +1283,6 @@ int __init da850_register_vpif_capture(struct vpif_capture_config
 
 static struct davinci_gpio_platform_data da850_gpio_platform_data = {
 	.ngpio = 144,
-	.intc_irq_num = DA850_N_CP_INTC_IRQ,
 };
 
 int __init da850_register_gpio(void)
diff --git a/arch/arm/mach-davinci/dm355.c b/arch/arm/mach-davinci/dm355.c
index ef9ff1f..5160aed 100644
--- a/arch/arm/mach-davinci/dm355.c
+++ b/arch/arm/mach-davinci/dm355.c
@@ -900,7 +900,6 @@ static struct resource dm355_gpio_resources[] = {
 
 static struct davinci_gpio_platform_data dm355_gpio_platform_data = {
 	.ngpio		= 104,
-	.intc_irq_num	= DAVINCI_N_AINTC_IRQ,
 };
 
 int __init dm355_gpio_register(void)
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index 1511a06..4fe29fa 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -713,7 +713,6 @@ static struct resource dm365_gpio_resources[] = {
 
 static struct davinci_gpio_platform_data dm365_gpio_platform_data = {
 	.ngpio		= 104,
-	.intc_irq_num	= DAVINCI_N_AINTC_IRQ,
 	.gpio_unbanked	= 8,
 };
 
diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
index 143a321..178cb68 100644
--- a/arch/arm/mach-davinci/dm644x.c
+++ b/arch/arm/mach-davinci/dm644x.c
@@ -786,7 +786,6 @@ static struct resource dm644_gpio_resources[] = {
 
 static struct davinci_gpio_platform_data dm644_gpio_platform_data = {
 	.ngpio		= 71,
-	.intc_irq_num	= DAVINCI_N_AINTC_IRQ,
 };
 
 int __init dm644x_gpio_register(void)
diff --git a/arch/arm/mach-davinci/dm646x.c b/arch/arm/mach-davinci/dm646x.c
index 2a73f29..01c576f 100644
--- a/arch/arm/mach-davinci/dm646x.c
+++ b/arch/arm/mach-davinci/dm646x.c
@@ -763,7 +763,6 @@ static struct resource dm646x_gpio_resources[] = {
 
 static struct davinci_gpio_platform_data dm646x_gpio_platform_data = {
 	.ngpio		= 43,
-	.intc_irq_num	= DAVINCI_N_AINTC_IRQ,
 };
 
 int __init dm646x_gpio_register(void)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 95c6df1..bcb6d8d 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -16,6 +16,7 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/irq.h>
+#include <linux/irqdomain.h>
 #include <linux/platform_device.h>
 #include <linux/platform_data/gpio-davinci.h>
 
@@ -292,7 +293,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 		__raw_writel(status, &g->intstat);
 
 		/* now demux them to the right lowlevel handler */
-		n = d->irq_base;
+		n = irq_find_mapping(d->irq_domain, 0);
 		if (irq & 1) {
 			n += 16;
 			status >>= 16;
@@ -313,10 +314,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip, unsigned offset)
 {
 	struct davinci_gpio_controller *d = chip2controller(chip);
 
-	if (d->irq_base >= 0)
-		return d->irq_base + offset;
-	else
-		return -ENODEV;
+	return irq_find_mapping(d->irq_domain, offset);
 }
 
 static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
@@ -373,6 +371,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
 	struct davinci_gpio_platform_data *pdata = dev->platform_data;
 	struct davinci_gpio_regs __iomem *g;
+	int gpio_irq = 0;
 
 	ngpio = pdata->ngpio;
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -402,9 +401,15 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	 */
 	for (gpio = 0, bank = 0; gpio < ngpio; bank++, gpio += 32) {
 		chips[bank].chip.to_irq = gpio_to_irq_banked;
-		chips[bank].irq_base = pdata->gpio_unbanked
-			? -EINVAL
-			: (pdata->intc_irq_num + gpio);
+		if (!pdata->gpio_unbanked) {
+			chips[bank].irq_domain =
+				irq_domain_add_linear(NULL, 32,
+						      &irq_domain_simple_ops,
+						      NULL);
+
+			if (!chips[bank].irq_domain)
+				return -ENOMEM;
+		}
 	}
 
 	/*
@@ -445,9 +450,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 	 * Or, AINTC can handle IRQs for banks of 16 GPIO IRQs, which we
 	 * then chain through our own handler.
 	 */
-	for (gpio = 0, irq = gpio_to_irq(0), bank = 0;
-			gpio < ngpio;
-			bank++, bank_irq++) {
+	for (gpio = 0, irq = 0, bank = 0; gpio < ngpio; bank++, bank_irq++) {
 		unsigned		i;
 
 		/* disabled by default, enabled only as needed */
@@ -465,12 +468,22 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
 		 */
 		irq_set_handler_data(bank_irq, &chips[gpio / 32]);
 
-		for (i = 0; i < 16 && gpio < ngpio; i++, irq++, gpio++) {
-			irq_set_chip(irq, &gpio_irqchip);
-			irq_set_chip_data(irq, (__force void *)g);
-			irq_set_handler_data(irq, (void *)__gpio_mask(gpio));
-			irq_set_handler(irq, handle_simple_irq);
-			set_irq_flags(irq, IRQF_VALID);
+		if (!(bank % 2))
+			irq = 0;
+		else
+			irq = 16;
+
+		for (i = 0; i < 16 && gpio < ngpio; i++, gpio++) {
+			int irqno =
+				irq_create_mapping(chips[gpio / 32].irq_domain,
+						   i + irq);
+
+			irq_set_chip(irqno, &gpio_irqchip);
+			irq_set_chip_data(irqno, (__force void *)g);
+			irq_set_handler_data(irqno, (void *)__gpio_mask(gpio));
+			irq_set_handler(irqno, handle_simple_irq);
+			set_irq_flags(irqno, IRQF_VALID);
+			gpio_irq++;
 		}
 
 		binten |= BIT(bank);
@@ -483,7 +496,7 @@ done:
 	 */
 	__raw_writel(binten, gpio_base + BINTEN);
 
-	printk(KERN_INFO "DaVinci: %d gpio irqs\n", irq - gpio_to_irq(0));
+	pr_info("DaVinci: %d gpio irqs\n", gpio_irq);
 
 	return 0;
 }
diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index 6efd202..fbe2f75 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -28,13 +28,12 @@ enum davinci_gpio_type {
 struct davinci_gpio_platform_data {
 	u32	ngpio;
 	u32	gpio_unbanked;
-	u32	intc_irq_num;
 };
 
 
 struct davinci_gpio_controller {
 	struct gpio_chip	chip;
-	int			irq_base;
+	struct irq_domain	*irq_domain;
 	/* Serialize access to GPIO registers */
 	spinlock_t		lock;
 	void __iomem		*regs;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/6] gpio: davinci: use irqdomain

[Grygorii Strashko]

Hi Prabhakar Lad,

On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This patch converts the davinci gpio driver to use irqdomain
> support.

This patch needs to be splitted in two:
1) add IRQ domain support
2) remove intc_irq_num

> 
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   arch/arm/mach-davinci/da830.c              |    1 -
>   arch/arm/mach-davinci/da850.c              |    1 -
>   arch/arm/mach-davinci/dm355.c              |    1 -
>   arch/arm/mach-davinci/dm365.c              |    1 -
>   arch/arm/mach-davinci/dm644x.c             |    1 -
>   arch/arm/mach-davinci/dm646x.c             |    1 -
>   drivers/gpio/gpio-davinci.c                |   49 ++++++++++++++++++----------
>   include/linux/platform_data/gpio-davinci.h |    3 +-
>   8 files changed, 32 insertions(+), 26 deletions(-)
> 

[...]

>   
>   int __init dm646x_gpio_register(void)
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 95c6df1..bcb6d8d 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -16,6 +16,7 @@
>   #include <linux/err.h>
>   #include <linux/io.h>
>   #include <linux/irq.h>
> +#include <linux/irqdomain.h>
>   #include <linux/platform_device.h>
>   #include <linux/platform_data/gpio-davinci.h>
>   
> @@ -292,7 +293,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>   		__raw_writel(status, &g->intstat);
>   
>   		/* now demux them to the right lowlevel handler */
> -		n = d->irq_base;
> +		n = irq_find_mapping(d->irq_domain, 0);

Sorry, but I don't understand why have you used <0> as hwirq?

All below logic may not work in case if we switch to use Linear IRQ domain :(
- irq_create_mapping() may return ANY Linux IRQ number. It means, for 
example, for bank0(ngpio=32)[0] it may return Linux_IRQ=200 or 201 or any other.
 Also, for bank3(ngpio=16)[0] it may return Linux_IRQ=150, etc.
- More over, if you call irq_create_mapping() 32 times you may NOT get
 sequential Linux_IRQ numbers.

So, the better sequence here can be smth. as below
(I can't verify it - my HW support only unbanked IRQs):

	if (irq & 1)
		mask <<= 16;

	while (1) {
		u32		status;
		int		bit;

		/* ack any irqs */
		status = __raw_readl(&g->intstat) & mask;
		if (!status)
			break;
		__raw_writel(status, &g->intstat);

		/* now demux them to the right lowlevel handler */
		while (status) {
			bit = __ffs(status);
                        status &= ~(1 << bit);
			generic_handle_irq(irq_find_mapping(d->irq_domain, bit));
		}
	}


>   		if (irq & 1) {
>   			n += 16;
>   			status >>= 16;
> @@ -313,10 +314,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip, unsigned offset)
>   {
>   	struct davinci_gpio_controller *d = chip2controller(chip);
>   
> -	if (d->irq_base >= 0)
> -		return d->irq_base + offset;
> -	else
> -		return -ENODEV;
> +	return irq_find_mapping(d->irq_domain, offset);

I think you can use irq_create_mapping() here instead of 
irq_find_mapping(), so IRQ will be mapped/allocated on demand.
Also, it seems, above code may crash in case if SoC has >1 GPIO banks and
gpio_unbanked > 0 and someone will call gpio_to_irq(>31).

>   }
>   
>   static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
> @@ -373,6 +371,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>   	struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
>   	struct davinci_gpio_platform_data *pdata = dev->platform_data;
>   	struct davinci_gpio_regs __iomem *g;
> +	int gpio_irq = 0;
>   
>   	ngpio = pdata->ngpio;
>   	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> @@ -402,9 +401,15 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>   	 */
>   	for (gpio = 0, bank = 0; gpio < ngpio; bank++, gpio += 32) {
>   		chips[bank].chip.to_irq = gpio_to_irq_banked;
> -		chips[bank].irq_base = pdata->gpio_unbanked
> -			? -EINVAL
> -			: (pdata->intc_irq_num + gpio);
> +		if (!pdata->gpio_unbanked) {
> +			chips[bank].irq_domain =
> +				irq_domain_add_linear(NULL, 32,

Use chips[i].chip.ngpio here instead of const 32?

> +						      &irq_domain_simple_ops,

Pass &davinci_gpio_irq_ops (see below)

> +						      NULL);

Pass &chips[bank] as host_data and use .map() callback (see below)

> +
> +			if (!chips[bank].irq_domain)
> +				return -ENOMEM;
> +		}
>   	}
>   
>   	/*
> @@ -445,9 +450,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>   	 * Or, AINTC can handle IRQs for banks of 16 GPIO IRQs, which we
>   	 * then chain through our own handler.
>   	 */
> -	for (gpio = 0, irq = gpio_to_irq(0), bank = 0;
> -			gpio < ngpio;
> -			bank++, bank_irq++) {
> +	for (gpio = 0, irq = 0, bank = 0; gpio < ngpio; bank++, bank_irq++) {
>   		unsigned		i;
>   
>   		/* disabled by default, enabled only as needed */
> @@ -465,12 +468,22 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>   		 */
>   		irq_set_handler_data(bank_irq, &chips[gpio / 32]);
>   
> -		for (i = 0; i < 16 && gpio < ngpio; i++, irq++, gpio++) {
> -			irq_set_chip(irq, &gpio_irqchip);
> -			irq_set_chip_data(irq, (__force void *)g);
> -			irq_set_handler_data(irq, (void *)__gpio_mask(gpio));
> -			irq_set_handler(irq, handle_simple_irq);
> -			set_irq_flags(irq, IRQF_VALID);
> +		if (!(bank % 2))
> +			irq = 0;
> +		else
> +			irq = 16;

As I mentioned above, I think, it is not good to play with IRQ numbers here.
Only chained IRQs and <binten> can be configured in this cycle.

> +
> +		for (i = 0; i < 16 && gpio < ngpio; i++, gpio++) {
> +			int irqno =
> +				irq_create_mapping(chips[gpio / 32].irq_domain,
> +						   i + irq);
> +
> +			irq_set_chip(irqno, &gpio_irqchip);
> +			irq_set_chip_data(irqno, (__force void *)g);
> +			irq_set_handler_data(irqno, (void *)__gpio_mask(gpio));
> +			irq_set_handler(irqno, handle_simple_irq);
> +			set_irq_flags(irqno, IRQF_VALID);

It makes no sense to manually create mapping. Usually it can be done in
.map() callback of IRQ domain. Like:

static int davinci_gpio_irq_map(struct irq_domain *d, unsigned int irq,
			    irq_hw_number_t hw)
{
	struct davinci_gpio_controller *chip = d->host_data;
	unsigned gpio = chip->chip.base + hw;

	irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq,
				      "davinci_gpio");
	irq_set_irq_type(irq, IRQ_TYPE_NONE);
	set_irq_flags(irq, IRQF_VALID);
...
	return 0;
}

static const struct irq_domain_ops davinci_gpio_irq_ops = {
	.map = davinci_gpio_irq_map,
	.xlate = irq_domain_xlate_onetwocell,
};


> +			gpio_irq++;
>   		}
>   
>   		binten |= BIT(bank);
> @@ -483,7 +496,7 @@ done:
>   	 */
>   	__raw_writel(binten, gpio_base + BINTEN);
>   
> -	printk(KERN_INFO "DaVinci: %d gpio irqs\n", irq - gpio_to_irq(0));
> +	pr_info("DaVinci: %d gpio irqs\n", gpio_irq);
>   
>   	return 0;
>   }
[...]
>   	spinlock_t		lock;
>   	void __iomem		*regs;
> 
start

Regards,
- grygorii

Re: [PATCH v4 3/6] gpio: davinci: use irqdomain

[Prabhakar Lad]

Hi grygorii,

Thanks for the review.

On Mon, Nov 4, 2013 at 11:57 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
> Hi Prabhakar Lad,
>
> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>
>> This patch converts the davinci gpio driver to use irqdomain
>> support.
>
> This patch needs to be splitted in two:
> 1) add IRQ domain support
> 2) remove intc_irq_num
>
OK

>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>> ---
>>   arch/arm/mach-davinci/da830.c              |    1 -
>>   arch/arm/mach-davinci/da850.c              |    1 -
>>   arch/arm/mach-davinci/dm355.c              |    1 -
>>   arch/arm/mach-davinci/dm365.c              |    1 -
>>   arch/arm/mach-davinci/dm644x.c             |    1 -
>>   arch/arm/mach-davinci/dm646x.c             |    1 -
>>   drivers/gpio/gpio-davinci.c                |   49 ++++++++++++++++++----------
>>   include/linux/platform_data/gpio-davinci.h |    3 +-
>>   8 files changed, 32 insertions(+), 26 deletions(-)
>>
>
> [...]
>
>>
>>   int __init dm646x_gpio_register(void)
>> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
>> index 95c6df1..bcb6d8d 100644
>> --- a/drivers/gpio/gpio-davinci.c
>> +++ b/drivers/gpio/gpio-davinci.c
>> @@ -16,6 +16,7 @@
>>   #include <linux/err.h>
>>   #include <linux/io.h>
>>   #include <linux/irq.h>
>> +#include <linux/irqdomain.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/platform_data/gpio-davinci.h>
>>
>> @@ -292,7 +293,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>>               __raw_writel(status, &g->intstat);
>>
>>               /* now demux them to the right lowlevel handler */
>> -             n = d->irq_base;
>> +             n = irq_find_mapping(d->irq_domain, 0);
>
> Sorry, but I don't understand why have you used <0> as hwirq?
>
> All below logic may not work in case if we switch to use Linear IRQ domain :(
> - irq_create_mapping() may return ANY Linux IRQ number. It means, for
> example, for bank0(ngpio=32)[0] it may return Linux_IRQ=200 or 201 or any other.
>  Also, for bank3(ngpio=16)[0] it may return Linux_IRQ=150, etc.
> - More over, if you call irq_create_mapping() 32 times you may NOT get
>  sequential Linux_IRQ numbers.
>
> So, the better sequence here can be smth. as below
> (I can't verify it - my HW support only unbanked IRQs):
>
Yeah below logic works fine for banked IRQs.

>         if (irq & 1)
>                 mask <<= 16;
>
>         while (1) {
>                 u32             status;
>                 int             bit;
>
>                 /* ack any irqs */
>                 status = __raw_readl(&g->intstat) & mask;
>                 if (!status)
>                         break;
>                 __raw_writel(status, &g->intstat);
>
>                 /* now demux them to the right lowlevel handler */
>                 while (status) {
>                         bit = __ffs(status);
>                         status &= ~(1 << bit);
>                         generic_handle_irq(irq_find_mapping(d->irq_domain, bit));
>                 }
>         }
>
>
>>               if (irq & 1) {
>>                       n += 16;
>>                       status >>= 16;
>> @@ -313,10 +314,7 @@ static int gpio_to_irq_banked(struct gpio_chip *chip, unsigned offset)
>>   {
>>       struct davinci_gpio_controller *d = chip2controller(chip);
>>
>> -     if (d->irq_base >= 0)
>> -             return d->irq_base + offset;
>> -     else
>> -             return -ENODEV;
>> +     return irq_find_mapping(d->irq_domain, offset);
>
> I think you can use irq_create_mapping() here instead of
> irq_find_mapping(), so IRQ will be mapped/allocated on demand.
> Also, it seems, above code may crash in case if SoC has >1 GPIO banks and
> gpio_unbanked > 0 and someone will call gpio_to_irq(>31).
>
Fixed it.

>>   }
>>
>>   static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
>> @@ -373,6 +371,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>>       struct davinci_gpio_controller *chips = platform_get_drvdata(pdev);
>>       struct davinci_gpio_platform_data *pdata = dev->platform_data;
>>       struct davinci_gpio_regs __iomem *g;
>> +     int gpio_irq = 0;
>>
>>       ngpio = pdata->ngpio;
>>       res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> @@ -402,9 +401,15 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>>        */
>>       for (gpio = 0, bank = 0; gpio < ngpio; bank++, gpio += 32) {
>>               chips[bank].chip.to_irq = gpio_to_irq_banked;
>> -             chips[bank].irq_base = pdata->gpio_unbanked
>> -                     ? -EINVAL
>> -                     : (pdata->intc_irq_num + gpio);
>> +             if (!pdata->gpio_unbanked) {
>> +                     chips[bank].irq_domain =
>> +                             irq_domain_add_linear(NULL, 32,
>
> Use chips[i].chip.ngpio here instead of const 32?
>
OK

>> +                                                   &irq_domain_simple_ops,
>
> Pass &davinci_gpio_irq_ops (see below)
>
>> +                                                   NULL);
>
> Pass &chips[bank] as host_data and use .map() callback (see below)
>
OK

>> +
>> +                     if (!chips[bank].irq_domain)
>> +                             return -ENOMEM;
>> +             }
>>       }
>>
>>       /*
>> @@ -445,9 +450,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>>        * Or, AINTC can handle IRQs for banks of 16 GPIO IRQs, which we
>>        * then chain through our own handler.
>>        */
>> -     for (gpio = 0, irq = gpio_to_irq(0), bank = 0;
>> -                     gpio < ngpio;
>> -                     bank++, bank_irq++) {
>> +     for (gpio = 0, irq = 0, bank = 0; gpio < ngpio; bank++, bank_irq++) {
>>               unsigned                i;
>>
>>               /* disabled by default, enabled only as needed */
>> @@ -465,12 +468,22 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>>                */
>>               irq_set_handler_data(bank_irq, &chips[gpio / 32]);
>>
>> -             for (i = 0; i < 16 && gpio < ngpio; i++, irq++, gpio++) {
>> -                     irq_set_chip(irq, &gpio_irqchip);
>> -                     irq_set_chip_data(irq, (__force void *)g);
>> -                     irq_set_handler_data(irq, (void *)__gpio_mask(gpio));
>> -                     irq_set_handler(irq, handle_simple_irq);
>> -                     set_irq_flags(irq, IRQF_VALID);
>> +             if (!(bank % 2))
>> +                     irq = 0;
>> +             else
>> +                     irq = 16;
>
> As I mentioned above, I think, it is not good to play with IRQ numbers here.
> Only chained IRQs and <binten> can be configured in this cycle.
>
Done

>> +
>> +             for (i = 0; i < 16 && gpio < ngpio; i++, gpio++) {
>> +                     int irqno =
>> +                             irq_create_mapping(chips[gpio / 32].irq_domain,
>> +                                                i + irq);
>> +
>> +                     irq_set_chip(irqno, &gpio_irqchip);
>> +                     irq_set_chip_data(irqno, (__force void *)g);
>> +                     irq_set_handler_data(irqno, (void *)__gpio_mask(gpio));
>> +                     irq_set_handler(irqno, handle_simple_irq);
>> +                     set_irq_flags(irqno, IRQF_VALID);
>
> It makes no sense to manually create mapping. Usually it can be done in
> .map() callback of IRQ domain. Like:
>
> static int davinci_gpio_irq_map(struct irq_domain *d, unsigned int irq,
>                             irq_hw_number_t hw)
> {
>         struct davinci_gpio_controller *chip = d->host_data;
>         unsigned gpio = chip->chip.base + hw;
>
>         irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq,
>                                       "davinci_gpio");
>         irq_set_irq_type(irq, IRQ_TYPE_NONE);
>         set_irq_flags(irq, IRQF_VALID);
> ...
>         return 0;
> }
>
> static const struct irq_domain_ops davinci_gpio_irq_ops = {
>         .map = davinci_gpio_irq_map,
>         .xlate = irq_domain_xlate_onetwocell,
> };
>
>
Fixed it.

Regards,
--Prabhakar Lad

[PATCH v4 5/6] ARM: davinci: da850: add GPIO DT node

[Lad, Prabhakar]

From: KV Sujith <sujithkv-l0cyMroinI0@public.gmane.org>

Add DT node for Davinci GPIO driver.

Signed-off-by: KV Sujith <sujithkv-l0cyMroinI0@public.gmane.org>
Signed-off-by: Philip Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/da850.dtsi |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 8d17346..d7b4cd6 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -8,6 +8,7 @@
  * option) any later version.
  */
 #include "skeleton.dtsi"
+#include <dt-bindings/interrupt-controller/irq.h>
 
 / {
 	arm {
@@ -256,6 +257,20 @@
 					36
 					>;
 		};
+		gpio: gpio@1e26000 {
+			compatible = "ti,dm6441-gpio";
+			gpio-controller;
+			reg = <0x226000 0x1000>;
+			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-irq-base = <101>;
+			ti,davinci-gpio-unbanked = <0>;
+			status = "disabled";
+		};
 	};
 	nand_cs3@62000000 {
 		compatible = "ti,davinci-nand";
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Lad, Prabhakar]

From: KV Sujith <sujithkv@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/.

Signed-off-by: KV Sujith <sujithkv@ti.com>
Signed-off-by: Philip Avinash <avinashphilip@ti.com>
[prabhakar.csengg@gmail.com: simplified the OF code, removed
		unnecessary DT property and also simplified
		the commit message]
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
 .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 ++++++++++++
 drivers/gpio/gpio-davinci.c                        |   54 ++++++++++++++++++--
 2 files changed, 83 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 0000000..55aae1c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -0,0 +1,32 @@
+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.
+
+- interrupts: Array of GPIO interrupt number.
+
+- ti,ngpio: The number of GPIO pins supported.
+
+- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt
+		             line to processor.
+
+Example:
+
+gpio: gpio@1e26000 {
+	compatible = "ti,dm6441-gpio";
+	gpio-controller;
+	reg = <0x226000 0x1000>;
+	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-irq-base = <101>;
+	ti,davinci-gpio-unbanked = <0>;
+};
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index bcb6d8d..bb20a39 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)
 	__raw_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
@@ -501,11 +540,20 @@ done:
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id davinci_gpio_ids[] = {
+	{ .compatible = "ti,dm6441-gpio", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
+#endif
+
 static struct platform_driver davinci_gpio_driver = {
 	.probe		= davinci_gpio_probe,
 	.driver		= {
-		.name	= "davinci_gpio",
-		.owner	= THIS_MODULE,
+		.name		= "davinci_gpio",
+		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(davinci_gpio_ids),
 	},
 };
 
-- 
1.7.9.5

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

[Grygorii Strashko]

Hi Prabhakar Lad,

On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
> From: KV Sujith <sujithkv-l0cyMroinI0@public.gmane.org>
>
> This patch adds OF parser support for davinci gpio
> driver and also appropriate documentation in gpio-davinci.txt
> located at Documentation/devicetree/bindings/gpio/.

I worry, do we need to have gpio_chip.of_xlate() callback implemented?
- From one side, Davinci GPIO controller in DT described by one entry
which defines number of supported GPIOs as "ti,ngpio = <144>;"

- From other side, on Linux level more than one gpio_chip objects are 
instantiated (one per each 32 GPIO).

How the standard GPIO biding will work in this case? .. And will they?

Linus, I'd very appreciate if you will be able to clarify this point.

>
> Signed-off-by: KV Sujith <sujithkv-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Philip Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
> [prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: simplified the OF code, removed
> 		unnecessary DT property and also simplified
> 		the commit message]
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>   .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 ++++++++++++
>   drivers/gpio/gpio-davinci.c                        |   54 ++++++++++++++++++--
>   2 files changed, 83 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 0000000..55aae1c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> @@ -0,0 +1,32 @@
> +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.
> +
> +- interrupts: Array of GPIO interrupt number.

May be meaning of <interrupts> property need to be extended, because,
as of now, only banked or unbanked IRQs are supported - and not both.

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

Should interrupt-controller; specifier be added here?

> +
> +Example:
> +
> +gpio: gpio@1e26000 {
> +	compatible = "ti,dm6441-gpio";
> +	gpio-controller;
> +	reg = <0x226000 0x1000>;
> +	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-irq-base = <101>;

         ^^ Is it still needed?

> +	ti,davinci-gpio-unbanked = <0>;
> +};
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index bcb6d8d..bb20a39 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)
>   	__raw_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
> @@ -501,11 +540,20 @@ done:
>   	return 0;
>   }
>
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id davinci_gpio_ids[] = {
> +	{ .compatible = "ti,dm6441-gpio", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
> +#endif
> +
>   static struct platform_driver davinci_gpio_driver = {
>   	.probe		= davinci_gpio_probe,
>   	.driver		= {
> -		.name	= "davinci_gpio",
> -		.owner	= THIS_MODULE,
> +		.name		= "davinci_gpio",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= of_match_ptr(davinci_gpio_ids),
>   	},
>   };
>
>
Regards,
- Grygorii

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

[Prabhakar Lad]

Hi Grygorii,

Thanks for the review.

On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
> Hi Prabhakar Lad,
>
>
> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>>
>> From: KV Sujith <sujithkv@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/.
>
>
> I worry, do we need to have gpio_chip.of_xlate() callback implemented?

I looked for the other OF GPIO implementations with same "ngpio"
property (marvel, msm) but I don’t see of_xlate() callback implemented.

> - From one side, Davinci GPIO controller in DT described by one entry
> which defines number of supported GPIOs as "ti,ngpio = <144>;"
>
> - From other side, on Linux level more than one gpio_chip objects are
> instantiated (one per each 32 GPIO).
>
> How the standard GPIO biding will work in this case? .. And will they?
>
> Linus, I'd very appreciate if you will be able to clarify this point.
>
>
>>
>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>> [prabhakar.csengg@gmail.com: simplified the OF code, removed
>>                 unnecessary DT property and also simplified
>>                 the commit message]
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>> ---
>>   .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 ++++++++++++
>>   drivers/gpio/gpio-davinci.c                        |   54
>> ++++++++++++++++++--
>>   2 files changed, 83 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 0000000..55aae1c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> @@ -0,0 +1,32 @@
>> +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.
>> +
>> +- interrupts: Array of GPIO interrupt number.
>
>
> May be meaning of <interrupts> property need to be extended, because,
> as of now, only banked or unbanked IRQs are supported - and not both.
>
>
OK

>> +
>> +- ti,ngpio: The number of GPIO pins supported.
>> +
>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
>> interrupt
>> +                            line to processor.
>
>
> Should interrupt-controller; specifier be added here?
>
No

>
>> +
>> +Example:
>> +
>> +gpio: gpio@1e26000 {
>> +       compatible = "ti,dm6441-gpio";
>> +       gpio-controller;
>> +       reg = <0x226000 0x1000>;
>> +       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-irq-base = <101>;
>
>
>         ^^ Is it still needed?
>
OOps missed to remove that.

Regards,
--Prabhakar Lad

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

[Grygorii Strashko]

On 11/05/2013 10:53 AM, Prabhakar Lad wrote:> Hi Grygorii,
 >
 > Thanks for the review.
 >
 > On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
 > <grygorii.strashko-l0cyMroinI0@public.gmane.org> wrote:
 >> Hi Prabhakar Lad,
 >>
 >>
 >> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
 >>>
 >>> From: KV Sujith <sujithkv-l0cyMroinI0@public.gmane.org>
 >>>
 >>> This patch adds OF parser support for davinci gpio
 >>> driver and also appropriate documentation in gpio-davinci.txt
 >>> located at Documentation/devicetree/bindings/gpio/.
 >>
 >>
 >> I worry, do we need to have gpio_chip.of_xlate() callback implemented?
 >
 > I looked for the other OF GPIO implementations with same "ngpio"
 > property (marvel, msm) but I don’t see of_xlate() callback implemented.

The question: will below definitions in DT work or not after this series?
  Will of_get_gpio()/of_get_named_gpio() work?

Example1 - leds:
	leds {
		compatible = "gpio-leds";
		debug0 {
			label = "green:debug0";
			gpios = <&gpio 29 GPIO_ACTIVE_HIGH>;
		};
	};

Example2 - any dev:
devA {
	compatible = "devA";
	gpios = <&gpio 120 GPIO_ACTIVE_LOW>;
}


 >
 >> - From one side, Davinci GPIO controller in DT described by one entry
 >> which defines number of supported GPIOs as "ti,ngpio = <144>;"
 >>
 >> - From other side, on Linux level more than one gpio_chip objects are
 >> instantiated (one per each 32 GPIO).
 >>
 >> How the standard GPIO biding will work in this case? .. And will they?
 >>
 >> Linus, I'd very appreciate if you will be able to clarify this point.
 >>
 >>
 >>>
 >>> Signed-off-by: KV Sujith <sujithkv-l0cyMroinI0@public.gmane.org>
 >>> Signed-off-by: Philip Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
 >>> [prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: simplified the OF code, removed
 >>>                  unnecessary DT property and also simplified
 >>>                  the commit message]
 >>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
 >>> ---
 >>>    .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 
++++++++++++
 >>>    drivers/gpio/gpio-davinci.c                        |   54
 >>> ++++++++++++++++++--
 >>>    2 files changed, 83 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 0000000..55aae1c
 >>> --- /dev/null
 >>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 >>> @@ -0,0 +1,32 @@
 >>> +Davinci GPIO controller bindings29
 >>> +
 >>> +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.
 >>> +
 >>> +- interrupts: Array of GPIO interrupt number.
 >>
 >>
 >> May be meaning of <interrupts> property need to be extended, because,
 >> as of now, only banked or unbanked IRQs are supported - and not both.
 >>
 >>
 > OK
 >
 >>> +
 >>> +- ti,ngpio: The number of GPIO pins supported.
 >>> +
 >>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an 
individual
 >>> interrupt
 >>> +                            line to processor.
 >>
 >>
 >> Should interrupt-controller; specifier be added here?
 >>
 > No

So, it would be impossible to map GPIO IRQ to device through DT. Right?
Like:
	devX@0 {
		compatible = "devX";
		interrupt-parent = <&gpio>;
		interrupts = <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */

	};


 >
 >>
 >>> +
 >>> +Example:
 >>> +
 >>> +gpio: gpio@1e26000 {
 >>> +       compatible = "ti,dm6441-gpio";
 >>> +       gpio-controller;
 >>> +       reg = <0x226000 0x1000>;
 >>> +       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-irq-base = <101>;
 >>
 >>
 >>          ^^ Is it still needed?
 >>
 > OOps missed to remove that.
 >
Regards,
-grygorii

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

[Prabhakar Lad]

Hi Grygorii,

On Tue, Nov 5, 2013 at 10:29 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
> On 11/05/2013 10:53 AM, Prabhakar Lad wrote:> Hi Grygorii,
>
>>
>> Thanks for the review.
>>
>> On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
>> <grygorii.strashko@ti.com> wrote:
>>> Hi Prabhakar Lad,
>>>
>>>
>>> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>>>>
>>>> From: KV Sujith <sujithkv@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/.
>>>
>>>
>>> I worry, do we need to have gpio_chip.of_xlate() callback implemented?
>>
>> I looked for the other OF GPIO implementations with same "ngpio"
>> property (marvel, msm) but I don’t see of_xlate() callback implemented.
>
> The question: will below definitions in DT work or not after this series?
>  Will of_get_gpio()/of_get_named_gpio() work?
>
> Example1 - leds:
>         leds {
>                 compatible = "gpio-leds";
>                 debug0 {
>                         label = "green:debug0";
>                         gpios = <&gpio 29 GPIO_ACTIVE_HIGH>;
>                 };
>         };
>
> Example2 - any dev:
> devA {
>         compatible = "devA";
>         gpios = <&gpio 120 GPIO_ACTIVE_LOW>;
>
> }
>
>
Agreed of_get_gpio()/of_get_named_gpio() wont work without
xlate callback implemented, but I think this can be added as a
incremental patch later.

>>
>>> - From one side, Davinci GPIO controller in DT described by one entry
>>> which defines number of supported GPIOs as "ti,ngpio = <144>;"
>>>
>>> - From other side, on Linux level more than one gpio_chip objects are
>>> instantiated (one per each 32 GPIO).
>>>
>>> How the standard GPIO biding will work in this case? .. And will they?
>>>
>>> Linus, I'd very appreciate if you will be able to clarify this point.
>>>
>>>
>>>>
>>>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>>>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>>>> [prabhakar.csengg@gmail.com: simplified the OF code, removed
>>>>                  unnecessary DT property and also simplified
>>>>                  the commit message]
>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>> ---
>>>>    .../devicetree/bindings/gpio/gpio-davinci.txt      |   32
>>>> ++++++++++++
>>>>    drivers/gpio/gpio-davinci.c                        |   54
>>>> ++++++++++++++++++--
>>>>    2 files changed, 83 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 0000000..55aae1c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>> @@ -0,0 +1,32 @@
>>>> +Davinci GPIO controller bindings29
>
>>>> +
>>>> +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.
>>>> +
>>>> +- interrupts: Array of GPIO interrupt number.
>>>
>>>
>>> May be meaning of <interrupts> property need to be extended, because,
>>> as of now, only banked or unbanked IRQs are supported - and not both.
>>>
>>>
>> OK
>>
>>>> +
>>>> +- ti,ngpio: The number of GPIO pins supported.
>>>> +
>>>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
>>>> interrupt
>>>> +                            line to processor.
>>>
>>>
>>> Should interrupt-controller; specifier be added here?
>>>
>> No
>
> So, it would be impossible to map GPIO IRQ to device through DT. Right?
> Like:
>         devX@0 {
>                 compatible = "devX";
>                 interrupt-parent = <&gpio>;
>                 interrupts = <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */
>
>
>         };
>
>
may be I took you wrong here, the interrupt-controller is inherited
property taken from its parent, so didn’t mention that in the documentation

Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Grygorii Strashko]

On 11/06/2013 12:08 PM, Prabhakar Lad wrote:
> Hi Grygorii,
>
> On Tue, Nov 5, 2013 at 10:29 PM, Grygorii Strashko
> <grygorii.strashko-l0cyMroinI0@public.gmane.org> wrote:
>> On 11/05/2013 10:53 AM, Prabhakar Lad wrote:> Hi Grygorii,
>>
>>>
>>> Thanks for the review.
>>>
>>> On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
>>> <grygorii.strashko-l0cyMroinI0@public.gmane.org> wrote:
>>>> Hi Prabhakar Lad,
>>>>
>>>>
>>>> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>>>>>
>>>>> From: KV Sujith <sujithkv-l0cyMroinI0@public.gmane.org>
>>>>>
>>>>> This patch adds OF parser support for davinci gpio
>>>>> driver and also appropriate documentation in gpio-davinci.txt
>>>>> located at Documentation/devicetree/bindings/gpio/.
>>>>
>>>>
>>>> I worry, do we need to have gpio_chip.of_xlate() callback implemented?
>>>
>>> I looked for the other OF GPIO implementations with same "ngpio"
>>> property (marvel, msm) but I don’t see of_xlate() callback implemented.
>>
>> The question: will below definitions in DT work or not after this series?
>>   Will of_get_gpio()/of_get_named_gpio() work?
>>
>> Example1 - leds:
>>          leds {
>>                  compatible = "gpio-leds";
>>                  debug0 {
>>                          label = "green:debug0";
>>                          gpios = <&gpio 29 GPIO_ACTIVE_HIGH>;
>>                  };
>>          };
>>
>> Example2 - any dev:
>> devA {
>>          compatible = "devA";
>>          gpios = <&gpio 120 GPIO_ACTIVE_LOW>;
>>
>> }
>>
>>
> Agreed of_get_gpio()/of_get_named_gpio() wont work without
> xlate callback implemented, but I think this can be added as a
> incremental patch later.
>
>>>
>>>> - From one side, Davinci GPIO controller in DT described by one entry
>>>> which defines number of supported GPIOs as "ti,ngpio = <144>;"
>>>>
>>>> - From other side, on Linux level more than one gpio_chip objects are
>>>> instantiated (one per each 32 GPIO).
>>>>
>>>> How the standard GPIO biding will work in this case? .. And will they?
>>>>
>>>> Linus, I'd very appreciate if you will be able to clarify this point.
>>>>
>>>>
>>>>>
>>>>> Signed-off-by: KV Sujith <sujithkv-l0cyMroinI0@public.gmane.org>
>>>>> Signed-off-by: Philip Avinash <avinashphilip-l0cyMroinI0@public.gmane.org>
>>>>> [prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: simplified the OF code, removed
>>>>>                   unnecessary DT property and also simplified
>>>>>                   the commit message]
>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>>     .../devicetree/bindings/gpio/gpio-davinci.txt      |   32
>>>>> ++++++++++++
>>>>>     drivers/gpio/gpio-davinci.c                        |   54
>>>>> ++++++++++++++++++--
>>>>>     2 files changed, 83 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 0000000..55aae1c
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>>> @@ -0,0 +1,32 @@
>>>>> +Davinci GPIO controller bindings29
>>
>>>>> +
>>>>> +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.
>>>>> +
>>>>> +- interrupts: Array of GPIO interrupt number.
>>>>
>>>>
>>>> May be meaning of <interrupts> property need to be extended, because,
>>>> as of now, only banked or unbanked IRQs are supported - and not both.
>>>>
>>>>
>>> OK
>>>
>>>>> +
>>>>> +- ti,ngpio: The number of GPIO pins supported.
>>>>> +
>>>>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
>>>>> interrupt
>>>>> +                            line to processor.
>>>>
>>>>
>>>> Should interrupt-controller; specifier be added here?
>>>>
>>> No
>>
>> So, it would be impossible to map GPIO IRQ to device through DT. Right?
>> Like:
>>          devX@0 {
>>                  compatible = "devX";
>>                  interrupt-parent = <&gpio>;
>>                  interrupts = <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */
>>
>>
>>          };
>>
>>
> may be I took you wrong here, the interrupt-controller is inherited
> property taken from its parent, so didn’t mention that in the documentation

The GPIO controller uses interrupts form parent controller INTC/GIC
from one side, but from other side it can provide interrupts to its
users.
And as result can be interrupt-controller.

INTC/GIC -> GPIO -> user

It could work for banked IRQs only now :)


>
> Regards,
> --Prabhakar Lad
>

[PATCH v4 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries DT node

[Lad, Prabhakar]

From: KV Sujith <sujithkv@ti.com>

Add GPIO DT node and pinmux entries for DA850 EVM. GPIO is
configurable differently on different boards. So add GPIO
pinmuxing in dts file.

Signed-off-by: KV Sujith <sujithkv@ti.com>
Signed-off-by: Philip Avinash <avinashphilip@ti.com>
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
 arch/arm/boot/dts/da850-evm.dts |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 588ce58..f82c129 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -17,6 +17,21 @@
 	soc {
 		pmx_core: pinmux@1c14120 {
 			status = "okay";
+
+			gpio_pins: pinmux_gpio_pins {
+				pinctrl-single,bits = <
+					/* GPIO2_4 GPIO2_6 */
+					0x18 0x00008080 0x0000f0f0
+					/* GPIO2_8 GPIO2_15 */
+					0x14 0x80000008 0xf000000f
+					/* GPIO3_12 GPIO3_13 */
+					0x1C 0x00008800 0x0000ff00
+					/* GPIO4_0 GPIO4_1 */
+					0x28 0x88000000 0xff000000
+					/* GPIO6_9 GPIO6_10 GPIO6_13 */
+					0x34 0x08800800 0x0ff00f00
+				>;
+			};
 		};
 		serial0: serial@1c42000 {
 			status = "okay";
@@ -101,6 +116,11 @@
 			pinctrl-names = "default";
 			pinctrl-0 = <&mii_pins>;
 		};
+		gpio: gpio@1e26000 {
+			status = "okay";
+			pinctrl-names = "default";
+			pinctrl-0 = <&gpio_pins>;
+		};
 	};
 	nand_cs3@62000000 {
 		status = "okay";
-- 
1.7.9.5