* [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_*
2013-11-21 18:15 [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Prabhakar Lad
@ 2013-11-21 18:15 ` Prabhakar Lad
2013-11-22 10:08 ` Taras Kondratiuk
2013-11-21 18:15 ` [PATCH v6 2/6] This patch converts the davinci gpio driver to use irqdomain support Prabhakar Lad
` (5 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-21 18:15 UTC (permalink / raw)
To: linux-arm-kernel
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
This patch replaces the __raw_readl/writel with
readl and writel, Altough the code runs on ARMv5
based SOCs, changing this will help copying the code
for other uses.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
drivers/gpio/gpio-davinci.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 84be701..1f33fcd 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -82,14 +82,14 @@ static inline int __davinci_direction(struct gpio_chip *chip,
u32 mask = 1 << offset;
spin_lock_irqsave(&d->lock, flags);
- temp = __raw_readl(&g->dir);
+ temp = readl(&g->dir);
if (out) {
temp &= ~mask;
- __raw_writel(mask, value ? &g->set_data : &g->clr_data);
+ writel(mask, value ? &g->set_data : &g->clr_data);
} else {
temp |= mask;
}
- __raw_writel(temp, &g->dir);
+ writel(temp, &g->dir);
spin_unlock_irqrestore(&d->lock, flags);
return 0;
@@ -118,7 +118,7 @@ static int davinci_gpio_get(struct gpio_chip *chip, unsigned offset)
struct davinci_gpio_controller *d = chip2controller(chip);
struct davinci_gpio_regs __iomem *g = d->regs;
- return (1 << offset) & __raw_readl(&g->in_data);
+ return (1 << offset) & readl(&g->in_data);
}
/*
@@ -130,7 +130,7 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
struct davinci_gpio_controller *d = chip2controller(chip);
struct davinci_gpio_regs __iomem *g = d->regs;
- __raw_writel((1 << offset), value ? &g->set_data : &g->clr_data);
+ writel((1 << offset), value ? &g->set_data : &g->clr_data);
}
static int davinci_gpio_probe(struct platform_device *pdev)
@@ -227,8 +227,8 @@ static void gpio_irq_disable(struct irq_data *d)
struct davinci_gpio_regs __iomem *g = irq2regs(d->irq);
u32 mask = (u32) irq_data_get_irq_handler_data(d);
- __raw_writel(mask, &g->clr_falling);
- __raw_writel(mask, &g->clr_rising);
+ writel(mask, &g->clr_falling);
+ writel(mask, &g->clr_rising);
}
static void gpio_irq_enable(struct irq_data *d)
@@ -242,9 +242,9 @@ static void gpio_irq_enable(struct irq_data *d)
status = IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING;
if (status & IRQ_TYPE_EDGE_FALLING)
- __raw_writel(mask, &g->set_falling);
+ writel(mask, &g->set_falling);
if (status & IRQ_TYPE_EDGE_RISING)
- __raw_writel(mask, &g->set_rising);
+ writel(mask, &g->set_rising);
}
static int gpio_irq_type(struct irq_data *d, unsigned trigger)
@@ -286,10 +286,10 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
int res;
/* ack any irqs */
- status = __raw_readl(&g->intstat) & mask;
+ status = readl(&g->intstat) & mask;
if (!status)
break;
- __raw_writel(status, &g->intstat);
+ writel(status, &g->intstat);
/* now demux them to the right lowlevel handler */
n = d->irq_base;
@@ -346,9 +346,9 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger)
if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
return -EINVAL;
- __raw_writel(mask, (trigger & IRQ_TYPE_EDGE_FALLING)
+ writel(mask, (trigger & IRQ_TYPE_EDGE_FALLING)
? &g->set_falling : &g->clr_falling);
- __raw_writel(mask, (trigger & IRQ_TYPE_EDGE_RISING)
+ writel(mask, (trigger & IRQ_TYPE_EDGE_RISING)
? &g->set_rising : &g->clr_rising);
return 0;
@@ -432,8 +432,8 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
/* default trigger: both edges */
g = gpio2regs(0);
- __raw_writel(~0, &g->set_falling);
- __raw_writel(~0, &g->set_rising);
+ writel(~0, &g->set_falling);
+ writel(~0, &g->set_rising);
/* set the direct IRQs up to use that irqchip */
for (gpio = 0; gpio < pdata->gpio_unbanked; gpio++, irq++) {
@@ -456,8 +456,8 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
/* disabled by default, enabled only as needed */
g = gpio2regs(gpio);
- __raw_writel(~0, &g->clr_falling);
- __raw_writel(~0, &g->clr_rising);
+ writel(~0, &g->clr_falling);
+ writel(~0, &g->clr_rising);
/* set up all irqs in this bank */
irq_set_chained_handler(bank_irq, gpio_irq_handler);
@@ -485,7 +485,7 @@ done:
* BINTEN -- per-bank interrupt enable. genirq would also let these
* bits be set/cleared dynamically.
*/
- __raw_writel(binten, gpio_base + BINTEN);
+ writel(binten, gpio_base + BINTEN);
printk(KERN_INFO "DaVinci: %d gpio irqs\n", irq - gpio_to_irq(0));
--
1.7.9.5
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_*
2013-11-21 18:15 ` [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_* Prabhakar Lad
@ 2013-11-22 10:08 ` Taras Kondratiuk
2013-11-25 4:12 ` Prabhakar Lad
0 siblings, 1 reply; 28+ messages in thread
From: Taras Kondratiuk @ 2013-11-22 10:08 UTC (permalink / raw)
To: linux-arm-kernel
On 21 November 2013 20:15, Prabhakar Lad <prabhakar.csengg@gmail.com> wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
> This patch replaces the __raw_readl/writel with
> readl and writel, Altough the code runs on ARMv5
> based SOCs, changing this will help copying the code
> for other uses.
This replacement has a functional impact: it adds memory barriers.
Please note this in the description.
Also please add a bit of explanation on why do you need to add barriers.
--
Regards,
Taras Kondratiuk
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_*
2013-11-22 10:08 ` Taras Kondratiuk
@ 2013-11-25 4:12 ` Prabhakar Lad
2013-11-25 10:34 ` Sekhar Nori
0 siblings, 1 reply; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-25 4:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi Taras,
On Fri, Nov 22, 2013 at 3:38 PM, Taras Kondratiuk
<taras.kondratiuk@linaro.org> wrote:
> On 21 November 2013 20:15, Prabhakar Lad <prabhakar.csengg@gmail.com> wrote:
>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>
>> This patch replaces the __raw_readl/writel with
>> readl and writel, Altough the code runs on ARMv5
>> based SOCs, changing this will help copying the code
>> for other uses.
>
> This replacement has a functional impact: it adds memory barriers.
> Please note this in the description.
> Also please add a bit of explanation on why do you need to add barriers.
>
Agreed this adds memory barriers, I'll add a note about it.
Thanks,
--Prabhakar Lad
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_*
2013-11-25 4:12 ` Prabhakar Lad
@ 2013-11-25 10:34 ` Sekhar Nori
2013-11-26 8:29 ` Prabhakar Lad
0 siblings, 1 reply; 28+ messages in thread
From: Sekhar Nori @ 2013-11-25 10:34 UTC (permalink / raw)
To: linux-arm-kernel
Prabhakar,
On Monday 25 November 2013 09:42 AM, Prabhakar Lad wrote:
> Hi Taras,
>
> On Fri, Nov 22, 2013 at 3:38 PM, Taras Kondratiuk
> <taras.kondratiuk@linaro.org> wrote:
>> On 21 November 2013 20:15, Prabhakar Lad <prabhakar.csengg@gmail.com> wrote:
>>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>>
>>> This patch replaces the __raw_readl/writel with
>>> readl and writel, Altough the code runs on ARMv5
>>> based SOCs, changing this will help copying the code
>>> for other uses.
>>
>> This replacement has a functional impact: it adds memory barriers.
>> Please note this in the description.
>> Also please add a bit of explanation on why do you need to add barriers.
>>
> Agreed this adds memory barriers, I'll add a note about it.
Well the barriers certainly make it easier to debug by having both
device and memory accesses happen in program order. That said, if there
is no pressing reason to add barriers, you can use
{readl|writel}_relaxed() instead. That will make the code protable
across endianess.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_*
2013-11-25 10:34 ` Sekhar Nori
@ 2013-11-26 8:29 ` Prabhakar Lad
0 siblings, 0 replies; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-26 8:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sekhar,
On Mon, Nov 25, 2013 at 4:04 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> Prabhakar,
>
> On Monday 25 November 2013 09:42 AM, Prabhakar Lad wrote:
>> Hi Taras,
>>
>> On Fri, Nov 22, 2013 at 3:38 PM, Taras Kondratiuk
>> <taras.kondratiuk@linaro.org> wrote:
>>> On 21 November 2013 20:15, Prabhakar Lad <prabhakar.csengg@gmail.com> wrote:
>>>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>>>
>>>> This patch replaces the __raw_readl/writel with
>>>> readl and writel, Altough the code runs on ARMv5
>>>> based SOCs, changing this will help copying the code
>>>> for other uses.
>>>
>>> This replacement has a functional impact: it adds memory barriers.
>>> Please note this in the description.
>>> Also please add a bit of explanation on why do you need to add barriers.
>>>
>> Agreed this adds memory barriers, I'll add a note about it.
>
> Well the barriers certainly make it easier to debug by having both
> device and memory accesses happen in program order. That said, if there
> is no pressing reason to add barriers, you can use
> {readl|writel}_relaxed() instead. That will make the code protable
> across endianess.
>
OK will use {readl|writel}_relaxed() instead.
Regards,
--Prabhakar Lad
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 2/6] This patch converts the davinci gpio driver to use irqdomain support.
2013-11-21 18:15 [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Prabhakar Lad
2013-11-21 18:15 ` [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_* Prabhakar Lad
@ 2013-11-21 18:15 ` Prabhakar Lad
2013-11-29 9:23 ` Linus Walleij
2013-12-15 12:36 ` Sekhar Nori
2013-11-21 18:15 ` [PATCH v6 3/6] gpio: davinci: remove unused variable intc_irq_num Prabhakar Lad
` (4 subsequent siblings)
6 siblings, 2 replies; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-21 18:15 UTC (permalink / raw)
To: linux-arm-kernel
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
[grygorii.strashko at ti.com:
- switch to use one irq-domain per all GPIO banks
- keep irq_create_mapping() call in gpio_to_irq_banked() as it
simply transformed to irq_find_mapping() if IRQ mapping exist
already]
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
drivers/gpio/gpio-davinci.c | 80 ++++++++++++++++------------
include/linux/platform_data/gpio-davinci.h | 2 +-
2 files changed, 48 insertions(+), 34 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 1f33fcd..4d6adbc 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>
@@ -282,8 +283,7 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
desc->irq_data.chip->irq_ack(&desc->irq_data);
while (1) {
u32 status;
- int n;
- int res;
+ int bit;
/* ack any irqs */
status = readl(&g->intstat) & mask;
@@ -292,17 +292,13 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
writel(status, &g->intstat);
/* now demux them to the right lowlevel handler */
- n = d->irq_base;
- if (irq & 1) {
- n += 16;
- status >>= 16;
- }
while (status) {
- res = ffs(status);
- n += res;
- generic_handle_irq(n - 1);
- status >>= res;
+ bit = __ffs(status);
+ status &= ~BIT(bit);
+ generic_handle_irq(
+ irq_find_mapping(d->irq_domain,
+ d->chip.base + bit));
}
}
desc->irq_data.chip->irq_unmask(&desc->irq_data);
@@ -313,10 +309,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_create_mapping(d->irq_domain, d->chip.base + offset);
}
static int gpio_to_irq_unbanked(struct gpio_chip *chip, unsigned offset)
@@ -354,6 +347,27 @@ static int gpio_irq_type_unbanked(struct irq_data *data, unsigned trigger)
return 0;
}
+static int
+davinci_gpio_irq_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ struct davinci_gpio_regs __iomem *g = gpio2regs(hw);
+
+ irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq,
+ "davinci_gpio");
+ irq_set_irq_type(irq, IRQ_TYPE_NONE);
+ irq_set_chip_data(irq, (__force void *)g);
+ irq_set_handler_data(irq, (void *)__gpio_mask(hw));
+ 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,
+};
+
/*
* NOTE: for suspend/resume, probably best to make a platform_device with
* suspend_late/resume_resume calls hooking into results of the set_wake()
@@ -373,6 +387,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;
+ struct irq_domain *irq_domain;
ngpio = pdata->ngpio;
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
@@ -396,6 +411,20 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
}
clk_prepare_enable(clk);
+ irq = irq_alloc_descs(-1, 0, ngpio, 0);
+ if (irq < 0) {
+ dev_err(dev, "Couldn't allocate IRQ numbers\n");
+ return -ENODEV;
+ }
+
+ irq_domain = irq_domain_add_legacy(NULL, ngpio, irq, 0,
+ &davinci_gpio_irq_ops,
+ chips);
+ if (!irq_domain) {
+ dev_err(dev, "Couldn't register an IRQ domain\n");
+ return -ENODEV;
+ }
+
/*
* Arrange gpio_to_irq() support, handling either direct IRQs or
* banked IRQs. Having GPIOs in the first GPIO bank use direct
@@ -404,9 +433,8 @@ 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;
}
/*
@@ -449,11 +477,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++) {
- unsigned i;
-
+ for (gpio = 0, bank = 0; gpio < ngpio; bank++, bank_irq++, gpio += 16) {
/* disabled by default, enabled only as needed */
g = gpio2regs(gpio);
writel(~0, &g->clr_falling);
@@ -469,14 +493,6 @@ 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);
- }
-
binten |= BIT(bank);
}
@@ -487,8 +503,6 @@ done:
*/
writel(binten, gpio_base + BINTEN);
- printk(KERN_INFO "DaVinci: %d gpio irqs\n", irq - gpio_to_irq(0));
-
return 0;
}
diff --git a/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index 6efd202..0c3551b 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -34,7 +34,7 @@ struct davinci_gpio_platform_data {
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v6 2/6] This patch converts the davinci gpio driver to use irqdomain support.
2013-11-21 18:15 ` [PATCH v6 2/6] This patch converts the davinci gpio driver to use irqdomain support Prabhakar Lad
@ 2013-11-29 9:23 ` Linus Walleij
2013-12-15 12:36 ` Sekhar Nori
1 sibling, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2013-11-29 9:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 21, 2013 at 7:15 PM, Prabhakar Lad
<prabhakar.csengg@gmail.com> wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> [grygorii.strashko at ti.com:
> - switch to use one irq-domain per all GPIO banks
> - keep irq_create_mapping() call in gpio_to_irq_banked() as it
> simply transformed to irq_find_mapping() if IRQ mapping exist
> already]
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 2/6] This patch converts the davinci gpio driver to use irqdomain support.
2013-11-21 18:15 ` [PATCH v6 2/6] This patch converts the davinci gpio driver to use irqdomain support Prabhakar Lad
2013-11-29 9:23 ` Linus Walleij
@ 2013-12-15 12:36 ` Sekhar Nori
1 sibling, 0 replies; 28+ messages in thread
From: Sekhar Nori @ 2013-12-15 12:36 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> [grygorii.strashko at ti.com:
> - switch to use one irq-domain per all GPIO banks
> - keep irq_create_mapping() call in gpio_to_irq_banked() as it
> simply transformed to irq_find_mapping() if IRQ mapping exist
> already]
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
A proper subject line is missing. I added the following as the subject:
gpio: davinci: convert to use irqdomain
and moved your current subject line to become the commit text.
> @@ -396,6 +411,20 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
> }
> clk_prepare_enable(clk);
>
> + irq = irq_alloc_descs(-1, 0, ngpio, 0);
> + if (irq < 0) {
> + dev_err(dev, "Couldn't allocate IRQ numbers\n");
> + return -ENODEV;
modified this to:
return irq;
since your have already received a more relevant error code. With these
modifications and Linus's ack, queuing for v3.14.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 3/6] gpio: davinci: remove unused variable intc_irq_num
2013-11-21 18:15 [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Prabhakar Lad
2013-11-21 18:15 ` [PATCH v6 1/6] gpio: davinci: use readl/writel instead of __raw_* Prabhakar Lad
2013-11-21 18:15 ` [PATCH v6 2/6] This patch converts the davinci gpio driver to use irqdomain support Prabhakar Lad
@ 2013-11-21 18:15 ` Prabhakar Lad
2013-11-29 7:43 ` Linus Walleij
2013-12-15 12:39 ` Sekhar Nori
2013-11-21 18:15 ` [PATCH v6 4/6] gpio: davinci: add OF support Prabhakar Lad
` (3 subsequent siblings)
6 siblings, 2 replies; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-21 18:15 UTC (permalink / raw)
To: linux-arm-kernel
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
As the davinci-gpio driver is migrated to use irqdomain
there is no need to pass the irq base for the gpio driver.
This patch removes this variable from davinci_gpio_platform_data
and also the refrences from the machine file.
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 -
include/linux/platform_data/gpio-davinci.h | 1 -
7 files changed, 7 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 536ce52..a1713cc 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 9c96520..2595097 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 72a3aa7..d83436d 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 d1b646c..5d289f4 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/include/linux/platform_data/gpio-davinci.h b/include/linux/platform_data/gpio-davinci.h
index 0c3551b..fbe2f75 100644
--- a/include/linux/platform_data/gpio-davinci.h
+++ b/include/linux/platform_data/gpio-davinci.h
@@ -28,7 +28,6 @@ enum davinci_gpio_type {
struct davinci_gpio_platform_data {
u32 ngpio;
u32 gpio_unbanked;
- u32 intc_irq_num;
};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v6 3/6] gpio: davinci: remove unused variable intc_irq_num
2013-11-21 18:15 ` [PATCH v6 3/6] gpio: davinci: remove unused variable intc_irq_num Prabhakar Lad
@ 2013-11-29 7:43 ` Linus Walleij
2013-12-15 12:39 ` Sekhar Nori
1 sibling, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2013-11-29 7:43 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 21, 2013 at 7:15 PM, Prabhakar Lad
<prabhakar.csengg@gmail.com> wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
> As the davinci-gpio driver is migrated to use irqdomain
> there is no need to pass the irq base for the gpio driver.
> This patch removes this variable from davinci_gpio_platform_data
> and also the refrences from the machine file.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
*Very* nice patch!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 3/6] gpio: davinci: remove unused variable intc_irq_num
2013-11-21 18:15 ` [PATCH v6 3/6] gpio: davinci: remove unused variable intc_irq_num Prabhakar Lad
2013-11-29 7:43 ` Linus Walleij
@ 2013-12-15 12:39 ` Sekhar Nori
1 sibling, 0 replies; 28+ messages in thread
From: Sekhar Nori @ 2013-12-15 12:39 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
> As the davinci-gpio driver is migrated to use irqdomain
> there is no need to pass the irq base for the gpio driver.
> This patch removes this variable from davinci_gpio_platform_data
> and also the refrences from the machine file.
>
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
Queuing for v3.14 along with Linus's ack.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-21 18:15 [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Prabhakar Lad
` (2 preceding siblings ...)
2013-11-21 18:15 ` [PATCH v6 3/6] gpio: davinci: remove unused variable intc_irq_num Prabhakar Lad
@ 2013-11-21 18:15 ` Prabhakar Lad
2013-11-25 11:00 ` Sekhar Nori
2013-11-21 18:15 ` [PATCH v6 5/6] ARM: davinci: da850: add GPIO DT node Prabhakar Lad
` (2 subsequent siblings)
6 siblings, 1 reply; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-21 18:15 UTC (permalink / raw)
To: linux-arm-kernel
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/.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Signed-off-by: KV Sujith <sujithkv@ti.com>
Signed-off-by: Philip Avinash <avinashphilip@ti.com>
[prabhakar.csengg at 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 | 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 0000000..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 at 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:
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-21 18:15 ` [PATCH v6 4/6] gpio: davinci: add OF support Prabhakar Lad
@ 2013-11-25 11:00 ` Sekhar Nori
2013-11-26 8:28 ` Prabhakar Lad
2013-11-26 12:33 ` Grygorii Strashko
0 siblings, 2 replies; 28+ messages in thread
From: Sekhar Nori @ 2013-11-25 11:00 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Acked-by: Rob Herring <rob.herring@calxeda.com>
> Signed-off-by: KV Sujith <sujithkv@ti.com>
> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> [prabhakar.csengg at 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 | 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 0000000..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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-25 11:00 ` Sekhar Nori
@ 2013-11-26 8:28 ` Prabhakar Lad
2013-11-26 12:33 ` Grygorii Strashko
1 sibling, 0 replies; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-26 8:28 UTC (permalink / raw)
To: linux-arm-kernel
Hi Sekhar,
Thanks for the review.
On Mon, Nov 25, 2013 at 4:30 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/.
>>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>> Acked-by: Rob Herring <rob.herring@calxeda.com>
>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>> [prabhakar.csengg at 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 | 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 0000000..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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-25 11:00 ` Sekhar Nori
2013-11-26 8:28 ` Prabhakar Lad
@ 2013-11-26 12:33 ` Grygorii Strashko
2013-11-26 17:12 ` Sekhar Nori
1 sibling, 1 reply; 28+ messages in thread
From: Grygorii Strashko @ 2013-11-26 12:33 UTC (permalink / raw)
To: linux-arm-kernel
On 11/25/2013 01:00 PM, Sekhar Nori wrote:
> On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/.
>>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>> Acked-by: Rob Herring <rob.herring@calxeda.com>
>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>> [prabhakar.csengg at 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 | 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 0000000..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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-26 12:33 ` Grygorii Strashko
@ 2013-11-26 17:12 ` Sekhar Nori
2013-11-26 19:41 ` Grygorii Strashko
2013-11-29 7:46 ` Linus Walleij
0 siblings, 2 replies; 28+ messages in thread
From: Sekhar Nori @ 2013-11-26 17:12 UTC (permalink / raw)
To: linux-arm-kernel
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 <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/.
>>>
>>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>> Acked-by: Rob Herring <rob.herring@calxeda.com>
>>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>>> [prabhakar.csengg at 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 | 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 0000000..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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-26 17:12 ` Sekhar Nori
@ 2013-11-26 19:41 ` Grygorii Strashko
2013-11-27 3:59 ` Sekhar Nori
2013-11-29 7:46 ` Linus Walleij
1 sibling, 1 reply; 28+ messages in thread
From: Grygorii Strashko @ 2013-11-26 19:41 UTC (permalink / raw)
To: linux-arm-kernel
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 <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/.
>>>>
>>>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Acked-by: Rob Herring <rob.herring@calxeda.com>
>>>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>>>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>>>> [prabhakar.csengg at 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 | 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 0000000..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 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 :)
I'm thinking more and more about new DT only compatible driver, so there
will be no problem with non-DT code ("no regression") and even about
moving the old driver back to the platform. :) Just thinking aloud.
Regards,
-grygorii
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-26 19:41 ` Grygorii Strashko
@ 2013-11-27 3:59 ` Sekhar Nori
0 siblings, 0 replies; 28+ messages in thread
From: Sekhar Nori @ 2013-11-27 3:59 UTC (permalink / raw)
To: linux-arm-kernel
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 <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/.
>>>>>
>>>>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>>>> Acked-by: Rob Herring <rob.herring@calxeda.com>
>>>>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>>>>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>>>>> [prabhakar.csengg at 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 | 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 0000000..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
> ;).
May be you can take care of the DT case, upload the patches to some tree
and I can help you handle the non-DT case?
>
>>
>>>
>>> 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 :)
>
> I'm thinking more and more about new DT only compatible driver, so there
> will be no problem with non-DT code ("no regression") and even about
> moving the old driver back to the platform. :) Just thinking aloud.
Having two drivers is really a step backwards. NAK :)
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-26 17:12 ` Sekhar Nori
2013-11-26 19:41 ` Grygorii Strashko
@ 2013-11-29 7:46 ` Linus Walleij
2013-11-29 8:09 ` Sekhar Nori
1 sibling, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2013-11-29 7:46 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Nov 26, 2013 at 6:12 PM, Sekhar Nori <nsekhar@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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 4/6] gpio: davinci: add OF support
2013-11-29 7:46 ` Linus Walleij
@ 2013-11-29 8:09 ` Sekhar Nori
0 siblings, 0 replies; 28+ messages in thread
From: Sekhar Nori @ 2013-11-29 8:09 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 29 November 2013 01:16 PM, Linus Walleij wrote:
> On Tue, Nov 26, 2013 at 6:12 PM, Sekhar Nori <nsekhar@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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 5/6] ARM: davinci: da850: add GPIO DT node
2013-11-21 18:15 [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Prabhakar Lad
` (3 preceding siblings ...)
2013-11-21 18:15 ` [PATCH v6 4/6] gpio: davinci: add OF support Prabhakar Lad
@ 2013-11-21 18:15 ` Prabhakar Lad
2013-12-15 13:05 ` Sekhar Nori
2013-11-21 18:15 ` [PATCH v6 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries " Prabhakar Lad
2013-11-29 7:47 ` [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Linus Walleij
6 siblings, 1 reply; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-21 18:15 UTC (permalink / raw)
To: linux-arm-kernel
From: KV Sujith <sujithkv@ti.com>
Add DT node for Davinci GPIO driver.
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.dtsi | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 8d17346..b695548 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,19 @@
36
>;
};
+ gpio: gpio at 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-unbanked = <0>;
+ status = "disabled";
+ };
};
nand_cs3 at 62000000 {
compatible = "ti,davinci-nand";
--
1.7.9.5
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v6 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries DT node
2013-11-21 18:15 [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Prabhakar Lad
` (4 preceding siblings ...)
2013-11-21 18:15 ` [PATCH v6 5/6] ARM: davinci: da850: add GPIO DT node Prabhakar Lad
@ 2013-11-21 18:15 ` Prabhakar Lad
2013-12-15 13:13 ` Sekhar Nori
2013-11-29 7:47 ` [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Linus Walleij
6 siblings, 1 reply; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-21 18:15 UTC (permalink / raw)
To: linux-arm-kernel
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 at 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 at 1c42000 {
status = "okay";
@@ -101,6 +116,11 @@
pinctrl-names = "default";
pinctrl-0 = <&mii_pins>;
};
+ gpio: gpio at 1e26000 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&gpio_pins>;
+ };
};
nand_cs3 at 62000000 {
status = "okay";
--
1.7.9.5
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v6 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries DT node
2013-11-21 18:15 ` [PATCH v6 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries " Prabhakar Lad
@ 2013-12-15 13:13 ` Sekhar Nori
2013-12-17 10:20 ` Linus Walleij
0 siblings, 1 reply; 28+ messages in thread
From: Sekhar Nori @ 2013-12-15 13:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
> 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 at 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
> + >;
> + };
> };
Shouldn't these pinmux entries be part of actual device
node which needs them to be muxed this way? For now, I
have committed the attached reduced patch.
Thanks,
Sekhar
---8<---
>From 3a9574f2aa4ffd9b867321a1f298893410bd3718 Mon Sep 17 00:00:00 2001
From: KV Sujith <sujithkv@ti.com>
Date: Thu, 21 Nov 2013 23:45:31 +0530
Subject: [PATCH 1/1] ARM: davinci: da850 evm: add GPIO pinumux entries DT node
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>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
arch/arm/boot/dts/da850-evm.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 588ce58..1e11e5a 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -101,6 +101,9 @@
pinctrl-names = "default";
pinctrl-0 = <&mii_pins>;
};
+ gpio: gpio at 1e26000 {
+ status = "okay";
+ };
};
nand_cs3 at 62000000 {
status = "okay";
--
1.7.10.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v6 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries DT node
2013-12-15 13:13 ` Sekhar Nori
@ 2013-12-17 10:20 ` Linus Walleij
0 siblings, 0 replies; 28+ messages in thread
From: Linus Walleij @ 2013-12-17 10:20 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Dec 15, 2013 at 2:13 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote:
>> From: KV Sujith <sujithkv@ti.com>
>> +
>> + 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
>> + >;
>> + };
>> };
>
> Shouldn't these pinmux entries be part of actual device
> node which needs them to be muxed this way?
The usual way to do it is to set up as states for the device,
or as a hog (on the pin controller itself).
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement
2013-11-21 18:15 [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Prabhakar Lad
` (5 preceding siblings ...)
2013-11-21 18:15 ` [PATCH v6 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries " Prabhakar Lad
@ 2013-11-29 7:47 ` Linus Walleij
2013-11-29 8:30 ` Prabhakar Lad
6 siblings, 1 reply; 28+ messages in thread
From: Linus Walleij @ 2013-11-29 7:47 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 21, 2013 at 7:15 PM, Prabhakar Lad
<prabhakar.csengg@gmail.com> wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
> This patch series does the following
> 1> Ports the driver to use irqdomain.
> 2> Adds dt binding support for gpio-davinci.
> 3> Adds DA850 dt support goio.
>
> Changes for v6:
> 1: GPIO driver now migrated to irq domain legacy.
> 2: Fixed review comments pointed by Grygorii.
> 3: Included Ack's.
This series is looking nice, I assume that Sekhar will take this through
the DaVinci tree once he's happy with it. I think I've ACKed all relevant
patches, else tell me.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement
2013-11-29 7:47 ` [PATCH v6 0/6] gpio: daVinci: cleanup and feature enhancement Linus Walleij
@ 2013-11-29 8:30 ` Prabhakar Lad
0 siblings, 0 replies; 28+ messages in thread
From: Prabhakar Lad @ 2013-11-29 8:30 UTC (permalink / raw)
To: linux-arm-kernel
Hi Linus,
On Fri, Nov 29, 2013 at 1:17 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Nov 21, 2013 at 7:15 PM, Prabhakar Lad
> <prabhakar.csengg@gmail.com> wrote:
>
>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>>
>> This patch series does the following
>> 1> Ports the driver to use irqdomain.
>> 2> Adds dt binding support for gpio-davinci.
>> 3> Adds DA850 dt support goio.
>>
>> Changes for v6:
>> 1: GPIO driver now migrated to irq domain legacy.
>> 2: Fixed review comments pointed by Grygorii.
>> 3: Included Ack's.
>
> This series is looking nice, I assume that Sekhar will take this through
> the DaVinci tree once he's happy with it. I think I've ACKed all relevant
> patches, else tell me.
>
While you are at it, you missed to ACK v6 2/6 patch [1]
[1] https://lkml.org/lkml/2013/11/21/425
Regards,
--Prabhakar
^ permalink raw reply [flat|nested] 28+ messages in thread