* [PATCH v3 0/2] generic irq chip and pl061 domain support
@ 2012-01-30 17:31 Rob Herring
2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring
2012-01-30 17:31 ` [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding Rob Herring
0 siblings, 2 replies; 11+ messages in thread
From: Rob Herring @ 2012-01-30 17:31 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: Grant Likely, shawn.guo, b-cousson, Rob Herring
From: Rob Herring <rob.herring@calxeda.com>
This series adds irq domain support for generic irq chip and updates
pl061 gpio to use it. This is has been significant re-worked from the
previous version based on Grant Likely's irq domain work and to make
the domain support optional as some arches don't use domains. A new
function irq_setup_generic_chip_domain is added to setup a generic irq
chip with a domain.
Shawn, can you test on i.MX again. I think your case with multiple generic
irq chips for 1 device node should work, but I can't test that.
This is based on Grant Likely's irq domain work and is available here:
git://sources.calxeda.com/kernel/linux.git pl061-domain-v3
Rob
Rob Herring (2):
irq: add irq_domain support to generic-chip
gpio: pl061: enable interrupts with DT style binding
.../devicetree/bindings/gpio/pl061-gpio.txt | 15 +++
drivers/gpio/gpio-pl061.c | 39 ++++---
include/linux/irq.h | 10 ++
kernel/irq/generic-chip.c | 134 +++++++++++++++++---
4 files changed, 166 insertions(+), 32 deletions(-)
--
1.7.5.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/2] irq: add irq_domain support to generic-chip
2012-01-30 17:31 [PATCH v3 0/2] generic irq chip and pl061 domain support Rob Herring
@ 2012-01-30 17:31 ` Rob Herring
2012-01-31 14:13 ` Shawn Guo
2012-02-01 0:02 ` Grant Likely
2012-01-30 17:31 ` [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding Rob Herring
1 sibling, 2 replies; 11+ messages in thread
From: Rob Herring @ 2012-01-30 17:31 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: Grant Likely, shawn.guo, b-cousson, Rob Herring, Thomas Gleixner
From: Rob Herring <rob.herring@calxeda.com>
Add irq domain support to irq generic-chip. This enables users of
generic-chip to support dynamic irq assignment needed for DT interrupt
binding.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
include/linux/irq.h | 10 +++
kernel/irq/generic-chip.c | 134 ++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 129 insertions(+), 15 deletions(-)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index bff29c5..482d198 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -665,6 +665,10 @@ struct irq_chip_generic {
void __iomem *reg_base;
unsigned int irq_base;
unsigned int irq_cnt;
+ struct irq_domain *domain;
+ unsigned int flags;
+ unsigned int irq_set;
+ unsigned int irq_clr;
u32 mask_cache;
u32 type_cache;
u32 polarity_cache;
@@ -707,6 +711,12 @@ irq_alloc_generic_chip(const char *name, int nr_ct, unsigned int irq_base,
void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
enum irq_gc_flags flags, unsigned int clr,
unsigned int set);
+
+struct device_node;
+void irq_setup_generic_chip_domain(struct irq_chip_generic *gc,
+ struct device_node *node, u32 msk,
+ enum irq_gc_flags flags, unsigned int clr,
+ unsigned int set);
int irq_setup_alt_chip(struct irq_data *d, unsigned int type);
void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
unsigned int clr, unsigned int set);
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index c89295a..2519663 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -5,6 +5,7 @@
*/
#include <linux/io.h>
#include <linux/irq.h>
+#include <linux/irqdomain.h>
#include <linux/slab.h>
#include <linux/export.h>
#include <linux/interrupt.h>
@@ -39,7 +40,7 @@ void irq_gc_noop(struct irq_data *d)
void irq_gc_mask_disable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = 1 << d->hwirq;
irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->disable);
@@ -57,7 +58,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
void irq_gc_mask_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = 1 << d->hwirq;
irq_gc_lock(gc);
gc->mask_cache |= mask;
@@ -75,7 +76,7 @@ void irq_gc_mask_set_bit(struct irq_data *d)
void irq_gc_mask_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = 1 << d->hwirq;
irq_gc_lock(gc);
gc->mask_cache &= ~mask;
@@ -93,7 +94,7 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
void irq_gc_unmask_enable_reg(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = 1 << d->hwirq;
irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->enable);
@@ -108,7 +109,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
void irq_gc_ack_set_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = 1 << d->hwirq;
irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
@@ -122,7 +123,7 @@ void irq_gc_ack_set_bit(struct irq_data *d)
void irq_gc_ack_clr_bit(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = ~(1 << (d->irq - gc->irq_base));
+ u32 mask = ~(1 << d->hwirq);
irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
@@ -136,7 +137,7 @@ void irq_gc_ack_clr_bit(struct irq_data *d)
void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = 1 << d->hwirq;
irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->mask);
@@ -151,7 +152,7 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
void irq_gc_eoi(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = 1 << d->hwirq;
irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->eoi);
@@ -169,7 +170,7 @@ void irq_gc_eoi(struct irq_data *d)
int irq_gc_set_wake(struct irq_data *d, unsigned int on)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << (d->irq - gc->irq_base);
+ u32 mask = 1 << d->hwirq;
if (!(mask & gc->wake_enabled))
return -EINVAL;
@@ -257,11 +258,99 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
irq_set_chip_and_handler(i, &ct->chip, ct->handler);
irq_set_chip_data(i, gc);
irq_modify_status(i, clr, set);
+ irq_get_irq_data(i)->hwirq = i - gc->irq_base;
}
gc->irq_cnt = i - gc->irq_base;
}
EXPORT_SYMBOL_GPL(irq_setup_generic_chip);
+#ifdef CONFIG_IRQ_DOMAIN
+static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np)
+{
+ struct irq_chip_generic *gc;
+
+ if (d->of_node != NULL && d->of_node == np) {
+ list_for_each_entry(gc, &gc_list, list) {
+ if ((gc == d->host_data) && (d == gc->domain))
+ return 1;
+ }
+ }
+ return 0;
+}
+
+static int irq_gc_irq_domain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ struct irq_chip_generic *gc = d->host_data;
+ struct irq_chip_type *ct = gc->chip_types;
+
+ if (gc->flags & IRQ_GC_INIT_NESTED_LOCK)
+ irq_set_lockdep_class(irq, &irq_nested_lock_class);
+
+ irq_set_chip_and_handler(irq, &ct->chip, ct->handler);
+ irq_set_chip_data(irq, gc);
+ irq_modify_status(irq, gc->irq_clr, gc->irq_set);
+
+ return 0;
+}
+
+static struct irq_domain_ops irq_gc_irq_domain_ops = {
+ .match = irq_gc_irq_domain_match,
+ .map = irq_gc_irq_domain_map,
+ .xlate = irq_domain_xlate_onetwocell,
+};
+
+/*
+ * irq_setup_generic_chip_domain - Setup a range of interrupts with a generic chip and domain
+ * @gc: Generic irq chip holding all data
+ * @node: Device tree node pointer for domain
+ * @msk: Bitmask holding the irqs to initialize relative to gc->irq_base
+ * @flags: Flags for initialization
+ * @clr: IRQ_* bits to clear
+ * @set: IRQ_* bits to set
+ *
+ * Set up max. 32 interrupts starting from gc->irq_base using an irq domain.
+ * Note, this initializes all interrupts to the primary irq_chip_type and its
+ * associated handler.
+ */
+void irq_setup_generic_chip_domain(struct irq_chip_generic *gc,
+ struct device_node *node, u32 msk,
+ enum irq_gc_flags flags, unsigned int clr,
+ unsigned int set)
+{
+ struct irq_chip_type *ct = gc->chip_types;
+
+ if (!node) {
+ irq_setup_generic_chip(gc, msk, flags, clr, set);
+ return;
+ }
+
+ raw_spin_lock(&gc_lock);
+ list_add_tail(&gc->list, &gc_list);
+ raw_spin_unlock(&gc_lock);
+
+ /* Init mask cache ? */
+ if (flags & IRQ_GC_INIT_MASK_CACHE)
+ gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
+
+ gc->flags = flags;
+ gc->irq_clr = clr;
+ gc->irq_set = set;
+
+ /* Users of domains should not use irq_base */
+ if ((int)gc->irq_base > 0)
+ gc->domain = irq_domain_add_legacy(node, fls(msk),
+ gc->irq_base, 0,
+ &irq_gc_irq_domain_ops, gc);
+ else {
+ gc->irq_base = 0;
+ gc->domain = irq_domain_add_linear(node, fls(msk),
+ &irq_gc_irq_domain_ops, gc);
+ }
+}
+EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
+#endif
+
/**
* irq_setup_alt_chip - Switch to alternative chip
* @d: irq_data for this interrupt
@@ -325,8 +414,13 @@ static int irq_gc_suspend(void)
list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
- if (ct->chip.irq_suspend)
- ct->chip.irq_suspend(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_suspend) {
+ int i;
+ int irq = gc->irq_base;
+ for (i = 0; !irq && i < 32; i++)
+ irq = irq_find_mapping(gc->domain, i);
+ ct->chip.irq_suspend(irq_get_irq_data(irq));
+ }
}
return 0;
}
@@ -338,8 +432,13 @@ static void irq_gc_resume(void)
list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
- if (ct->chip.irq_resume)
- ct->chip.irq_resume(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_resume) {
+ int i;
+ int irq = gc->irq_base;
+ for (i = 0; !irq && i < 32; i++)
+ irq = irq_find_mapping(gc->domain, i);
+ ct->chip.irq_resume(irq_get_irq_data(irq));
+ }
}
}
#else
@@ -354,8 +453,13 @@ static void irq_gc_shutdown(void)
list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
- if (ct->chip.irq_pm_shutdown)
- ct->chip.irq_pm_shutdown(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_pm_shutdown) {
+ int i;
+ int irq = gc->irq_base;
+ for (i = 0; !irq && i < 32; i++)
+ irq = irq_find_mapping(gc->domain, i);
+ ct->chip.irq_pm_shutdown(irq_get_irq_data(irq));
+ }
}
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding
2012-01-30 17:31 [PATCH v3 0/2] generic irq chip and pl061 domain support Rob Herring
2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring
@ 2012-01-30 17:31 ` Rob Herring
2012-01-31 14:36 ` Shawn Guo
1 sibling, 1 reply; 11+ messages in thread
From: Rob Herring @ 2012-01-30 17:31 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: Grant Likely, shawn.guo, b-cousson, Rob Herring, Linus Walleij
From: Rob Herring <rob.herring@calxeda.com>
Enable DT interrupt binding support for pl061 gpio lines. If the gpio
node has an interrupt-controller property, then it will be setup to
handle interrupts on gpio lines.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Linus Walleij <linus.ml.walleij@gmail.com>
---
.../devicetree/bindings/gpio/pl061-gpio.txt | 15 ++++++++
drivers/gpio/gpio-pl061.c | 39 +++++++++++---------
2 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/Documentation/devicetree/bindings/gpio/pl061-gpio.txt b/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
index a2c416b..9671d4e 100644
--- a/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
+++ b/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
@@ -8,3 +8,18 @@ Required properties:
- gpio-controller : Marks the device node as a GPIO controller.
- interrupts : Interrupt mapping for GPIO IRQ.
+Optional properties:
+- interrupt-controller : Identifies the node as an interrupt controller. Must
+ be present if using gpios lines for interrupts.
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The type shall be a <u32> and the value shall be 2.
+
+ The 1st cell contains the interrupt number 0-7 corresponding to the gpio
+ line.
+
+ The 2nd cell is the flags, encoding trigger type and level flags.
+ 1 = low-to-high edge triggered
+ 2 = high-to-low edge triggered
+ 4 = active high level-sensitive
+ 8 = active low level-sensitive
+
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 77c9cc7..1657adb 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -15,6 +15,8 @@
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of.h>
#include <linux/bitops.h>
#include <linux/workqueue.h>
#include <linux/gpio.h>
@@ -56,7 +58,6 @@ struct pl061_gpio {
spinlock_t lock; /* GPIO registers */
void __iomem *base;
- int irq_base;
struct irq_chip_generic *irq_gc;
struct gpio_chip gc;
@@ -126,18 +127,16 @@ static void pl061_set_value(struct gpio_chip *gc, unsigned offset, int value)
static int pl061_to_irq(struct gpio_chip *gc, unsigned offset)
{
struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
-
- if (chip->irq_base <= 0)
- return -EINVAL;
-
- return chip->irq_base + offset;
+ if (!chip->irq_gc)
+ return -ENXIO;
+ return irq_find_mapping(chip->irq_gc->domain, offset);
}
static int pl061_irq_type(struct irq_data *d, unsigned trigger)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
struct pl061_gpio *chip = gc->private;
- int offset = d->irq - chip->irq_base;
+ int offset = d->hwirq;
unsigned long flags;
u8 gpiois, gpioibe, gpioiev;
@@ -197,7 +196,8 @@ static void pl061_irq_handler(unsigned irq, struct irq_desc *desc)
chained_irq_exit(irqchip, desc);
}
-static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
+static void __init pl061_init_gc(struct pl061_gpio *chip,
+ struct device_node *node, int irq_base)
{
struct irq_chip_type *ct;
@@ -212,15 +212,17 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
ct->chip.irq_set_wake = irq_gc_set_wake;
ct->regs.mask = GPIOIE;
- irq_setup_generic_chip(chip->irq_gc, IRQ_MSK(PL061_GPIO_NR),
- IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
+ irq_setup_generic_chip_domain(chip->irq_gc, node,
+ IRQ_MSK(PL061_GPIO_NR),
+ IRQ_GC_INIT_NESTED_LOCK,
+ IRQ_NOREQUEST, 0);
}
static int pl061_probe(struct amba_device *dev, const struct amba_id *id)
{
struct pl061_platform_data *pdata;
struct pl061_gpio *chip;
- int ret, irq, i;
+ int ret, irq, i, irq_base;
chip = kzalloc(sizeof(*chip), GFP_KERNEL);
if (chip == NULL)
@@ -229,10 +231,13 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id)
pdata = dev->dev.platform_data;
if (pdata) {
chip->gc.base = pdata->gpio_base;
- chip->irq_base = pdata->irq_base;
+ irq_base = pdata->irq_base;
} else if (dev->dev.of_node) {
chip->gc.base = -1;
- chip->irq_base = 0;
+ if (of_get_property(dev->dev.of_node, "interrupt-controller", NULL))
+ irq_base = -1;
+ else
+ irq_base = 0;
} else {
ret = -ENODEV;
goto free_mem;
@@ -267,13 +272,13 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id)
goto iounmap;
/*
- * irq_chip support
+ * irq_chip support. If irq_base is 0, then we don't support interrupts
+ * on gpio lines and just return now. Otherwise setup the interrupts.
*/
-
- if (chip->irq_base <= 0)
+ if (!irq_base)
return 0;
- pl061_init_gc(chip, chip->irq_base);
+ pl061_init_gc(chip, of_node_get(dev->dev.of_node), irq_base);
writeb(0, chip->base + GPIOIE); /* disable irqs */
irq = dev->irq[0];
--
1.7.5.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip
2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring
@ 2012-01-31 14:13 ` Shawn Guo
2012-01-31 14:32 ` Rob Herring
2012-02-01 0:02 ` Grant Likely
1 sibling, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2012-01-31 14:13 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson,
Rob Herring, Thomas Gleixner
On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote:
...
> +#ifdef CONFIG_IRQ_DOMAIN
> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np)
> +{
> + struct irq_chip_generic *gc;
> +
> + if (d->of_node != NULL && d->of_node == np) {
> + list_for_each_entry(gc, &gc_list, list) {
> + if ((gc == d->host_data) && (d == gc->domain))
> + return 1;
> + }
> + }
IIRC, we talked about this a little bit, but I'm still unsure how this
works for imx5 tzic case, where we have the same one tzic device_node
for 4 irqdomains representing 128 irq lines. It seems to me the match
function here will always find the first irqdomain of the 4 for any
of those 128 tzic irqs.
The following is my code change against your branch for testing. Am I
missing anything?
8<---
diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
index e1b5edf..45abf11 100644
--- a/arch/arm/mach-imx/imx51-dt.c
+++ b/arch/arm/mach-imx/imx51-dt.c
@@ -44,13 +44,6 @@ static const struct of_dev_auxdata
imx51_auxdata_lookup[] __initconst = {
{ /* sentinel */ }
};
-static int __init imx51_tzic_add_irq_domain(struct device_node *np,
- struct device_node *interrupt_parent)
-{
- irq_domain_add_legacy(np, 32, 0, 0, &irq_domain_simple_ops, NULL);
- return 0;
-}
-
static int __init imx51_gpio_add_irq_domain(struct device_node *np,
struct device_node *interrupt_parent)
{
@@ -63,7 +56,6 @@ static int __init imx51_gpio_add_irq_domain(struct
device_node *np,
}
static const struct of_device_id imx51_irq_match[] __initconst = {
- { .compatible = "fsl,imx51-tzic", .data = imx51_tzic_add_irq_domain, },
{ .compatible = "fsl,imx51-gpio", .data = imx51_gpio_add_irq_domain, },
{ /* sentinel */ }
};
diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
index 98308ec..ffb615d 100644
--- a/arch/arm/plat-mxc/tzic.c
+++ b/arch/arm/plat-mxc/tzic.c
@@ -122,7 +122,9 @@ static __init void tzic_init_gc(unsigned int irq_start)
ct->regs.disable = TZIC_ENCLEAR0(idx);
ct->regs.enable = TZIC_ENSET0(idx);
- irq_setup_generic_chip(gc, IRQ_MSK(32), 0, IRQ_NOREQUEST, 0);
+ irq_setup_generic_chip_domain(gc,
+ of_find_compatible_node(NULL, NULL, "fsl,tzic"),
+ IRQ_MSK(32), 0, IRQ_NOREQUEST, 0);
}
asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs)
--->8
> + return 0;
> +}
> +
...
> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc,
> + struct device_node *node, u32 msk,
> + enum irq_gc_flags flags, unsigned int clr,
> + unsigned int set)
> +{
> + struct irq_chip_type *ct = gc->chip_types;
> +
> + if (!node) {
> + irq_setup_generic_chip(gc, msk, flags, clr, set);
> + return;
> + }
> +
> + raw_spin_lock(&gc_lock);
> + list_add_tail(&gc->list, &gc_list);
> + raw_spin_unlock(&gc_lock);
> +
> + /* Init mask cache ? */
> + if (flags & IRQ_GC_INIT_MASK_CACHE)
> + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
> +
> + gc->flags = flags;
> + gc->irq_clr = clr;
> + gc->irq_set = set;
> +
> + /* Users of domains should not use irq_base */
> + if ((int)gc->irq_base > 0)
> + gc->domain = irq_domain_add_legacy(node, fls(msk),
> + gc->irq_base, 0,
> + &irq_gc_irq_domain_ops, gc);
> + else {
> + gc->irq_base = 0;
> + gc->domain = irq_domain_add_linear(node, fls(msk),
> + &irq_gc_irq_domain_ops, gc);
> + }
We have 4 generic_chips for tzic with irq_base as 0, 32, 64, 96. In
this case, we end up with having the first domain as the linear, and
the other 3 as the legacy?
> +}
> +EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
> +#endif
--
Regards,
Shawn
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip
2012-01-31 14:13 ` Shawn Guo
@ 2012-01-31 14:32 ` Rob Herring
2012-01-31 15:06 ` Shawn Guo
0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2012-01-31 14:32 UTC (permalink / raw)
To: Shawn Guo
Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Thomas Gleixner
On 01/31/2012 08:13 AM, Shawn Guo wrote:
> On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote:
> ...
>> +#ifdef CONFIG_IRQ_DOMAIN
>> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np)
>> +{
>> + struct irq_chip_generic *gc;
>> +
>> + if (d->of_node != NULL && d->of_node == np) {
>> + list_for_each_entry(gc, &gc_list, list) {
>> + if ((gc == d->host_data) && (d == gc->domain))
>> + return 1;
>> + }
>> + }
>
> IIRC, we talked about this a little bit, but I'm still unsure how this
> works for imx5 tzic case, where we have the same one tzic device_node
> for 4 irqdomains representing 128 irq lines. It seems to me the match
> function here will always find the first irqdomain of the 4 for any
> of those 128 tzic irqs.
>
> The following is my code change against your branch for testing. Am I
> missing anything?
The irq domain code is a bit different now, so it's matching differently
than before. See the match function. The host_data ptr for a domain is
set to the gc ptr. I then check that the gc->domain matches the domain
passed in. So the fact that 4 domains point to 1 device_node doesn't matter.
>
> 8<---
> diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
> index e1b5edf..45abf11 100644
> --- a/arch/arm/mach-imx/imx51-dt.c
> +++ b/arch/arm/mach-imx/imx51-dt.c
> @@ -44,13 +44,6 @@ static const struct of_dev_auxdata
> imx51_auxdata_lookup[] __initconst = {
> { /* sentinel */ }
> };
>
> -static int __init imx51_tzic_add_irq_domain(struct device_node *np,
> - struct device_node *interrupt_parent)
> -{
> - irq_domain_add_legacy(np, 32, 0, 0, &irq_domain_simple_ops, NULL);
> - return 0;
> -}
> -
> static int __init imx51_gpio_add_irq_domain(struct device_node *np,
> struct device_node *interrupt_parent)
> {
> @@ -63,7 +56,6 @@ static int __init imx51_gpio_add_irq_domain(struct
> device_node *np,
> }
>
> static const struct of_device_id imx51_irq_match[] __initconst = {
> - { .compatible = "fsl,imx51-tzic", .data = imx51_tzic_add_irq_domain, },
> { .compatible = "fsl,imx51-gpio", .data = imx51_gpio_add_irq_domain, },
> { /* sentinel */ }
> };
>
> diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
> index 98308ec..ffb615d 100644
> --- a/arch/arm/plat-mxc/tzic.c
> +++ b/arch/arm/plat-mxc/tzic.c
> @@ -122,7 +122,9 @@ static __init void tzic_init_gc(unsigned int irq_start)
> ct->regs.disable = TZIC_ENCLEAR0(idx);
> ct->regs.enable = TZIC_ENSET0(idx);
>
> - irq_setup_generic_chip(gc, IRQ_MSK(32), 0, IRQ_NOREQUEST, 0);
> + irq_setup_generic_chip_domain(gc,
> + of_find_compatible_node(NULL, NULL, "fsl,tzic"),
> + IRQ_MSK(32), 0, IRQ_NOREQUEST, 0);
Looks right, but I wouldn't lookup the node ptr every time.
> }
>
> asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs)
> --->8
>
>> + return 0;
>> +}
>> +
> ...
>> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc,
>> + struct device_node *node, u32 msk,
>> + enum irq_gc_flags flags, unsigned int clr,
>> + unsigned int set)
>> +{
>> + struct irq_chip_type *ct = gc->chip_types;
>> +
>> + if (!node) {
>> + irq_setup_generic_chip(gc, msk, flags, clr, set);
>> + return;
>> + }
>> +
>> + raw_spin_lock(&gc_lock);
>> + list_add_tail(&gc->list, &gc_list);
>> + raw_spin_unlock(&gc_lock);
>> +
>> + /* Init mask cache ? */
>> + if (flags & IRQ_GC_INIT_MASK_CACHE)
>> + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
>> +
>> + gc->flags = flags;
>> + gc->irq_clr = clr;
>> + gc->irq_set = set;
>> +
>> + /* Users of domains should not use irq_base */
>> + if ((int)gc->irq_base > 0)
>> + gc->domain = irq_domain_add_legacy(node, fls(msk),
>> + gc->irq_base, 0,
>> + &irq_gc_irq_domain_ops, gc);
>> + else {
>> + gc->irq_base = 0;
>> + gc->domain = irq_domain_add_linear(node, fls(msk),
>> + &irq_gc_irq_domain_ops, gc);
>> + }
>
> We have 4 generic_chips for tzic with irq_base as 0, 32, 64, 96. In
> this case, we end up with having the first domain as the linear, and
> the other 3 as the legacy?
Umm, yes. So it should be '>= 0' instead until we stop using IRQ0. I
could base it on NULL node ptr instead...
For the DT case, you want irq_base to be -1.
Rob
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding
2012-01-30 17:31 ` [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding Rob Herring
@ 2012-01-31 14:36 ` Shawn Guo
2012-01-31 14:44 ` Rob Herring
0 siblings, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2012-01-31 14:36 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson,
Rob Herring, Linus Walleij
On Mon, Jan 30, 2012 at 11:31:39AM -0600, Rob Herring wrote:
...
> -static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
> +static void __init pl061_init_gc(struct pl061_gpio *chip,
> + struct device_node *node, int irq_base)
> {
> struct irq_chip_type *ct;
>
> @@ -212,15 +212,17 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
> ct->chip.irq_set_wake = irq_gc_set_wake;
> ct->regs.mask = GPIOIE;
>
> - irq_setup_generic_chip(chip->irq_gc, IRQ_MSK(PL061_GPIO_NR),
> - IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
> + irq_setup_generic_chip_domain(chip->irq_gc, node,
> + IRQ_MSK(PL061_GPIO_NR),
> + IRQ_GC_INIT_NESTED_LOCK,
> + IRQ_NOREQUEST, 0);
> }
The function irq_setup_generic_chip_domain() is wrapped by
#ifdef CONFIG_IRQ_DOMAIN in patch #1. Is it true that pl061 driver
will never work with !IRQ_DOMAIN case?
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding
2012-01-31 14:36 ` Shawn Guo
@ 2012-01-31 14:44 ` Rob Herring
2012-02-01 0:07 ` Grant Likely
0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2012-01-31 14:44 UTC (permalink / raw)
To: Shawn Guo
Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Linus Walleij
On 01/31/2012 08:36 AM, Shawn Guo wrote:
> On Mon, Jan 30, 2012 at 11:31:39AM -0600, Rob Herring wrote:
> ...
>> -static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
>> +static void __init pl061_init_gc(struct pl061_gpio *chip,
>> + struct device_node *node, int irq_base)
>> {
>> struct irq_chip_type *ct;
>>
>> @@ -212,15 +212,17 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
>> ct->chip.irq_set_wake = irq_gc_set_wake;
>> ct->regs.mask = GPIOIE;
>>
>> - irq_setup_generic_chip(chip->irq_gc, IRQ_MSK(PL061_GPIO_NR),
>> - IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
>> + irq_setup_generic_chip_domain(chip->irq_gc, node,
>> + IRQ_MSK(PL061_GPIO_NR),
>> + IRQ_GC_INIT_NESTED_LOCK,
>> + IRQ_NOREQUEST, 0);
>> }
>
> The function irq_setup_generic_chip_domain() is wrapped by
> #ifdef CONFIG_IRQ_DOMAIN in patch #1. Is it true that pl061 driver
> will never work with !IRQ_DOMAIN case?
You're right unless Grant thinks IRQ_DOMAIN should always be enabled for
ARM? Otherwise, I'll add something like this for !IRQ_DOMAIN:
static inline void irq_setup_generic_chip_domain(
struct irq_chip_generic *gc,
struct device_node *node, u32 msk,
enum irq_gc_flags flags, unsigned int clr,
unsigned int set)
{
irq_setup_generic_chip(gc, msk, flags, clr, set);
}
Rob
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip
2012-01-31 14:32 ` Rob Herring
@ 2012-01-31 15:06 ` Shawn Guo
2012-01-31 15:37 ` Rob Herring
0 siblings, 1 reply; 11+ messages in thread
From: Shawn Guo @ 2012-01-31 15:06 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Thomas Gleixner
On Tue, Jan 31, 2012 at 08:32:29AM -0600, Rob Herring wrote:
> On 01/31/2012 08:13 AM, Shawn Guo wrote:
> > On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote:
> > ...
> >> +#ifdef CONFIG_IRQ_DOMAIN
> >> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np)
> >> +{
> >> + struct irq_chip_generic *gc;
> >> +
> >> + if (d->of_node != NULL && d->of_node == np) {
> >> + list_for_each_entry(gc, &gc_list, list) {
> >> + if ((gc == d->host_data) && (d == gc->domain))
> >> + return 1;
> >> + }
> >> + }
> >
> > IIRC, we talked about this a little bit, but I'm still unsure how this
> > works for imx5 tzic case, where we have the same one tzic device_node
> > for 4 irqdomains representing 128 irq lines. It seems to me the match
> > function here will always find the first irqdomain of the 4 for any
> > of those 128 tzic irqs.
> >
> > The following is my code change against your branch for testing. Am I
> > missing anything?
>
> The irq domain code is a bit different now, so it's matching differently
> than before. See the match function. The host_data ptr for a domain is
> set to the gc ptr. I then check that the gc->domain matches the domain
> passed in. So the fact that 4 domains point to 1 device_node doesn't matter.
>
I do not quite follow on that. The gc->domain always matches the
domain passed in anyway for those 4 pairs of domain/gc. That said,
the condition is all true for any of those 4 tzic domains.
if ((gc == d->host_data) && (d == gc->domain))
It does not help on identifying the correct domain from those 4 with
given device_node, which is exactly same for those 4 domains.
> >
> > 8<---
> > diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
> > index e1b5edf..45abf11 100644
> > --- a/arch/arm/mach-imx/imx51-dt.c
> > +++ b/arch/arm/mach-imx/imx51-dt.c
> > @@ -44,13 +44,6 @@ static const struct of_dev_auxdata
> > imx51_auxdata_lookup[] __initconst = {
> > { /* sentinel */ }
> > };
> >
> > -static int __init imx51_tzic_add_irq_domain(struct device_node *np,
> > - struct device_node *interrupt_parent)
> > -{
> > - irq_domain_add_legacy(np, 32, 0, 0, &irq_domain_simple_ops, NULL);
> > - return 0;
> > -}
> > -
> > static int __init imx51_gpio_add_irq_domain(struct device_node *np,
> > struct device_node *interrupt_parent)
> > {
> > @@ -63,7 +56,6 @@ static int __init imx51_gpio_add_irq_domain(struct
> > device_node *np,
> > }
> >
> > static const struct of_device_id imx51_irq_match[] __initconst = {
> > - { .compatible = "fsl,imx51-tzic", .data = imx51_tzic_add_irq_domain, },
> > { .compatible = "fsl,imx51-gpio", .data = imx51_gpio_add_irq_domain, },
> > { /* sentinel */ }
> > };
> >
> > diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
> > index 98308ec..ffb615d 100644
> > --- a/arch/arm/plat-mxc/tzic.c
> > +++ b/arch/arm/plat-mxc/tzic.c
> > @@ -122,7 +122,9 @@ static __init void tzic_init_gc(unsigned int irq_start)
> > ct->regs.disable = TZIC_ENCLEAR0(idx);
> > ct->regs.enable = TZIC_ENSET0(idx);
> >
> > - irq_setup_generic_chip(gc, IRQ_MSK(32), 0, IRQ_NOREQUEST, 0);
> > + irq_setup_generic_chip_domain(gc,
> > + of_find_compatible_node(NULL, NULL, "fsl,tzic"),
> > + IRQ_MSK(32), 0, IRQ_NOREQUEST, 0);
>
> Looks right, but I wouldn't lookup the node ptr every time.
>
Yeah, will avoid it in the final patch.
> > }
> >
> > asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs)
> > --->8
> >
> >> + return 0;
> >> +}
> >> +
> > ...
> >> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc,
> >> + struct device_node *node, u32 msk,
> >> + enum irq_gc_flags flags, unsigned int clr,
> >> + unsigned int set)
> >> +{
> >> + struct irq_chip_type *ct = gc->chip_types;
> >> +
> >> + if (!node) {
> >> + irq_setup_generic_chip(gc, msk, flags, clr, set);
> >> + return;
> >> + }
> >> +
> >> + raw_spin_lock(&gc_lock);
> >> + list_add_tail(&gc->list, &gc_list);
> >> + raw_spin_unlock(&gc_lock);
> >> +
> >> + /* Init mask cache ? */
> >> + if (flags & IRQ_GC_INIT_MASK_CACHE)
> >> + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
> >> +
> >> + gc->flags = flags;
> >> + gc->irq_clr = clr;
> >> + gc->irq_set = set;
> >> +
> >> + /* Users of domains should not use irq_base */
> >> + if ((int)gc->irq_base > 0)
> >> + gc->domain = irq_domain_add_legacy(node, fls(msk),
> >> + gc->irq_base, 0,
> >> + &irq_gc_irq_domain_ops, gc);
> >> + else {
> >> + gc->irq_base = 0;
> >> + gc->domain = irq_domain_add_linear(node, fls(msk),
> >> + &irq_gc_irq_domain_ops, gc);
> >> + }
> >
> > We have 4 generic_chips for tzic with irq_base as 0, 32, 64, 96. In
> > this case, we end up with having the first domain as the linear, and
> > the other 3 as the legacy?
>
> Umm, yes. So it should be '>= 0' instead until we stop using IRQ0. I
> could base it on NULL node ptr instead...
>
> For the DT case, you want irq_base to be -1.
>
Unless we have to, I would keep the same tzic code for dt and non-dt
to save #ifdef CONFIG_OF.
--
Regards,
Shawn
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip
2012-01-31 15:06 ` Shawn Guo
@ 2012-01-31 15:37 ` Rob Herring
0 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2012-01-31 15:37 UTC (permalink / raw)
To: Shawn Guo
Cc: linux-arm-kernel, linux-kernel, Grant Likely, b-cousson, Thomas Gleixner
On 01/31/2012 09:06 AM, Shawn Guo wrote:
> On Tue, Jan 31, 2012 at 08:32:29AM -0600, Rob Herring wrote:
>> On 01/31/2012 08:13 AM, Shawn Guo wrote:
>>> On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote:
>>> ...
>>>> +#ifdef CONFIG_IRQ_DOMAIN
>>>> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np)
>>>> +{
>>>> + struct irq_chip_generic *gc;
>>>> +
>>>> + if (d->of_node != NULL && d->of_node == np) {
>>>> + list_for_each_entry(gc, &gc_list, list) {
>>>> + if ((gc == d->host_data) && (d == gc->domain))
>>>> + return 1;
>>>> + }
>>>> + }
>>>
>>> IIRC, we talked about this a little bit, but I'm still unsure how this
>>> works for imx5 tzic case, where we have the same one tzic device_node
>>> for 4 irqdomains representing 128 irq lines. It seems to me the match
>>> function here will always find the first irqdomain of the 4 for any
>>> of those 128 tzic irqs.
>>>
>>> The following is my code change against your branch for testing. Am I
>>> missing anything?
>>
>> The irq domain code is a bit different now, so it's matching differently
>> than before. See the match function. The host_data ptr for a domain is
>> set to the gc ptr. I then check that the gc->domain matches the domain
>> passed in. So the fact that 4 domains point to 1 device_node doesn't matter.
>>
> I do not quite follow on that. The gc->domain always matches the
> domain passed in anyway for those 4 pairs of domain/gc. That said,
> the condition is all true for any of those 4 tzic domains.
>
> if ((gc == d->host_data) && (d == gc->domain))
>
> It does not help on identifying the correct domain from those 4 with
> given device_node, which is exactly same for those 4 domains.
>
Crap. You're right. It worked in my head... ;)
Let me think about this some more.
>>>
>>> 8<---
>>> diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
>>> index e1b5edf..45abf11 100644
>>> --- a/arch/arm/mach-imx/imx51-dt.c
>>> +++ b/arch/arm/mach-imx/imx51-dt.c
>>> @@ -44,13 +44,6 @@ static const struct of_dev_auxdata
>>> imx51_auxdata_lookup[] __initconst = {
>>> { /* sentinel */ }
>>> };
>>>
>>> -static int __init imx51_tzic_add_irq_domain(struct device_node *np,
>>> - struct device_node *interrupt_parent)
>>> -{
>>> - irq_domain_add_legacy(np, 32, 0, 0, &irq_domain_simple_ops, NULL);
>>> - return 0;
>>> -}
>>> -
>>> static int __init imx51_gpio_add_irq_domain(struct device_node *np,
>>> struct device_node *interrupt_parent)
>>> {
>>> @@ -63,7 +56,6 @@ static int __init imx51_gpio_add_irq_domain(struct
>>> device_node *np,
>>> }
>>>
>>> static const struct of_device_id imx51_irq_match[] __initconst = {
>>> - { .compatible = "fsl,imx51-tzic", .data = imx51_tzic_add_irq_domain, },
>>> { .compatible = "fsl,imx51-gpio", .data = imx51_gpio_add_irq_domain, },
>>> { /* sentinel */ }
>>> };
>>>
>>> diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
>>> index 98308ec..ffb615d 100644
>>> --- a/arch/arm/plat-mxc/tzic.c
>>> +++ b/arch/arm/plat-mxc/tzic.c
>>> @@ -122,7 +122,9 @@ static __init void tzic_init_gc(unsigned int irq_start)
>>> ct->regs.disable = TZIC_ENCLEAR0(idx);
>>> ct->regs.enable = TZIC_ENSET0(idx);
>>>
>>> - irq_setup_generic_chip(gc, IRQ_MSK(32), 0, IRQ_NOREQUEST, 0);
>>> + irq_setup_generic_chip_domain(gc,
>>> + of_find_compatible_node(NULL, NULL, "fsl,tzic"),
>>> + IRQ_MSK(32), 0, IRQ_NOREQUEST, 0);
>>
>> Looks right, but I wouldn't lookup the node ptr every time.
>>
> Yeah, will avoid it in the final patch.
>
>>> }
>>>
>>> asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs)
>>> --->8
>>>
>>>> + return 0;
>>>> +}
>>>> +
>>> ...
>>>> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc,
>>>> + struct device_node *node, u32 msk,
>>>> + enum irq_gc_flags flags, unsigned int clr,
>>>> + unsigned int set)
>>>> +{
>>>> + struct irq_chip_type *ct = gc->chip_types;
>>>> +
>>>> + if (!node) {
>>>> + irq_setup_generic_chip(gc, msk, flags, clr, set);
>>>> + return;
>>>> + }
>>>> +
>>>> + raw_spin_lock(&gc_lock);
>>>> + list_add_tail(&gc->list, &gc_list);
>>>> + raw_spin_unlock(&gc_lock);
>>>> +
>>>> + /* Init mask cache ? */
>>>> + if (flags & IRQ_GC_INIT_MASK_CACHE)
>>>> + gc->mask_cache = irq_reg_readl(gc->reg_base + ct->regs.mask);
>>>> +
>>>> + gc->flags = flags;
>>>> + gc->irq_clr = clr;
>>>> + gc->irq_set = set;
>>>> +
>>>> + /* Users of domains should not use irq_base */
>>>> + if ((int)gc->irq_base > 0)
>>>> + gc->domain = irq_domain_add_legacy(node, fls(msk),
>>>> + gc->irq_base, 0,
>>>> + &irq_gc_irq_domain_ops, gc);
>>>> + else {
>>>> + gc->irq_base = 0;
>>>> + gc->domain = irq_domain_add_linear(node, fls(msk),
>>>> + &irq_gc_irq_domain_ops, gc);
>>>> + }
>>>
>>> We have 4 generic_chips for tzic with irq_base as 0, 32, 64, 96. In
>>> this case, we end up with having the first domain as the linear, and
>>> the other 3 as the legacy?
>>
>> Umm, yes. So it should be '>= 0' instead until we stop using IRQ0. I
>> could base it on NULL node ptr instead...
>>
>> For the DT case, you want irq_base to be -1.
>>
> Unless we have to, I would keep the same tzic code for dt and non-dt
> to save #ifdef CONFIG_OF.
You need to stop using Linux vIRQ0 as a valid vIRQ# and with DT, all
knowledge about irq_base should be removed. Whether that can be
accomplished with the same code is a separate issue.
Rob
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] irq: add irq_domain support to generic-chip
2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring
2012-01-31 14:13 ` Shawn Guo
@ 2012-02-01 0:02 ` Grant Likely
1 sibling, 0 replies; 11+ messages in thread
From: Grant Likely @ 2012-02-01 0:02 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, linux-kernel, shawn.guo, b-cousson,
Rob Herring, Thomas Gleixner
On Mon, Jan 30, 2012 at 11:31:38AM -0600, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Add irq domain support to irq generic-chip. This enables users of
> generic-chip to support dynamic irq assignment needed for DT interrupt
> binding.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
> @@ -39,7 +40,7 @@ void irq_gc_noop(struct irq_data *d)
> void irq_gc_mask_disable_reg(struct irq_data *d)
> {
> struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> - u32 mask = 1 << (d->irq - gc->irq_base);
> + u32 mask = 1 << d->hwirq;
As discussed on IRC, there needs to be a 1:N relationship between
an irq_domain and generic chips, but doing that means that hwirq no
longer directly maps to the bit in the register.
This could be solved however if a mod is applied to the hwirq number
and if we're careful to line up hwirqs to those mod boundaries:
u32 mask = 1 << (d->hwirq % gc->bank_size);
> }
> gc->irq_cnt = i - gc->irq_base;
> }
> EXPORT_SYMBOL_GPL(irq_setup_generic_chip);
>
> +#ifdef CONFIG_IRQ_DOMAIN
> +static int irq_gc_irq_domain_match(struct irq_domain *d, struct device_node *np)
> +{
> + struct irq_chip_generic *gc;
> +
> + if (d->of_node != NULL && d->of_node == np) {
> + list_for_each_entry(gc, &gc_list, list) {
> + if ((gc == d->host_data) && (d == gc->domain))
> + return 1;
> + }
> + }
> + return 0;
> +}
> +
> +static int irq_gc_irq_domain_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hw)
> +{
> + struct irq_chip_generic *gc = d->host_data;
> + struct irq_chip_type *ct = gc->chip_types;
> +
> + if (gc->flags & IRQ_GC_INIT_NESTED_LOCK)
> + irq_set_lockdep_class(irq, &irq_nested_lock_class);
> +
> + irq_set_chip_and_handler(irq, &ct->chip, ct->handler);
> + irq_set_chip_data(irq, gc);
> + irq_modify_status(irq, gc->irq_clr, gc->irq_set);
> +
> + return 0;
> +}
> +
> +static struct irq_domain_ops irq_gc_irq_domain_ops = {
> + .match = irq_gc_irq_domain_match,
> + .map = irq_gc_irq_domain_map,
> + .xlate = irq_domain_xlate_onetwocell,
> +};
> +
> +/*
> + * irq_setup_generic_chip_domain - Setup a range of interrupts with a generic chip and domain
> + * @gc: Generic irq chip holding all data
> + * @node: Device tree node pointer for domain
> + * @msk: Bitmask holding the irqs to initialize relative to gc->irq_base
> + * @flags: Flags for initialization
> + * @clr: IRQ_* bits to clear
> + * @set: IRQ_* bits to set
> + *
> + * Set up max. 32 interrupts starting from gc->irq_base using an irq domain.
> + * Note, this initializes all interrupts to the primary irq_chip_type and its
> + * associated handler.
> + */
> +void irq_setup_generic_chip_domain(struct irq_chip_generic *gc,
> + struct device_node *node, u32 msk,
> + enum irq_gc_flags flags, unsigned int clr,
> + unsigned int set)
> +{
> + struct irq_chip_type *ct = gc->chip_types;
> +
> + if (!node) {
> + irq_setup_generic_chip(gc, msk, flags, clr, set);
> + return;
> + }
Calling irq_setup_generic_chip() should always be performed,
regardless of whether or not a node is passed in. However, the msk
field should be passed as 0 so that none of the irqs get configured
before they are allocated. I'd like to see all the domain code paths
look the same, regardless of whether a node pointer is provided.
The only quirk here is that if a node isn't provided, then that
probably means the range of irqs needs to be fixed; which means using
the legacy map in the current implementation. We should sit down and
talk about this next week at Connect.
g.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding
2012-01-31 14:44 ` Rob Herring
@ 2012-02-01 0:07 ` Grant Likely
0 siblings, 0 replies; 11+ messages in thread
From: Grant Likely @ 2012-02-01 0:07 UTC (permalink / raw)
To: Rob Herring
Cc: Shawn Guo, linux-arm-kernel, linux-kernel, b-cousson, Linus Walleij
On Tue, Jan 31, 2012 at 08:44:19AM -0600, Rob Herring wrote:
> On 01/31/2012 08:36 AM, Shawn Guo wrote:
> > On Mon, Jan 30, 2012 at 11:31:39AM -0600, Rob Herring wrote:
> > ...
> >> -static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
> >> +static void __init pl061_init_gc(struct pl061_gpio *chip,
> >> + struct device_node *node, int irq_base)
> >> {
> >> struct irq_chip_type *ct;
> >>
> >> @@ -212,15 +212,17 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
> >> ct->chip.irq_set_wake = irq_gc_set_wake;
> >> ct->regs.mask = GPIOIE;
> >>
> >> - irq_setup_generic_chip(chip->irq_gc, IRQ_MSK(PL061_GPIO_NR),
> >> - IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
> >> + irq_setup_generic_chip_domain(chip->irq_gc, node,
> >> + IRQ_MSK(PL061_GPIO_NR),
> >> + IRQ_GC_INIT_NESTED_LOCK,
> >> + IRQ_NOREQUEST, 0);
> >> }
> >
> > The function irq_setup_generic_chip_domain() is wrapped by
> > #ifdef CONFIG_IRQ_DOMAIN in patch #1. Is it true that pl061 driver
> > will never work with !IRQ_DOMAIN case?
>
> You're right unless Grant thinks IRQ_DOMAIN should always be enabled for
> ARM? Otherwise, I'll add something like this for !IRQ_DOMAIN:
I think always having IRQ_DOMAIN on are is where we want to get to; or at least
have platforms that use it depend on it or select it. I don't want to see
interrupt controllers both use irq_domain and also their own open-coded
translation mechanism. That's just a fail.
If irq_domain isn't acceptable to be enabled all the time, then it needs to be
refactored until it is.
g.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-02-01 0:07 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-30 17:31 [PATCH v3 0/2] generic irq chip and pl061 domain support Rob Herring
2012-01-30 17:31 ` [PATCH v3 1/2] irq: add irq_domain support to generic-chip Rob Herring
2012-01-31 14:13 ` Shawn Guo
2012-01-31 14:32 ` Rob Herring
2012-01-31 15:06 ` Shawn Guo
2012-01-31 15:37 ` Rob Herring
2012-02-01 0:02 ` Grant Likely
2012-01-30 17:31 ` [PATCH v3 2/2] gpio: pl061: enable interrupts with DT style binding Rob Herring
2012-01-31 14:36 ` Shawn Guo
2012-01-31 14:44 ` Rob Herring
2012-02-01 0:07 ` Grant Likely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).