2012-02-03 22:35:22

by Rob Herring

[permalink] [raw]
Subject: [PATCH v4 0/4] generic irq chip domain support

From: Rob Herring <[email protected]>

This series adds irq domain support for generic irq chip and updates
pl061 gpio and imx tzic to use it.

Once again, this is has been significantly re-worked from the previous
version. It's simply not possible to add domain support in a transparent
way. So now a wrapper function is added to setup a domain and alloc and
setup N gen irq chips.

There was some discussion about supporting multiple banks of less than 32
irqs per bank. Looking at all users of generic irq chip, this doesn't
appear to be needed. All users are either a single generic irq chip per
node or have 32 irqs per bank. Both are supported by this patch series.
If bank masking is needed, it can be added later.

This is based on Grant Likely's irq domain work and is available here:

git://sources.calxeda.com/kernel/linux.git pl061-domain-v4

Rob

Rob Herring (4):
ARM: kconfig: always select IRQ_DOMAIN
irq: add irq_domain support to generic-chip
ARM: imx: add irq domain support to tzic
gpio: pl061: enable interrupts with DT style binding

.../devicetree/bindings/gpio/pl061-gpio.txt | 15 ++
arch/arm/Kconfig | 2 +-
arch/arm/common/Kconfig | 2 -
arch/arm/plat-mxc/tzic.c | 23 ++--
drivers/gpio/gpio-pl061.c | 48 +++---
include/linux/irq.h | 15 ++
kernel/irq/generic-chip.c | 154 +++++++++++++++++---
7 files changed, 198 insertions(+), 61 deletions(-)

--
1.7.5.4


2012-02-03 22:35:27

by Rob Herring

[permalink] [raw]
Subject: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

From: Rob Herring <[email protected]>

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 <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
include/linux/irq.h | 15 +++++
kernel/irq/generic-chip.c | 154 ++++++++++++++++++++++++++++++++++++++-------
2 files changed, 147 insertions(+), 22 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index bff29c5..d721abc 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -664,7 +664,12 @@ struct irq_chip_generic {
raw_spinlock_t lock;
void __iomem *reg_base;
unsigned int irq_base;
+ unsigned int hwirq_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 +712,16 @@ 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;
+int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
+ int num_ct, unsigned int irq_base,
+ void __iomem *reg_base,
+ irq_flow_handler_t handler, u32 hwirq_cnt,
+ enum irq_gc_flags flags,
+ unsigned int clr, unsigned int set,
+ void (*gc_init_cb)(struct irq_chip_generic *),
+ void *private);
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..ffbe6fe 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) % 32;

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) % 32;

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) % 32;

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) % 32;

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) % 32;

irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
@@ -122,10 +123,10 @@ 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) % 32;

irq_gc_lock(gc);
- irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+ irq_reg_writel(~mask, gc->reg_base + cur_regs(d)->ack);
irq_gc_unlock(gc);
}

@@ -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) % 32;

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) % 32;

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) % 32;

if (!(mask & gc->wake_enabled))
return -EINVAL;
@@ -220,6 +221,18 @@ EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
*/
static struct lock_class_key irq_nested_lock_class;

+static void irq_gc_setup_irq(struct irq_chip_generic *gc, int irq)
+{
+ 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);
+}
+
/**
* irq_setup_generic_chip - Setup a range of interrupts with a generic chip
* @gc: Generic irq chip holding all data
@@ -247,21 +260,115 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
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;
+
for (i = gc->irq_base; msk; msk >>= 1, i++) {
if (!(msk & 0x01))
continue;

- if (flags & IRQ_GC_INIT_NESTED_LOCK)
- irq_set_lockdep_class(i, &irq_nested_lock_class);
-
- irq_set_chip_and_handler(i, &ct->chip, ct->handler);
- irq_set_chip_data(i, gc);
- irq_modify_status(i, clr, set);
+ irq_gc_setup_irq(gc, i);
+ 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_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ struct irq_chip_generic **gc_array = d->host_data;
+ struct irq_chip_generic *gc = gc_array[hw / 32];
+
+ /* We need a valid irq number for suspend/resume functions */
+ if ((int)gc->irq_base == -1)
+ gc->irq_base = irq;
+ irq_gc_setup_irq(gc, irq);
+ return 0;
+}
+
+static struct irq_domain_ops irq_gc_irq_domain_ops = {
+ .map = irq_gc_irq_domain_map,
+ .xlate = irq_domain_xlate_onetwocell,
+};
+
+/*
+ * irq_setup_generic_chip_domain - Setup a domain and N generic chips
+ * @name: Name of the irq chip
+ * @node: Device tree node pointer for domain
+ * @num_ct: Number of irq_chip_type instances associated with this
+ * @irq_base: Interrupt base nr for this chip
+ * @reg_base: Register base address (virtual)
+ * @handler: Default flow handler associated with this chip
+ * @hwirq_cnt: Number of hw irqs for the domain
+ * @flags: Flags for initialization
+ * @clr: IRQ_* bits to clear
+ * @set: IRQ_* bits to set
+ * @gc_init_cb: Init callback function called for each generic irq chip created
+ * @private Ptr to caller private data
+ *
+ * Set up an irq domain and N banks of 32 interrupts starting from gc->irq_base.
+ * Note, this initializes all interrupts to the primary irq_chip_type and its
+ * associated handler.
+ */
+int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
+ int num_ct, unsigned int irq_base,
+ void __iomem *reg_base,
+ irq_flow_handler_t handler, u32 hwirq_cnt,
+ enum irq_gc_flags flags, unsigned int clr,
+ unsigned int set,
+ void (*gc_init_cb)(struct irq_chip_generic *),
+ void *private)
+{
+ int ret, i;
+ u32 msk = 0;
+ struct irq_chip_generic **gc;
+ struct irq_domain *d;
+ int num_gc = (hwirq_cnt + 31) / 32;
+
+ gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
+ if (!gc)
+ return -ENOMEM;
+
+ if (node)
+ d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
+ else {
+ msk = IRQ_MSK(32);
+ d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
+ &irq_gc_irq_domain_ops, gc);
+ }
+
+ for (i = 0; i < num_gc; i++) {
+ gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
+ reg_base, handler);
+ if (!gc[i]) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ gc[i]->hwirq_base = i * 32;
+ gc[i]->domain = d;
+ gc[i]->private = private;
+
+ gc_init_cb(gc[i]);
+
+ irq_setup_generic_chip(gc[i], msk, flags, clr, set);
+
+ irq_base += node ? 0 : 32;
+ }
+
+ return 0;
+
+ err:
+ for ( ; i >= 0; i--)
+ kfree(gc[i]);
+ kfree(gc);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
+#endif
+
/**
* irq_setup_alt_chip - Switch to alternative chip
* @d: irq_data for this interrupt
@@ -324,9 +431,10 @@ static int irq_gc_suspend(void)

list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
+ struct irq_data *data = irq_get_irq_data(gc->irq_base);

- if (ct->chip.irq_suspend)
- ct->chip.irq_suspend(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_suspend && data)
+ ct->chip.irq_suspend(data);
}
return 0;
}
@@ -337,9 +445,10 @@ static void irq_gc_resume(void)

list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
+ struct irq_data *data = irq_get_irq_data(gc->irq_base);

- if (ct->chip.irq_resume)
- ct->chip.irq_resume(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_resume && data)
+ ct->chip.irq_resume(data);
}
}
#else
@@ -353,9 +462,10 @@ static void irq_gc_shutdown(void)

list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
+ struct irq_data *data = irq_get_irq_data(gc->irq_base);

- if (ct->chip.irq_pm_shutdown)
- ct->chip.irq_pm_shutdown(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_pm_shutdown && data)
+ ct->chip.irq_pm_shutdown(data);
}
}

--
1.7.5.4

2012-02-03 22:35:25

by Rob Herring

[permalink] [raw]
Subject: [PATCH v4 3/4] ARM: imx: add irq domain support to tzic

From: Rob Herring <[email protected]>

Add irq domain support to tzic. This is needed to enable DT.

Signed-off-by: Rob Herring <[email protected]>
---
arch/arm/plat-mxc/tzic.c | 23 ++++++++++-------------
1 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
index 98308ec..25c10bb 100644
--- a/arch/arm/plat-mxc/tzic.c
+++ b/arch/arm/plat-mxc/tzic.c
@@ -77,7 +77,7 @@ static int tzic_set_irq_fiq(unsigned int irq, unsigned int type)
static void tzic_irq_suspend(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- int idx = gc->irq_base >> 5;
+ int idx = d->hwirq / 32;

__raw_writel(gc->wake_active, tzic_base + TZIC_WAKEUP0(idx));
}
@@ -85,7 +85,7 @@ static void tzic_irq_suspend(struct irq_data *d)
static void tzic_irq_resume(struct irq_data *d)
{
struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
- int idx = gc->irq_base >> 5;
+ int idx = d->hwirq / 32;

__raw_writel(__raw_readl(tzic_base + TZIC_ENSET0(idx)),
tzic_base + TZIC_WAKEUP0(idx));
@@ -102,18 +102,13 @@ static struct mxc_extra_irq tzic_extra_irq = {
#endif
};

-static __init void tzic_init_gc(unsigned int irq_start)
+static __init void tzic_init_gc(struct irq_chip_generic *gc)
{
- struct irq_chip_generic *gc;
- struct irq_chip_type *ct;
- int idx = irq_start >> 5;
+ struct irq_chip_type *ct = gc->chip_types;
+ int idx = gc->hwirq_base / 32;

- gc = irq_alloc_generic_chip("tzic", 1, irq_start, tzic_base,
- handle_level_irq);
- gc->private = &tzic_extra_irq;
gc->wake_enabled = IRQ_MSK(32);

- ct = gc->chip_types;
ct->chip.irq_mask = irq_gc_mask_disable_reg;
ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
ct->chip.irq_set_wake = irq_gc_set_wake;
@@ -122,7 +117,7 @@ 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);
+ return 0;
}

asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs)
@@ -175,8 +170,10 @@ void __init tzic_init_irq(void __iomem *irqbase)

/* all IRQ no FIQ Warning :: No selection */

- for (i = 0; i < TZIC_NUM_IRQS; i += 32)
- tzic_init_gc(i);
+ irq_setup_generic_chip_domain("tzic", NULL, 1, 0, tzic_base,
+ handle_level_irq, TZIC_NUM_IRQS, 0,
+ IRQ_NOREQUEST, 0,
+ tzic_init_gc, &tzic_extra_irq);

#ifdef CONFIG_FIQ
/* Initialize FIQ */
--
1.7.5.4

2012-02-03 22:35:53

by Rob Herring

[permalink] [raw]
Subject: [PATCH v4 4/4] gpio: pl061: enable interrupts with DT style binding

From: Rob Herring <[email protected]>

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 <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Linus Walleij <[email protected]>
---
.../devicetree/bindings/gpio/pl061-gpio.txt | 15 ++++++
drivers/gpio/gpio-pl061.c | 48 ++++++++++---------
2 files changed, 40 insertions(+), 23 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..b271a17 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,30 +196,25 @@ 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 irq_chip_generic *gc)
{
- struct irq_chip_type *ct;
+ struct irq_chip_type *ct = gc->chip_types;
+ struct pl061_gpio *chip = gc->private;

- chip->irq_gc = irq_alloc_generic_chip("gpio-pl061", 1, irq_base,
- chip->base, handle_simple_irq);
- chip->irq_gc->private = chip;
+ chip->irq_gc = gc;

- ct = chip->irq_gc->chip_types;
ct->chip.irq_mask = irq_gc_mask_clr_bit;
ct->chip.irq_unmask = irq_gc_mask_set_bit;
ct->chip.irq_set_type = pl061_irq_type;
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);
}

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 +223,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 +264,18 @@ 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);
+ irq_setup_generic_chip_domain("gpio-pl061",
+ of_node_get(dev->dev.of_node),
+ 1, irq_base, chip->base,
+ handle_simple_irq,
+ PL061_GPIO_NR, IRQ_GC_INIT_NESTED_LOCK,
+ IRQ_NOREQUEST, 0, pl061_init_gc, chip);

writeb(0, chip->base + GPIOIE); /* disable irqs */
irq = dev->irq[0];
--
1.7.5.4

2012-02-03 22:36:17

by Rob Herring

[permalink] [raw]
Subject: [PATCH v4 1/4] ARM: kconfig: always select IRQ_DOMAIN

From: Rob Herring <[email protected]>

irqdomains should ultimately be needed for all platforms, so enable it
unconditionally for ARM.

Signed-off-by: Rob Herring <[email protected]>
---
arch/arm/Kconfig | 2 +-
arch/arm/common/Kconfig | 2 --
2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 24626b0..b21b396 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -32,6 +32,7 @@ config ARM
select GENERIC_IRQ_SHOW
select CPU_PM if (SUSPEND || CPU_IDLE)
select GENERIC_PCI_IOMAP
+ select IRQ_DOMAIN
help
The ARM series is a line of low-power-consumption RISC chip designs
licensed by ARM Ltd and targeted at embedded applications and
@@ -1862,7 +1863,6 @@ config USE_OF
bool "Flattened Device Tree support"
select OF
select OF_EARLY_FLATTREE
- select IRQ_DOMAIN
help
Include support for flattened device tree machine descriptions.

diff --git a/arch/arm/common/Kconfig b/arch/arm/common/Kconfig
index 81a933e..2d3fc0c 100644
--- a/arch/arm/common/Kconfig
+++ b/arch/arm/common/Kconfig
@@ -1,5 +1,4 @@
config ARM_GIC
- select IRQ_DOMAIN
select MULTI_IRQ_HANDLER
bool

@@ -7,7 +6,6 @@ config GIC_NON_BANKED
bool

config ARM_VIC
- select IRQ_DOMAIN
select MULTI_IRQ_HANDLER
bool

--
1.7.5.4

2012-02-03 23:47:39

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

On Fri, Feb 3, 2012 at 3:35 PM, Rob Herring <[email protected]> wrote:
> From: Rob Herring <[email protected]>
>
> 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 <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> ?include/linux/irq.h ? ? ? | ? 15 +++++
> ?kernel/irq/generic-chip.c | ?154 ++++++++++++++++++++++++++++++++++++++-------
> ?2 files changed, 147 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index bff29c5..d721abc 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -664,7 +664,12 @@ struct irq_chip_generic {
> ? ? ? ?raw_spinlock_t ? ? ? ? ?lock;
> ? ? ? ?void __iomem ? ? ? ? ? ?*reg_base;
> ? ? ? ?unsigned int ? ? ? ? ? ?irq_base;
> + ? ? ? unsigned int ? ? ? ? ? ?hwirq_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 +712,16 @@ 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;
> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int num_ct, unsigned int irq_base,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void __iomem *reg_base,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? irq_flow_handler_t handler, u32 hwirq_cnt,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum irq_gc_flags flags,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int clr, unsigned int set,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void (*gc_init_cb)(struct irq_chip_generic *),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void *private);
> ?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..ffbe6fe 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) % 32;

Shouldn't this be: 1 << (d->hwirq & 32)? The way it is written will
result in mask == 0 for all hwirq >= 32.

> ? ? ? ?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) % 32;
>
> ? ? ? ?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) % 32;
>
> ? ? ? ?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) % 32;
>
> ? ? ? ?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) % 32;
>
> ? ? ? ?irq_gc_lock(gc);
> ? ? ? ?irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
> @@ -122,10 +123,10 @@ 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) % 32;
>
> ? ? ? ?irq_gc_lock(gc);
> - ? ? ? irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
> + ? ? ? irq_reg_writel(~mask, gc->reg_base + cur_regs(d)->ack);
> ? ? ? ?irq_gc_unlock(gc);
> ?}
>
> @@ -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) % 32;
>
> ? ? ? ?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) % 32;
>
> ? ? ? ?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) % 32;
>
> ? ? ? ?if (!(mask & gc->wake_enabled))
> ? ? ? ? ? ? ? ?return -EINVAL;
> @@ -220,6 +221,18 @@ EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
> ?*/
> ?static struct lock_class_key irq_nested_lock_class;
>
> +static void irq_gc_setup_irq(struct irq_chip_generic *gc, int irq)
> +{
> + ? ? ? 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);
> +}
> +
> ?/**
> ?* irq_setup_generic_chip - Setup a range of interrupts with a generic chip
> ?* @gc: ? ? ? ? ? ? ? ?Generic irq chip holding all data
> @@ -247,21 +260,115 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
> ? ? ? ?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;
> +
> ? ? ? ?for (i = gc->irq_base; msk; msk >>= 1, i++) {
> ? ? ? ? ? ? ? ?if (!(msk & 0x01))
> ? ? ? ? ? ? ? ? ? ? ? ?continue;
>
> - ? ? ? ? ? ? ? if (flags & IRQ_GC_INIT_NESTED_LOCK)
> - ? ? ? ? ? ? ? ? ? ? ? irq_set_lockdep_class(i, &irq_nested_lock_class);
> -
> - ? ? ? ? ? ? ? irq_set_chip_and_handler(i, &ct->chip, ct->handler);
> - ? ? ? ? ? ? ? irq_set_chip_data(i, gc);
> - ? ? ? ? ? ? ? irq_modify_status(i, clr, set);
> + ? ? ? ? ? ? ? irq_gc_setup_irq(gc, i);
> + ? ? ? ? ? ? ? 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_map(struct irq_domain *d, unsigned int irq,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?irq_hw_number_t hw)
> +{
> + ? ? ? struct irq_chip_generic **gc_array = d->host_data;
> + ? ? ? struct irq_chip_generic *gc = gc_array[hw / 32];
> +
> + ? ? ? /* We need a valid irq number for suspend/resume functions */
> + ? ? ? if ((int)gc->irq_base == -1)
> + ? ? ? ? ? ? ? gc->irq_base = irq;
> + ? ? ? irq_gc_setup_irq(gc, irq);
> + ? ? ? return 0;
> +}
> +
> +static struct irq_domain_ops irq_gc_irq_domain_ops = {
> + ? ? ? .map = irq_gc_irq_domain_map,
> + ? ? ? .xlate = irq_domain_xlate_onetwocell,

Can we hard-enforce _xlate_twocell here? Are there any users using
just one cell? Alternately, can we make the caller specify if one or
two cells are expected?

The irq_domain_ops structure should be const.

> +};
> +
> +/*
> + * irq_setup_generic_chip_domain - Setup a domain and N generic chips
> + * @name: ? ? ?Name of the irq chip
> + * @node: ? ? ?Device tree node pointer for domain
> + * @num_ct: ? ?Number of irq_chip_type instances associated with this
> + * @irq_base: ?Interrupt base nr for this chip
> + * @reg_base: ?Register base address (virtual)
> + * @handler: ? Default flow handler associated with this chip
> + * @hwirq_cnt: Number of hw irqs for the domain
> + * @flags: ? ? Flags for initialization
> + * @clr: ? ? ? IRQ_* bits to clear
> + * @set: ? ? ? IRQ_* bits to set
> + * @gc_init_cb: ? ? ? ?Init callback function called for each generic irq chip created
> + * @private ? ?Ptr to caller private data
> + *
> + * Set up an irq domain and N banks of 32 interrupts starting from gc->irq_base.
> + * Note, this initializes all interrupts to the primary irq_chip_type and its
> + * associated handler.
> + */
> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int num_ct, unsigned int irq_base,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void __iomem *reg_base,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? irq_flow_handler_t handler, u32 hwirq_cnt,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum irq_gc_flags flags, unsigned int clr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int set,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void (*gc_init_cb)(struct irq_chip_generic *),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void *private)
> +{
> + ? ? ? int ret, i;
> + ? ? ? u32 msk = 0;
> + ? ? ? struct irq_chip_generic **gc;
> + ? ? ? struct irq_domain *d;
> + ? ? ? int num_gc = (hwirq_cnt + 31) / 32;

Use the ALIGN() macro.

> +
> + ? ? ? gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
> + ? ? ? if (!gc)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? if (node)
> + ? ? ? ? ? ? ? d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
> + ? ? ? else {
> + ? ? ? ? ? ? ? msk = IRQ_MSK(32);
> + ? ? ? ? ? ? ? d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &irq_gc_irq_domain_ops, gc);
> + ? ? ? }
> +
> + ? ? ? for (i = 0; i < num_gc; i++) {
> + ? ? ? ? ? ? ? gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg_base, handler);
> + ? ? ? ? ? ? ? if (!gc[i]) {
> + ? ? ? ? ? ? ? ? ? ? ? ret = -ENOMEM;
> + ? ? ? ? ? ? ? ? ? ? ? goto err;
> + ? ? ? ? ? ? ? }
> + ? ? ? ? ? ? ? gc[i]->hwirq_base = i * 32;
> + ? ? ? ? ? ? ? gc[i]->domain = d;
> + ? ? ? ? ? ? ? gc[i]->private = private;
> +
> + ? ? ? ? ? ? ? gc_init_cb(gc[i]);
> +
> + ? ? ? ? ? ? ? irq_setup_generic_chip(gc[i], msk, flags, clr, set);
> +
> + ? ? ? ? ? ? ? irq_base += node ? 0 : 32;
> + ? ? ? }
> +
> + ? ? ? return 0;
> +
> + err:
> + ? ? ? for ( ; i >= 0; i--)
> + ? ? ? ? ? ? ? kfree(gc[i]);
> + ? ? ? kfree(gc);
> + ? ? ? return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
> +#endif
> +
> ?/**
> ?* irq_setup_alt_chip - Switch to alternative chip
> ?* @d: ? ? ? ? irq_data for this interrupt
> @@ -324,9 +431,10 @@ static int irq_gc_suspend(void)
>
> ? ? ? ?list_for_each_entry(gc, &gc_list, list) {
> ? ? ? ? ? ? ? ?struct irq_chip_type *ct = gc->chip_types;
> + ? ? ? ? ? ? ? struct irq_data *data = irq_get_irq_data(gc->irq_base);
>
> - ? ? ? ? ? ? ? if (ct->chip.irq_suspend)
> - ? ? ? ? ? ? ? ? ? ? ? ct->chip.irq_suspend(irq_get_irq_data(gc->irq_base));
> + ? ? ? ? ? ? ? if (ct->chip.irq_suspend && data)
> + ? ? ? ? ? ? ? ? ? ? ? ct->chip.irq_suspend(data);
> ? ? ? ?}
> ? ? ? ?return 0;
> ?}
> @@ -337,9 +445,10 @@ static void irq_gc_resume(void)
>
> ? ? ? ?list_for_each_entry(gc, &gc_list, list) {
> ? ? ? ? ? ? ? ?struct irq_chip_type *ct = gc->chip_types;
> + ? ? ? ? ? ? ? struct irq_data *data = irq_get_irq_data(gc->irq_base);
>
> - ? ? ? ? ? ? ? if (ct->chip.irq_resume)
> - ? ? ? ? ? ? ? ? ? ? ? ct->chip.irq_resume(irq_get_irq_data(gc->irq_base));
> + ? ? ? ? ? ? ? if (ct->chip.irq_resume && data)
> + ? ? ? ? ? ? ? ? ? ? ? ct->chip.irq_resume(data);
> ? ? ? ?}
> ?}
> ?#else
> @@ -353,9 +462,10 @@ static void irq_gc_shutdown(void)
>
> ? ? ? ?list_for_each_entry(gc, &gc_list, list) {
> ? ? ? ? ? ? ? ?struct irq_chip_type *ct = gc->chip_types;
> + ? ? ? ? ? ? ? struct irq_data *data = irq_get_irq_data(gc->irq_base);
>
> - ? ? ? ? ? ? ? if (ct->chip.irq_pm_shutdown)
> - ? ? ? ? ? ? ? ? ? ? ? ct->chip.irq_pm_shutdown(irq_get_irq_data(gc->irq_base));
> + ? ? ? ? ? ? ? if (ct->chip.irq_pm_shutdown && data)
> + ? ? ? ? ? ? ? ? ? ? ? ct->chip.irq_pm_shutdown(data);
> ? ? ? ?}
> ?}
>
> --
> 1.7.5.4
>



--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

2012-02-04 13:55:33

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

On Fri, Feb 03, 2012 at 04:35:10PM -0600, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> 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 <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> include/linux/irq.h | 15 +++++
> kernel/irq/generic-chip.c | 154 ++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 147 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index bff29c5..d721abc 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -664,7 +664,12 @@ struct irq_chip_generic {
> raw_spinlock_t lock;
> void __iomem *reg_base;
> unsigned int irq_base;
> + unsigned int hwirq_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 +712,16 @@ 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;
> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> + int num_ct, unsigned int irq_base,
> + void __iomem *reg_base,
> + irq_flow_handler_t handler, u32 hwirq_cnt,
> + enum irq_gc_flags flags,
> + unsigned int clr, unsigned int set,
> + void (*gc_init_cb)(struct irq_chip_generic *),
> + void *private);
> 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..ffbe6fe 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) % 32;
>
> 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) % 32;
>
> 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) % 32;
>
> 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) % 32;
>
> 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) % 32;
>
> irq_gc_lock(gc);
> irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
> @@ -122,10 +123,10 @@ 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) % 32;
>
> irq_gc_lock(gc);
> - irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
> + irq_reg_writel(~mask, gc->reg_base + cur_regs(d)->ack);
> irq_gc_unlock(gc);
> }
>
> @@ -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) % 32;
>
> 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) % 32;
>
> 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) % 32;
>
> if (!(mask & gc->wake_enabled))
> return -EINVAL;
> @@ -220,6 +221,18 @@ EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
> */
> static struct lock_class_key irq_nested_lock_class;
>
> +static void irq_gc_setup_irq(struct irq_chip_generic *gc, int irq)
> +{
> + 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);
> +}
> +
> /**
> * irq_setup_generic_chip - Setup a range of interrupts with a generic chip
> * @gc: Generic irq chip holding all data
> @@ -247,21 +260,115 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
> 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;
> +
> for (i = gc->irq_base; msk; msk >>= 1, i++) {
> if (!(msk & 0x01))
> continue;
>
> - if (flags & IRQ_GC_INIT_NESTED_LOCK)
> - irq_set_lockdep_class(i, &irq_nested_lock_class);
> -
> - irq_set_chip_and_handler(i, &ct->chip, ct->handler);
> - irq_set_chip_data(i, gc);
> - irq_modify_status(i, clr, set);
> + irq_gc_setup_irq(gc, i);
> + 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_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hw)
> +{
> + struct irq_chip_generic **gc_array = d->host_data;
> + struct irq_chip_generic *gc = gc_array[hw / 32];
> +
This .map callback gets already called in irq_domain_add_legacy(),
where gc_array has not been populated yet with the implementation
below ...

> + /* We need a valid irq number for suspend/resume functions */
> + if ((int)gc->irq_base == -1)
> + gc->irq_base = irq;
> + irq_gc_setup_irq(gc, irq);
> + return 0;
> +}
> +
> +static struct irq_domain_ops irq_gc_irq_domain_ops = {
> + .map = irq_gc_irq_domain_map,
> + .xlate = irq_domain_xlate_onetwocell,
> +};
> +
> +/*
> + * irq_setup_generic_chip_domain - Setup a domain and N generic chips
> + * @name: Name of the irq chip
> + * @node: Device tree node pointer for domain
> + * @num_ct: Number of irq_chip_type instances associated with this
> + * @irq_base: Interrupt base nr for this chip
> + * @reg_base: Register base address (virtual)
> + * @handler: Default flow handler associated with this chip
> + * @hwirq_cnt: Number of hw irqs for the domain
> + * @flags: Flags for initialization
> + * @clr: IRQ_* bits to clear
> + * @set: IRQ_* bits to set
> + * @gc_init_cb: Init callback function called for each generic irq chip created
> + * @private Ptr to caller private data
> + *
> + * Set up an irq domain and N banks of 32 interrupts starting from gc->irq_base.
> + * Note, this initializes all interrupts to the primary irq_chip_type and its
> + * associated handler.
> + */
> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> + int num_ct, unsigned int irq_base,
> + void __iomem *reg_base,
> + irq_flow_handler_t handler, u32 hwirq_cnt,
> + enum irq_gc_flags flags, unsigned int clr,
> + unsigned int set,
> + void (*gc_init_cb)(struct irq_chip_generic *),
> + void *private)
> +{
> + int ret, i;
> + u32 msk = 0;
> + struct irq_chip_generic **gc;
> + struct irq_domain *d;
> + int num_gc = (hwirq_cnt + 31) / 32;
> +
> + gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
> + if (!gc)
> + return -ENOMEM;
> +
> + if (node)
> + d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
> + else {
> + msk = IRQ_MSK(32);
> + d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> + &irq_gc_irq_domain_ops, gc);
> + }
> +
... irq_domain_add_legacy() is called here for !node case ...

> + for (i = 0; i < num_gc; i++) {
> + gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
> + reg_base, handler);

... while gc[i] only gets populated here ...

The following change seems fixing the problem for me.

8<---
@@ -323,6 +324,7 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
void *private)
{
int ret, i;
+ unsigned int irqbase = irq_base;
u32 msk = 0;
struct irq_chip_generic **gc;
struct irq_domain *d;
@@ -332,16 +334,8 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
if (!gc)
return -ENOMEM;

- if (node)
- d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
- else {
- msk = IRQ_MSK(32);
- d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
- &irq_gc_irq_domain_ops, gc);
- }
-
for (i = 0; i < num_gc; i++) {
- gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
+ gc[i] = irq_alloc_generic_chip(name, num_ct, irqbase,
reg_base, handler);
if (!gc[i]) {
ret = -ENOMEM;
@@ -355,7 +349,15 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,

irq_setup_generic_chip(gc[i], msk, flags, clr, set);

- irq_base += node ? 0 : 32;
+ irqbase += node ? 0 : 32;
+ }
+
+ if (node)
+ d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
+ else {
+ msk = IRQ_MSK(32);
+ d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
+ &irq_gc_irq_domain_ops, gc);
}

return 0;
--->8

Regards,
Shawn

> + if (!gc[i]) {
> + ret = -ENOMEM;
> + goto err;
> + }
> + gc[i]->hwirq_base = i * 32;
> + gc[i]->domain = d;
> + gc[i]->private = private;
> +
> + gc_init_cb(gc[i]);
> +
> + irq_setup_generic_chip(gc[i], msk, flags, clr, set);
> +
> + irq_base += node ? 0 : 32;
> + }
> +
> + return 0;
> +
> + err:
> + for ( ; i >= 0; i--)
> + kfree(gc[i]);
> + kfree(gc);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
> +#endif
> +
> /**
> * irq_setup_alt_chip - Switch to alternative chip
> * @d: irq_data for this interrupt
> @@ -324,9 +431,10 @@ static int irq_gc_suspend(void)
>
> list_for_each_entry(gc, &gc_list, list) {
> struct irq_chip_type *ct = gc->chip_types;
> + struct irq_data *data = irq_get_irq_data(gc->irq_base);
>
> - if (ct->chip.irq_suspend)
> - ct->chip.irq_suspend(irq_get_irq_data(gc->irq_base));
> + if (ct->chip.irq_suspend && data)
> + ct->chip.irq_suspend(data);
> }
> return 0;
> }
> @@ -337,9 +445,10 @@ static void irq_gc_resume(void)
>
> list_for_each_entry(gc, &gc_list, list) {
> struct irq_chip_type *ct = gc->chip_types;
> + struct irq_data *data = irq_get_irq_data(gc->irq_base);
>
> - if (ct->chip.irq_resume)
> - ct->chip.irq_resume(irq_get_irq_data(gc->irq_base));
> + if (ct->chip.irq_resume && data)
> + ct->chip.irq_resume(data);
> }
> }
> #else
> @@ -353,9 +462,10 @@ static void irq_gc_shutdown(void)
>
> list_for_each_entry(gc, &gc_list, list) {
> struct irq_chip_type *ct = gc->chip_types;
> + struct irq_data *data = irq_get_irq_data(gc->irq_base);
>
> - if (ct->chip.irq_pm_shutdown)
> - ct->chip.irq_pm_shutdown(irq_get_irq_data(gc->irq_base));
> + if (ct->chip.irq_pm_shutdown && data)
> + ct->chip.irq_pm_shutdown(data);
> }
> }
>
> --
> 1.7.5.4
>

2012-02-04 14:06:15

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

On Sat, Feb 4, 2012 at 7:08 AM, Shawn Guo <[email protected]> wrote:
> On Fri, Feb 03, 2012 at 04:35:10PM -0600, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> 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 <[email protected]>
>> Cc: Grant Likely <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> ---
>> ?include/linux/irq.h ? ? ? | ? 15 +++++
>> ?kernel/irq/generic-chip.c | ?154 ++++++++++++++++++++++++++++++++++++++-------
>> ?2 files changed, 147 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index bff29c5..d721abc 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -664,7 +664,12 @@ struct irq_chip_generic {
>> ? ? ? raw_spinlock_t ? ? ? ? ?lock;
>> ? ? ? void __iomem ? ? ? ? ? ?*reg_base;
>> ? ? ? unsigned int ? ? ? ? ? ?irq_base;
>> + ? ? unsigned int ? ? ? ? ? ?hwirq_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 +712,16 @@ 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;
>> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int num_ct, unsigned int irq_base,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void __iomem *reg_base,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? irq_flow_handler_t handler, u32 hwirq_cnt,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum irq_gc_flags flags,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int clr, unsigned int set,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void (*gc_init_cb)(struct irq_chip_generic *),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void *private);
>> ?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..ffbe6fe 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) % 32;
>>
>> ? ? ? 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) % 32;
>>
>> ? ? ? 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) % 32;
>>
>> ? ? ? 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) % 32;
>>
>> ? ? ? 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) % 32;
>>
>> ? ? ? irq_gc_lock(gc);
>> ? ? ? irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
>> @@ -122,10 +123,10 @@ 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) % 32;
>>
>> ? ? ? irq_gc_lock(gc);
>> - ? ? irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
>> + ? ? irq_reg_writel(~mask, gc->reg_base + cur_regs(d)->ack);
>> ? ? ? irq_gc_unlock(gc);
>> ?}
>>
>> @@ -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) % 32;
>>
>> ? ? ? 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) % 32;
>>
>> ? ? ? 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) % 32;
>>
>> ? ? ? if (!(mask & gc->wake_enabled))
>> ? ? ? ? ? ? ? return -EINVAL;
>> @@ -220,6 +221,18 @@ EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
>> ? */
>> ?static struct lock_class_key irq_nested_lock_class;
>>
>> +static void irq_gc_setup_irq(struct irq_chip_generic *gc, int irq)
>> +{
>> + ? ? 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);
>> +}
>> +
>> ?/**
>> ? * irq_setup_generic_chip - Setup a range of interrupts with a generic chip
>> ? * @gc: ? ? ? ? ? ? ?Generic irq chip holding all data
>> @@ -247,21 +260,115 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>> ? ? ? 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;
>> +
>> ? ? ? for (i = gc->irq_base; msk; msk >>= 1, i++) {
>> ? ? ? ? ? ? ? if (!(msk & 0x01))
>> ? ? ? ? ? ? ? ? ? ? ? continue;
>>
>> - ? ? ? ? ? ? if (flags & IRQ_GC_INIT_NESTED_LOCK)
>> - ? ? ? ? ? ? ? ? ? ? irq_set_lockdep_class(i, &irq_nested_lock_class);
>> -
>> - ? ? ? ? ? ? irq_set_chip_and_handler(i, &ct->chip, ct->handler);
>> - ? ? ? ? ? ? irq_set_chip_data(i, gc);
>> - ? ? ? ? ? ? irq_modify_status(i, clr, set);
>> + ? ? ? ? ? ? irq_gc_setup_irq(gc, i);
>> + ? ? ? ? ? ? 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_map(struct irq_domain *d, unsigned int irq,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?irq_hw_number_t hw)
>> +{
>> + ? ? struct irq_chip_generic **gc_array = d->host_data;
>> + ? ? struct irq_chip_generic *gc = gc_array[hw / 32];
>> +
> This .map callback gets already called in irq_domain_add_legacy(),
> where gc_array has not been populated yet ?with the implementation
> below ...
>
>> + ? ? /* We need a valid irq number for suspend/resume functions */
>> + ? ? if ((int)gc->irq_base == -1)
>> + ? ? ? ? ? ? gc->irq_base = irq;
>> + ? ? irq_gc_setup_irq(gc, irq);
>> + ? ? return 0;
>> +}
>> +
>> +static struct irq_domain_ops irq_gc_irq_domain_ops = {
>> + ? ? .map = irq_gc_irq_domain_map,
>> + ? ? .xlate = irq_domain_xlate_onetwocell,
>> +};
>> +
>> +/*
>> + * irq_setup_generic_chip_domain - Setup a domain and N generic chips
>> + * @name: ? ?Name of the irq chip
>> + * @node: ? ?Device tree node pointer for domain
>> + * @num_ct: ?Number of irq_chip_type instances associated with this
>> + * @irq_base: ? ? ? ?Interrupt base nr for this chip
>> + * @reg_base: ? ? ? ?Register base address (virtual)
>> + * @handler: Default flow handler associated with this chip
>> + * @hwirq_cnt: ? ? ? Number of hw irqs for the domain
>> + * @flags: ? Flags for initialization
>> + * @clr: ? ? IRQ_* bits to clear
>> + * @set: ? ? IRQ_* bits to set
>> + * @gc_init_cb: ? ? ?Init callback function called for each generic irq chip created
>> + * @private ?Ptr to caller private data
>> + *
>> + * Set up an irq domain and N banks of 32 interrupts starting from gc->irq_base.
>> + * Note, this initializes all interrupts to the primary irq_chip_type and its
>> + * associated handler.
>> + */
>> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? int num_ct, unsigned int irq_base,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? void __iomem *reg_base,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? irq_flow_handler_t handler, u32 hwirq_cnt,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum irq_gc_flags flags, unsigned int clr,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int set,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? void (*gc_init_cb)(struct irq_chip_generic *),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? void *private)
>> +{
>> + ? ? int ret, i;
>> + ? ? u32 msk = 0;
>> + ? ? struct irq_chip_generic **gc;
>> + ? ? struct irq_domain *d;
>> + ? ? int num_gc = (hwirq_cnt + 31) / 32;
>> +
>> + ? ? gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
>> + ? ? if (!gc)
>> + ? ? ? ? ? ? return -ENOMEM;
>> +
>> + ? ? if (node)
>> + ? ? ? ? ? ? d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
>> + ? ? else {
>> + ? ? ? ? ? ? msk = IRQ_MSK(32);
>> + ? ? ? ? ? ? d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &irq_gc_irq_domain_ops, gc);
>> + ? ? }
>> +
> ... irq_domain_add_legacy() is called here for !node case ...
>
>> + ? ? for (i = 0; i < num_gc; i++) {
>> + ? ? ? ? ? ? gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg_base, handler);
>
> ... while gc[i] only gets populated here ...
>
> The following change seems fixing the problem for me.
>
> 8<---
> @@ -323,6 +324,7 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *private)
> ?{
> ? ? ? ?int ret, i;
> + ? ? ? unsigned int irqbase = irq_base;
> ? ? ? ?u32 msk = 0;
> ? ? ? ?struct irq_chip_generic **gc;
> ? ? ? ?struct irq_domain *d;
> @@ -332,16 +334,8 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> ? ? ? ?if (!gc)
> ? ? ? ? ? ? ? ?return -ENOMEM;
>
> - ? ? ? if (node)
> - ? ? ? ? ? ? ? d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
> - ? ? ? else {
> - ? ? ? ? ? ? ? msk = IRQ_MSK(32);
> - ? ? ? ? ? ? ? d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &irq_gc_irq_domain_ops, gc);
> - ? ? ? }
> -
> ? ? ? ?for (i = 0; i < num_gc; i++) {
> - ? ? ? ? ? ? ? gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
> + ? ? ? ? ? ? ? gc[i] = irq_alloc_generic_chip(name, num_ct, irqbase,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? reg_base, handler);
> ? ? ? ? ? ? ? ?if (!gc[i]) {
> ? ? ? ? ? ? ? ? ? ? ? ?ret = -ENOMEM;
> @@ -355,7 +349,15 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>
> ? ? ? ? ? ? ? ?irq_setup_generic_chip(gc[i], msk, flags, clr, set);
>
> - ? ? ? ? ? ? ? irq_base += node ? 0 : 32;
> + ? ? ? ? ? ? ? irqbase += node ? 0 : 32;
> + ? ? ? }
> +
> + ? ? ? if (node)
> + ? ? ? ? ? ? ? d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
> + ? ? ? else {
> + ? ? ? ? ? ? ? msk = IRQ_MSK(32);
> + ? ? ? ? ? ? ? d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &irq_gc_irq_domain_ops, gc);
> ? ? ? ?}
>
> ? ? ? ?return 0;
> --->8
>
> Regards,
> Shawn

That looks like the correct fix to me. The irq chip does indeed need
to be set up before calling irq_domain_add_legacy().

g.

2012-02-04 14:07:31

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] ARM: imx: add irq domain support to tzic

On Fri, Feb 03, 2012 at 04:35:11PM -0600, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> Add irq domain support to tzic. This is needed to enable DT.
>
> Signed-off-by: Rob Herring <[email protected]>
> ---
> arch/arm/plat-mxc/tzic.c | 23 ++++++++++-------------
> 1 files changed, 10 insertions(+), 13 deletions(-)
>

CC arch/arm/plat-mxc/tzic.o
arch/arm/plat-mxc/tzic.c: In function ‘tzic_irq_resume’:
arch/arm/plat-mxc/tzic.c:87:27: warning: unused variable ‘gc’ [-Wunused-variable]
arch/arm/plat-mxc/tzic.c: In function ‘tzic_init_gc’:
arch/arm/plat-mxc/tzic.c:120:2: warning: ‘return’ with a value, in function returning void [enabled by default]

Regards,
Shawn

> diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
> index 98308ec..25c10bb 100644
> --- a/arch/arm/plat-mxc/tzic.c
> +++ b/arch/arm/plat-mxc/tzic.c
> @@ -77,7 +77,7 @@ static int tzic_set_irq_fiq(unsigned int irq, unsigned int type)
> static void tzic_irq_suspend(struct irq_data *d)
> {
> struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> - int idx = gc->irq_base >> 5;
> + int idx = d->hwirq / 32;
>
> __raw_writel(gc->wake_active, tzic_base + TZIC_WAKEUP0(idx));
> }
> @@ -85,7 +85,7 @@ static void tzic_irq_suspend(struct irq_data *d)
> static void tzic_irq_resume(struct irq_data *d)
> {
> struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> - int idx = gc->irq_base >> 5;
> + int idx = d->hwirq / 32;
>
> __raw_writel(__raw_readl(tzic_base + TZIC_ENSET0(idx)),
> tzic_base + TZIC_WAKEUP0(idx));
> @@ -102,18 +102,13 @@ static struct mxc_extra_irq tzic_extra_irq = {
> #endif
> };
>
> -static __init void tzic_init_gc(unsigned int irq_start)
> +static __init void tzic_init_gc(struct irq_chip_generic *gc)
> {
> - struct irq_chip_generic *gc;
> - struct irq_chip_type *ct;
> - int idx = irq_start >> 5;
> + struct irq_chip_type *ct = gc->chip_types;
> + int idx = gc->hwirq_base / 32;
>
> - gc = irq_alloc_generic_chip("tzic", 1, irq_start, tzic_base,
> - handle_level_irq);
> - gc->private = &tzic_extra_irq;
> gc->wake_enabled = IRQ_MSK(32);
>
> - ct = gc->chip_types;
> ct->chip.irq_mask = irq_gc_mask_disable_reg;
> ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
> ct->chip.irq_set_wake = irq_gc_set_wake;
> @@ -122,7 +117,7 @@ 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);
> + return 0;
> }
>
> asmlinkage void __exception_irq_entry tzic_handle_irq(struct pt_regs *regs)
> @@ -175,8 +170,10 @@ void __init tzic_init_irq(void __iomem *irqbase)
>
> /* all IRQ no FIQ Warning :: No selection */
>
> - for (i = 0; i < TZIC_NUM_IRQS; i += 32)
> - tzic_init_gc(i);
> + irq_setup_generic_chip_domain("tzic", NULL, 1, 0, tzic_base,
> + handle_level_irq, TZIC_NUM_IRQS, 0,
> + IRQ_NOREQUEST, 0,
> + tzic_init_gc, &tzic_extra_irq);
>
> #ifdef CONFIG_FIQ
> /* Initialize FIQ */
> --
> 1.7.5.4
>

2012-02-07 04:55:01

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

Shawn,

On 02/04/2012 08:08 AM, Shawn Guo wrote:
> On Fri, Feb 03, 2012 at 04:35:10PM -0600, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> 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 <[email protected]>
>> Cc: Grant Likely <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> ---
>> include/linux/irq.h | 15 +++++
>> kernel/irq/generic-chip.c | 154 ++++++++++++++++++++++++++++++++++++++-------
>> 2 files changed, 147 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index bff29c5..d721abc 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -664,7 +664,12 @@ struct irq_chip_generic {
>> raw_spinlock_t lock;
>> void __iomem *reg_base;
>> unsigned int irq_base;
>> + unsigned int hwirq_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 +712,16 @@ 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;
>> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>> + int num_ct, unsigned int irq_base,
>> + void __iomem *reg_base,
>> + irq_flow_handler_t handler, u32 hwirq_cnt,
>> + enum irq_gc_flags flags,
>> + unsigned int clr, unsigned int set,
>> + void (*gc_init_cb)(struct irq_chip_generic *),
>> + void *private);
>> 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..ffbe6fe 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) % 32;
>>
>> 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) % 32;
>>
>> 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) % 32;
>>
>> 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) % 32;
>>
>> 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) % 32;
>>
>> irq_gc_lock(gc);
>> irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
>> @@ -122,10 +123,10 @@ 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) % 32;
>>
>> irq_gc_lock(gc);
>> - irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
>> + irq_reg_writel(~mask, gc->reg_base + cur_regs(d)->ack);
>> irq_gc_unlock(gc);
>> }
>>
>> @@ -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) % 32;
>>
>> 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) % 32;
>>
>> 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) % 32;
>>
>> if (!(mask & gc->wake_enabled))
>> return -EINVAL;
>> @@ -220,6 +221,18 @@ EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
>> */
>> static struct lock_class_key irq_nested_lock_class;
>>
>> +static void irq_gc_setup_irq(struct irq_chip_generic *gc, int irq)
>> +{
>> + 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);
>> +}
>> +
>> /**
>> * irq_setup_generic_chip - Setup a range of interrupts with a generic chip
>> * @gc: Generic irq chip holding all data
>> @@ -247,21 +260,115 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>> 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;
>> +
>> for (i = gc->irq_base; msk; msk >>= 1, i++) {
>> if (!(msk & 0x01))
>> continue;
>>
>> - if (flags & IRQ_GC_INIT_NESTED_LOCK)
>> - irq_set_lockdep_class(i, &irq_nested_lock_class);
>> -
>> - irq_set_chip_and_handler(i, &ct->chip, ct->handler);
>> - irq_set_chip_data(i, gc);
>> - irq_modify_status(i, clr, set);
>> + irq_gc_setup_irq(gc, i);
>> + 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_map(struct irq_domain *d, unsigned int irq,
>> + irq_hw_number_t hw)
>> +{
>> + struct irq_chip_generic **gc_array = d->host_data;
>> + struct irq_chip_generic *gc = gc_array[hw / 32];
>> +
> This .map callback gets already called in irq_domain_add_legacy(),
> where gc_array has not been populated yet with the implementation
> below ...
>
>> + /* We need a valid irq number for suspend/resume functions */
>> + if ((int)gc->irq_base == -1)
>> + gc->irq_base = irq;
>> + irq_gc_setup_irq(gc, irq);
>> + return 0;
>> +}
>> +
>> +static struct irq_domain_ops irq_gc_irq_domain_ops = {
>> + .map = irq_gc_irq_domain_map,
>> + .xlate = irq_domain_xlate_onetwocell,
>> +};
>> +
>> +/*
>> + * irq_setup_generic_chip_domain - Setup a domain and N generic chips
>> + * @name: Name of the irq chip
>> + * @node: Device tree node pointer for domain
>> + * @num_ct: Number of irq_chip_type instances associated with this
>> + * @irq_base: Interrupt base nr for this chip
>> + * @reg_base: Register base address (virtual)
>> + * @handler: Default flow handler associated with this chip
>> + * @hwirq_cnt: Number of hw irqs for the domain
>> + * @flags: Flags for initialization
>> + * @clr: IRQ_* bits to clear
>> + * @set: IRQ_* bits to set
>> + * @gc_init_cb: Init callback function called for each generic irq chip created
>> + * @private Ptr to caller private data
>> + *
>> + * Set up an irq domain and N banks of 32 interrupts starting from gc->irq_base.
>> + * Note, this initializes all interrupts to the primary irq_chip_type and its
>> + * associated handler.
>> + */
>> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>> + int num_ct, unsigned int irq_base,
>> + void __iomem *reg_base,
>> + irq_flow_handler_t handler, u32 hwirq_cnt,
>> + enum irq_gc_flags flags, unsigned int clr,
>> + unsigned int set,
>> + void (*gc_init_cb)(struct irq_chip_generic *),
>> + void *private)
>> +{
>> + int ret, i;
>> + u32 msk = 0;
>> + struct irq_chip_generic **gc;
>> + struct irq_domain *d;
>> + int num_gc = (hwirq_cnt + 31) / 32;
>> +
>> + gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
>> + if (!gc)
>> + return -ENOMEM;
>> +
>> + if (node)
>> + d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
>> + else {
>> + msk = IRQ_MSK(32);
>> + d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
>> + &irq_gc_irq_domain_ops, gc);
>> + }
>> +
> ... irq_domain_add_legacy() is called here for !node case ...
>
>> + for (i = 0; i < num_gc; i++) {
>> + gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
>> + reg_base, handler);
>
> ... while gc[i] only gets populated here ...
>
> The following change seems fixing the problem for me.
>
> 8<---
> @@ -323,6 +324,7 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> void *private)
> {
> int ret, i;
> + unsigned int irqbase = irq_base;
> u32 msk = 0;
> struct irq_chip_generic **gc;
> struct irq_domain *d;
> @@ -332,16 +334,8 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> if (!gc)
> return -ENOMEM;
>
> - if (node)
> - d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
> - else {
> - msk = IRQ_MSK(32);
> - d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> - &irq_gc_irq_domain_ops, gc);
> - }
> -
> for (i = 0; i < num_gc; i++) {
> - gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
> + gc[i] = irq_alloc_generic_chip(name, num_ct, irqbase,
> reg_base, handler);
> if (!gc[i]) {
> ret = -ENOMEM;
> @@ -355,7 +349,15 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>
> irq_setup_generic_chip(gc[i], msk, flags, clr, set);
>
> - irq_base += node ? 0 : 32;
> + irqbase += node ? 0 : 32;
> + }
> +
> + if (node)
> + d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
> + else {
> + msk = IRQ_MSK(32);
> + d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> + &irq_gc_irq_domain_ops, gc);
> }
>

This isn't quite right either. The domain ptr is not getting set and the
hwirq mapping is getting skipped.

I've pushed a v5 branch. Can you please test it out on mx51.

Grant, is there some reason a legacy domain cannot setup the
irq_data.hwirq itself? No other code should be setting this up.

Rob

2012-02-07 05:25:34

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

On Mon, Feb 6, 2012 at 9:54 PM, Rob Herring <[email protected]> wrote:
> Shawn,
>
> On 02/04/2012 08:08 AM, Shawn Guo wrote:
>> On Fri, Feb 03, 2012 at 04:35:10PM -0600, Rob Herring wrote:
>>> From: Rob Herring <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Cc: Grant Likely <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> ---
>>> ?include/linux/irq.h ? ? ? | ? 15 +++++
>>> ?kernel/irq/generic-chip.c | ?154 ++++++++++++++++++++++++++++++++++++++-------
>>> ?2 files changed, 147 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>>> index bff29c5..d721abc 100644
>>> --- a/include/linux/irq.h
>>> +++ b/include/linux/irq.h
>>> @@ -664,7 +664,12 @@ struct irq_chip_generic {
>>> ? ? ?raw_spinlock_t ? ? ? ? ?lock;
>>> ? ? ?void __iomem ? ? ? ? ? ?*reg_base;
>>> ? ? ?unsigned int ? ? ? ? ? ?irq_base;
>>> + ? ?unsigned int ? ? ? ? ? ?hwirq_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 +712,16 @@ 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;
>>> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int num_ct, unsigned int irq_base,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void __iomem *reg_base,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?irq_flow_handler_t handler, u32 hwirq_cnt,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?enum irq_gc_flags flags,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int clr, unsigned int set,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void (*gc_init_cb)(struct irq_chip_generic *),
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *private);
>>> ?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..ffbe6fe 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) % 32;
>>>
>>> ? ? ?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) % 32;
>>>
>>> ? ? ?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) % 32;
>>>
>>> ? ? ?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) % 32;
>>>
>>> ? ? ?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) % 32;
>>>
>>> ? ? ?irq_gc_lock(gc);
>>> ? ? ?irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
>>> @@ -122,10 +123,10 @@ 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) % 32;
>>>
>>> ? ? ?irq_gc_lock(gc);
>>> - ? ?irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
>>> + ? ?irq_reg_writel(~mask, gc->reg_base + cur_regs(d)->ack);
>>> ? ? ?irq_gc_unlock(gc);
>>> ?}
>>>
>>> @@ -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) % 32;
>>>
>>> ? ? ?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) % 32;
>>>
>>> ? ? ?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) % 32;
>>>
>>> ? ? ?if (!(mask & gc->wake_enabled))
>>> ? ? ? ? ? ? ?return -EINVAL;
>>> @@ -220,6 +221,18 @@ EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
>>> ? */
>>> ?static struct lock_class_key irq_nested_lock_class;
>>>
>>> +static void irq_gc_setup_irq(struct irq_chip_generic *gc, int irq)
>>> +{
>>> + ? ?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);
>>> +}
>>> +
>>> ?/**
>>> ? * irq_setup_generic_chip - Setup a range of interrupts with a generic chip
>>> ? * @gc: ? ? ? ? ? ? Generic irq chip holding all data
>>> @@ -247,21 +260,115 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>>> ? ? ?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;
>>> +
>>> ? ? ?for (i = gc->irq_base; msk; msk >>= 1, i++) {
>>> ? ? ? ? ? ? ?if (!(msk & 0x01))
>>> ? ? ? ? ? ? ? ? ? ? ?continue;
>>>
>>> - ? ? ? ? ? ?if (flags & IRQ_GC_INIT_NESTED_LOCK)
>>> - ? ? ? ? ? ? ? ? ? ?irq_set_lockdep_class(i, &irq_nested_lock_class);
>>> -
>>> - ? ? ? ? ? ?irq_set_chip_and_handler(i, &ct->chip, ct->handler);
>>> - ? ? ? ? ? ?irq_set_chip_data(i, gc);
>>> - ? ? ? ? ? ?irq_modify_status(i, clr, set);
>>> + ? ? ? ? ? ?irq_gc_setup_irq(gc, i);
>>> + ? ? ? ? ? ?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_map(struct irq_domain *d, unsigned int irq,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? irq_hw_number_t hw)
>>> +{
>>> + ? ?struct irq_chip_generic **gc_array = d->host_data;
>>> + ? ?struct irq_chip_generic *gc = gc_array[hw / 32];
>>> +
>> This .map callback gets already called in irq_domain_add_legacy(),
>> where gc_array has not been populated yet ?with the implementation
>> below ...
>>
>>> + ? ?/* We need a valid irq number for suspend/resume functions */
>>> + ? ?if ((int)gc->irq_base == -1)
>>> + ? ? ? ? ? ?gc->irq_base = irq;
>>> + ? ?irq_gc_setup_irq(gc, irq);
>>> + ? ?return 0;
>>> +}
>>> +
>>> +static struct irq_domain_ops irq_gc_irq_domain_ops = {
>>> + ? ?.map = irq_gc_irq_domain_map,
>>> + ? ?.xlate = irq_domain_xlate_onetwocell,
>>> +};
>>> +
>>> +/*
>>> + * irq_setup_generic_chip_domain - Setup a domain and N generic chips
>>> + * @name: ? Name of the irq chip
>>> + * @node: ? Device tree node pointer for domain
>>> + * @num_ct: Number of irq_chip_type instances associated with this
>>> + * @irq_base: ? ? ? Interrupt base nr for this chip
>>> + * @reg_base: ? ? ? Register base address (virtual)
>>> + * @handler: ? ? ? ?Default flow handler associated with this chip
>>> + * @hwirq_cnt: ? ? ?Number of hw irqs for the domain
>>> + * @flags: ?Flags for initialization
>>> + * @clr: ? ?IRQ_* bits to clear
>>> + * @set: ? ?IRQ_* bits to set
>>> + * @gc_init_cb: ? ? Init callback function called for each generic irq chip created
>>> + * @private Ptr to caller private data
>>> + *
>>> + * Set up an irq domain and N banks of 32 interrupts starting from gc->irq_base.
>>> + * Note, this initializes all interrupts to the primary irq_chip_type and its
>>> + * associated handler.
>>> + */
>>> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?int num_ct, unsigned int irq_base,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?void __iomem *reg_base,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?irq_flow_handler_t handler, u32 hwirq_cnt,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?enum irq_gc_flags flags, unsigned int clr,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int set,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?void (*gc_init_cb)(struct irq_chip_generic *),
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?void *private)
>>> +{
>>> + ? ?int ret, i;
>>> + ? ?u32 msk = 0;
>>> + ? ?struct irq_chip_generic **gc;
>>> + ? ?struct irq_domain *d;
>>> + ? ?int num_gc = (hwirq_cnt + 31) / 32;
>>> +
>>> + ? ?gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
>>> + ? ?if (!gc)
>>> + ? ? ? ? ? ?return -ENOMEM;
>>> +
>>> + ? ?if (node)
>>> + ? ? ? ? ? ?d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
>>> + ? ?else {
>>> + ? ? ? ? ? ?msk = IRQ_MSK(32);
>>> + ? ? ? ? ? ?d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&irq_gc_irq_domain_ops, gc);
>>> + ? ?}
>>> +
>> ... irq_domain_add_legacy() is called here for !node case ...
>>
>>> + ? ?for (i = 0; i < num_gc; i++) {
>>> + ? ? ? ? ? ?gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? reg_base, handler);
>>
>> ... while gc[i] only gets populated here ...
>>
>> The following change seems fixing the problem for me.
>>
>> 8<---
>> @@ -323,6 +324,7 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? void *private)
>> ?{
>> ? ? ? ? int ret, i;
>> + ? ? ? unsigned int irqbase = irq_base;
>> ? ? ? ? u32 msk = 0;
>> ? ? ? ? struct irq_chip_generic **gc;
>> ? ? ? ? struct irq_domain *d;
>> @@ -332,16 +334,8 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>> ? ? ? ? if (!gc)
>> ? ? ? ? ? ? ? ? return -ENOMEM;
>>
>> - ? ? ? if (node)
>> - ? ? ? ? ? ? ? d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
>> - ? ? ? else {
>> - ? ? ? ? ? ? ? msk = IRQ_MSK(32);
>> - ? ? ? ? ? ? ? d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &irq_gc_irq_domain_ops, gc);
>> - ? ? ? }
>> -
>> ? ? ? ? for (i = 0; i < num_gc; i++) {
>> - ? ? ? ? ? ? ? gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
>> + ? ? ? ? ? ? ? gc[i] = irq_alloc_generic_chip(name, num_ct, irqbase,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?reg_base, handler);
>> ? ? ? ? ? ? ? ? if (!gc[i]) {
>> ? ? ? ? ? ? ? ? ? ? ? ? ret = -ENOMEM;
>> @@ -355,7 +349,15 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>>
>> ? ? ? ? ? ? ? ? irq_setup_generic_chip(gc[i], msk, flags, clr, set);
>>
>> - ? ? ? ? ? ? ? irq_base += node ? 0 : 32;
>> + ? ? ? ? ? ? ? irqbase += node ? 0 : 32;
>> + ? ? ? }
>> +
>> + ? ? ? if (node)
>> + ? ? ? ? ? ? ? d = irq_domain_add_linear(node, hwirq_cnt, &irq_gc_irq_domain_ops, gc);
>> + ? ? ? else {
>> + ? ? ? ? ? ? ? msk = IRQ_MSK(32);
>> + ? ? ? ? ? ? ? d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? &irq_gc_irq_domain_ops, gc);
>> ? ? ? ? }
>>
>
> This isn't quite right either. The domain ptr is not getting set and the
> hwirq mapping is getting skipped.
>
> I've pushed a v5 branch. Can you please test it out on mx51.
>
> Grant, is there some reason a legacy domain cannot setup the
> irq_data.hwirq itself? No other code should be setting this up.

It is supposed to be setting it up. irq_domain_add_legacy() loops
over the range of hwirqs and sets both the domain and the hwirq
mapping. Can you insert some debug printks into that function and
trace through what is actually getting called?

g.

2012-02-08 06:16:59

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

On Fri, Feb 03, 2012 at 04:47:17PM -0700, Grant Likely wrote:
> On Fri, Feb 3, 2012 at 3:35 PM, Rob Herring <[email protected]> wrote:
...
> > +static struct irq_domain_ops irq_gc_irq_domain_ops = {
> > + ? ? ? .map = irq_gc_irq_domain_map,
> > + ? ? ? .xlate = irq_domain_xlate_onetwocell,
>
> Can we hard-enforce _xlate_twocell here? Are there any users using
> just one cell? Alternately, can we make the caller specify if one or
> two cells are expected?
>
I did not notice this comment until I tried Rob's v5 branch. The imx5
TZIC uses just one cell.

--
Regards,
Shawn

2012-02-08 07:15:48

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

On Mon, Feb 06, 2012 at 10:54:56PM -0600, Rob Herring wrote:
...
> I've pushed a v5 branch. Can you please test it out on mx51.
>
Unfortunately, it still does not work. The following are the changes
I made to get it work for both non-dt and dt boot.

Since you do not have hardware to test, I would suggest you leave the
tzic changes to me. I would post a series to migrate tzic and gpio-mxc
shortly after your series becomes stable, and you can fold it into your
series if it looks good to you then.

BTW, the arch/arm/mach-mx5 folder has been merged into mach-imx since
3.3-rc2. The following changes are based on a merging of your branch
into 3.3-rc2.

---8<-----
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 6663986..a5fda43 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -171,6 +171,12 @@
status = "disabled";
};

+ gpt@73fa0000 {
+ compatible = "fsl,imx51-gpt", "fsl,gpt";
+ reg = <0x73fa0000 0x4000>;
+ interrupts = <39>;
+ };
+
uart1: uart@73fbc000 {
compatible = "fsl,imx51-uart", "fsl,imx21-uart";
reg = <0x73fbc000 0x4000>;
diff --git a/arch/arm/mach-imx/clock-mx51-mx53.c b/arch/arm/mach-imx/clock-mx51-mx53.c
index 0847050..4f2dc66 100644
--- a/arch/arm/mach-imx/clock-mx51-mx53.c
+++ b/arch/arm/mach-imx/clock-mx51-mx53.c
@@ -16,6 +16,7 @@
#include <linux/io.h>
#include <linux/clkdev.h>
#include <linux/of.h>
+#include <linux/of_irq.h>

#include <asm/div64.h>

@@ -1555,10 +1556,12 @@ static void clk_tree_init(void)
__raw_writel(reg, MXC_CCM_CBCDR);
}

+static int get_timer_irq(void);
+
int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,
unsigned long ckih1, unsigned long ckih2)
{
- int i;
+ int i, gpt_irq;

external_low_reference = ckil;
external_high_reference = ckih1;
@@ -1592,6 +1595,9 @@ int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,
clk_set_rate(&esdhc2_clk, 166250000);

/* System timer */
+ gpt_irq = get_timer_irq();
+ if (!gpt_irq)
+ gpt_irq = MX51_INT_GPT;
mxc_timer_init(&gpt_clk, MX51_IO_ADDRESS(MX51_GPT1_BASE_ADDR),
MX51_INT_GPT);
return 0;
@@ -1600,7 +1606,7 @@ int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,
int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
unsigned long ckih1, unsigned long ckih2)
{
- int i;
+ int i, gpt_irq;

external_low_reference = ckil;
external_high_reference = ckih1;
@@ -1629,8 +1635,11 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
clk_set_rate(&esdhc3_mx53_clk, 200000000);

/* System timer */
+ gpt_irq = get_timer_irq();
+ if (!gpt_irq)
+ gpt_irq = MX53_INT_GPT;
mxc_timer_init(&gpt_clk, MX53_IO_ADDRESS(MX53_GPT1_BASE_ADDR),
- MX53_INT_GPT);
+ gpt_irq);
return 0;
}

@@ -1672,4 +1681,15 @@ int __init mx53_clocks_init_dt(void)
clk_get_freq_dt(&ckil, &osc, &ckih1, &ckih2);
return mx53_clocks_init(ckil, osc, ckih1, ckih2);
}
+
+static inline int get_timer_irq(void)
+{
+ return irq_of_parse_and_map(
+ of_find_compatible_node(NULL, NULL, "fsl,gpt"), 0);
+}
+#else
+static inline int get_timer_irq(void)
+{
+ return 0;
+}
#endif
diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
index cc4bb16..98eb415 100644
--- a/arch/arm/mach-imx/imx51-dt.c
+++ b/arch/arm/mach-imx/imx51-dt.c
@@ -12,6 +12,7 @@

#include <linux/irq.h>
#include <linux/irqdomain.h>
+#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <asm/mach/arch.h>
diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
index 1962188..def0090 100644
--- a/arch/arm/plat-mxc/tzic.c
+++ b/arch/arm/plat-mxc/tzic.c
@@ -167,7 +167,9 @@ void __init tzic_init_irq(void __iomem *irqbase)

/* all IRQ no FIQ Warning :: No selection */

- irq_setup_generic_chip_domain("tzic", NULL, 1, 0, tzic_base,
+ irq_setup_generic_chip_domain("tzic",
+ of_find_compatible_node(NULL, NULL, "fsl,tzic"),
+ 1, 0, tzic_base,
handle_level_irq, TZIC_NUM_IRQS, 0,
IRQ_NOREQUEST, 0,
tzic_init_gc, &tzic_extra_irq);
diff --git a/include/linux/of.h b/include/linux/of.h
index a75a831..92cf6ad 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -281,6 +281,14 @@ static inline struct property *of_find_property(const struct device_node *np,
return NULL;
}

+static inline struct device_node *of_find_compatible_node(
+ struct device_node *from,
+ const char *type,
+ const char *compat)
+{
+ return NULL;
+}
+
static inline int of_property_read_u32_array(const struct device_node *np,
const char *propname,
u32 *out_values, size_t sz)
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index fed1cf7..c9e7a58 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -291,7 +291,7 @@ static int irq_gc_irq_domain_map(struct irq_domain *d, unsigned int irq,

static const struct irq_domain_ops irq_gc_irq_domain_ops = {
.map = irq_gc_irq_domain_map,
- .xlate = irq_domain_xlate_twocell,
+ .xlate = irq_domain_xlate_onetwocell,
};

/*
@@ -344,13 +344,6 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
gc_init_cb(gc[i]);

irq_setup_generic_chip(gc[i], 0, flags, clr, set);
- if (node)
- continue;
-
- /* Additional setup for legacy domains */
- for (hwirq = 0; hwirq < 32; irq_base++, hwirq++)
- irq_get_irq_data(irq_base)->hwirq =
- gc[i]->hwirq_base + hwirq;
}

if (node)
---->8----

> Grant, is there some reason a legacy domain cannot setup the
> irq_data.hwirq itself? No other code should be setting this up.
>
I'm not sure this is true, since it works for my non-dt case with your
'Addtional setup ...' code above removed.

--
Regards,
Shawn

2012-02-08 16:49:41

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 2/4] irq: add irq_domain support to generic-chip

On 02/08/2012 01:15 AM, Shawn Guo wrote:
> On Mon, Feb 06, 2012 at 10:54:56PM -0600, Rob Herring wrote:
> ...
>> I've pushed a v5 branch. Can you please test it out on mx51.
>>
> Unfortunately, it still does not work. The following are the changes
> I made to get it work for both non-dt and dt boot.
>
> Since you do not have hardware to test, I would suggest you leave the
> tzic changes to me. I would post a series to migrate tzic and gpio-mxc
> shortly after your series becomes stable, and you can fold it into your
> series if it looks good to you then.

Certainly fine by me. My plan is to send this to Grant.

> BTW, the arch/arm/mach-mx5 folder has been merged into mach-imx since
> 3.3-rc2. The following changes are based on a merging of your branch
> into 3.3-rc2.
>
> ---8<-----
> diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
> index 6663986..a5fda43 100644
> --- a/arch/arm/boot/dts/imx51.dtsi
> +++ b/arch/arm/boot/dts/imx51.dtsi
> @@ -171,6 +171,12 @@
> status = "disabled";
> };
>
> + gpt@73fa0000 {
> + compatible = "fsl,imx51-gpt", "fsl,gpt";
> + reg = <0x73fa0000 0x4000>;
> + interrupts = <39>;
> + };
> +
> uart1: uart@73fbc000 {
> compatible = "fsl,imx51-uart", "fsl,imx21-uart";
> reg = <0x73fbc000 0x4000>;
> diff --git a/arch/arm/mach-imx/clock-mx51-mx53.c b/arch/arm/mach-imx/clock-mx51-mx53.c
> index 0847050..4f2dc66 100644
> --- a/arch/arm/mach-imx/clock-mx51-mx53.c
> +++ b/arch/arm/mach-imx/clock-mx51-mx53.c
> @@ -16,6 +16,7 @@
> #include <linux/io.h>
> #include <linux/clkdev.h>
> #include <linux/of.h>
> +#include <linux/of_irq.h>
>
> #include <asm/div64.h>
>
> @@ -1555,10 +1556,12 @@ static void clk_tree_init(void)
> __raw_writel(reg, MXC_CCM_CBCDR);
> }
>
> +static int get_timer_irq(void);
> +
> int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,
> unsigned long ckih1, unsigned long ckih2)
> {
> - int i;
> + int i, gpt_irq;
>
> external_low_reference = ckil;
> external_high_reference = ckih1;
> @@ -1592,6 +1595,9 @@ int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,
> clk_set_rate(&esdhc2_clk, 166250000);
>
> /* System timer */
> + gpt_irq = get_timer_irq();
> + if (!gpt_irq)
> + gpt_irq = MX51_INT_GPT;
> mxc_timer_init(&gpt_clk, MX51_IO_ADDRESS(MX51_GPT1_BASE_ADDR),
> MX51_INT_GPT);
> return 0;
> @@ -1600,7 +1606,7 @@ int __init mx51_clocks_init(unsigned long ckil, unsigned long osc,
> int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
> unsigned long ckih1, unsigned long ckih2)
> {
> - int i;
> + int i, gpt_irq;
>
> external_low_reference = ckil;
> external_high_reference = ckih1;
> @@ -1629,8 +1635,11 @@ int __init mx53_clocks_init(unsigned long ckil, unsigned long osc,
> clk_set_rate(&esdhc3_mx53_clk, 200000000);
>
> /* System timer */
> + gpt_irq = get_timer_irq();
> + if (!gpt_irq)
> + gpt_irq = MX53_INT_GPT;
> mxc_timer_init(&gpt_clk, MX53_IO_ADDRESS(MX53_GPT1_BASE_ADDR),
> - MX53_INT_GPT);
> + gpt_irq);
> return 0;
> }
>
> @@ -1672,4 +1681,15 @@ int __init mx53_clocks_init_dt(void)
> clk_get_freq_dt(&ckil, &osc, &ckih1, &ckih2);
> return mx53_clocks_init(ckil, osc, ckih1, ckih2);
> }
> +
> +static inline int get_timer_irq(void)
> +{
> + return irq_of_parse_and_map(
> + of_find_compatible_node(NULL, NULL, "fsl,gpt"), 0);
> +}
> +#else
> +static inline int get_timer_irq(void)
> +{
> + return 0;
> +}
> #endif
> diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
> index cc4bb16..98eb415 100644
> --- a/arch/arm/mach-imx/imx51-dt.c
> +++ b/arch/arm/mach-imx/imx51-dt.c
> @@ -12,6 +12,7 @@
>
> #include <linux/irq.h>
> #include <linux/irqdomain.h>
> +#include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/of_platform.h>
> #include <asm/mach/arch.h>
> diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
> index 1962188..def0090 100644
> --- a/arch/arm/plat-mxc/tzic.c
> +++ b/arch/arm/plat-mxc/tzic.c
> @@ -167,7 +167,9 @@ void __init tzic_init_irq(void __iomem *irqbase)
>
> /* all IRQ no FIQ Warning :: No selection */
>
> - irq_setup_generic_chip_domain("tzic", NULL, 1, 0, tzic_base,
> + irq_setup_generic_chip_domain("tzic",
> + of_find_compatible_node(NULL, NULL, "fsl,tzic"),
> + 1, 0, tzic_base,
> handle_level_irq, TZIC_NUM_IRQS, 0,
> IRQ_NOREQUEST, 0,
> tzic_init_gc, &tzic_extra_irq);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index a75a831..92cf6ad 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -281,6 +281,14 @@ static inline struct property *of_find_property(const struct device_node *np,
> return NULL;
> }
>
> +static inline struct device_node *of_find_compatible_node(
> + struct device_node *from,
> + const char *type,
> + const char *compat)
> +{
> + return NULL;
> +}
> +
> static inline int of_property_read_u32_array(const struct device_node *np,
> const char *propname,
> u32 *out_values, size_t sz)
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index fed1cf7..c9e7a58 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -291,7 +291,7 @@ static int irq_gc_irq_domain_map(struct irq_domain *d, unsigned int irq,
>
> static const struct irq_domain_ops irq_gc_irq_domain_ops = {
> .map = irq_gc_irq_domain_map,
> - .xlate = irq_domain_xlate_twocell,
> + .xlate = irq_domain_xlate_onetwocell,
> };
>
> /*
> @@ -344,13 +344,6 @@ int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> gc_init_cb(gc[i]);
>
> irq_setup_generic_chip(gc[i], 0, flags, clr, set);
> - if (node)
> - continue;
> -
> - /* Additional setup for legacy domains */
> - for (hwirq = 0; hwirq < 32; irq_base++, hwirq++)
> - irq_get_irq_data(irq_base)->hwirq =
> - gc[i]->hwirq_base + hwirq;
> }
>
> if (node)
> ---->8----
>
>> Grant, is there some reason a legacy domain cannot setup the
>> irq_data.hwirq itself? No other code should be setting this up.
>>
> I'm not sure this is true, since it works for my non-dt case with your
> 'Addtional setup ...' code above removed.

Ahh, right.

Rob

2012-02-08 22:55:40

by Rob Herring

[permalink] [raw]
Subject: [PATCH v6] irq: add irq_domain support to generic-chip

From: Rob Herring <[email protected]>

Add irq domain support to irq generic-chip. This enables users of
generic-chip to support dynamic irq assignment needed for DT interrupt
binding.

Thanks to Shawn Guo for fixes and testing.

Signed-off-by: Rob Herring <[email protected]>
Cc: Grant Likely <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
Here's the latest version. This has fixes from Shawn Guo, so should be
working. This version is also available here:

git://sources.calxeda.com/kernel/linux.git pl061-domain-v6.

include/linux/irq.h | 15 +++++
kernel/irq/generic-chip.c | 152 ++++++++++++++++++++++++++++++++++++++-------
2 files changed, 145 insertions(+), 22 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index bff29c5..d721abc 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -664,7 +664,12 @@ struct irq_chip_generic {
raw_spinlock_t lock;
void __iomem *reg_base;
unsigned int irq_base;
+ unsigned int hwirq_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 +712,16 @@ 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;
+int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
+ int num_ct, unsigned int irq_base,
+ void __iomem *reg_base,
+ irq_flow_handler_t handler, u32 hwirq_cnt,
+ enum irq_gc_flags flags,
+ unsigned int clr, unsigned int set,
+ void (*gc_init_cb)(struct irq_chip_generic *),
+ void *private);
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..3ac7fa1 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 % 32);

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 % 32);

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 % 32);

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 % 32);

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 % 32);

irq_gc_lock(gc);
irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
@@ -122,10 +123,10 @@ 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 % 32);

irq_gc_lock(gc);
- irq_reg_writel(mask, gc->reg_base + cur_regs(d)->ack);
+ irq_reg_writel(~mask, gc->reg_base + cur_regs(d)->ack);
irq_gc_unlock(gc);
}

@@ -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 % 32);

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 % 32);

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 % 32);

if (!(mask & gc->wake_enabled))
return -EINVAL;
@@ -220,6 +221,18 @@ EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
*/
static struct lock_class_key irq_nested_lock_class;

+static void irq_gc_setup_irq(struct irq_chip_generic *gc, int irq)
+{
+ 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);
+}
+
/**
* irq_setup_generic_chip - Setup a range of interrupts with a generic chip
* @gc: Generic irq chip holding all data
@@ -247,21 +260,113 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
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;
+
for (i = gc->irq_base; msk; msk >>= 1, i++) {
if (!(msk & 0x01))
continue;

- if (flags & IRQ_GC_INIT_NESTED_LOCK)
- irq_set_lockdep_class(i, &irq_nested_lock_class);
-
- irq_set_chip_and_handler(i, &ct->chip, ct->handler);
- irq_set_chip_data(i, gc);
- irq_modify_status(i, clr, set);
+ irq_gc_setup_irq(gc, i);
+ 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_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ struct irq_chip_generic **gc_array = d->host_data;
+ struct irq_chip_generic *gc = gc_array[hw / 32];
+
+ /* We need a valid irq number for suspend/resume functions */
+ if ((int)gc->irq_base == -1)
+ gc->irq_base = irq;
+ irq_gc_setup_irq(gc, irq);
+ return 0;
+}
+
+static const struct irq_domain_ops irq_gc_irq_domain_ops = {
+ .map = irq_gc_irq_domain_map,
+ .xlate = irq_domain_xlate_onetwocell,
+};
+
+/*
+ * irq_setup_generic_chip_domain - Setup a domain and N generic chips
+ * @name: Name of the irq chip
+ * @node: Device tree node pointer for domain
+ * @num_ct: Number of irq_chip_type instances associated with this
+ * @irq_base: Interrupt base nr for this chip
+ * @reg_base: Register base address (virtual)
+ * @handler: Default flow handler associated with this chip
+ * @hwirq_cnt: Number of hw irqs for the domain
+ * @flags: Flags for initialization
+ * @clr: IRQ_* bits to clear
+ * @set: IRQ_* bits to set
+ * @gc_init_cb: Init callback function called for each generic irq chip created
+ * @private Ptr to caller private data
+ *
+ * Set up an irq domain and N banks of 32 interrupts starting from gc->irq_base.
+ * Note, this initializes all interrupts to the primary irq_chip_type and its
+ * associated handler.
+ */
+int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
+ int num_ct, unsigned int irq_base,
+ void __iomem *reg_base,
+ irq_flow_handler_t handler, u32 hwirq_cnt,
+ enum irq_gc_flags flags, unsigned int clr,
+ unsigned int set,
+ void (*gc_init_cb)(struct irq_chip_generic *),
+ void *private)
+{
+ int ret, i;
+ struct irq_chip_generic **gc;
+ struct irq_domain *d;
+ int num_gc = ALIGN(hwirq_cnt, 32) / 32;
+
+ gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
+ if (!gc)
+ return -ENOMEM;
+
+ for (i = 0; i < num_gc; i++) {
+ gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
+ reg_base, handler);
+ if (!gc[i]) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ gc[i]->hwirq_base = i * 32;
+ gc[i]->private = private;
+
+ gc_init_cb(gc[i]);
+
+ irq_setup_generic_chip(gc[i], 0, flags, clr, set);
+ }
+
+ if (node)
+ d = irq_domain_add_linear(node, hwirq_cnt,
+ &irq_gc_irq_domain_ops, gc);
+ else
+ d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
+ &irq_gc_irq_domain_ops, gc);
+
+ for (i = 0; i < num_gc; i++)
+ gc[i]->domain = d;
+
+ return 0;
+
+ err:
+ for ( ; i >= 0; i--)
+ kfree(gc[i]);
+ kfree(gc);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
+#endif
+
/**
* irq_setup_alt_chip - Switch to alternative chip
* @d: irq_data for this interrupt
@@ -324,9 +429,10 @@ static int irq_gc_suspend(void)

list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
+ struct irq_data *data = irq_get_irq_data(gc->irq_base);

- if (ct->chip.irq_suspend)
- ct->chip.irq_suspend(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_suspend && data)
+ ct->chip.irq_suspend(data);
}
return 0;
}
@@ -337,9 +443,10 @@ static void irq_gc_resume(void)

list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
+ struct irq_data *data = irq_get_irq_data(gc->irq_base);

- if (ct->chip.irq_resume)
- ct->chip.irq_resume(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_resume && data)
+ ct->chip.irq_resume(data);
}
}
#else
@@ -353,9 +460,10 @@ static void irq_gc_shutdown(void)

list_for_each_entry(gc, &gc_list, list) {
struct irq_chip_type *ct = gc->chip_types;
+ struct irq_data *data = irq_get_irq_data(gc->irq_base);

- if (ct->chip.irq_pm_shutdown)
- ct->chip.irq_pm_shutdown(irq_get_irq_data(gc->irq_base));
+ if (ct->chip.irq_pm_shutdown && data)
+ ct->chip.irq_pm_shutdown(data);
}
}

--
1.7.5.4

2012-02-09 19:48:15

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v6] irq: add irq_domain support to generic-chip

On Wed, Feb 08, 2012 at 04:55:22PM -0600, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> Add irq domain support to irq generic-chip. This enables users of
> generic-chip to support dynamic irq assignment needed for DT interrupt
> binding.
>
> Thanks to Shawn Guo for fixes and testing.
>
> Signed-off-by: Rob Herring <[email protected]>
> Cc: Grant Likely <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> ---
> Here's the latest version. This has fixes from Shawn Guo, so should be
> working. This version is also available here:
>
Rob, yes it's working. But I think it should be working in a better
way after I play it a little bit more. See comment below.
...
> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
> + int num_ct, unsigned int irq_base,
> + void __iomem *reg_base,
> + irq_flow_handler_t handler, u32 hwirq_cnt,
> + enum irq_gc_flags flags, unsigned int clr,
> + unsigned int set,
> + void (*gc_init_cb)(struct irq_chip_generic *),
> + void *private)
> +{
> + int ret, i;
> + struct irq_chip_generic **gc;
> + struct irq_domain *d;
> + int num_gc = ALIGN(hwirq_cnt, 32) / 32;
> +
> + gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
> + if (!gc)
> + return -ENOMEM;
> +
> + for (i = 0; i < num_gc; i++) {
> + gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
> + reg_base, handler);
> + if (!gc[i]) {
> + ret = -ENOMEM;
> + goto err;
> + }
> + gc[i]->hwirq_base = i * 32;
> + gc[i]->private = private;
> +
> + gc_init_cb(gc[i]);
> +
> + irq_setup_generic_chip(gc[i], 0, flags, clr, set);
> + }
> +
> + if (node)
> + d = irq_domain_add_linear(node, hwirq_cnt,
> + &irq_gc_irq_domain_ops, gc);
> + else
> + d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> + &irq_gc_irq_domain_ops, gc);
> +
I do not think it's good to make this decision based on whether it's
dt or non-dt case. That said, it's fairly valid and actually
encouraged that non-dt users use linear domain too, and also the
irqdomain core has no limit on dt users to use legacy domain. Instead,
it's much more reasonable to make this decision based on if irq_base,
something like you did in your v3 patch.


- if (node)
+ if ((int) irq_base < 0)

Otherwise,

Acked-by: Shawn Guo <[email protected]>

Tested on imx51 with tzic and gpio interrupt controller.

Regards,
Shawn

> + for (i = 0; i < num_gc; i++)
> + gc[i]->domain = d;
> +
> + return 0;
> +
> + err:
> + for ( ; i >= 0; i--)
> + kfree(gc[i]);
> + kfree(gc);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
> +#endif
> +

2012-02-09 20:04:47

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] gpio: pl061: enable interrupts with DT style binding

On Fri, Feb 03, 2012 at 04:35:12PM -0600, Rob Herring wrote:
...
> @@ -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);

If I understand the driver correctly, it will add a linear domain for
dt case. Do you have code somewhere creating the mapping before this
irq_find_mapping gets called here? The reason I'm asking this is I
have to call irq_create_mapping rather than irq_find_mapping here to
get imx gpio driver working with linear domain, otherwise the
irq_find_mapping call will fail.

Regards,
Shawn

> }

2012-02-09 22:03:24

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] gpio: pl061: enable interrupts with DT style binding


On 02/09/2012 02:04 PM, Shawn Guo wrote:
> On Fri, Feb 03, 2012 at 04:35:12PM -0600, Rob Herring wrote:
> ...
>> @@ -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);
>
> If I understand the driver correctly, it will add a linear domain for
> dt case. Do you have code somewhere creating the mapping before this
> irq_find_mapping gets called here? The reason I'm asking this is I
> have to call irq_create_mapping rather than irq_find_mapping here to
> get imx gpio driver working with linear domain, otherwise the
> irq_find_mapping call will fail.
>

Right, the user has to call irq_of_parse_and_map (which calls
irq_create_mapping ultimately). Interrupts are allocated on demand. The
dts needs to declare the gpio controller as an interrupt-controller and
the node using the gpio line needs to set its interrupt parent and
interrupt connection

Rob

> Regards,
> Shawn
>
>> }

2012-02-09 22:31:19

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v6] irq: add irq_domain support to generic-chip

On 02/09/2012 01:48 PM, Shawn Guo wrote:
> On Wed, Feb 08, 2012 at 04:55:22PM -0600, Rob Herring wrote:
>> From: Rob Herring <[email protected]>
>>
>> Add irq domain support to irq generic-chip. This enables users of
>> generic-chip to support dynamic irq assignment needed for DT interrupt
>> binding.
>>
>> Thanks to Shawn Guo for fixes and testing.
>>
>> Signed-off-by: Rob Herring <[email protected]>
>> Cc: Grant Likely <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> ---
>> Here's the latest version. This has fixes from Shawn Guo, so should be
>> working. This version is also available here:
>>
> Rob, yes it's working. But I think it should be working in a better
> way after I play it a little bit more. See comment below.
> ...
>> +int irq_setup_generic_chip_domain(const char *name, struct device_node *node,
>> + int num_ct, unsigned int irq_base,
>> + void __iomem *reg_base,
>> + irq_flow_handler_t handler, u32 hwirq_cnt,
>> + enum irq_gc_flags flags, unsigned int clr,
>> + unsigned int set,
>> + void (*gc_init_cb)(struct irq_chip_generic *),
>> + void *private)
>> +{
>> + int ret, i;
>> + struct irq_chip_generic **gc;
>> + struct irq_domain *d;
>> + int num_gc = ALIGN(hwirq_cnt, 32) / 32;
>> +
>> + gc = kzalloc(num_gc * sizeof(struct irq_chip_generic *), GFP_KERNEL);
>> + if (!gc)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < num_gc; i++) {
>> + gc[i] = irq_alloc_generic_chip(name, num_ct, irq_base,
>> + reg_base, handler);
>> + if (!gc[i]) {
>> + ret = -ENOMEM;
>> + goto err;
>> + }
>> + gc[i]->hwirq_base = i * 32;
>> + gc[i]->private = private;
>> +
>> + gc_init_cb(gc[i]);
>> +
>> + irq_setup_generic_chip(gc[i], 0, flags, clr, set);
>> + }
>> +
>> + if (node)
>> + d = irq_domain_add_linear(node, hwirq_cnt,
>> + &irq_gc_irq_domain_ops, gc);
>> + else
>> + d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
>> + &irq_gc_irq_domain_ops, gc);
>> +
> I do not think it's good to make this decision based on whether it's
> dt or non-dt case. That said, it's fairly valid and actually
> encouraged that non-dt users use linear domain too, and also the
> irqdomain core has no limit on dt users to use legacy domain. Instead,
> it's much more reasonable to make this decision based on if irq_base,
> something like you did in your v3 patch.
>
>
> - if (node)
> + if ((int) irq_base < 0)
>

Really, we want to discourage/limit the use of legacy domains. This is
only used for ISA irqs on powerpc. Unfortunately, there's not really a
way to use linear domains for non-DT ATM. But maybe we can fix that.

> Otherwise,
>
> Acked-by: Shawn Guo <[email protected]>
>
> Tested on imx51 with tzic and gpio interrupt controller.

Thanks!

Rob
>
> Regards,
> Shawn
>
>> + for (i = 0; i < num_gc; i++)
>> + gc[i]->domain = d;
>> +
>> + return 0;
>> +
>> + err:
>> + for ( ; i >= 0; i--)
>> + kfree(gc[i]);
>> + kfree(gc);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(irq_setup_generic_chip_domain);
>> +#endif
>> +

2012-02-09 23:36:11

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v6] irq: add irq_domain support to generic-chip

On Thu, Feb 09, 2012 at 04:31:04PM -0600, Rob Herring wrote:
> On 02/09/2012 01:48 PM, Shawn Guo wrote:
> > On Wed, Feb 08, 2012 at 04:55:22PM -0600, Rob Herring wrote:
...
> >> + if (node)
> >> + d = irq_domain_add_linear(node, hwirq_cnt,
> >> + &irq_gc_irq_domain_ops, gc);
> >> + else
> >> + d = irq_domain_add_legacy(node, hwirq_cnt, irq_base, 0,
> >> + &irq_gc_irq_domain_ops, gc);
> >> +
> > I do not think it's good to make this decision based on whether it's
> > dt or non-dt case. That said, it's fairly valid and actually
> > encouraged that non-dt users use linear domain too, and also the
> > irqdomain core has no limit on dt users to use legacy domain. Instead,
> > it's much more reasonable to make this decision based on if irq_base,
> > something like you did in your v3 patch.
> >
> >
> > - if (node)
> > + if ((int) irq_base < 0)
> >
>
> Really, we want to discourage/limit the use of legacy domains.

That's exactly the point I want to make. Your current code forces
non-dt users to use legacy domain, since they always get 'node' as
NULL. We should check irq_base, so that non-dt users can chose to
use linear domain by passing irq_base as something like -1.

Regards,
Shawn

> This is
> only used for ISA irqs on powerpc. Unfortunately, there's not really a
> way to use linear domains for non-DT ATM. But maybe we can fix that.
>

2012-02-09 23:58:12

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] gpio: pl061: enable interrupts with DT style binding

On Thu, Feb 09, 2012 at 04:03:10PM -0600, Rob Herring wrote:
>
> On 02/09/2012 02:04 PM, Shawn Guo wrote:
> > On Fri, Feb 03, 2012 at 04:35:12PM -0600, Rob Herring wrote:
> > ...
> >> @@ -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);
> >
> > If I understand the driver correctly, it will add a linear domain for
> > dt case. Do you have code somewhere creating the mapping before this
> > irq_find_mapping gets called here? The reason I'm asking this is I
> > have to call irq_create_mapping rather than irq_find_mapping here to
> > get imx gpio driver working with linear domain, otherwise the
> > irq_find_mapping call will fail.
> >
>
> Right, the user has to call irq_of_parse_and_map (which calls
> irq_create_mapping ultimately). Interrupts are allocated on demand. The
> dts needs to declare the gpio controller as an interrupt-controller and
> the node using the gpio line needs to set its interrupt parent and
> interrupt connection
>
Yes, that's how dt users use irq. But since I'm trying to make the
imx gpio irq_domain as linear for both non-dt and dt users. Calling
irq_create_mapping here may make sense for me, since it will not
require all these non-dt users change the way they use gpio irq.

Even for dt users, there may have some case that can not work in the
way we expect.

soc {
#address-cells = <1>;
#size-cells = <1>;
compatible = "simple-bus";
interrupt-parent = <&tzic>;
ranges;

esdhc@70008000 { /* ESDHC2 */
compatible = "fsl,imx51-esdhc";
reg = <0x70008000 0x4000>;
interrupts = <2>;
cd-gpios = <&gpio1 6 0>;
wp-gpios = <&gpio1 5 0>;
};
};

In above SD example, irq_of_parse_and_map will just work out the SD
controller internal irq to tzic. How can we work out the card-detection
irq to gpio controller in the same way?

--
Regards,
Shawn

2012-02-10 00:17:50

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] gpio: pl061: enable interrupts with DT style binding

On 02/09/2012 05:58 PM, Shawn Guo wrote:
> On Thu, Feb 09, 2012 at 04:03:10PM -0600, Rob Herring wrote:
>>
>> On 02/09/2012 02:04 PM, Shawn Guo wrote:
>>> On Fri, Feb 03, 2012 at 04:35:12PM -0600, Rob Herring wrote:
>>> ...
>>>> @@ -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);
>>>
>>> If I understand the driver correctly, it will add a linear domain for
>>> dt case. Do you have code somewhere creating the mapping before this
>>> irq_find_mapping gets called here? The reason I'm asking this is I
>>> have to call irq_create_mapping rather than irq_find_mapping here to
>>> get imx gpio driver working with linear domain, otherwise the
>>> irq_find_mapping call will fail.
>>>
>>
>> Right, the user has to call irq_of_parse_and_map (which calls
>> irq_create_mapping ultimately). Interrupts are allocated on demand. The
>> dts needs to declare the gpio controller as an interrupt-controller and
>> the node using the gpio line needs to set its interrupt parent and
>> interrupt connection
>>
> Yes, that's how dt users use irq. But since I'm trying to make the
> imx gpio irq_domain as linear for both non-dt and dt users. Calling
> irq_create_mapping here may make sense for me, since it will not
> require all these non-dt users change the way they use gpio irq.
>

I wouldn't try to use linear for non-DT at this point.

> Even for dt users, there may have some case that can not work in the
> way we expect.
>
> soc {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "simple-bus";
> interrupt-parent = <&tzic>;
> ranges;
>
> esdhc@70008000 { /* ESDHC2 */
> compatible = "fsl,imx51-esdhc";
> reg = <0x70008000 0x4000>;
> interrupts = <2>;
> cd-gpios = <&gpio1 6 0>;
> wp-gpios = <&gpio1 5 0>;
> };
> };
>
> In above SD example, irq_of_parse_and_map will just work out the SD
> controller internal irq to tzic. How can we work out the card-detection
> irq to gpio controller in the same way?
>

That's the limitation in the interrupt binding. Normally, an interrupt
nexus is used for this.

Rob

2012-02-10 16:37:53

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH v4 4/4] gpio: pl061: enable interrupts with DT style binding

On Thu, Feb 09, 2012 at 06:17:44PM -0600, Rob Herring wrote:
> On 02/09/2012 05:58 PM, Shawn Guo wrote:
> > On Thu, Feb 09, 2012 at 04:03:10PM -0600, Rob Herring wrote:
> >>
> >> On 02/09/2012 02:04 PM, Shawn Guo wrote:
> >>> On Fri, Feb 03, 2012 at 04:35:12PM -0600, Rob Herring wrote:
> >>> ...
> >>>> @@ -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);
> >>>
> >>> If I understand the driver correctly, it will add a linear domain for
> >>> dt case. Do you have code somewhere creating the mapping before this
> >>> irq_find_mapping gets called here? The reason I'm asking this is I
> >>> have to call irq_create_mapping rather than irq_find_mapping here to
> >>> get imx gpio driver working with linear domain, otherwise the
> >>> irq_find_mapping call will fail.
> >>>
> >>
> >> Right, the user has to call irq_of_parse_and_map (which calls
> >> irq_create_mapping ultimately). Interrupts are allocated on demand. The
> >> dts needs to declare the gpio controller as an interrupt-controller and
> >> the node using the gpio line needs to set its interrupt parent and
> >> interrupt connection
> >>
> > Yes, that's how dt users use irq. But since I'm trying to make the
> > imx gpio irq_domain as linear for both non-dt and dt users. Calling
> > irq_create_mapping here may make sense for me, since it will not
> > require all these non-dt users change the way they use gpio irq.
> >
>
> I wouldn't try to use linear for non-DT at this point.
>
Sure, I'm not convincing you to do that but just explaining what I'm
doing here.

--
Regards,
Shawn