[PATCH v4 0/4] generic irq chip domain support

[PATCH v4 0/4] generic irq chip domain support

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

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

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

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

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

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 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

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

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

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

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/irq.h       |   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

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

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

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

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
 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

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

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

Enable DT interrupt binding support for pl061 gpio lines. If the gpio
node has an interrupt-controller property, then it will be setup to
handle interrupts on gpio lines.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Linus Walleij <linus.ml.walleij@gmail.com>
---
 .../devicetree/bindings/gpio/pl061-gpio.txt        |   15 ++++++
 drivers/gpio/gpio-pl061.c                          |   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

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

[Grant Likely]

On Fri, Feb 3, 2012 at 3:35 PM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Add irq domain support to irq generic-chip. This enables users of
> generic-chip to support dynamic irq assignment needed for DT interrupt
> binding.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
>  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.

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

[Grant Likely]

On Sat, Feb 4, 2012 at 7:08 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> On Fri, Feb 03, 2012 at 04:35:10PM -0600, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> Add irq domain support to irq generic-chip. This enables users of
>> generic-chip to support dynamic irq assignment needed for DT interrupt
>> binding.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> ---
>>  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.

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

[Shawn Guo]

On Fri, Feb 03, 2012 at 04:35:10PM -0600, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Add irq domain support to irq generic-chip. This enables users of
> generic-chip to support dynamic irq assignment needed for DT interrupt
> binding.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
>  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
> 

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

[Shawn Guo]

On Fri, Feb 03, 2012 at 04:35:11PM -0600, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Add irq domain support to tzic. This is needed to enable DT.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
>  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
> 

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

[Rob Herring]

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 <rob.herring@calxeda.com>
>>
>> Add irq domain support to irq generic-chip. This enables users of
>> generic-chip to support dynamic irq assignment needed for DT interrupt
>> binding.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> ---
>>  include/linux/irq.h       |   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

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

[Grant Likely]

On Mon, Feb 6, 2012 at 9:54 PM, Rob Herring <robherring2@gmail.com> 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 <rob.herring@calxeda.com>
>>>
>>> Add irq domain support to irq generic-chip. This enables users of
>>> generic-chip to support dynamic irq assignment needed for DT interrupt
>>> binding.
>>>
>>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> ---
>>>  include/linux/irq.h       |   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.

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

[Shawn Guo]

On Fri, Feb 03, 2012 at 04:47:17PM -0700, Grant Likely wrote:
> On Fri, Feb 3, 2012 at 3:35 PM, Rob Herring <robherring2@gmail.com> 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

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

[Shawn Guo]

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

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

[Rob Herring]

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

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

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

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

Thanks to Shawn Guo for fixes and testing.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
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

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

[Shawn Guo]

On Wed, Feb 08, 2012 at 04:55:22PM -0600, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Add irq domain support to irq generic-chip. This enables users of
> generic-chip to support dynamic irq assignment needed for DT interrupt
> binding.
> 
> Thanks to Shawn Guo for fixes and testing.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
> 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 <shawn.guo@linaro.org>

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
> +

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

[Shawn Guo]

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

>  }

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

[Rob Herring]

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
> 
>>  }

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

[Rob Herring]

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 <rob.herring@calxeda.com>
>>
>> Add irq domain support to irq generic-chip. This enables users of
>> generic-chip to support dynamic irq assignment needed for DT interrupt
>> binding.
>>
>> Thanks to Shawn Guo for fixes and testing.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> ---
>> 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 <shawn.guo@linaro.org>
> 
> 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
>> +

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

[Shawn Guo]

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.
> 

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

[Shawn Guo]

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

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

[Rob Herring]

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

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

[Shawn Guo]

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

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

[Thomas Abraham]

Hi Rob,

On 9 February 2012 04:25, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Add irq domain support to irq generic-chip. This enables users of
> generic-chip to support dynamic irq assignment needed for DT interrupt
> binding.
>
> Thanks to Shawn Guo for fixes and testing.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
> 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/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

[...]

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

Is there anyway to use a custom xlate function instead of the fixed
'irq_domain_xlate_onetwocell' xlate function. I am using this patchset
with Exynos4 interrupt combiner controller whose interrupt specifier
format is two cell but has different meaning. The first cell is the
interrupt combiner number and the second cell is the interrupt number
within that combiner.

Thanks,
Thomas.

[...]

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

[Grant Likely]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

On Fri, 13 Apr 2012 10:53:33 +0530, Thomas Abraham <thomas.abraham@linaro.org> wrote:
> Hi Rob,
> 
> On 9 February 2012 04:25, Rob Herring <robherring2@gmail.com> wrote:
> > From: Rob Herring <rob.herring@calxeda.com>
> >
> > Add irq domain support to irq generic-chip. This enables users of
> > generic-chip to support dynamic irq assignment needed for DT interrupt
> > binding.
> >
> > Thanks to Shawn Guo for fixes and testing.
> >
> > Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > ---
> > 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/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
> 
> [...]
> 
> > +static const struct irq_domain_ops irq_gc_irq_domain_ops = {
> > +       .map = irq_gc_irq_domain_map,
> > +       .xlate = irq_domain_xlate_onetwocell,
> > +};
> 
> Is there anyway to use a custom xlate function instead of the fixed
> 'irq_domain_xlate_onetwocell' xlate function. I am using this patchset
> with Exynos4 interrupt combiner controller whose interrupt specifier
> format is two cell but has different meaning. The first cell is the
> interrupt combiner number and the second cell is the interrupt number
> within that combiner.

I think it would be good to rework the API so that irq_domain +
generic_chip works like a library and setup helper function that
allows a driver to provide custom implementations for things like
xlate before the irqdomain and generic chips get registered.

g.