* [PATCH v4 0/6] gpio: daVinci: Fixes and feature enhancement
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio, Lad Prabhakar,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 0/6] gpio: daVinci: Fixes and feature enhancement
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-02 15:39 ` Lad, Prabhakar
@ 2013-11-02 15:39 ` Lad, Prabhakar
-1 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio, Lad Prabhakar,
Grygorii Strashko
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 2/6] gpio: davinci: remove unnecessary printk
2013-11-02 15:39 ` Lad, Prabhakar
@ 2013-11-02 15:39 ` Lad, Prabhakar
-1 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio, Lad Prabhakar,
Grygorii Strashko
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 2/6] gpio: davinci: remove unnecessary printk
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 3/6] gpio: davinci: use irqdomain
2013-11-02 15:39 ` Lad, Prabhakar
(?)
@ 2013-11-02 15:39 ` Lad, Prabhakar
-1 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Lad Prabhakar,
Grygorii Strashko
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 3/6] gpio: davinci: use irqdomain
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio, Lad Prabhakar,
Grygorii Strashko
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
This patch converts the davinci gpio driver to use irqdomain
support.
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(-)
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 3/6] gpio: davinci: use irqdomain
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: linux-arm-kernel
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
This patch converts the davinci gpio driver to use irqdomain
support.
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(-)
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 4/6] gpio: davinci: add OF support
2013-11-02 15:39 ` Lad, Prabhakar
@ 2013-11-02 15:39 ` Lad, Prabhakar
-1 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio, Lad Prabhakar,
Grygorii Strashko
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 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/.
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 | 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 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-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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 5/6] ARM: davinci: da850: add GPIO DT node
2013-11-02 15:39 ` Lad, Prabhakar
(?)
@ 2013-11-02 15:39 ` Lad, Prabhakar
-1 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Lad Prabhakar,
Grygorii Strashko
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 5/6] ARM: davinci: da850: add GPIO DT node
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio, Lad Prabhakar,
Grygorii Strashko
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 | 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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 5/6] ARM: davinci: da850: add GPIO DT node
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 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 | 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 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-irq-base = <101>;
+ 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] 59+ messages in thread
* [PATCH v4 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries DT node
2013-11-02 15:39 ` Lad, Prabhakar
@ 2013-11-02 15:39 ` Lad, Prabhakar
-1 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 UTC (permalink / raw)
To: Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio, Lad Prabhakar,
Grygorii Strashko
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries DT node
@ 2013-11-02 15:39 ` Lad, Prabhakar
0 siblings, 0 replies; 59+ messages in thread
From: Lad, Prabhakar @ 2013-11-02 15:39 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] 59+ messages in thread
* Re: [PATCH v4 3/6] gpio: davinci: use irqdomain
2013-11-02 15:39 ` Lad, Prabhakar
(?)
@ 2013-11-04 18:27 ` Grygorii Strashko
-1 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-04 18:27 UTC (permalink / raw)
To: Lad, Prabhakar, Sekhar Nori, Linus Walleij,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Mark Rutland, Alex Elder,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Russell King, Pawel Moll, Ian Campbell, Stephen Warren,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Landley, Grant Likely,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 3/6] gpio: davinci: use irqdomain
@ 2013-11-04 18:27 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-04 18:27 UTC (permalink / raw)
To: Lad, Prabhakar, Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio
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
>
> 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):
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 3/6] gpio: davinci: use irqdomain
@ 2013-11-04 18:27 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-04 18:27 UTC (permalink / raw)
To: linux-arm-kernel
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
>
> 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):
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
2013-11-02 15:39 ` Lad, Prabhakar
(?)
@ 2013-11-04 18:28 ` Grygorii Strashko
-1 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-04 18:28 UTC (permalink / raw)
To: Lad, Prabhakar, Sekhar Nori, Linus Walleij,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Mark Rutland, Alex Elder,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Russell King, Pawel Moll, Ian Campbell, Stephen Warren,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Landley, Grant Likely,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-04 18:28 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-04 18:28 UTC (permalink / raw)
To: Lad, Prabhakar, Sekhar Nori, Linus Walleij, devicetree
Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
Ian Campbell, Rob Landley, Russell King, Grant Likely,
Alex Elder, linux-doc, linux-kernel, linux-arm-kernel,
davinci-linux-open-source, linux-gpio
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?
- 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.
> +
> +- 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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-04 18:28 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-04 18:28 UTC (permalink / raw)
To: linux-arm-kernel
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?
- 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 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 | 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 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-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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 3/6] gpio: davinci: use irqdomain
2013-11-04 18:27 ` Grygorii Strashko
@ 2013-11-05 8:00 ` Prabhakar Lad
-1 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-05 8:00 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Sekhar Nori, Linus Walleij, devicetree, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
Russell King, Grant Likely, Alex Elder, LDOC, LKML, LAK, dlos,
linux-gpio
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 3/6] gpio: davinci: use irqdomain
@ 2013-11-05 8:00 ` Prabhakar Lad
0 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-05 8:00 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
2013-11-04 18:28 ` Grygorii Strashko
@ 2013-11-05 8:53 ` Prabhakar Lad
-1 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-05 8:53 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Sekhar Nori, Linus Walleij, devicetree, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
Russell King, Grant Likely, Alex Elder, LDOC, LKML, LAK, dlos,
linux-gpio
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-05 8:53 ` Prabhakar Lad
0 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-05 8:53 UTC (permalink / raw)
To: linux-arm-kernel
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 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 | 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 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-irq-base = <101>;
>
>
> ^^ Is it still needed?
>
OOps missed to remove that.
Regards,
--Prabhakar Lad
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-02 15:39 ` Lad, Prabhakar
(?)
@ 2013-11-05 12:39 ` Linus Walleij
-1 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-05 12:39 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-05 12:39 ` Linus Walleij
0 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-05 12:39 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-05 12:39 ` Linus Walleij
0 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-05 12:39 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
2013-11-05 8:53 ` Prabhakar Lad
(?)
@ 2013-11-05 16:59 ` Grygorii Strashko
-1 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-05 16:59 UTC (permalink / raw)
To: Prabhakar Lad
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, dlos,
Russell King, LDOC, Pawel Moll, Stephen Warren, Linus Walleij,
Ian Campbell, LKML, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Landley, Alex Elder,
Grant Likely, LAK
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-05 16:59 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-05 16:59 UTC (permalink / raw)
To: Prabhakar Lad
Cc: Sekhar Nori, Linus Walleij, devicetree, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
Russell King, Grant Likely, Alex Elder, LDOC, LKML, LAK, dlos,
linux-gpio
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>;
}
>
>> - 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 */
};
>
>>
>>> +
>>> +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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-05 16:59 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-05 16:59 UTC (permalink / raw)
To: linux-arm-kernel
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>;
}
>
>> - 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 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 | 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 at 0 {
compatible = "devX";
interrupt-parent = <&gpio>;
interrupts = <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */
};
>
>>
>>> +
>>> +Example:
>>> +
>>> +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-irq-base = <101>;
>>
>>
>> ^^ Is it still needed?
>>
> OOps missed to remove that.
>
Regards,
-grygorii
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-05 12:39 ` Linus Walleij
(?)
@ 2013-11-06 9:33 ` Prabhakar Lad
-1 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 9:33 UTC (permalink / raw)
To: Linus Walleij
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 9:33 ` Prabhakar Lad
0 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 9:33 UTC (permalink / raw)
To: Linus Walleij
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 9:33 ` Prabhakar Lad
0 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 9:33 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-06 9:33 ` Prabhakar Lad
(?)
@ 2013-11-06 9:56 ` Linus Walleij
-1 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-06 9:56 UTC (permalink / raw)
To: Prabhakar Lad
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 9:56 ` Linus Walleij
0 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-06 9:56 UTC (permalink / raw)
To: Prabhakar Lad
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 9:56 ` Linus Walleij
0 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-06 9:56 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
2013-11-05 16:59 ` Grygorii Strashko
(?)
@ 2013-11-06 10:08 ` Prabhakar Lad
-1 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 10:08 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Sekhar Nori, Linus Walleij, devicetree, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
Russell King, Grant Likely, Alex Elder, LDOC, LKML, LAK, dlos,
linux-gpio
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-06 10:08 ` Prabhakar Lad
0 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 10:08 UTC (permalink / raw)
To: Grygorii Strashko
Cc: Sekhar Nori, Linus Walleij, devicetree, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
Russell King, Grant Likely, Alex Elder, LDOC, LKML, LAK, dlos,
linux-gpio
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-06 10:08 ` Prabhakar Lad
0 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 10:08 UTC (permalink / raw)
To: linux-arm-kernel
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 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 | 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 at 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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-06 9:56 ` Linus Walleij
(?)
@ 2013-11-06 10:15 ` Prabhakar Lad
-1 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 10:15 UTC (permalink / raw)
To: Linus Walleij
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 10:15 ` Prabhakar Lad
0 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 10:15 UTC (permalink / raw)
To: Linus Walleij
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 10:15 ` Prabhakar Lad
0 siblings, 0 replies; 59+ messages in thread
From: Prabhakar Lad @ 2013-11-06 10:15 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-06 10:15 ` Prabhakar Lad
(?)
@ 2013-11-06 10:49 ` Sekhar Nori
-1 siblings, 0 replies; 59+ messages in thread
From: Sekhar Nori @ 2013-11-06 10:49 UTC (permalink / raw)
To: Prabhakar Lad, Linus Walleij
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Russell King, Pawel Moll, linux-doc-u79uwXL29TY76Z2rM5mHXA,
Stephen Warren, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Landley, Alex Elder,
Grant Likely, Ian Campbell,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 10:49 ` Sekhar Nori
0 siblings, 0 replies; 59+ messages in thread
From: Sekhar Nori @ 2013-11-06 10:49 UTC (permalink / raw)
To: Prabhakar Lad, Linus Walleij
Cc: Mark Rutland, devicetree, davinci-linux-open-source,
Grygorii Strashko, Russell King, linux-doc, Pawel Moll,
Stephen Warren, Ian Campbell, linux-kernel, Rob Herring,
linux-gpio, Rob Landley, Alex Elder, Grant Likely,
linux-arm-kernel
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@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.
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 10:49 ` Sekhar Nori
0 siblings, 0 replies; 59+ messages in thread
From: Sekhar Nori @ 2013-11-06 10:49 UTC (permalink / raw)
To: linux-arm-kernel
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@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.
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-06 10:49 ` Sekhar Nori
(?)
@ 2013-11-06 11:33 ` Grygorii Strashko
-1 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-06 11:33 UTC (permalink / raw)
To: Sekhar Nori, Prabhakar Lad, Linus Walleij
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Russell King, Pawel Moll, linux-doc-u79uwXL29TY76Z2rM5mHXA,
Stephen Warren, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Landley, Alex Elder,
Grant Likely, Ian Campbell,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 11:33 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-06 11:33 UTC (permalink / raw)
To: Sekhar Nori, Prabhakar Lad, Linus Walleij
Cc: Mark Rutland, devicetree, davinci-linux-open-source,
Russell King, linux-doc, Pawel Moll, Stephen Warren,
Ian Campbell, linux-kernel, Rob Herring, linux-gpio, Rob Landley,
Alex Elder, Grant Likely, linux-arm-kernel
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@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.
>
> 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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 11:33 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-06 11:33 UTC (permalink / raw)
To: linux-arm-kernel
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@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.
>
> 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
^ permalink raw reply related [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-06 10:15 ` Prabhakar Lad
(?)
@ 2013-11-06 11:36 ` Linus Walleij
-1 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-06 11:36 UTC (permalink / raw)
To: Prabhakar Lad
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 11:36 ` Linus Walleij
0 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-06 11:36 UTC (permalink / raw)
To: Prabhakar Lad
Cc: Sekhar Nori, devicetree, Rob Herring, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Rob Landley, Russell King,
Grant Likely, Alex Elder, linux-doc, linux-kernel,
linux-arm-kernel, davinci-linux-open-source, linux-gpio,
Grygorii Strashko
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 11:36 ` Linus Walleij
0 siblings, 0 replies; 59+ messages in thread
From: Linus Walleij @ 2013-11-06 11:36 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
2013-11-06 11:33 ` Grygorii Strashko
(?)
@ 2013-11-06 11:42 ` Sekhar Nori
-1 siblings, 0 replies; 59+ messages in thread
From: Sekhar Nori @ 2013-11-06 11:42 UTC (permalink / raw)
To: Grygorii Strashko, Prabhakar Lad, Linus Walleij
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Russell King, Pawel Moll, linux-doc-u79uwXL29TY76Z2rM5mHXA,
Stephen Warren, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Landley, Alex Elder,
Grant Likely, Ian Campbell,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 11:42 ` Sekhar Nori
0 siblings, 0 replies; 59+ messages in thread
From: Sekhar Nori @ 2013-11-06 11:42 UTC (permalink / raw)
To: Grygorii Strashko, Prabhakar Lad, Linus Walleij
Cc: Mark Rutland, devicetree, davinci-linux-open-source,
Russell King, linux-doc, Pawel Moll, Stephen Warren,
Ian Campbell, linux-kernel, Rob Herring, linux-gpio, Rob Landley,
Alex Elder, Grant Likely, linux-arm-kernel
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@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.
>>
>> 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
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio
@ 2013-11-06 11:42 ` Sekhar Nori
0 siblings, 0 replies; 59+ messages in thread
From: Sekhar Nori @ 2013-11-06 11:42 UTC (permalink / raw)
To: linux-arm-kernel
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@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.
>>
>> 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
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
2013-11-06 10:08 ` Prabhakar Lad
(?)
@ 2013-11-06 11:55 ` Grygorii Strashko
-1 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-06 11:55 UTC (permalink / raw)
To: Prabhakar Lad
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, dlos,
Russell King, LDOC, Pawel Moll, Stephen Warren, Linus Walleij,
Ian Campbell, LKML, Rob Herring,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, Rob Landley, Alex Elder,
Grant Likely, LAK
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
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* Re: [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-06 11:55 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-06 11:55 UTC (permalink / raw)
To: Prabhakar Lad
Cc: Sekhar Nori, Linus Walleij, devicetree, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
Russell King, Grant Likely, Alex Elder, LDOC, LKML, LAK, dlos,
linux-gpio
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@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
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
>
^ permalink raw reply [flat|nested] 59+ messages in thread
* [PATCH v4 4/6] gpio: davinci: add OF support
@ 2013-11-06 11:55 ` Grygorii Strashko
0 siblings, 0 replies; 59+ messages in thread
From: Grygorii Strashko @ 2013-11-06 11:55 UTC (permalink / raw)
To: linux-arm-kernel
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@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 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 | 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 at 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
>
^ permalink raw reply [flat|nested] 59+ messages in thread
end of thread, other threads:[~2013-11-06 11:58 UTC | newest]
Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-02 15:39 [PATCH v4 0/6] gpio: daVinci: Fixes and feature enhancement Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
2013-11-02 15:39 ` [PATCH v4 1/6] gpio: davinci: Fixed a check for unbanked gpio Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
2013-11-05 12:39 ` Linus Walleij
2013-11-05 12:39 ` Linus Walleij
2013-11-05 12:39 ` Linus Walleij
2013-11-06 9:33 ` Prabhakar Lad
2013-11-06 9:33 ` Prabhakar Lad
2013-11-06 9:33 ` Prabhakar Lad
2013-11-06 9:56 ` Linus Walleij
2013-11-06 9:56 ` Linus Walleij
2013-11-06 9:56 ` Linus Walleij
2013-11-06 10:15 ` Prabhakar Lad
2013-11-06 10:15 ` Prabhakar Lad
2013-11-06 10:15 ` Prabhakar Lad
[not found] ` <CA+V-a8tZTm9bh+arQ+uFyTVHxfdf07bWaM_Sdqo08TO=UCMMUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-11-06 10:49 ` Sekhar Nori
2013-11-06 10:49 ` Sekhar Nori
2013-11-06 10:49 ` Sekhar Nori
[not found] ` <527A1EC7.20400-l0cyMroinI0@public.gmane.org>
2013-11-06 11:33 ` Grygorii Strashko
2013-11-06 11:33 ` Grygorii Strashko
2013-11-06 11:33 ` Grygorii Strashko
[not found] ` <527A28EF.9070601-l0cyMroinI0@public.gmane.org>
2013-11-06 11:42 ` Sekhar Nori
2013-11-06 11:42 ` Sekhar Nori
2013-11-06 11:42 ` Sekhar Nori
2013-11-06 11:36 ` Linus Walleij
2013-11-06 11:36 ` Linus Walleij
2013-11-06 11:36 ` Linus Walleij
2013-11-02 15:39 ` [PATCH v4 2/6] gpio: davinci: remove unnecessary printk Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
[not found] ` <1383406775-14902-1-git-send-email-prabhakar.csenng-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-11-02 15:39 ` [PATCH v4 3/6] gpio: davinci: use irqdomain Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
[not found] ` <1383406775-14902-4-git-send-email-prabhakar.csenng-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-11-04 18:27 ` Grygorii Strashko
2013-11-04 18:27 ` Grygorii Strashko
2013-11-04 18:27 ` Grygorii Strashko
2013-11-05 8:00 ` Prabhakar Lad
2013-11-05 8:00 ` Prabhakar Lad
2013-11-02 15:39 ` [PATCH v4 5/6] ARM: davinci: da850: add GPIO DT node Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
2013-11-02 15:39 ` [PATCH v4 4/6] gpio: davinci: add OF support Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
[not found] ` <1383406775-14902-5-git-send-email-prabhakar.csenng-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-11-04 18:28 ` Grygorii Strashko
2013-11-04 18:28 ` Grygorii Strashko
2013-11-04 18:28 ` Grygorii Strashko
2013-11-05 8:53 ` Prabhakar Lad
2013-11-05 8:53 ` Prabhakar Lad
[not found] ` <CA+V-a8uV0g36LoQtxFjR56SaMxj7KOE2ztC+pfT_Pn1wGcqEmw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-11-05 16:59 ` Grygorii Strashko
2013-11-05 16:59 ` Grygorii Strashko
2013-11-05 16:59 ` Grygorii Strashko
2013-11-06 10:08 ` Prabhakar Lad
2013-11-06 10:08 ` Prabhakar Lad
2013-11-06 10:08 ` Prabhakar Lad
[not found] ` <CA+V-a8tJ7yDJOPamoCiaX6Twnnqxp_o_L0rzo2dEeSDS2VoYLw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-11-06 11:55 ` Grygorii Strashko
2013-11-06 11:55 ` Grygorii Strashko
2013-11-06 11:55 ` Grygorii Strashko
2013-11-02 15:39 ` [PATCH v4 6/6] ARM: davinci: da850 evm: add GPIO pinumux entries DT node Lad, Prabhakar
2013-11-02 15:39 ` Lad, Prabhakar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.