[PATCH 0/9] ARM: at91: irqdomain and device tree for AIC and GPIO

[PATCH 0/9] ARM: at91: irqdomain and device tree for AIC and GPIO

[Nicolas Ferre]

Hi,

This series adds irqdomain and device tree support for both the
interrupt and GPIO controllers of AT91 SoC.

The series has already been sent some time ago but has been
reworked to address Rob and Grant comments. The reworked patches
are marked with corresponding "vX" tags...

The series can go on top of Grant's patch series and depends on it:
"[PATCH v3 00/25] irq_domain generalization and refinement".

It also depends on the material already sent for at91 (base, pm_cleanup,
device-board, 9x5 branches on arm-soc).

The 3.3-rc3 is needed if you whish to test them (ioremap fix by Russell).

The USB driver modification will need a "Acked-by" from someone at linux-usb.

Note that I have remove all "Acked-by" form first round as the patches are
deeply modified.

All this is available here:
git://github.com/at91linux/linux-at91.git  nfe/at91-3.4-base2+3.3-rc3+irqdomain+aic_gpio6

Nicolas Ferre (9):
      ARM: at91/aic: add irq domain and device tree support
      ARM: at91/snapper9260: move gpio_to_irq out of structure initialization
      ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio
      ARM: at91/gpio: change comments and one variable name
      ARM: at91/gpio: add irqdomain and DT support
      ARM: at91/gpio: non-DT builds do not have gpio_chip.of_node field
      ARM: at91/gpio: add .to_irq gpio_chip handler
      ARM: at91/gpio: remove the static specification of gpio_chip.base
      ARM: at91/board-dt: remove AIC irq domain from board file

 .../devicetree/bindings/arm/atmel-aic.txt          |   38 +++
 .../devicetree/bindings/gpio/gpio_atmel.txt        |   20 ++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/at91sam9g20.dtsi                 |   48 +++-
 arch/arm/boot/dts/at91sam9g45.dtsi                 |   66 ++++-
 arch/arm/mach-at91/board-dt.c                      |    8 -
 arch/arm/mach-at91/board-snapper9260.c             |   10 +-
 arch/arm/mach-at91/gpio.c                          |  287 +++++++++++++++-----
 arch/arm/mach-at91/include/mach/gpio.h             |   12 -
 arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
 arch/arm/mach-at91/irq.c                           |   74 ++++-
 drivers/usb/host/ohci-at91.c                       |    5 +-
 12 files changed, 453 insertions(+), 119 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_atmel.txt

[PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support

[Nicolas Ferre]

Add an irqdomain for the AIC interrupt controller.
The device tree support is mapping the registers and
is using the irq_domain_add_legacy() to manage hwirq
translation.
The documentation is describing the meaning of the
two cells required for using this "interrupt-controller"
in a device tree node.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
v5: - rebased on top of Grant's "irq_domain generalization and refinement"
      patch series
    - use of irq_domain_add_legacy() API

v4: - use irq_alloc_descs() to find irq_base
    - add a new constant AIC_BASE_IRQ that will allow to skip
      first interrupt numbers (in the future)

v3: - change number of cells to define an AIC interrupt (irq trigger type)
    - change current .dtsi files to match specification
    - use irq_domain_simple_ops (for DT mapping)

v2: - use of_irq_init() function for device tree probing
    - add documentation
    - use own simple struct irq_domain_ops


 .../devicetree/bindings/arm/atmel-aic.txt          |   38 ++++++++++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/at91sam9g20.dtsi                 |   18 +++---
 arch/arm/boot/dts/at91sam9g45.dtsi                 |   16 ++--
 arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
 arch/arm/mach-at91/irq.c                           |   74 ++++++++++++++++----
 6 files changed, 119 insertions(+), 31 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt

diff --git a/Documentation/devicetree/bindings/arm/atmel-aic.txt b/Documentation/devicetree/bindings/arm/atmel-aic.txt
new file mode 100644
index 0000000..289feb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/atmel-aic.txt
@@ -0,0 +1,38 @@
+* Advanced Interrupt Controller (AIC)
+
+Required properties:
+- compatible: Should be "atmel,<chip>-aic"
+- interrupt-controller: Identifies the node as an interrupt controller.
+- interrupt-parent: For single AIC system, it is an empty property.
+- #interrupt-cells: The number of cells to define the interrupts. It sould be 2.
+  The first cell is the GPIO number.
+  The second cell is used to specify flags:
+    bits[3:0] 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.
+      Valid combinations are 1, 2, 3, 4, 8.
+      Default flag for internal sources should be set to 4 (active high).
+- reg: Should contain AIC registers location and length
+
+Examples:
+	/*
+	 * AIC
+	 */
+	aic: interrupt-controller@fffff000 {
+		compatible = "atmel,at91rm9200-aic";
+		interrupt-controller;
+		interrupt-parent;
+		#interrupt-cells = <2>;
+		reg = <0xfffff000 0x200>;
+	};
+
+	/*
+	 * An interrupt generating device that is wired to an AIC.
+	 */
+	dma: dma-controller@ffffec00 {
+		compatible = "atmel,at91sam9g45-dma";
+		reg = <0xffffec00 0x200>;
+		interrupts = <21 4>;
+	};
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 92c9c79..cabd8f5 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -322,6 +322,7 @@ config ARCH_AT91
 	select ARCH_REQUIRE_GPIOLIB
 	select HAVE_CLK
 	select CLKDEV_LOOKUP
+	select IRQ_DOMAIN
 	help
 	  This enables support for systems based on the Atmel AT91RM9200,
 	  AT91SAM9 processors.
diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
index 07603b8..9a0aee7 100644
--- a/arch/arm/boot/dts/at91sam9g20.dtsi
+++ b/arch/arm/boot/dts/at91sam9g20.dtsi
@@ -47,7 +47,7 @@
 			ranges;
 
 			aic: interrupt-controller@fffff000 {
-				#interrupt-cells = <1>;
+				#interrupt-cells = <2>;
 				compatible = "atmel,at91rm9200-aic";
 				interrupt-controller;
 				interrupt-parent;
@@ -57,14 +57,14 @@
 			dbgu: serial@fffff200 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffff200 0x200>;
-				interrupts = <1>;
+				interrupts = <1 4>;
 				status = "disabled";
 			};
 
 			usart0: serial@fffb0000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffb0000 0x200>;
-				interrupts = <6>;
+				interrupts = <6 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -73,7 +73,7 @@
 			usart1: serial@fffb4000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffb4000 0x200>;
-				interrupts = <7>;
+				interrupts = <7 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -82,7 +82,7 @@
 			usart2: serial@fffb8000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffb8000 0x200>;
-				interrupts = <8>;
+				interrupts = <8 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -91,7 +91,7 @@
 			usart3: serial@fffd0000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffd0000 0x200>;
-				interrupts = <23>;
+				interrupts = <23 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -100,7 +100,7 @@
 			usart4: serial@fffd4000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffd4000 0x200>;
-				interrupts = <24>;
+				interrupts = <24 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -109,7 +109,7 @@
 			usart5: serial@fffd8000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffd8000 0x200>;
-				interrupts = <25>;
+				interrupts = <25 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -118,7 +118,7 @@
 			macb0: ethernet@fffc4000 {
 				compatible = "cdns,at32ap7000-macb", "cdns,macb";
 				reg = <0xfffc4000 0x100>;
-				interrupts = <21>;
+				interrupts = <21 4>;
 				status = "disabled";
 			};
 		};
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index fffa005..67f94d3 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -46,7 +46,7 @@
 			ranges;
 
 			aic: interrupt-controller@fffff000 {
-				#interrupt-cells = <1>;
+				#interrupt-cells = <2>;
 				compatible = "atmel,at91rm9200-aic";
 				interrupt-controller;
 				interrupt-parent;
@@ -56,20 +56,20 @@
 			dma: dma-controller@ffffec00 {
 				compatible = "atmel,at91sam9g45-dma";
 				reg = <0xffffec00 0x200>;
-				interrupts = <21>;
+				interrupts = <21 4>;
 			};
 
 			dbgu: serial@ffffee00 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xffffee00 0x200>;
-				interrupts = <1>;
+				interrupts = <1 4>;
 				status = "disabled";
 			};
 
 			usart0: serial@fff8c000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfff8c000 0x200>;
-				interrupts = <7>;
+				interrupts = <7 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -78,7 +78,7 @@
 			usart1: serial@fff90000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfff90000 0x200>;
-				interrupts = <8>;
+				interrupts = <8 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -87,7 +87,7 @@
 			usart2: serial@fff94000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfff94000 0x200>;
-				interrupts = <9>;
+				interrupts = <9 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -96,7 +96,7 @@
 			usart3: serial@fff98000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfff98000 0x200>;
-				interrupts = <10>;
+				interrupts = <10 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";
@@ -105,7 +105,7 @@
 			macb0: ethernet@fffbc000 {
 				compatible = "cdns,at32ap7000-macb", "cdns,macb";
 				reg = <0xfffbc000 0x100>;
-				interrupts = <25>;
+				interrupts = <25 4>;
 				status = "disabled";
 			};
 		};
diff --git a/arch/arm/mach-at91/include/mach/irqs.h b/arch/arm/mach-at91/include/mach/irqs.h
index ac8b7df..e3ee0fc 100644
--- a/arch/arm/mach-at91/include/mach/irqs.h
+++ b/arch/arm/mach-at91/include/mach/irqs.h
@@ -25,6 +25,7 @@
 #include <mach/at91_aic.h>
 
 #define NR_AIC_IRQS 32
+#define AIC_BASE_IRQ 0
 
 
 /*
@@ -40,7 +41,7 @@
  * symbols in gpio.h for ones handled indirectly as GPIOs.
  * We make provision for 5 banks of GPIO.
  */
-#define	NR_IRQS		(NR_AIC_IRQS + (5 * 32))
+#define	NR_IRQS		(AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))
 
 /* FIQ is AIC source 0. */
 #define FIQ_START AT91_ID_FIQ
diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
index be6b639..880da98 100644
--- a/arch/arm/mach-at91/irq.c
+++ b/arch/arm/mach-at91/irq.c
@@ -24,6 +24,12 @@
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/types.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/err.h>
 
 #include <mach/hardware.h>
 #include <asm/irq.h>
@@ -34,22 +40,24 @@
 #include <asm/mach/map.h>
 
 void __iomem *at91_aic_base;
+static struct irq_domain *at91_aic_domain;
+static struct device_node *at91_aic_np;
 
 static void at91_aic_mask_irq(struct irq_data *d)
 {
 	/* Disable interrupt on AIC */
-	at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
+	at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
 }
 
 static void at91_aic_unmask_irq(struct irq_data *d)
 {
 	/* Enable interrupt on AIC */
-	at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
+	at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
 }
 
 unsigned int at91_extern_irq;
 
-#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
+#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
 
 static int at91_aic_set_type(struct irq_data *d, unsigned type)
 {
@@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
 		srctype = AT91_AIC_SRCTYPE_RISING;
 		break;
 	case IRQ_TYPE_LEVEL_LOW:
-		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
+		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
 			srctype = AT91_AIC_SRCTYPE_LOW;
 		else
 			return -EINVAL;
 		break;
 	case IRQ_TYPE_EDGE_FALLING:
-		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
+		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
 			srctype = AT91_AIC_SRCTYPE_FALLING;
 		else
 			return -EINVAL;
@@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
 		return -EINVAL;
 	}
 
-	smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
-	at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
+	smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
+	at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
 	return 0;
 }
 
@@ -90,13 +98,13 @@ static u32 backups;
 
 static int at91_aic_set_wake(struct irq_data *d, unsigned value)
 {
-	if (unlikely(d->irq >= 32))
+	if (unlikely(d->hwirq >= NR_AIC_IRQS))
 		return -EINVAL;
 
 	if (value)
-		wakeups |= (1 << d->irq);
+		wakeups |= (1 << d->hwirq);
 	else
-		wakeups &= ~(1 << d->irq);
+		wakeups &= ~(1 << d->hwirq);
 
 	return 0;
 }
@@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
 	.irq_set_wake	= at91_aic_set_wake,
 };
 
+#if defined(CONFIG_OF)
+static int __init __at91_aic_of_init(struct device_node *node,
+				     struct device_node *parent)
+{
+	at91_aic_base = of_iomap(node, 0);
+	at91_aic_np = node;
+
+	return 0;
+}
+
+static const struct of_device_id aic_ids[] __initconst = {
+	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
+	{ /*sentinel*/ }
+};
+
+static void __init at91_aic_of_init(void)
+{
+	of_irq_init(aic_ids);
+}
+#else
+static void __init at91_aic_of_init(void) {}
+#endif
+
 /*
  * Initialize the AIC interrupt controller.
  */
 void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
 {
 	unsigned int i;
+	int irq_base;
 
-	at91_aic_base = ioremap(AT91_AIC, 512);
+	if (of_have_populated_dt())
+		at91_aic_of_init();
+	else
+		at91_aic_base = ioremap(AT91_AIC, 512);
 
 	if (!at91_aic_base)
-		panic("Impossible to ioremap AT91_AIC\n");
+		panic("Unable to ioremap AIC registers\n");
+
+	/* Add irq domain for AIC */
+	irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);
+	if (irq_base < 0) {
+		WARN(1, "Cannot allocate irq_descs, assuming pre-allocated\n");
+		irq_base = AIC_BASE_IRQ;
+	}
+	at91_aic_domain = irq_domain_add_legacy(at91_aic_np, NR_AIC_IRQS,
+						irq_base, 0,
+						&irq_domain_simple_ops, NULL);
+
+	if (!at91_aic_domain)
+		panic("Unable to add AIC irq domain\n");
 
 	/*
 	 * The IVR is used by macro get_irqnr_and_base to read and verify.
 	 * The irq number is NR_AIC_IRQS when a spurious interrupt has occurred.
 	 */
 	for (i = 0; i < NR_AIC_IRQS; i++) {
-		/* Put irq number in Source Vector Register: */
+		/* Put hardware irq number in Source Vector Register: */
 		at91_aic_write(AT91_AIC_SVR(i), i);
 		/* Active Low interrupt, with the specified priority */
 		at91_aic_write(AT91_AIC_SMR(i), AT91_AIC_SRCTYPE_LOW | priority[i]);
-- 
1.7.9

[PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization

[Nicolas Ferre]

gpio_to_irq() implementation will be moved from a macro to a
plain function: we cannot use it in a structure initialization
anymore.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: rmallon@gmail.com
---
 arch/arm/mach-at91/board-snapper9260.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-at91/board-snapper9260.c b/arch/arm/mach-at91/board-snapper9260.c
index 4770db0..3c2e3fc 100644
--- a/arch/arm/mach-at91/board-snapper9260.c
+++ b/arch/arm/mach-at91/board-snapper9260.c
@@ -145,11 +145,11 @@ static struct i2c_board_info __initdata snapper9260_i2c_devices[] = {
 		/* Audio codec */
 		I2C_BOARD_INFO("tlv320aic23", 0x1a),
 	},
-	{
+};
+
+static struct i2c_board_info __initdata snapper9260_i2c_isl1208 = {
 		/* RTC */
 		I2C_BOARD_INFO("isl1208", 0x6f),
-		.irq = gpio_to_irq(AT91_PIN_PA31),
-	},
 };
 
 static void __init snapper9260_add_device_nand(void)
@@ -163,6 +163,10 @@ static void __init snapper9260_board_init(void)
 {
 	at91_add_device_i2c(snapper9260_i2c_devices,
 			    ARRAY_SIZE(snapper9260_i2c_devices));
+
+	snapper9260_i2c_isl1208.irq = gpio_to_irq(AT91_PIN_PA31);
+	i2c_register_board_info(0, &snapper9260_i2c_isl1208, 1);
+
 	at91_add_device_serial();
 	at91_add_device_usbh(&snapper9260_usbh_data);
 	at91_add_device_udc(&snapper9260_udc_data);
-- 
1.7.9

[PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio

[Nicolas Ferre]

irq_to_gpio() macro will be removed from AT91 GPIO interrupt
controller. So we replace it with the use of gpio_to_irq()
and a reworked test.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/host/ohci-at91.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
index 77afabc..8e855eb 100644
--- a/drivers/usb/host/ohci-at91.c
+++ b/drivers/usb/host/ohci-at91.c
@@ -448,10 +448,11 @@ static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
 
 	/* From the GPIO notifying the over-current situation, find
 	 * out the corresponding port */
-	gpio = irq_to_gpio(irq);
 	for (port = 0; port < ARRAY_SIZE(pdata->overcurrent_pin); port++) {
-		if (pdata->overcurrent_pin[port] == gpio)
+		if (gpio_to_irq(pdata->overcurrent_pin[port]) == irq) {
+			gpio = pdata->overcurrent_pin[port];
 			break;
+		}
 	}
 
 	if (port == ARRAY_SIZE(pdata->overcurrent_pin)) {
-- 
1.7.9

[PATCH 4/9] ARM: at91/gpio: change comments and one variable name

[Nicolas Ferre]

What was true only on at91sam9263 about the sharing of a single AIC
IRQ line for several GPIO banks is now used by several Atmel SoCs.

Change a variable name to allow better understanding while
introducing IRQ domains in following patches.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/gpio.c |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index 74d6783..b762afc 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -29,8 +29,8 @@
 struct at91_gpio_chip {
 	struct gpio_chip	chip;
 	struct at91_gpio_chip	*next;		/* Bank sharing same clock */
-	int			id;		/* ID of register bank */
-	void __iomem		*regbase;	/* Base of register bank */
+	int			pioc_hwirq;	/* PIO bank interrupt identifier on AIC */
+	void __iomem		*regbase;	/* PIO bank virtual address */
 	struct clk		*clock;		/* associated clock */
 };
 
@@ -285,7 +285,7 @@ static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
 	else
 		wakeups[bank] &= ~mask;
 
-	irq_set_irq_wake(gpio_chip[bank].id, state);
+	irq_set_irq_wake(gpio_chip[bank].pioc_hwirq, state);
 
 	return 0;
 }
@@ -499,7 +499,7 @@ void __init at91_gpio_irq_setup(void)
 	for (pioc = 0, this = gpio_chip, prev = NULL;
 			pioc++ < gpio_banks;
 			prev = this, this++) {
-		unsigned	id = this->id;
+		unsigned	pioc_hwirq = this->pioc_hwirq;
 		unsigned	i;
 
 		__raw_writel(~0, this->regbase + PIO_IDR);
@@ -518,14 +518,14 @@ void __init at91_gpio_irq_setup(void)
 		}
 
 		/* The toplevel handler handles one bank of GPIOs, except
-		 * AT91SAM9263_ID_PIOCDE handles three... PIOC is first in
-		 * the list, so we only set up that handler.
+		 * on some SoC it can handles up to three...
+		 * We only set up the handler for the first of the list.
 		 */
 		if (prev && prev->next == this)
 			continue;
 
-		irq_set_chip_data(id, this);
-		irq_set_chained_handler(id, gpio_irq_handler);
+		irq_set_chip_data(pioc_hwirq, this);
+		irq_set_chained_handler(pioc_hwirq, gpio_irq_handler);
 	}
 	pr_info("AT91: %d gpio irqs in %d banks\n", irq - gpio_to_irq(0), gpio_banks);
 }
@@ -615,7 +615,7 @@ void __init at91_gpio_init(struct at91_gpio_bank *data, int nr_banks)
 	for (i = 0; i < nr_banks; i++) {
 		at91_gpio = &gpio_chip[i];
 
-		at91_gpio->id = data[i].id;
+		at91_gpio->pioc_hwirq = data[i].pioc_hwirq;
 		at91_gpio->chip.base = i * 32;
 
 		at91_gpio->regbase = ioremap(data[i].regbase, 512);
@@ -633,8 +633,11 @@ void __init at91_gpio_init(struct at91_gpio_bank *data, int nr_banks)
 		/* enable PIO controller's clock */
 		clk_enable(at91_gpio->clock);
 
-		/* AT91SAM9263_ID_PIOCDE groups PIOC, PIOD, PIOE */
-		if (last && last->id == at91_gpio->id)
+		/*
+		 * GPIO controller are grouped on some SoC:
+		 * PIOC, PIOD and PIOE can share the same IRQ line
+		 */
+		if (last && last->pioc_hwirq == at91_gpio->pioc_hwirq)
 			last->next = at91_gpio;
 		last = at91_gpio;
 
-- 
1.7.9

[PATCH v5 5/9] ARM: at91/gpio: add irqdomain and DT support

[Nicolas Ferre]

Add "legacy" type of irqdomain to preserve old-style numbering
and allow smooth transition for both DT and non-DT cases.

Original idea and code by Jean-Christophe Plagniol-Villard.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
v5: - merge irqdomain and device tree modifications
    - use "legacy" type of irqdomain (call to irq_domain_add_legacy())
    - use more explicit variable names

v4: - at91_gpio_setup_clk() returns an error code

v3: - Corrections proposed by Russell King:
      - use of clk_prepare()/clk_unprepare()
      - helper function to setup the clock for both DT/non DT init functions

v2: - no BUG_ON() anymore in case of an "interrupts" property missing
      just stop configuring the gpio bank
    - review error path in both of_at91_gpio_init_one() and
      at91_gpio_init_one()
    - use for_each_compatible_node() iterator instead of a homemade loop

 .../devicetree/bindings/gpio/gpio_atmel.txt        |   20 ++
 arch/arm/boot/dts/at91sam9g20.dtsi                 |   30 +++
 arch/arm/boot/dts/at91sam9g45.dtsi                 |   50 +++++
 arch/arm/mach-at91/gpio.c                          |  233 ++++++++++++++++----
 4 files changed, 287 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_atmel.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio_atmel.txt b/Documentation/devicetree/bindings/gpio/gpio_atmel.txt
new file mode 100644
index 0000000..a7bcaec
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio_atmel.txt
@@ -0,0 +1,20 @@
+* Atmel GPIO controller (PIO)
+
+Required properties:
+- compatible: "atmel,at91rm9200-gpio"
+- reg: Should contain GPIO controller registers location and length
+- interrupts: Should be the port interrupt shared by all the pins.
+- #gpio-cells: Should be two.  The first cell is the pin number and
+  the second cell is used to specify optional parameters (currently
+  unused).
+- gpio-controller: Marks the device node as a GPIO controller.
+
+Example:
+	pioA: gpio@fffff200 {
+		compatible = "atmel,at91rm9200-gpio";
+		reg = <0xfffff200 0x100>;
+		interrupts = <2 4>;
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
+
diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
index 9a0aee7..325989a 100644
--- a/arch/arm/boot/dts/at91sam9g20.dtsi
+++ b/arch/arm/boot/dts/at91sam9g20.dtsi
@@ -23,6 +23,9 @@
 		serial4 = &usart3;
 		serial5 = &usart4;
 		serial6 = &usart5;
+		gpio0 = &pioA;
+		gpio1 = &pioB;
+		gpio2 = &pioC;
 	};
 	cpus {
 		cpu@0 {
@@ -54,6 +57,33 @@
 				reg = <0xfffff000 0x200>;
 			};
 
+			pioA: gpio@fffff400 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff400 0x100>;
+				interrupts = <2 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+				interrupt-controller;
+			};
+
+			pioB: gpio@fffff600 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff600 0x100>;
+				interrupts = <3 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+				interrupt-controller;
+			};
+
+			pioC: gpio@fffff800 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff800 0x100>;
+				interrupts = <4 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+				interrupt-controller;
+			};
+
 			dbgu: serial@fffff200 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffff200 0x200>;
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 67f94d3..a9dbbb5 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -22,6 +22,11 @@
 		serial2 = &usart1;
 		serial3 = &usart2;
 		serial4 = &usart3;
+		gpio0 = &pioA;
+		gpio1 = &pioB;
+		gpio2 = &pioC;
+		gpio3 = &pioD;
+		gpio4 = &pioE;
 	};
 	cpus {
 		cpu@0 {
@@ -59,6 +64,51 @@
 				interrupts = <21 4>;
 			};
 
+			pioA: gpio@fffff200 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff200 0x100>;
+				interrupts = <2 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+				interrupt-controller;
+			};
+
+			pioB: gpio@fffff400 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff400 0x100>;
+				interrupts = <3 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+				interrupt-controller;
+			};
+
+			pioC: gpio@fffff600 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff600 0x100>;
+				interrupts = <4 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+				interrupt-controller;
+			};
+
+			pioD: gpio@fffff800 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff800 0x100>;
+				interrupts = <5 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+				interrupt-controller;
+			};
+
+			pioE: gpio@fffffa00 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffffa00 0x100>;
+				interrupts = <5 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+				interrupt-controller;
+			};
+
 			dbgu: serial@ffffee00 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xffffee00 0x200>;
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index b762afc..89e683a 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -20,6 +20,9 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/io.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 
 #include <mach/hardware.h>
 #include <mach/at91_pio.h>
@@ -30,8 +33,10 @@ struct at91_gpio_chip {
 	struct gpio_chip	chip;
 	struct at91_gpio_chip	*next;		/* Bank sharing same clock */
 	int			pioc_hwirq;	/* PIO bank interrupt identifier on AIC */
+	int			pioc_idx;	/* PIO bank index */
 	void __iomem		*regbase;	/* PIO bank virtual address */
 	struct clk		*clock;		/* associated clock */
+	struct irq_domain	*domain;	/* associated irq domain */
 };
 
 #define to_at91_gpio_chip(c) container_of(c, struct at91_gpio_chip, chip)
@@ -273,9 +278,9 @@ static u32 backups[MAX_GPIO_BANKS];
 
 static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
 {
-	unsigned	pin = irq_to_gpio(d->irq);
-	unsigned	mask = pin_to_mask(pin);
-	unsigned	bank = pin / 32;
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+	unsigned	mask = 1 << d->hwirq;
+	unsigned	bank = at91_gpio->pioc_idx;
 
 	if (unlikely(bank >= MAX_GPIO_BANKS))
 		return -EINVAL;
@@ -301,9 +306,10 @@ void at91_gpio_suspend(void)
 		__raw_writel(backups[i], pio + PIO_IDR);
 		__raw_writel(wakeups[i], pio + PIO_IER);
 
-		if (!wakeups[i])
+		if (!wakeups[i]) {
+			clk_unprepare(gpio_chip[i].clock);
 			clk_disable(gpio_chip[i].clock);
-		else {
+		} else {
 #ifdef CONFIG_PM_DEBUG
 			printk(KERN_DEBUG "GPIO-%c may wake for %08x\n", 'A'+i, wakeups[i]);
 #endif
@@ -318,8 +324,10 @@ void at91_gpio_resume(void)
 	for (i = 0; i < gpio_banks; i++) {
 		void __iomem	*pio = gpio_chip[i].regbase;
 
-		if (!wakeups[i])
-			clk_enable(gpio_chip[i].clock);
+		if (!wakeups[i]) {
+			if (clk_prepare(gpio_chip[i].clock) == 0)
+				clk_enable(gpio_chip[i].clock);
+		}
 
 		__raw_writel(wakeups[i], pio + PIO_IDR);
 		__raw_writel(backups[i], pio + PIO_IER);
@@ -344,9 +352,9 @@ void at91_gpio_resume(void)
 
 static void gpio_irq_mask(struct irq_data *d)
 {
-	unsigned	pin = irq_to_gpio(d->irq);
-	void __iomem	*pio = pin_to_controller(pin);
-	unsigned	mask = pin_to_mask(pin);
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+	void __iomem	*pio = at91_gpio->regbase;
+	unsigned	mask = 1 << d->hwirq;
 
 	if (pio)
 		__raw_writel(mask, pio + PIO_IDR);
@@ -354,9 +362,9 @@ static void gpio_irq_mask(struct irq_data *d)
 
 static void gpio_irq_unmask(struct irq_data *d)
 {
-	unsigned	pin = irq_to_gpio(d->irq);
-	void __iomem	*pio = pin_to_controller(pin);
-	unsigned	mask = pin_to_mask(pin);
+	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(d);
+	void __iomem	*pio = at91_gpio->regbase;
+	unsigned	mask = 1 << d->hwirq;
 
 	if (pio)
 		__raw_writel(mask, pio + PIO_IER);
@@ -384,7 +392,7 @@ static struct irq_chip gpio_irqchip = {
 
 static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 {
-	unsigned	irq_pin;
+	unsigned	virq;
 	struct irq_data *idata = irq_desc_get_irq_data(desc);
 	struct irq_chip *chip = irq_data_get_irq_chip(idata);
 	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(idata);
@@ -407,12 +415,12 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 			continue;
 		}
 
-		irq_pin = gpio_to_irq(at91_gpio->chip.base);
+		virq = gpio_to_irq(at91_gpio->chip.base);
 
 		while (isr) {
 			if (isr & 1)
-				generic_handle_irq(irq_pin);
-			irq_pin++;
+				generic_handle_irq(virq);
+			virq++;
 			isr >>= 1;
 		}
 	}
@@ -483,6 +491,26 @@ postcore_initcall(at91_gpio_debugfs_init);
 /*--------------------------------------------------------------------------*/
 
 /*
+ * irqdomain initialization: pile up irqdomains on top of AIC range
+ */
+static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
+{
+	int irq_base;
+
+	irq_base = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
+	if (irq_base < 0)
+		panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
+			at91_gpio->pioc_idx, irq_base);
+	at91_gpio->domain = irq_domain_add_legacy(at91_gpio->chip.of_node,
+						  at91_gpio->chip.ngpio,
+						  irq_base, 0,
+						  &irq_domain_simple_ops, NULL);
+	if (!at91_gpio->domain)
+		panic("at91_gpio.%d: couldn't allocate irq domain.\n",
+			at91_gpio->pioc_idx);
+}
+
+/*
  * This lock class tells lockdep that GPIO irqs are in a different
  * category than their parents, so it won't report false recursion.
  */
@@ -493,28 +521,35 @@ static struct lock_class_key gpio_lock_class;
  */
 void __init at91_gpio_irq_setup(void)
 {
-	unsigned		pioc, irq = gpio_to_irq(0);
+	unsigned		pioc;
+	int			gpio_irqnbr = 0;
 	struct at91_gpio_chip	*this, *prev;
 
 	for (pioc = 0, this = gpio_chip, prev = NULL;
 			pioc++ < gpio_banks;
 			prev = this, this++) {
 		unsigned	pioc_hwirq = this->pioc_hwirq;
-		unsigned	i;
+		int		offset;
 
 		__raw_writel(~0, this->regbase + PIO_IDR);
 
-		for (i = 0, irq = gpio_to_irq(this->chip.base); i < 32;
-		     i++, irq++) {
-			irq_set_lockdep_class(irq, &gpio_lock_class);
+		/* setup irq domain for this GPIO controller */
+		at91_gpio_irqdomain(this);
+
+		for (offset = 0; offset < this->chip.ngpio; offset++) {
+			unsigned int virq = irq_find_mapping(this->domain, offset);
+			irq_set_lockdep_class(virq, &gpio_lock_class);
 
 			/*
 			 * Can use the "simple" and not "edge" handler since it's
 			 * shorter, and the AIC handles interrupts sanely.
 			 */
-			irq_set_chip_and_handler(irq, &gpio_irqchip,
+			irq_set_chip_and_handler(virq, &gpio_irqchip,
 						 handle_simple_irq);
-			set_irq_flags(irq, IRQF_VALID);
+			set_irq_flags(virq, IRQF_VALID);
+			irq_set_chip_data(virq, this);
+
+			gpio_irqnbr++;
 		}
 
 		/* The toplevel handler handles one bank of GPIOs, except
@@ -527,7 +562,7 @@ void __init at91_gpio_irq_setup(void)
 		irq_set_chip_data(pioc_hwirq, this);
 		irq_set_chained_handler(pioc_hwirq, gpio_irq_handler);
 	}
-	pr_info("AT91: %d gpio irqs in %d banks\n", irq - gpio_to_irq(0), gpio_banks);
+	pr_info("AT91: %d gpio irqs in %d banks\n", gpio_irqnbr, gpio_banks);
 }
 
 /* gpiolib support */
@@ -600,39 +635,145 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	}
 }
 
+static int __init at91_gpio_setup_clk(int idx)
+{
+	struct at91_gpio_chip *at91_gpio = &gpio_chip[idx];
+
+	/* retreive PIO controller's clock */
+	at91_gpio->clock = clk_get_sys(NULL, at91_gpio->chip.label);
+	if (IS_ERR(at91_gpio->clock)) {
+		pr_err("at91_gpio.%d, failed to get clock, ignoring.\n", idx);
+		goto err;
+	}
+
+	if (clk_prepare(at91_gpio->clock))
+		goto clk_prep_err;
+
+	/* enable PIO controller's clock */
+	if (clk_enable(at91_gpio->clock)) {
+		pr_err("at91_gpio.%d, failed to enable clock, ignoring.\n", idx);
+		goto clk_err;
+	}
+
+	return 0;
+
+clk_err:
+	clk_unprepare(at91_gpio->clock);
+clk_prep_err:
+	clk_put(at91_gpio->clock);
+err:
+	return -EINVAL;
+}
+
+#ifdef CONFIG_OF_GPIO
+static void __init of_at91_gpio_init_one(struct device_node *np)
+{
+	int alias_idx;
+	struct at91_gpio_chip *at91_gpio;
+
+	if (!np)
+		return;
+
+	alias_idx = of_alias_get_id(np, "gpio");
+	if (alias_idx >= MAX_GPIO_BANKS) {
+		pr_err("at91_gpio, failed alias idx(%d) > MAX_GPIO_BANKS(%d), ignoring.\n",
+						alias_idx, MAX_GPIO_BANKS);
+		return;
+	}
+
+	at91_gpio = &gpio_chip[alias_idx];
+	at91_gpio->chip.base = alias_idx * at91_gpio->chip.ngpio;
+
+	at91_gpio->regbase = of_iomap(np, 0);
+	if (!at91_gpio->regbase) {
+		pr_err("at91_gpio.%d, failed to map registers, ignoring.\n",
+								alias_idx);
+		return;
+	}
+
+	/* Get the interrupts property */
+	if (of_property_read_u32(np, "interrupts", &at91_gpio->pioc_hwirq)) {
+		pr_err("at91_gpio.%d, failed to get interrupts property, ignoring.\n",
+								alias_idx);
+		goto ioremap_err;
+	}
+
+	/* Setup clock */
+	if (at91_gpio_setup_clk(alias_idx))
+		goto ioremap_err;
+
+	at91_gpio->chip.of_node = np;
+	gpio_banks = max(gpio_banks, alias_idx + 1);
+	at91_gpio->pioc_idx = alias_idx;
+	return;
+
+ioremap_err:
+	iounmap(at91_gpio->regbase);
+}
+
+static int __init of_at91_gpio_init(void)
+{
+	struct device_node *np = NULL;
+
+	/*
+	 * This isn't ideal, but it gets things hooked up until this
+	 * driver is converted into a platform_device
+	 */
+	for_each_compatible_node(np, NULL, "atmel,at91rm9200-gpio")
+		of_at91_gpio_init_one(np);
+
+	return gpio_banks > 0 ? 0 : -EINVAL;
+}
+#else
+static int __init of_at91_gpio_init(void)
+{
+	return -EINVAL;
+}
+#endif
+
+static void __init at91_gpio_init_one(int idx, u32 regbase, int pioc_hwirq)
+{
+	struct at91_gpio_chip *at91_gpio = &gpio_chip[idx];
+
+	at91_gpio->chip.base = idx * at91_gpio->chip.ngpio;
+	at91_gpio->pioc_hwirq = pioc_hwirq;
+	at91_gpio->pioc_idx = idx;
+
+	at91_gpio->regbase = ioremap(regbase, 512);
+	if (!at91_gpio->regbase) {
+		pr_err("at91_gpio.%d, failed to map registers, ignoring.\n", idx);
+		return;
+	}
+
+	if (at91_gpio_setup_clk(idx))
+		goto ioremap_err;
+
+	gpio_banks = max(gpio_banks, idx + 1);
+	return;
+
+ioremap_err:
+	iounmap(at91_gpio->regbase);
+}
+
 /*
  * Called from the processor-specific init to enable GPIO pin support.
  */
 void __init at91_gpio_init(struct at91_gpio_bank *data, int nr_banks)
 {
-	unsigned		i;
+	unsigned i;
 	struct at91_gpio_chip *at91_gpio, *last = NULL;
 
 	BUG_ON(nr_banks > MAX_GPIO_BANKS);
 
-	gpio_banks = nr_banks;
+	if (of_at91_gpio_init() < 0) {
+		/* No GPIO controller found in device tree */
+		for (i = 0; i < nr_banks; i++)
+			at91_gpio_init_one(i, data[i].regbase, data[i].id);
+	}
 
-	for (i = 0; i < nr_banks; i++) {
+	for (i = 0; i < gpio_banks; i++) {
 		at91_gpio = &gpio_chip[i];
 
-		at91_gpio->pioc_hwirq = data[i].pioc_hwirq;
-		at91_gpio->chip.base = i * 32;
-
-		at91_gpio->regbase = ioremap(data[i].regbase, 512);
-		if (!at91_gpio->regbase) {
-			pr_err("at91_gpio.%d, failed to map registers, ignoring.\n", i);
-			continue;
-		}
-
-		at91_gpio->clock = clk_get_sys(NULL, at91_gpio->chip.label);
-		if (!at91_gpio->clock) {
-			pr_err("at91_gpio.%d, failed to get clock, ignoring.\n", i);
-			continue;
-		}
-
-		/* enable PIO controller's clock */
-		clk_enable(at91_gpio->clock);
-
 		/*
 		 * GPIO controller are grouped on some SoC:
 		 * PIOC, PIOD and PIOE can share the same IRQ line
-- 
1.7.9

[PATCH 6/9] ARM: at91/gpio: non-DT builds do not have gpio_chip.of_node field

[Nicolas Ferre]

Protect build failure in case of non-DT configuration: the
gpio_chip structure does not have a of_node field in case of
!CONFIG_OF_GPIO.

Keep this in a separate patch as it can be reverted if the
field is added for both DT/non-DT cases.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/gpio.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index 89e683a..0dc3f5e 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -496,12 +496,17 @@ postcore_initcall(at91_gpio_debugfs_init);
 static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
 {
 	int irq_base;
+#if defined(CONFIG_OF)
+	struct device_node *of_node = at91_gpio->chip.of_node;
+#else
+	struct device_node *of_node = NULL;
+#endif
 
 	irq_base = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
 	if (irq_base < 0)
 		panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
 			at91_gpio->pioc_idx, irq_base);
-	at91_gpio->domain = irq_domain_add_legacy(at91_gpio->chip.of_node,
+	at91_gpio->domain = irq_domain_add_legacy(of_node,
 						  at91_gpio->chip.ngpio,
 						  irq_base, 0,
 						  &irq_domain_simple_ops, NULL);
-- 
1.7.9

[PATCH 7/9] ARM: at91/gpio: add .to_irq gpio_chip handler

[Nicolas Ferre]

Replace the gpio_to_irq() macro by a plain gpiolib .to_irq() handler.
This call is using the irqdomain to translate hardware to Linux
IRQ numbers.
The irq_to_gpio() macro is completely removed.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/gpio.c              |   13 +++++++++++++
 arch/arm/mach-at91/include/mach/gpio.h |   12 ------------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index 0dc3f5e..e8f5831 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -11,6 +11,7 @@
 
 #include <linux/clk.h>
 #include <linux/errno.h>
+#include <linux/device.h>
 #include <linux/gpio.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
@@ -48,6 +49,7 @@ static int at91_gpiolib_direction_output(struct gpio_chip *chip,
 					 unsigned offset, int val);
 static int at91_gpiolib_direction_input(struct gpio_chip *chip,
 					unsigned offset);
+static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
 
 #define AT91_GPIO_CHIP(name, base_gpio, nr_gpio)			\
 	{								\
@@ -59,6 +61,7 @@ static int at91_gpiolib_direction_input(struct gpio_chip *chip,
 			.set		  = at91_gpiolib_set,		\
 			.dbg_show	  = at91_gpiolib_dbg_show,	\
 			.base		  = base_gpio,			\
+			.to_irq		  = at91_gpiolib_to_irq,	\
 			.ngpio		  = nr_gpio,			\
 		},							\
 	}
@@ -640,6 +643,16 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	}
 }
 
+static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
+	int virq = irq_find_mapping(at91_gpio->domain, offset);
+
+	dev_dbg(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
+				chip->label, offset + chip->base, virq);
+	return virq;
+}
+
 static int __init at91_gpio_setup_clk(int idx)
 {
 	struct at91_gpio_chip *at91_gpio = &gpio_chip[idx];
diff --git a/arch/arm/mach-at91/include/mach/gpio.h b/arch/arm/mach-at91/include/mach/gpio.h
index e3fd225..7cf009b 100644
--- a/arch/arm/mach-at91/include/mach/gpio.h
+++ b/arch/arm/mach-at91/include/mach/gpio.h
@@ -204,18 +204,6 @@ extern int at91_get_gpio_value(unsigned pin);
 extern void at91_gpio_suspend(void);
 extern void at91_gpio_resume(void);
 
-/*-------------------------------------------------------------------------*/
-
-/* wrappers for "new style" GPIO calls. the old AT91-specific ones should
- * eventually be removed (along with this errno.h inclusion), and the
- * gpio request/free calls should probably be implemented.
- */
-
-#include <asm/errno.h>
-
-#define gpio_to_irq(gpio) (gpio + NR_AIC_IRQS)
-#define irq_to_gpio(irq)  (irq - NR_AIC_IRQS)
-
 #endif	/* __ASSEMBLY__ */
 
 #endif
-- 
1.7.9

[PATCH 8/9] ARM: at91/gpio: remove the static specification of gpio_chip.base

[Nicolas Ferre]

This value is determined at runtime using device tree or platform data
information.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/gpio.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index e8f5831..ecf6bbb 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -51,7 +51,7 @@ static int at91_gpiolib_direction_input(struct gpio_chip *chip,
 					unsigned offset);
 static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
 
-#define AT91_GPIO_CHIP(name, base_gpio, nr_gpio)			\
+#define AT91_GPIO_CHIP(name, nr_gpio)					\
 	{								\
 		.chip = {						\
 			.label		  = name,			\
@@ -60,18 +60,17 @@ static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset);
 			.get		  = at91_gpiolib_get,		\
 			.set		  = at91_gpiolib_set,		\
 			.dbg_show	  = at91_gpiolib_dbg_show,	\
-			.base		  = base_gpio,			\
 			.to_irq		  = at91_gpiolib_to_irq,	\
 			.ngpio		  = nr_gpio,			\
 		},							\
 	}
 
 static struct at91_gpio_chip gpio_chip[] = {
-	AT91_GPIO_CHIP("pioA", 0x00, 32),
-	AT91_GPIO_CHIP("pioB", 0x20, 32),
-	AT91_GPIO_CHIP("pioC", 0x40, 32),
-	AT91_GPIO_CHIP("pioD", 0x60, 32),
-	AT91_GPIO_CHIP("pioE", 0x80, 32),
+	AT91_GPIO_CHIP("pioA", 32),
+	AT91_GPIO_CHIP("pioB", 32),
+	AT91_GPIO_CHIP("pioC", 32),
+	AT91_GPIO_CHIP("pioD", 32),
+	AT91_GPIO_CHIP("pioE", 32),
 };
 
 static int gpio_banks;
-- 
1.7.9

[PATCH 9/9] ARM: at91/board-dt: remove AIC irq domain from board file

[Nicolas Ferre]

Adding of irqdomain in AIC code make the specification
of the irq domain in board file useless.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 arch/arm/mach-at91/board-dt.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
index 0579315..6120978 100644
--- a/arch/arm/mach-at91/board-dt.c
+++ b/arch/arm/mach-at91/board-dt.c
@@ -15,8 +15,6 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/gpio.h>
-#include <linux/irqdomain.h>
-#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 
 #include <mach/hardware.h>
@@ -88,14 +86,8 @@ static void __init ek_add_device_nand(void)
 	at91_add_device_nand(&ek_nand_data);
 }
 
-static const struct of_device_id aic_of_match[] __initconst = {
-	{ .compatible = "atmel,at91rm9200-aic", },
-	{},
-};
-
 static void __init at91_dt_init_irq(void)
 {
-	irq_domain_generate_simple(aic_of_match, 0xfffff000, 0);
 	at91_init_irq_default();
 }
 
-- 
1.7.9

Re: [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization

[Ryan Mallon]

On 14/02/12 01:43, Nicolas Ferre wrote:

> gpio_to_irq() implementation will be moved from a macro to a
> plain function: we cannot use it in a structure initialization
> anymore.

What was the reason for the change? It was originally a macro for 
exactly this reason.

Anyway,

Reviewed-by: Ryan Mallon <rmallon@gmail.com>

> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: rmallon@gmail.com
> ---
>  arch/arm/mach-at91/board-snapper9260.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/board-snapper9260.c b/arch/arm/mach-at91/board-snapper9260.c
> index 4770db0..3c2e3fc 100644
> --- a/arch/arm/mach-at91/board-snapper9260.c
> +++ b/arch/arm/mach-at91/board-snapper9260.c
> @@ -145,11 +145,11 @@ static struct i2c_board_info __initdata snapper9260_i2c_devices[] = {
>  		/* Audio codec */
>  		I2C_BOARD_INFO("tlv320aic23", 0x1a),
>  	},
> -	{
> +};
> +
> +static struct i2c_board_info __initdata snapper9260_i2c_isl1208 = {
>  		/* RTC */
>  		I2C_BOARD_INFO("isl1208", 0x6f),
> -		.irq = gpio_to_irq(AT91_PIN_PA31),
> -	},
>  };
>  
>  static void __init snapper9260_add_device_nand(void)
> @@ -163,6 +163,10 @@ static void __init snapper9260_board_init(void)
>  {
>  	at91_add_device_i2c(snapper9260_i2c_devices,
>  			    ARRAY_SIZE(snapper9260_i2c_devices));
> +
> +	snapper9260_i2c_isl1208.irq = gpio_to_irq(AT91_PIN_PA31);
> +	i2c_register_board_info(0, &snapper9260_i2c_isl1208, 1);
> +
>  	at91_add_device_serial();
>  	at91_add_device_usbh(&snapper9260_usbh_data);
>  	at91_add_device_udc(&snapper9260_udc_data);

Re: [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support

[Rob Herring]

On 02/13/2012 08:43 AM, Nicolas Ferre wrote:
> Add an irqdomain for the AIC interrupt controller.
> The device tree support is mapping the registers and
> is using the irq_domain_add_legacy() to manage hwirq
> translation.
> The documentation is describing the meaning of the
> two cells required for using this "interrupt-controller"
> in a device tree node.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> v5: - rebased on top of Grant's "irq_domain generalization and refinement"
>       patch series
>     - use of irq_domain_add_legacy() API
> 
> v4: - use irq_alloc_descs() to find irq_base
>     - add a new constant AIC_BASE_IRQ that will allow to skip
>       first interrupt numbers (in the future)
> 
> v3: - change number of cells to define an AIC interrupt (irq trigger type)
>     - change current .dtsi files to match specification
>     - use irq_domain_simple_ops (for DT mapping)
> 
> v2: - use of_irq_init() function for device tree probing
>     - add documentation
>     - use own simple struct irq_domain_ops
> 
> 
>  .../devicetree/bindings/arm/atmel-aic.txt          |   38 ++++++++++
>  arch/arm/Kconfig                                   |    1 +
>  arch/arm/boot/dts/at91sam9g20.dtsi                 |   18 +++---
>  arch/arm/boot/dts/at91sam9g45.dtsi                 |   16 ++--
>  arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
>  arch/arm/mach-at91/irq.c                           |   74 ++++++++++++++++----
>  6 files changed, 119 insertions(+), 31 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/atmel-aic.txt b/Documentation/devicetree/bindings/arm/atmel-aic.txt
> new file mode 100644
> index 0000000..289feb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/atmel-aic.txt
> @@ -0,0 +1,38 @@
> +* Advanced Interrupt Controller (AIC)
> +
> +Required properties:
> +- compatible: Should be "atmel,<chip>-aic"
> +- interrupt-controller: Identifies the node as an interrupt controller.
> +- interrupt-parent: For single AIC system, it is an empty property.
> +- #interrupt-cells: The number of cells to define the interrupts. It sould be 2.
> +  The first cell is the GPIO number.

s/GPIO/IRQ/

> +  The second cell is used to specify flags:
> +    bits[3:0] 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.
> +      Valid combinations are 1, 2, 3, 4, 8.
> +      Default flag for internal sources should be set to 4 (active high).
> +- reg: Should contain AIC registers location and length
> +
> +Examples:
> +	/*
> +	 * AIC
> +	 */
> +	aic: interrupt-controller@fffff000 {
> +		compatible = "atmel,at91rm9200-aic";
> +		interrupt-controller;
> +		interrupt-parent;
> +		#interrupt-cells = <2>;
> +		reg = <0xfffff000 0x200>;
> +	};
> +
> +	/*
> +	 * An interrupt generating device that is wired to an AIC.
> +	 */
> +	dma: dma-controller@ffffec00 {
> +		compatible = "atmel,at91sam9g45-dma";
> +		reg = <0xffffec00 0x200>;
> +		interrupts = <21 4>;
> +	};
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 92c9c79..cabd8f5 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -322,6 +322,7 @@ config ARCH_AT91
>  	select ARCH_REQUIRE_GPIOLIB
>  	select HAVE_CLK
>  	select CLKDEV_LOOKUP
> +	select IRQ_DOMAIN

This can likely go away assuming my change to select IRQ_DOMAIN for all
of ARM goes upstream.

>  	help
>  	  This enables support for systems based on the Atmel AT91RM9200,
>  	  AT91SAM9 processors.
> diff --git a/arch/arm/boot/dts/at91sam9g20.dtsi b/arch/arm/boot/dts/at91sam9g20.dtsi
> index 07603b8..9a0aee7 100644
> --- a/arch/arm/boot/dts/at91sam9g20.dtsi
> +++ b/arch/arm/boot/dts/at91sam9g20.dtsi
> @@ -47,7 +47,7 @@
>  			ranges;
>  
>  			aic: interrupt-controller@fffff000 {
> -				#interrupt-cells = <1>;
> +				#interrupt-cells = <2>;
>  				compatible = "atmel,at91rm9200-aic";
>  				interrupt-controller;
>  				interrupt-parent;
> @@ -57,14 +57,14 @@
>  			dbgu: serial@fffff200 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfffff200 0x200>;
> -				interrupts = <1>;
> +				interrupts = <1 4>;
>  				status = "disabled";
>  			};
>  
>  			usart0: serial@fffb0000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfffb0000 0x200>;
> -				interrupts = <6>;
> +				interrupts = <6 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -73,7 +73,7 @@
>  			usart1: serial@fffb4000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfffb4000 0x200>;
> -				interrupts = <7>;
> +				interrupts = <7 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -82,7 +82,7 @@
>  			usart2: serial@fffb8000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfffb8000 0x200>;
> -				interrupts = <8>;
> +				interrupts = <8 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -91,7 +91,7 @@
>  			usart3: serial@fffd0000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfffd0000 0x200>;
> -				interrupts = <23>;
> +				interrupts = <23 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -100,7 +100,7 @@
>  			usart4: serial@fffd4000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfffd4000 0x200>;
> -				interrupts = <24>;
> +				interrupts = <24 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -109,7 +109,7 @@
>  			usart5: serial@fffd8000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfffd8000 0x200>;
> -				interrupts = <25>;
> +				interrupts = <25 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -118,7 +118,7 @@
>  			macb0: ethernet@fffc4000 {
>  				compatible = "cdns,at32ap7000-macb", "cdns,macb";
>  				reg = <0xfffc4000 0x100>;
> -				interrupts = <21>;
> +				interrupts = <21 4>;
>  				status = "disabled";
>  			};
>  		};
> diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
> index fffa005..67f94d3 100644
> --- a/arch/arm/boot/dts/at91sam9g45.dtsi
> +++ b/arch/arm/boot/dts/at91sam9g45.dtsi
> @@ -46,7 +46,7 @@
>  			ranges;
>  
>  			aic: interrupt-controller@fffff000 {
> -				#interrupt-cells = <1>;
> +				#interrupt-cells = <2>;
>  				compatible = "atmel,at91rm9200-aic";
>  				interrupt-controller;
>  				interrupt-parent;
> @@ -56,20 +56,20 @@
>  			dma: dma-controller@ffffec00 {
>  				compatible = "atmel,at91sam9g45-dma";
>  				reg = <0xffffec00 0x200>;
> -				interrupts = <21>;
> +				interrupts = <21 4>;
>  			};
>  
>  			dbgu: serial@ffffee00 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xffffee00 0x200>;
> -				interrupts = <1>;
> +				interrupts = <1 4>;
>  				status = "disabled";
>  			};
>  
>  			usart0: serial@fff8c000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfff8c000 0x200>;
> -				interrupts = <7>;
> +				interrupts = <7 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -78,7 +78,7 @@
>  			usart1: serial@fff90000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfff90000 0x200>;
> -				interrupts = <8>;
> +				interrupts = <8 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -87,7 +87,7 @@
>  			usart2: serial@fff94000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfff94000 0x200>;
> -				interrupts = <9>;
> +				interrupts = <9 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -96,7 +96,7 @@
>  			usart3: serial@fff98000 {
>  				compatible = "atmel,at91sam9260-usart";
>  				reg = <0xfff98000 0x200>;
> -				interrupts = <10>;
> +				interrupts = <10 4>;
>  				atmel,use-dma-rx;
>  				atmel,use-dma-tx;
>  				status = "disabled";
> @@ -105,7 +105,7 @@
>  			macb0: ethernet@fffbc000 {
>  				compatible = "cdns,at32ap7000-macb", "cdns,macb";
>  				reg = <0xfffbc000 0x100>;
> -				interrupts = <25>;
> +				interrupts = <25 4>;
>  				status = "disabled";
>  			};
>  		};
> diff --git a/arch/arm/mach-at91/include/mach/irqs.h b/arch/arm/mach-at91/include/mach/irqs.h
> index ac8b7df..e3ee0fc 100644
> --- a/arch/arm/mach-at91/include/mach/irqs.h
> +++ b/arch/arm/mach-at91/include/mach/irqs.h
> @@ -25,6 +25,7 @@
>  #include <mach/at91_aic.h>
>  
>  #define NR_AIC_IRQS 32
> +#define AIC_BASE_IRQ 0
>  
>  
>  /*
> @@ -40,7 +41,7 @@
>   * symbols in gpio.h for ones handled indirectly as GPIOs.
>   * We make provision for 5 banks of GPIO.
>   */
> -#define	NR_IRQS		(NR_AIC_IRQS + (5 * 32))
> +#define	NR_IRQS		(AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))

Why? You're not changing anything.

You should turn on SPARSE_IRQ at some point and then this value goes away.

>  
>  /* FIQ is AIC source 0. */
>  #define FIQ_START AT91_ID_FIQ
> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
> index be6b639..880da98 100644
> --- a/arch/arm/mach-at91/irq.c
> +++ b/arch/arm/mach-at91/irq.c
> @@ -24,6 +24,12 @@
>  #include <linux/module.h>
>  #include <linux/mm.h>
>  #include <linux/types.h>
> +#include <linux/irq.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/err.h>
>  
>  #include <mach/hardware.h>
>  #include <asm/irq.h>
> @@ -34,22 +40,24 @@
>  #include <asm/mach/map.h>
>  
>  void __iomem *at91_aic_base;
> +static struct irq_domain *at91_aic_domain;
> +static struct device_node *at91_aic_np;
>  
>  static void at91_aic_mask_irq(struct irq_data *d)
>  {
>  	/* Disable interrupt on AIC */
> -	at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
> +	at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
>  }
>  
>  static void at91_aic_unmask_irq(struct irq_data *d)
>  {
>  	/* Enable interrupt on AIC */
> -	at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
> +	at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
>  }
>  
>  unsigned int at91_extern_irq;
>  
> -#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
> +#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
>  
>  static int at91_aic_set_type(struct irq_data *d, unsigned type)
>  {
> @@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>  		srctype = AT91_AIC_SRCTYPE_RISING;
>  		break;
>  	case IRQ_TYPE_LEVEL_LOW:
> -		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
> +		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
>  			srctype = AT91_AIC_SRCTYPE_LOW;
>  		else
>  			return -EINVAL;
>  		break;
>  	case IRQ_TYPE_EDGE_FALLING:
> -		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
> +		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
>  			srctype = AT91_AIC_SRCTYPE_FALLING;
>  		else
>  			return -EINVAL;
> @@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>  		return -EINVAL;
>  	}
>  
> -	smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
> -	at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
> +	smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
> +	at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
>  	return 0;
>  }
>  
> @@ -90,13 +98,13 @@ static u32 backups;
>  
>  static int at91_aic_set_wake(struct irq_data *d, unsigned value)
>  {
> -	if (unlikely(d->irq >= 32))
> +	if (unlikely(d->hwirq >= NR_AIC_IRQS))
>  		return -EINVAL;
>  
>  	if (value)
> -		wakeups |= (1 << d->irq);
> +		wakeups |= (1 << d->hwirq);
>  	else
> -		wakeups &= ~(1 << d->irq);
> +		wakeups &= ~(1 << d->hwirq);
>  
>  	return 0;
>  }
> @@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
>  	.irq_set_wake	= at91_aic_set_wake,
>  };
>  
> +#if defined(CONFIG_OF)
> +static int __init __at91_aic_of_init(struct device_node *node,
> +				     struct device_node *parent)
> +{
> +	at91_aic_base = of_iomap(node, 0);
> +	at91_aic_np = node;
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id aic_ids[] __initconst = {
> +	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
> +	{ /*sentinel*/ }
> +};
> +
> +static void __init at91_aic_of_init(void)
> +{
> +	of_irq_init(aic_ids);

This call and match str belongs in your board file (init_irq function)
and you should be doing all setup in __at91_aic_of_init.

> +}
> +#else
> +static void __init at91_aic_of_init(void) {}
> +#endif
> +
>  /*
>   * Initialize the AIC interrupt controller.
>   */
>  void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])

Perhaps the priority should be a cell in your interrupts property?

>  {
>  	unsigned int i;
> +	int irq_base;
>  
> -	at91_aic_base = ioremap(AT91_AIC, 512);
> +	if (of_have_populated_dt())
> +		at91_aic_of_init();

You should get to this point because of_irq_init called you and you
should already have a node ptr at point.

> +	else
> +		at91_aic_base = ioremap(AT91_AIC, 512);
>  
>  	if (!at91_aic_base)
> -		panic("Impossible to ioremap AT91_AIC\n");
> +		panic("Unable to ioremap AIC registers\n");
> +
> +	/* Add irq domain for AIC */
> +	irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);

Really, you don't want to use irq0, but that would break your irq entry
code.

Rob

> +	if (irq_base < 0) {
> +		WARN(1, "Cannot allocate irq_descs, assuming pre-allocated\n");
> +		irq_base = AIC_BASE_IRQ;
> +	}
> +	at91_aic_domain = irq_domain_add_legacy(at91_aic_np, NR_AIC_IRQS,
> +						irq_base, 0,
> +						&irq_domain_simple_ops, NULL);
> +
> +	if (!at91_aic_domain)
> +		panic("Unable to add AIC irq domain\n");
>  
>  	/*
>  	 * The IVR is used by macro get_irqnr_and_base to read and verify.
>  	 * The irq number is NR_AIC_IRQS when a spurious interrupt has occurred.
>  	 */
>  	for (i = 0; i < NR_AIC_IRQS; i++) {
> -		/* Put irq number in Source Vector Register: */
> +		/* Put hardware irq number in Source Vector Register: */
>  		at91_aic_write(AT91_AIC_SVR(i), i);
>  		/* Active Low interrupt, with the specified priority */
>  		at91_aic_write(AT91_AIC_SMR(i), AT91_AIC_SRCTYPE_LOW | priority[i]);

Re: [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization

[Nicolas Ferre]

On 02/13/2012 09:02 PM, Ryan Mallon :
> On 14/02/12 01:43, Nicolas Ferre wrote:
> 
>> gpio_to_irq() implementation will be moved from a macro to a
>> plain function: we cannot use it in a structure initialization
>> anymore.
> 
> What was the reason for the change? It was originally a macro for 
> exactly this reason.

Linux vIRQ numbers will be allocated dynamically when switching to
irqdomains. That will prevent the use of a static operation in a macro.

> Reviewed-by: Ryan Mallon <rmallon@gmail.com>

Thanks.

>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> Cc: rmallon@gmail.com
>> ---
>>  arch/arm/mach-at91/board-snapper9260.c |   10 +++++++---
>>  1 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/board-snapper9260.c b/arch/arm/mach-at91/board-snapper9260.c
>> index 4770db0..3c2e3fc 100644
>> --- a/arch/arm/mach-at91/board-snapper9260.c
>> +++ b/arch/arm/mach-at91/board-snapper9260.c
>> @@ -145,11 +145,11 @@ static struct i2c_board_info __initdata snapper9260_i2c_devices[] = {
>>  		/* Audio codec */
>>  		I2C_BOARD_INFO("tlv320aic23", 0x1a),
>>  	},
>> -	{
>> +};
>> +
>> +static struct i2c_board_info __initdata snapper9260_i2c_isl1208 = {
>>  		/* RTC */
>>  		I2C_BOARD_INFO("isl1208", 0x6f),
>> -		.irq = gpio_to_irq(AT91_PIN_PA31),
>> -	},
>>  };
>>  
>>  static void __init snapper9260_add_device_nand(void)
>> @@ -163,6 +163,10 @@ static void __init snapper9260_board_init(void)
>>  {
>>  	at91_add_device_i2c(snapper9260_i2c_devices,
>>  			    ARRAY_SIZE(snapper9260_i2c_devices));
>> +
>> +	snapper9260_i2c_isl1208.irq = gpio_to_irq(AT91_PIN_PA31);
>> +	i2c_register_board_info(0, &snapper9260_i2c_isl1208, 1);
>> +
>>  	at91_add_device_serial();
>>  	at91_add_device_usbh(&snapper9260_usbh_data);
>>  	at91_add_device_udc(&snapper9260_udc_data);
> 
> 
> 


-- 
Nicolas Ferre

Re: [PATCH 2/9] ARM: at91/snapper9260: move gpio_to_irq out of structure initialization

[Russell King - ARM Linux]

On Tue, Feb 14, 2012 at 07:02:20AM +1100, Ryan Mallon wrote:
> On 14/02/12 01:43, Nicolas Ferre wrote:
> 
> > gpio_to_irq() implementation will be moved from a macro to a
> > plain function: we cannot use it in a structure initialization
> > anymore.
> 
> What was the reason for the change? It was originally a macro for 
> exactly this reason.

gpio_to_irq() is supposed to work not only for built-in GPIOs, but also
GPIOs provided by other devices.  Having it as a definition which only
works for the SoC GPIOs is broken.

Re: [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support

[Nicolas Ferre]

On 02/13/2012 11:10 PM, Rob Herring :
> On 02/13/2012 08:43 AM, Nicolas Ferre wrote:
>> Add an irqdomain for the AIC interrupt controller.
>> The device tree support is mapping the registers and
>> is using the irq_domain_add_legacy() to manage hwirq
>> translation.
>> The documentation is describing the meaning of the
>> two cells required for using this "interrupt-controller"
>> in a device tree node.
>>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> ---
>> v5: - rebased on top of Grant's "irq_domain generalization and refinement"
>>       patch series
>>     - use of irq_domain_add_legacy() API
>>
>> v4: - use irq_alloc_descs() to find irq_base
>>     - add a new constant AIC_BASE_IRQ that will allow to skip
>>       first interrupt numbers (in the future)
>>
>> v3: - change number of cells to define an AIC interrupt (irq trigger type)
>>     - change current .dtsi files to match specification
>>     - use irq_domain_simple_ops (for DT mapping)
>>
>> v2: - use of_irq_init() function for device tree probing
>>     - add documentation
>>     - use own simple struct irq_domain_ops
>>
>>
>>  .../devicetree/bindings/arm/atmel-aic.txt          |   38 ++++++++++
>>  arch/arm/Kconfig                                   |    1 +
>>  arch/arm/boot/dts/at91sam9g20.dtsi                 |   18 +++---
>>  arch/arm/boot/dts/at91sam9g45.dtsi                 |   16 ++--
>>  arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
>>  arch/arm/mach-at91/irq.c                           |   74 ++++++++++++++++----
>>  6 files changed, 119 insertions(+), 31 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt

[..]

>> --- a/arch/arm/mach-at91/include/mach/irqs.h
>> +++ b/arch/arm/mach-at91/include/mach/irqs.h
>> @@ -25,6 +25,7 @@
>>  #include <mach/at91_aic.h>
>>  
>>  #define NR_AIC_IRQS 32
>> +#define AIC_BASE_IRQ 0
>>  
>>  
>>  /*
>> @@ -40,7 +41,7 @@
>>   * symbols in gpio.h for ones handled indirectly as GPIOs.
>>   * We make provision for 5 banks of GPIO.
>>   */
>> -#define	NR_IRQS		(NR_AIC_IRQS + (5 * 32))
>> +#define	NR_IRQS		(AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))
> 
> Why? You're not changing anything.
> 
> You should turn on SPARSE_IRQ at some point and then this value goes away.

Yes, my intention is to move to this step by step. I have tried hard to
rework completely the AIC driver without success so far. I plan:
- move the AIC_BASE_IRQ away from irq0
  => this is why I introduce a constant here
- use SPARSE_IRQ
- move to generic irq & irq domain helpers (move DT case to "linear")

*once* the code for handling all this is stabilized and integrated in
mainline.

This rework is designed for 3.4 and a lot of code depend on this for all
our at91 DT move.


>>  /* FIQ is AIC source 0. */
>>  #define FIQ_START AT91_ID_FIQ
>> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
>> index be6b639..880da98 100644
>> --- a/arch/arm/mach-at91/irq.c
>> +++ b/arch/arm/mach-at91/irq.c
>> @@ -24,6 +24,12 @@
>>  #include <linux/module.h>
>>  #include <linux/mm.h>
>>  #include <linux/types.h>
>> +#include <linux/irq.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/err.h>
>>  
>>  #include <mach/hardware.h>
>>  #include <asm/irq.h>
>> @@ -34,22 +40,24 @@
>>  #include <asm/mach/map.h>
>>  
>>  void __iomem *at91_aic_base;
>> +static struct irq_domain *at91_aic_domain;
>> +static struct device_node *at91_aic_np;
>>  
>>  static void at91_aic_mask_irq(struct irq_data *d)
>>  {
>>  	/* Disable interrupt on AIC */
>> -	at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
>> +	at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
>>  }
>>  
>>  static void at91_aic_unmask_irq(struct irq_data *d)
>>  {
>>  	/* Enable interrupt on AIC */
>> -	at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
>> +	at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
>>  }
>>  
>>  unsigned int at91_extern_irq;
>>  
>> -#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
>> +#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
>>  
>>  static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>  {
>> @@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>  		srctype = AT91_AIC_SRCTYPE_RISING;
>>  		break;
>>  	case IRQ_TYPE_LEVEL_LOW:
>> -		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
>> +		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
>>  			srctype = AT91_AIC_SRCTYPE_LOW;
>>  		else
>>  			return -EINVAL;
>>  		break;
>>  	case IRQ_TYPE_EDGE_FALLING:
>> -		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
>> +		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
>>  			srctype = AT91_AIC_SRCTYPE_FALLING;
>>  		else
>>  			return -EINVAL;
>> @@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>  		return -EINVAL;
>>  	}
>>  
>> -	smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
>> -	at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
>> +	smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
>> +	at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
>>  	return 0;
>>  }
>>  
>> @@ -90,13 +98,13 @@ static u32 backups;
>>  
>>  static int at91_aic_set_wake(struct irq_data *d, unsigned value)
>>  {
>> -	if (unlikely(d->irq >= 32))
>> +	if (unlikely(d->hwirq >= NR_AIC_IRQS))
>>  		return -EINVAL;
>>  
>>  	if (value)
>> -		wakeups |= (1 << d->irq);
>> +		wakeups |= (1 << d->hwirq);
>>  	else
>> -		wakeups &= ~(1 << d->irq);
>> +		wakeups &= ~(1 << d->hwirq);
>>  
>>  	return 0;
>>  }
>> @@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
>>  	.irq_set_wake	= at91_aic_set_wake,
>>  };
>>  
>> +#if defined(CONFIG_OF)
>> +static int __init __at91_aic_of_init(struct device_node *node,
>> +				     struct device_node *parent)
>> +{
>> +	at91_aic_base = of_iomap(node, 0);
>> +	at91_aic_np = node;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id aic_ids[] __initconst = {
>> +	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
>> +	{ /*sentinel*/ }
>> +};
>> +
>> +static void __init at91_aic_of_init(void)
>> +{
>> +	of_irq_init(aic_ids);
> 
> This call and match str belongs in your board file (init_irq function)
> and you should be doing all setup in __at91_aic_of_init.

It has already been discussed here:
http://article.gmane.org/gmane.linux.drivers.devicetree/9834

I feel that this self-contained driver is the best solution for the
moment. Once the priorities will be encoded in the device tree (and
moreover once they will be useful for threaded interrupts), we may
re-consider this.

>> +}
>> +#else
>> +static void __init at91_aic_of_init(void) {}
>> +#endif
>> +
>>  /*
>>   * Initialize the AIC interrupt controller.
>>   */
>>  void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
> 
> Perhaps the priority should be a cell in your interrupts property?

Yes, it is a good idea. But that will prevent me from using a standard
irq_domain simple xlate operation... Maybe we can think about this a bit
more and implement it in the future.

>>  {
>>  	unsigned int i;
>> +	int irq_base;
>>  
>> -	at91_aic_base = ioremap(AT91_AIC, 512);
>> +	if (of_have_populated_dt())
>> +		at91_aic_of_init();
> 
> You should get to this point because of_irq_init called you and you
> should already have a node ptr at point.
> 
>> +	else
>> +		at91_aic_base = ioremap(AT91_AIC, 512);
>>  
>>  	if (!at91_aic_base)
>> -		panic("Impossible to ioremap AT91_AIC\n");
>> +		panic("Unable to ioremap AIC registers\n");
>> +
>> +	/* Add irq domain for AIC */
>> +	irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);
> 
> Really, you don't want to use irq0, but that would break your irq entry
> code.

Exactly. I have the code for switching to MULTI_IRQ_HANDLER ready. But
that will require a little modification of each board file. This
scattered modification is maybe a bit late for 3.4 inclusion...

Best regards,
-- 
Nicolas Ferre

Re: [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support

[Rob Herring]

On 02/14/2012 04:24 AM, Nicolas Ferre wrote:
> On 02/13/2012 11:10 PM, Rob Herring :
>> On 02/13/2012 08:43 AM, Nicolas Ferre wrote:
>>> Add an irqdomain for the AIC interrupt controller.
>>> The device tree support is mapping the registers and
>>> is using the irq_domain_add_legacy() to manage hwirq
>>> translation.
>>> The documentation is describing the meaning of the
>>> two cells required for using this "interrupt-controller"
>>> in a device tree node.
>>>
>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>> ---
>>> v5: - rebased on top of Grant's "irq_domain generalization and refinement"
>>>       patch series
>>>     - use of irq_domain_add_legacy() API
>>>
>>> v4: - use irq_alloc_descs() to find irq_base
>>>     - add a new constant AIC_BASE_IRQ that will allow to skip
>>>       first interrupt numbers (in the future)
>>>
>>> v3: - change number of cells to define an AIC interrupt (irq trigger type)
>>>     - change current .dtsi files to match specification
>>>     - use irq_domain_simple_ops (for DT mapping)
>>>
>>> v2: - use of_irq_init() function for device tree probing
>>>     - add documentation
>>>     - use own simple struct irq_domain_ops
>>>
>>>
>>>  .../devicetree/bindings/arm/atmel-aic.txt          |   38 ++++++++++
>>>  arch/arm/Kconfig                                   |    1 +
>>>  arch/arm/boot/dts/at91sam9g20.dtsi                 |   18 +++---
>>>  arch/arm/boot/dts/at91sam9g45.dtsi                 |   16 ++--
>>>  arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
>>>  arch/arm/mach-at91/irq.c                           |   74 ++++++++++++++++----
>>>  6 files changed, 119 insertions(+), 31 deletions(-)
>>>  create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
> 
> [..]
> 
>>> --- a/arch/arm/mach-at91/include/mach/irqs.h
>>> +++ b/arch/arm/mach-at91/include/mach/irqs.h
>>> @@ -25,6 +25,7 @@
>>>  #include <mach/at91_aic.h>
>>>  
>>>  #define NR_AIC_IRQS 32
>>> +#define AIC_BASE_IRQ 0
>>>  
>>>  
>>>  /*
>>> @@ -40,7 +41,7 @@
>>>   * symbols in gpio.h for ones handled indirectly as GPIOs.
>>>   * We make provision for 5 banks of GPIO.
>>>   */
>>> -#define	NR_IRQS		(NR_AIC_IRQS + (5 * 32))
>>> +#define	NR_IRQS		(AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))
>>
>> Why? You're not changing anything.
>>
>> You should turn on SPARSE_IRQ at some point and then this value goes away.
> 
> Yes, my intention is to move to this step by step. I have tried hard to
> rework completely the AIC driver without success so far. I plan:
> - move the AIC_BASE_IRQ away from irq0
>   => this is why I introduce a constant here
> - use SPARSE_IRQ
> - move to generic irq & irq domain helpers (move DT case to "linear")
> 
> *once* the code for handling all this is stabilized and integrated in
> mainline.
> 
> This rework is designed for 3.4 and a lot of code depend on this for all
> our at91 DT move.

In that case, NR_IRQS will be removed. You will get the default of 16
(NR_LEGACY_IRQS) and they will get reserved so irq_alloc_descs will skip
over them. So there is no reason for you to have AIC_BASE_IRQ.

> 
> 
>>>  /* FIQ is AIC source 0. */
>>>  #define FIQ_START AT91_ID_FIQ
>>> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
>>> index be6b639..880da98 100644
>>> --- a/arch/arm/mach-at91/irq.c
>>> +++ b/arch/arm/mach-at91/irq.c
>>> @@ -24,6 +24,12 @@
>>>  #include <linux/module.h>
>>>  #include <linux/mm.h>
>>>  #include <linux/types.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/irqdomain.h>
>>> +#include <linux/err.h>
>>>  
>>>  #include <mach/hardware.h>
>>>  #include <asm/irq.h>
>>> @@ -34,22 +40,24 @@
>>>  #include <asm/mach/map.h>
>>>  
>>>  void __iomem *at91_aic_base;
>>> +static struct irq_domain *at91_aic_domain;
>>> +static struct device_node *at91_aic_np;
>>>  
>>>  static void at91_aic_mask_irq(struct irq_data *d)
>>>  {
>>>  	/* Disable interrupt on AIC */
>>> -	at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
>>> +	at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
>>>  }
>>>  
>>>  static void at91_aic_unmask_irq(struct irq_data *d)
>>>  {
>>>  	/* Enable interrupt on AIC */
>>> -	at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
>>> +	at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
>>>  }
>>>  
>>>  unsigned int at91_extern_irq;
>>>  
>>> -#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
>>> +#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
>>>  
>>>  static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>  {
>>> @@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>  		srctype = AT91_AIC_SRCTYPE_RISING;
>>>  		break;
>>>  	case IRQ_TYPE_LEVEL_LOW:
>>> -		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
>>> +		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
>>>  			srctype = AT91_AIC_SRCTYPE_LOW;
>>>  		else
>>>  			return -EINVAL;
>>>  		break;
>>>  	case IRQ_TYPE_EDGE_FALLING:
>>> -		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
>>> +		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
>>>  			srctype = AT91_AIC_SRCTYPE_FALLING;
>>>  		else
>>>  			return -EINVAL;
>>> @@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>  		return -EINVAL;
>>>  	}
>>>  
>>> -	smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
>>> -	at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
>>> +	smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
>>> +	at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
>>>  	return 0;
>>>  }
>>>  
>>> @@ -90,13 +98,13 @@ static u32 backups;
>>>  
>>>  static int at91_aic_set_wake(struct irq_data *d, unsigned value)
>>>  {
>>> -	if (unlikely(d->irq >= 32))
>>> +	if (unlikely(d->hwirq >= NR_AIC_IRQS))
>>>  		return -EINVAL;
>>>  
>>>  	if (value)
>>> -		wakeups |= (1 << d->irq);
>>> +		wakeups |= (1 << d->hwirq);
>>>  	else
>>> -		wakeups &= ~(1 << d->irq);
>>> +		wakeups &= ~(1 << d->hwirq);
>>>  
>>>  	return 0;
>>>  }
>>> @@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
>>>  	.irq_set_wake	= at91_aic_set_wake,
>>>  };
>>>  
>>> +#if defined(CONFIG_OF)
>>> +static int __init __at91_aic_of_init(struct device_node *node,
>>> +				     struct device_node *parent)
>>> +{
>>> +	at91_aic_base = of_iomap(node, 0);
>>> +	at91_aic_np = node;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct of_device_id aic_ids[] __initconst = {
>>> +	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
>>> +	{ /*sentinel*/ }
>>> +};
>>> +
>>> +static void __init at91_aic_of_init(void)
>>> +{
>>> +	of_irq_init(aic_ids);
>>
>> This call and match str belongs in your board file (init_irq function)
>> and you should be doing all setup in __at91_aic_of_init.
> 
> It has already been discussed here:
> http://article.gmane.org/gmane.linux.drivers.devicetree/9834
> 
> I feel that this self-contained driver is the best solution for the
> moment. Once the priorities will be encoded in the device tree (and
> moreover once they will be useful for threaded interrupts), we may
> re-consider this.
> 

Can you setup the priorities after init with a separate function? Then
you can follow standard init flow and afterwards set the priorities.

BTW, We're probably going to start moving irqchips to a common
drivers/irqchips. So it's important that init flow be consistent with
other irqchips.

>>> +}
>>> +#else
>>> +static void __init at91_aic_of_init(void) {}
>>> +#endif
>>> +
>>>  /*
>>>   * Initialize the AIC interrupt controller.
>>>   */
>>>  void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>>
>> Perhaps the priority should be a cell in your interrupts property?
> 
> Yes, it is a good idea. But that will prevent me from using a standard
> irq_domain simple xlate operation... Maybe we can think about this a bit
> more and implement it in the future.

That's not a good argument. What Linux has or doesn't have should not
really influence your binding. What is the right thing to do?

You can put the priority into the upper half word of cell 2. The trigger
type is masked. But you would still need a custom translate function to
extract the priority and set it.

You could perhaps just put all the priorities as a property of the
interrupt controller node. Then you don't have to increase the cell size.

>>>  {
>>>  	unsigned int i;
>>> +	int irq_base;
>>>  
>>> -	at91_aic_base = ioremap(AT91_AIC, 512);
>>> +	if (of_have_populated_dt())
>>> +		at91_aic_of_init();
>>
>> You should get to this point because of_irq_init called you and you
>> should already have a node ptr at point.
>>
>>> +	else
>>> +		at91_aic_base = ioremap(AT91_AIC, 512);
>>>  
>>>  	if (!at91_aic_base)
>>> -		panic("Impossible to ioremap AT91_AIC\n");
>>> +		panic("Unable to ioremap AIC registers\n");
>>> +
>>> +	/* Add irq domain for AIC */
>>> +	irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);
>>
>> Really, you don't want to use irq0, but that would break your irq entry
>> code.
> 
> Exactly. I have the code for switching to MULTI_IRQ_HANDLER ready. But
> that will require a little modification of each board file. This
> scattered modification is maybe a bit late for 3.4 inclusion...
> 

Okay. It's fine for now. Just making sure you're aware of the issue.

Rob

Re: [PATCH v5 1/9] ARM: at91/aic: add irq domain and device tree support

[Nicolas Ferre]

On 02/14/2012 03:11 PM, Rob Herring :
> On 02/14/2012 04:24 AM, Nicolas Ferre wrote:
>> On 02/13/2012 11:10 PM, Rob Herring :
>>> On 02/13/2012 08:43 AM, Nicolas Ferre wrote:
>>>> Add an irqdomain for the AIC interrupt controller.
>>>> The device tree support is mapping the registers and
>>>> is using the irq_domain_add_legacy() to manage hwirq
>>>> translation.
>>>> The documentation is describing the meaning of the
>>>> two cells required for using this "interrupt-controller"
>>>> in a device tree node.
>>>>
>>>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>>> ---
>>>> v5: - rebased on top of Grant's "irq_domain generalization and refinement"
>>>>       patch series
>>>>     - use of irq_domain_add_legacy() API
>>>>
>>>> v4: - use irq_alloc_descs() to find irq_base
>>>>     - add a new constant AIC_BASE_IRQ that will allow to skip
>>>>       first interrupt numbers (in the future)
>>>>
>>>> v3: - change number of cells to define an AIC interrupt (irq trigger type)
>>>>     - change current .dtsi files to match specification
>>>>     - use irq_domain_simple_ops (for DT mapping)
>>>>
>>>> v2: - use of_irq_init() function for device tree probing
>>>>     - add documentation
>>>>     - use own simple struct irq_domain_ops
>>>>
>>>>
>>>>  .../devicetree/bindings/arm/atmel-aic.txt          |   38 ++++++++++
>>>>  arch/arm/Kconfig                                   |    1 +
>>>>  arch/arm/boot/dts/at91sam9g20.dtsi                 |   18 +++---
>>>>  arch/arm/boot/dts/at91sam9g45.dtsi                 |   16 ++--
>>>>  arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
>>>>  arch/arm/mach-at91/irq.c                           |   74 ++++++++++++++++----
>>>>  6 files changed, 119 insertions(+), 31 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
>>
>> [..]
>>
>>>> --- a/arch/arm/mach-at91/include/mach/irqs.h
>>>> +++ b/arch/arm/mach-at91/include/mach/irqs.h
>>>> @@ -25,6 +25,7 @@
>>>>  #include <mach/at91_aic.h>
>>>>  
>>>>  #define NR_AIC_IRQS 32
>>>> +#define AIC_BASE_IRQ 0
>>>>  
>>>>  
>>>>  /*
>>>> @@ -40,7 +41,7 @@
>>>>   * symbols in gpio.h for ones handled indirectly as GPIOs.
>>>>   * We make provision for 5 banks of GPIO.
>>>>   */
>>>> -#define	NR_IRQS		(NR_AIC_IRQS + (5 * 32))
>>>> +#define	NR_IRQS		(AIC_BASE_IRQ + NR_AIC_IRQS + (5 * 32))
>>>
>>> Why? You're not changing anything.
>>>
>>> You should turn on SPARSE_IRQ at some point and then this value goes away.
>>
>> Yes, my intention is to move to this step by step. I have tried hard to
>> rework completely the AIC driver without success so far. I plan:
>> - move the AIC_BASE_IRQ away from irq0
>>   => this is why I introduce a constant here
>> - use SPARSE_IRQ
>> - move to generic irq & irq domain helpers (move DT case to "linear")
>>
>> *once* the code for handling all this is stabilized and integrated in
>> mainline.
>>
>> This rework is designed for 3.4 and a lot of code depend on this for all
>> our at91 DT move.
> 
> In that case, NR_IRQS will be removed. You will get the default of 16
> (NR_LEGACY_IRQS) and they will get reserved so irq_alloc_descs will skip
> over them. So there is no reason for you to have AIC_BASE_IRQ.

Fair enough, that indeed makes a lot of sense: I remove it.

>>>>  /* FIQ is AIC source 0. */
>>>>  #define FIQ_START AT91_ID_FIQ
>>>> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
>>>> index be6b639..880da98 100644
>>>> --- a/arch/arm/mach-at91/irq.c
>>>> +++ b/arch/arm/mach-at91/irq.c
>>>> @@ -24,6 +24,12 @@
>>>>  #include <linux/module.h>
>>>>  #include <linux/mm.h>
>>>>  #include <linux/types.h>
>>>> +#include <linux/irq.h>
>>>> +#include <linux/of.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/of_irq.h>
>>>> +#include <linux/irqdomain.h>
>>>> +#include <linux/err.h>
>>>>  
>>>>  #include <mach/hardware.h>
>>>>  #include <asm/irq.h>
>>>> @@ -34,22 +40,24 @@
>>>>  #include <asm/mach/map.h>
>>>>  
>>>>  void __iomem *at91_aic_base;
>>>> +static struct irq_domain *at91_aic_domain;
>>>> +static struct device_node *at91_aic_np;
>>>>  
>>>>  static void at91_aic_mask_irq(struct irq_data *d)
>>>>  {
>>>>  	/* Disable interrupt on AIC */
>>>> -	at91_aic_write(AT91_AIC_IDCR, 1 << d->irq);
>>>> +	at91_aic_write(AT91_AIC_IDCR, 1 << d->hwirq);
>>>>  }
>>>>  
>>>>  static void at91_aic_unmask_irq(struct irq_data *d)
>>>>  {
>>>>  	/* Enable interrupt on AIC */
>>>> -	at91_aic_write(AT91_AIC_IECR, 1 << d->irq);
>>>> +	at91_aic_write(AT91_AIC_IECR, 1 << d->hwirq);
>>>>  }
>>>>  
>>>>  unsigned int at91_extern_irq;
>>>>  
>>>> -#define is_extern_irq(irq) ((1 << (irq)) & at91_extern_irq)
>>>> +#define is_extern_irq(hwirq) ((1 << (hwirq)) & at91_extern_irq)
>>>>  
>>>>  static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>>  {
>>>> @@ -63,13 +71,13 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>>  		srctype = AT91_AIC_SRCTYPE_RISING;
>>>>  		break;
>>>>  	case IRQ_TYPE_LEVEL_LOW:
>>>> -		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
>>>> +		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
>>>>  			srctype = AT91_AIC_SRCTYPE_LOW;
>>>>  		else
>>>>  			return -EINVAL;
>>>>  		break;
>>>>  	case IRQ_TYPE_EDGE_FALLING:
>>>> -		if ((d->irq == AT91_ID_FIQ) || is_extern_irq(d->irq))		/* only supported on external interrupts */
>>>> +		if ((d->hwirq == AT91_ID_FIQ) || is_extern_irq(d->hwirq))		/* only supported on external interrupts */
>>>>  			srctype = AT91_AIC_SRCTYPE_FALLING;
>>>>  		else
>>>>  			return -EINVAL;
>>>> @@ -78,8 +86,8 @@ static int at91_aic_set_type(struct irq_data *d, unsigned type)
>>>>  		return -EINVAL;
>>>>  	}
>>>>  
>>>> -	smr = at91_aic_read(AT91_AIC_SMR(d->irq)) & ~AT91_AIC_SRCTYPE;
>>>> -	at91_aic_write(AT91_AIC_SMR(d->irq), smr | srctype);
>>>> +	smr = at91_aic_read(AT91_AIC_SMR(d->hwirq)) & ~AT91_AIC_SRCTYPE;
>>>> +	at91_aic_write(AT91_AIC_SMR(d->hwirq), smr | srctype);
>>>>  	return 0;
>>>>  }
>>>>  
>>>> @@ -90,13 +98,13 @@ static u32 backups;
>>>>  
>>>>  static int at91_aic_set_wake(struct irq_data *d, unsigned value)
>>>>  {
>>>> -	if (unlikely(d->irq >= 32))
>>>> +	if (unlikely(d->hwirq >= NR_AIC_IRQS))
>>>>  		return -EINVAL;
>>>>  
>>>>  	if (value)
>>>> -		wakeups |= (1 << d->irq);
>>>> +		wakeups |= (1 << d->hwirq);
>>>>  	else
>>>> -		wakeups &= ~(1 << d->irq);
>>>> +		wakeups &= ~(1 << d->hwirq);
>>>>  
>>>>  	return 0;
>>>>  }
>>>> @@ -127,24 +135,64 @@ static struct irq_chip at91_aic_chip = {
>>>>  	.irq_set_wake	= at91_aic_set_wake,
>>>>  };
>>>>  
>>>> +#if defined(CONFIG_OF)
>>>> +static int __init __at91_aic_of_init(struct device_node *node,
>>>> +				     struct device_node *parent)
>>>> +{
>>>> +	at91_aic_base = of_iomap(node, 0);
>>>> +	at91_aic_np = node;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id aic_ids[] __initconst = {
>>>> +	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
>>>> +	{ /*sentinel*/ }
>>>> +};
>>>> +
>>>> +static void __init at91_aic_of_init(void)
>>>> +{
>>>> +	of_irq_init(aic_ids);
>>>
>>> This call and match str belongs in your board file (init_irq function)
>>> and you should be doing all setup in __at91_aic_of_init.
>>
>> It has already been discussed here:
>> http://article.gmane.org/gmane.linux.drivers.devicetree/9834
>>
>> I feel that this self-contained driver is the best solution for the
>> moment. Once the priorities will be encoded in the device tree (and
>> moreover once they will be useful for threaded interrupts), we may
>> re-consider this.
>>
> 
> Can you setup the priorities after init with a separate function? Then
> you can follow standard init flow and afterwards set the priorities.
> 
> BTW, We're probably going to start moving irqchips to a common
> drivers/irqchips. So it's important that init flow be consistent with
> other irqchips.

That is indeed a strong argument.

I have made a bit of renewal of the initialization process which should
now match the standard flow. I post real-soon-now an additional patch
that goes on top of this series to address your advice.

In this new patch, the priorities are not managed at the moment but we
plan to use a "default" priority table which will be part of the AIC
node and, as you suggested, an additional optional cell in the
"interrupts" property itself.
Before implementing this, I would like to have your feedback on the
whole series (together with its brand-new additional patch).


>>>> +}
>>>> +#else
>>>> +static void __init at91_aic_of_init(void) {}
>>>> +#endif
>>>> +
>>>>  /*
>>>>   * Initialize the AIC interrupt controller.
>>>>   */
>>>>  void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>>>
>>> Perhaps the priority should be a cell in your interrupts property?
>>
>> Yes, it is a good idea. But that will prevent me from using a standard
>> irq_domain simple xlate operation... Maybe we can think about this a bit
>> more and implement it in the future.
> 
> That's not a good argument. What Linux has or doesn't have should not
> really influence your binding. What is the right thing to do?
> 
> You can put the priority into the upper half word of cell 2. The trigger
> type is masked. But you would still need a custom translate function to
> extract the priority and set it.
> 
> You could perhaps just put all the priorities as a property of the
> interrupt controller node. Then you don't have to increase the cell size.

Thank you for your thoughts about this: I will implement it soon (Cf.
above).


>>>>  {
>>>>  	unsigned int i;
>>>> +	int irq_base;
>>>>  
>>>> -	at91_aic_base = ioremap(AT91_AIC, 512);
>>>> +	if (of_have_populated_dt())
>>>> +		at91_aic_of_init();
>>>
>>> You should get to this point because of_irq_init called you and you
>>> should already have a node ptr at point.
>>>
>>>> +	else
>>>> +		at91_aic_base = ioremap(AT91_AIC, 512);
>>>>  
>>>>  	if (!at91_aic_base)
>>>> -		panic("Impossible to ioremap AT91_AIC\n");
>>>> +		panic("Unable to ioremap AIC registers\n");
>>>> +
>>>> +	/* Add irq domain for AIC */
>>>> +	irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ, NR_AIC_IRQS, 0);
>>>
>>> Really, you don't want to use irq0, but that would break your irq entry
>>> code.
>>
>> Exactly. I have the code for switching to MULTI_IRQ_HANDLER ready. But
>> that will require a little modification of each board file. This
>> scattered modification is maybe a bit late for 3.4 inclusion...
>>
> 
> Okay. It's fine for now. Just making sure you're aware of the issue.
> 
> Rob
> 

Best regards,
-- 
Nicolas Ferre

[PATCH] ARM: at91: AIC and GPIO IRQ device tree initilization

[Nicolas Ferre]

Both AIC and GPIO controllers are now using the standard of_irq_init()
function to initialize IRQs in case of DT use.
The DT specific initialization functions are now separated from the
non-DT case and are now using "linear" irq domains.
The .map() irqdomain operation is responsible for positioning the IRQ
handlers. In AIC case, the Linux IRQ number is directly programmed in
the hardware to avoid an additional reverse mapping operation.
The AIC position its irq domain as the "default" irq domain.

For DT case, the priority is not yet filled in the SMR. It will be the
subject of another patch.

Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
Hi,

This patch goes on top of previous series:
"[PATCH 0/9] ARM: at91: irqdomain and device tree for AIC and GPIO"
(which goes on top of Grant's "irq_domain generalization and rework").

It should address all advices by Rob Herring and I hope it is the best
way to deal with irq domains in all DT/non-DT cases.

A simple concern is that the IRQ latency may be bigger because of the
irq_create_mapping() call in .to_irq(). This is called in the hot path
of GPIO IRQ handler...

Best regards,


 arch/arm/mach-at91/board-dt.c |   11 +++-
 arch/arm/mach-at91/generic.h  |    6 ++
 arch/arm/mach-at91/gpio.c     |  124 +++++++++++++++++++++++++++++++----------
 arch/arm/mach-at91/irq.c      |   92 ++++++++++++++++++++-----------
 4 files changed, 171 insertions(+), 62 deletions(-)

diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
index 6120978..e0df4e5 100644
--- a/arch/arm/mach-at91/board-dt.c
+++ b/arch/arm/mach-at91/board-dt.c
@@ -15,6 +15,8 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/gpio.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 
 #include <mach/hardware.h>
@@ -86,9 +88,16 @@ static void __init ek_add_device_nand(void)
 	at91_add_device_nand(&ek_nand_data);
 }
 
+static const struct of_device_id irq_of_match[] __initconst = {
+
+	{ .compatible = "atmel,at91rm9200-aic", .data = at91_aic_of_init },
+	{ .compatible = "atmel,at91rm9200-gpio", .data = at91_gpio_of_irq_setup },
+	{ /*sentinel*/ }
+};
+
 static void __init at91_dt_init_irq(void)
 {
-	at91_init_irq_default();
+	of_irq_init(irq_of_match);
 }
 
 static void __init at91_dt_device_init(void)
diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
index 78cd605..c5d16e5 100644
--- a/arch/arm/mach-at91/generic.h
+++ b/arch/arm/mach-at91/generic.h
@@ -9,6 +9,7 @@
  */
 
 #include <linux/clkdev.h>
+#include <linux/of.h>
 
  /* Map io */
 extern void __init at91_map_io(void);
@@ -25,6 +26,9 @@ extern void __init at91_init_irq_default(void);
 extern void __init at91_init_interrupts(unsigned int priority[]);
 extern void __init at91x40_init_interrupts(unsigned int priority[]);
 extern void __init at91_aic_init(unsigned int priority[]);
+extern int  __init at91_aic_of_init(struct device_node *node,
+				    struct device_node *parent);
+
 
  /* Timer */
 struct sys_timer;
@@ -75,5 +79,7 @@ struct at91_gpio_bank {
 };
 extern void __init at91_gpio_init(struct at91_gpio_bank *, int nr_banks);
 extern void __init at91_gpio_irq_setup(void);
+extern int  __init at91_gpio_of_irq_setup(struct device_node *node,
+					  struct device_node *parent);
 
 extern int at91_extern_irq;
diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
index fbcb47e..64395a5 100644
--- a/arch/arm/mach-at91/gpio.c
+++ b/arch/arm/mach-at91/gpio.c
@@ -27,6 +27,7 @@
 #include <linux/irqdomain.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/of_gpio.h>
 
 #include <mach/hardware.h>
 #include <mach/at91_pio.h>
@@ -37,6 +38,7 @@ struct at91_gpio_chip {
 	struct gpio_chip	chip;
 	struct at91_gpio_chip	*next;		/* Bank sharing same clock */
 	int			pioc_hwirq;	/* PIO bank interrupt identifier on AIC */
+	int			pioc_virq;	/* PIO bank Linux virtual interrupt */
 	int			pioc_idx;	/* PIO bank index */
 	void __iomem		*regbase;	/* PIO bank virtual address */
 	struct clk		*clock;		/* associated clock */
@@ -295,7 +297,7 @@ static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
 	else
 		wakeups[bank] &= ~mask;
 
-	irq_set_irq_wake(gpio_chip[bank].pioc_hwirq, state);
+	irq_set_irq_wake(at91_gpio->pioc_virq, state);
 
 	return 0;
 }
@@ -397,12 +399,12 @@ static struct irq_chip gpio_irqchip = {
 
 static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 {
-	unsigned	virq;
 	struct irq_data *idata = irq_desc_get_irq_data(desc);
 	struct irq_chip *chip = irq_data_get_irq_chip(idata);
 	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(idata);
 	void __iomem	*pio = at91_gpio->regbase;
-	u32		isr;
+	unsigned long	isr;
+	int		n;
 
 	/* temporarily mask (level sensitive) parent IRQ */
 	chip->irq_ack(idata);
@@ -420,13 +422,10 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 			continue;
 		}
 
-		virq = gpio_to_irq(at91_gpio->chip.base);
-
-		while (isr) {
-			if (isr & 1)
-				generic_handle_irq(virq);
-			virq++;
-			isr >>= 1;
+		n = find_first_bit(&isr, BITS_PER_LONG);
+		while (n < BITS_PER_LONG) {
+			generic_handle_irq(at91_gpiolib_to_irq(&at91_gpio->chip, n));
+			n = find_next_bit(&isr, BITS_PER_LONG, n + 1);
 		}
 	}
 	chip->irq_unmask(idata);
@@ -496,23 +495,91 @@ postcore_initcall(at91_gpio_debugfs_init);
 /*--------------------------------------------------------------------------*/
 
 /*
+ * This lock class tells lockdep that GPIO irqs are in a different
+ * category than their parents, so it won't report false recursion.
+ */
+static struct lock_class_key gpio_lock_class;
+
+#if defined(CONFIG_OF)
+static int at91_gpio_irq_map(struct irq_domain *h, unsigned int virq,
+							irq_hw_number_t hw)
+{
+	struct at91_gpio_chip	*at91_gpio = h->host_data;
+
+	irq_set_lockdep_class(virq, &gpio_lock_class);
+
+	/*
+	 * Can use the "simple" and not "edge" handler since it's
+	 * shorter, and the AIC handles interrupts sanely.
+	 */
+	irq_set_chip_and_handler(virq, &gpio_irqchip,
+				 handle_simple_irq);
+	set_irq_flags(virq, IRQF_VALID);
+	irq_set_chip_data(virq, at91_gpio);
+
+	return 0;
+}
+
+static struct irq_domain_ops at91_gpio_ops = {
+	.map	= at91_gpio_irq_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+int __init at91_gpio_of_irq_setup(struct device_node *node,
+				     struct device_node *parent)
+{
+	struct at91_gpio_chip	*prev = NULL;
+	int			alias_idx = of_alias_get_id(node, "gpio");
+	struct at91_gpio_chip	*at91_gpio = &gpio_chip[alias_idx];
+
+	/* Disable irqs of this PIO controller */
+	__raw_writel(~0, at91_gpio->regbase + PIO_IDR);
+
+	/* Setup irq domain */
+	at91_gpio->domain = irq_domain_add_linear(node, at91_gpio->chip.ngpio,
+						&at91_gpio_ops, at91_gpio);
+	if (!at91_gpio->domain)
+		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
+			at91_gpio->pioc_idx);
+
+	/* Setup chained handler */
+	if (at91_gpio->pioc_idx)
+		prev = &gpio_chip[at91_gpio->pioc_idx - 1];
+
+	/* The toplevel handler handles one bank of GPIOs, except
+	 * on some SoC it can handles up to three...
+	 * We only set up the handler for the first of the list.
+	 */
+	if (prev && prev->next == at91_gpio)
+		return 0;
+
+	at91_gpio->pioc_virq = irq_create_mapping(irq_find_host(parent),
+							at91_gpio->pioc_hwirq);
+	irq_set_chip_data(at91_gpio->pioc_virq, at91_gpio);
+	irq_set_chained_handler(at91_gpio->pioc_virq, gpio_irq_handler);
+
+	return 0;
+}
+#else
+int __init at91_gpio_of_irq_setup(struct device_node *node,
+				     struct device_node *parent)
+{
+	return -EINVAL;
+}
+#endif
+
+/*
  * irqdomain initialization: pile up irqdomains on top of AIC range
  */
 static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
 {
 	int irq_base;
-#if defined(CONFIG_OF)
-	struct device_node *of_node = at91_gpio->chip.of_node;
-#else
-	struct device_node *of_node = NULL;
-#endif
 
 	irq_base = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
 	if (irq_base < 0)
 		panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
 			at91_gpio->pioc_idx, irq_base);
-	at91_gpio->domain = irq_domain_add_legacy(of_node,
-						  at91_gpio->chip.ngpio,
+	at91_gpio->domain = irq_domain_add_legacy(NULL, at91_gpio->chip.ngpio,
 						  irq_base, 0,
 						  &irq_domain_simple_ops, NULL);
 	if (!at91_gpio->domain)
@@ -521,12 +588,6 @@ static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
 }
 
 /*
- * This lock class tells lockdep that GPIO irqs are in a different
- * category than their parents, so it won't report false recursion.
- */
-static struct lock_class_key gpio_lock_class;
-
-/*
  * Called from the processor-specific init to enable GPIO interrupt support.
  */
 void __init at91_gpio_irq_setup(void)
@@ -538,8 +599,7 @@ void __init at91_gpio_irq_setup(void)
 	for (pioc = 0, this = gpio_chip, prev = NULL;
 			pioc++ < gpio_banks;
 			prev = this, this++) {
-		unsigned	pioc_hwirq = this->pioc_hwirq;
-		int		offset;
+		int offset;
 
 		__raw_writel(~0, this->regbase + PIO_IDR);
 
@@ -547,7 +607,7 @@ void __init at91_gpio_irq_setup(void)
 		at91_gpio_irqdomain(this);
 
 		for (offset = 0; offset < this->chip.ngpio; offset++) {
-			unsigned int virq = irq_find_mapping(this->domain, offset);
+			unsigned int virq = irq_create_mapping(this->domain, offset);
 			irq_set_lockdep_class(virq, &gpio_lock_class);
 
 			/*
@@ -569,8 +629,9 @@ void __init at91_gpio_irq_setup(void)
 		if (prev && prev->next == this)
 			continue;
 
-		irq_set_chip_data(pioc_hwirq, this);
-		irq_set_chained_handler(pioc_hwirq, gpio_irq_handler);
+		this->pioc_virq = irq_create_mapping(NULL, this->pioc_hwirq);
+		irq_set_chip_data(this->pioc_virq, this);
+		irq_set_chained_handler(this->pioc_virq, gpio_irq_handler);
 	}
 	pr_info("AT91: %d gpio irqs in %d banks\n", gpio_irqnbr, gpio_banks);
 }
@@ -648,7 +709,12 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset)
 {
 	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
-	int virq = irq_find_mapping(at91_gpio->domain, offset);
+	int virq;
+
+	if (offset < chip->ngpio)
+		virq = irq_create_mapping(at91_gpio->domain, offset);
+	else
+		virq = -ENXIO;
 
 	dev_dbg(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
 				chip->label, offset + chip->base, virq);
diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
index 46682fa..2e4ee09 100644
--- a/arch/arm/mach-at91/irq.c
+++ b/arch/arm/mach-at91/irq.c
@@ -135,27 +135,72 @@ static struct irq_chip at91_aic_chip = {
 	.irq_set_wake	= at91_aic_set_wake,
 };
 
+static void __init at91_aic_hw_init(unsigned int spu_vector)
+{
+	int i;
+
+	/*
+	 * Perform 8 End Of Interrupt Command to make sure AIC
+	 * will not Lock out nIRQ
+	 */
+	for (i = 0; i < 8; i++)
+		at91_aic_write(AT91_AIC_EOICR, 0);
+
+	/*
+	 * Spurious Interrupt ID in Spurious Vector Register.
+	 * When there is no current interrupt, the IRQ Vector Register
+	 * reads the value stored in AIC_SPU
+	 */
+	at91_aic_write(AT91_AIC_SPU, spu_vector);
+
+	/* No debugging in AIC: Debug (Protect) Control Register */
+	at91_aic_write(AT91_AIC_DCR, 0);
+
+	/* Disable and clear all interrupts initially */
+	at91_aic_write(AT91_AIC_IDCR, 0xFFFFFFFF);
+	at91_aic_write(AT91_AIC_ICCR, 0xFFFFFFFF);
+}
+
+
+
 #if defined(CONFIG_OF)
-static int __init __at91_aic_of_init(struct device_node *node,
-				     struct device_node *parent)
+static int at91_aic_irq_map(struct irq_domain *h, unsigned int virq,
+							irq_hw_number_t hw)
 {
-	at91_aic_base = of_iomap(node, 0);
-	at91_aic_np = node;
+	/* Put virq number in Source Vector Register */
+	at91_aic_write(AT91_AIC_SVR(hw), virq);
+
+	/* Active Low interrupt, without priority */
+	at91_aic_write(AT91_AIC_SMR(hw), AT91_AIC_SRCTYPE_LOW);
+
+	irq_set_chip_and_handler(virq, &at91_aic_chip, handle_level_irq);
+	set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
 
 	return 0;
 }
 
-static const struct of_device_id aic_ids[] __initconst = {
-	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
-	{ /*sentinel*/ }
+static struct irq_domain_ops at91_aic_irq_ops = {
+	.map	= at91_aic_irq_map,
+	.xlate	= irq_domain_xlate_twocell,
 };
 
-static void __init at91_aic_of_init(void)
+int __init at91_aic_of_init(struct device_node *node,
+				     struct device_node *parent)
 {
-	of_irq_init(aic_ids);
+	at91_aic_base = of_iomap(node, 0);
+	at91_aic_np = node;
+
+	at91_aic_domain = irq_domain_add_linear(at91_aic_np, NR_AIC_IRQS,
+						&at91_aic_irq_ops, NULL);
+	if (!at91_aic_domain)
+		panic("Unable to add AIC irq domain (DT)\n");
+
+	irq_set_default_host(at91_aic_domain);
+
+	at91_aic_hw_init(NR_AIC_IRQS);
+
+	return 0;
 }
-#else
-static void __init at91_aic_of_init(void) {}
 #endif
 
 /*
@@ -166,11 +211,7 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
 	unsigned int i;
 	int irq_base;
 
-	if (of_have_populated_dt())
-		at91_aic_of_init();
-	else
-		at91_aic_base = ioremap(AT91_AIC, 512);
-
+	at91_aic_base = ioremap(AT91_AIC, 512);
 	if (!at91_aic_base)
 		panic("Unable to ioremap AIC registers\n");
 
@@ -187,6 +228,8 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
 	if (!at91_aic_domain)
 		panic("Unable to add AIC irq domain\n");
 
+	irq_set_default_host(at91_aic_domain);
+
 	/*
 	 * The IVR is used by macro get_irqnr_and_base to read and verify.
 	 * The irq number is NR_AIC_IRQS when a spurious interrupt has occurred.
@@ -199,22 +242,7 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
 
 		irq_set_chip_and_handler(i, &at91_aic_chip, handle_level_irq);
 		set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
-
-		/* Perform 8 End Of Interrupt Command to make sure AIC will not Lock out nIRQ */
-		if (i < 8)
-			at91_aic_write(AT91_AIC_EOICR, 0);
 	}
 
-	/*
-	 * Spurious Interrupt ID in Spurious Vector Register is NR_AIC_IRQS
-	 * When there is no current interrupt, the IRQ Vector Register reads the value stored in AIC_SPU
-	 */
-	at91_aic_write(AT91_AIC_SPU, NR_AIC_IRQS);
-
-	/* No debugging in AIC: Debug (Protect) Control Register */
-	at91_aic_write(AT91_AIC_DCR, 0);
-
-	/* Disable and clear all interrupts initially */
-	at91_aic_write(AT91_AIC_IDCR, 0xFFFFFFFF);
-	at91_aic_write(AT91_AIC_ICCR, 0xFFFFFFFF);
+	at91_aic_hw_init(NR_AIC_IRQS);
 }
-- 
1.7.9

Re: [PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio

[Nicolas Ferre]

On 02/13/2012 03:43 PM, Nicolas Ferre :
> irq_to_gpio() macro will be removed from AT91 GPIO interrupt
> controller. So we replace it with the use of gpio_to_irq()
> and a reworked test.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: linux-usb@vger.kernel.org

Hi Greg,

I would like to queue this patch through arm-soc git tree (with all AT91
material of this patch series).

So, can I have your "Acked-by" on this one?

> ---
>  drivers/usb/host/ohci-at91.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c
> index 77afabc..8e855eb 100644
> --- a/drivers/usb/host/ohci-at91.c
> +++ b/drivers/usb/host/ohci-at91.c
> @@ -448,10 +448,11 @@ static irqreturn_t ohci_hcd_at91_overcurrent_irq(int irq, void *data)
>  
>  	/* From the GPIO notifying the over-current situation, find
>  	 * out the corresponding port */
> -	gpio = irq_to_gpio(irq);
>  	for (port = 0; port < ARRAY_SIZE(pdata->overcurrent_pin); port++) {
> -		if (pdata->overcurrent_pin[port] == gpio)
> +		if (gpio_to_irq(pdata->overcurrent_pin[port]) == irq) {
> +			gpio = pdata->overcurrent_pin[port];
>  			break;
> +		}
>  	}
>  
>  	if (port == ARRAY_SIZE(pdata->overcurrent_pin)) {

Thanks, best regards,
-- 
Nicolas Ferre

Re: [PATCH 3/9] ARM/USB: at91/ohci-at91: remove the use of irq_to_gpio

[Greg Kroah-Hartman]

On Fri, Feb 17, 2012 at 10:41:32AM +0100, Nicolas Ferre wrote:
> On 02/13/2012 03:43 PM, Nicolas Ferre :
> > irq_to_gpio() macro will be removed from AT91 GPIO interrupt
> > controller. So we replace it with the use of gpio_to_irq()
> > and a reworked test.
> > 
> > Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> > Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Cc: linux-usb@vger.kernel.org
> 
> Hi Greg,
> 
> I would like to queue this patch through arm-soc git tree (with all AT91
> material of this patch series).
> 
> So, can I have your "Acked-by" on this one?

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH] ARM: at91: AIC and GPIO IRQ device tree initilization

[Nicolas Ferre]

On 02/17/2012 10:27 AM, Nicolas Ferre :
> Both AIC and GPIO controllers are now using the standard of_irq_init()
> function to initialize IRQs in case of DT use.
> The DT specific initialization functions are now separated from the
> non-DT case and are now using "linear" irq domains.
> The .map() irqdomain operation is responsible for positioning the IRQ
> handlers. In AIC case, the Linux IRQ number is directly programmed in
> the hardware to avoid an additional reverse mapping operation.
> The AIC position its irq domain as the "default" irq domain.
> 
> For DT case, the priority is not yet filled in the SMR. It will be the
> subject of another patch.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> Hi,
> 
> This patch goes on top of previous series:
> "[PATCH 0/9] ARM: at91: irqdomain and device tree for AIC and GPIO"
> (which goes on top of Grant's "irq_domain generalization and rework").
> 
> It should address all advices by Rob Herring and I hope it is the best
> way to deal with irq domains in all DT/non-DT cases.
> 
> A simple concern is that the IRQ latency may be bigger because of the
> irq_create_mapping() call in .to_irq(). This is called in the hot path
> of GPIO IRQ handler...
> 
> Best regards,

Grant, Rob,

I would like to have some feedback from you so that we will be able to
build our DT related work on top of this series.

If you agree with this work, we may need to send it through arm-soc git
tree. This will require to establish a dependency with Grant's irqdomain
branch: do you think that it is stable enough?

Thanks for your advices, bye,

>  arch/arm/mach-at91/board-dt.c |   11 +++-
>  arch/arm/mach-at91/generic.h  |    6 ++
>  arch/arm/mach-at91/gpio.c     |  124 +++++++++++++++++++++++++++++++----------
>  arch/arm/mach-at91/irq.c      |   92 ++++++++++++++++++++-----------
>  4 files changed, 171 insertions(+), 62 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
> index 6120978..e0df4e5 100644
> --- a/arch/arm/mach-at91/board-dt.c
> +++ b/arch/arm/mach-at91/board-dt.c
> @@ -15,6 +15,8 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/gpio.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  
>  #include <mach/hardware.h>
> @@ -86,9 +88,16 @@ static void __init ek_add_device_nand(void)
>  	at91_add_device_nand(&ek_nand_data);
>  }
>  
> +static const struct of_device_id irq_of_match[] __initconst = {
> +
> +	{ .compatible = "atmel,at91rm9200-aic", .data = at91_aic_of_init },
> +	{ .compatible = "atmel,at91rm9200-gpio", .data = at91_gpio_of_irq_setup },
> +	{ /*sentinel*/ }
> +};
> +
>  static void __init at91_dt_init_irq(void)
>  {
> -	at91_init_irq_default();
> +	of_irq_init(irq_of_match);
>  }
>  
>  static void __init at91_dt_device_init(void)
> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> index 78cd605..c5d16e5 100644
> --- a/arch/arm/mach-at91/generic.h
> +++ b/arch/arm/mach-at91/generic.h
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <linux/clkdev.h>
> +#include <linux/of.h>
>  
>   /* Map io */
>  extern void __init at91_map_io(void);
> @@ -25,6 +26,9 @@ extern void __init at91_init_irq_default(void);
>  extern void __init at91_init_interrupts(unsigned int priority[]);
>  extern void __init at91x40_init_interrupts(unsigned int priority[]);
>  extern void __init at91_aic_init(unsigned int priority[]);
> +extern int  __init at91_aic_of_init(struct device_node *node,
> +				    struct device_node *parent);
> +
>  
>   /* Timer */
>  struct sys_timer;
> @@ -75,5 +79,7 @@ struct at91_gpio_bank {
>  };
>  extern void __init at91_gpio_init(struct at91_gpio_bank *, int nr_banks);
>  extern void __init at91_gpio_irq_setup(void);
> +extern int  __init at91_gpio_of_irq_setup(struct device_node *node,
> +					  struct device_node *parent);
>  
>  extern int at91_extern_irq;
> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> index fbcb47e..64395a5 100644
> --- a/arch/arm/mach-at91/gpio.c
> +++ b/arch/arm/mach-at91/gpio.c
> @@ -27,6 +27,7 @@
>  #include <linux/irqdomain.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/of_gpio.h>
>  
>  #include <mach/hardware.h>
>  #include <mach/at91_pio.h>
> @@ -37,6 +38,7 @@ struct at91_gpio_chip {
>  	struct gpio_chip	chip;
>  	struct at91_gpio_chip	*next;		/* Bank sharing same clock */
>  	int			pioc_hwirq;	/* PIO bank interrupt identifier on AIC */
> +	int			pioc_virq;	/* PIO bank Linux virtual interrupt */
>  	int			pioc_idx;	/* PIO bank index */
>  	void __iomem		*regbase;	/* PIO bank virtual address */
>  	struct clk		*clock;		/* associated clock */
> @@ -295,7 +297,7 @@ static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
>  	else
>  		wakeups[bank] &= ~mask;
>  
> -	irq_set_irq_wake(gpio_chip[bank].pioc_hwirq, state);
> +	irq_set_irq_wake(at91_gpio->pioc_virq, state);
>  
>  	return 0;
>  }
> @@ -397,12 +399,12 @@ static struct irq_chip gpio_irqchip = {
>  
>  static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>  {
> -	unsigned	virq;
>  	struct irq_data *idata = irq_desc_get_irq_data(desc);
>  	struct irq_chip *chip = irq_data_get_irq_chip(idata);
>  	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(idata);
>  	void __iomem	*pio = at91_gpio->regbase;
> -	u32		isr;
> +	unsigned long	isr;
> +	int		n;
>  
>  	/* temporarily mask (level sensitive) parent IRQ */
>  	chip->irq_ack(idata);
> @@ -420,13 +422,10 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>  			continue;
>  		}
>  
> -		virq = gpio_to_irq(at91_gpio->chip.base);
> -
> -		while (isr) {
> -			if (isr & 1)
> -				generic_handle_irq(virq);
> -			virq++;
> -			isr >>= 1;
> +		n = find_first_bit(&isr, BITS_PER_LONG);
> +		while (n < BITS_PER_LONG) {
> +			generic_handle_irq(at91_gpiolib_to_irq(&at91_gpio->chip, n));
> +			n = find_next_bit(&isr, BITS_PER_LONG, n + 1);
>  		}
>  	}
>  	chip->irq_unmask(idata);
> @@ -496,23 +495,91 @@ postcore_initcall(at91_gpio_debugfs_init);
>  /*--------------------------------------------------------------------------*/
>  
>  /*
> + * This lock class tells lockdep that GPIO irqs are in a different
> + * category than their parents, so it won't report false recursion.
> + */
> +static struct lock_class_key gpio_lock_class;
> +
> +#if defined(CONFIG_OF)
> +static int at91_gpio_irq_map(struct irq_domain *h, unsigned int virq,
> +							irq_hw_number_t hw)
> +{
> +	struct at91_gpio_chip	*at91_gpio = h->host_data;
> +
> +	irq_set_lockdep_class(virq, &gpio_lock_class);
> +
> +	/*
> +	 * Can use the "simple" and not "edge" handler since it's
> +	 * shorter, and the AIC handles interrupts sanely.
> +	 */
> +	irq_set_chip_and_handler(virq, &gpio_irqchip,
> +				 handle_simple_irq);
> +	set_irq_flags(virq, IRQF_VALID);
> +	irq_set_chip_data(virq, at91_gpio);
> +
> +	return 0;
> +}
> +
> +static struct irq_domain_ops at91_gpio_ops = {
> +	.map	= at91_gpio_irq_map,
> +	.xlate	= irq_domain_xlate_twocell,
> +};
> +
> +int __init at91_gpio_of_irq_setup(struct device_node *node,
> +				     struct device_node *parent)
> +{
> +	struct at91_gpio_chip	*prev = NULL;
> +	int			alias_idx = of_alias_get_id(node, "gpio");
> +	struct at91_gpio_chip	*at91_gpio = &gpio_chip[alias_idx];
> +
> +	/* Disable irqs of this PIO controller */
> +	__raw_writel(~0, at91_gpio->regbase + PIO_IDR);
> +
> +	/* Setup irq domain */
> +	at91_gpio->domain = irq_domain_add_linear(node, at91_gpio->chip.ngpio,
> +						&at91_gpio_ops, at91_gpio);
> +	if (!at91_gpio->domain)
> +		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
> +			at91_gpio->pioc_idx);
> +
> +	/* Setup chained handler */
> +	if (at91_gpio->pioc_idx)
> +		prev = &gpio_chip[at91_gpio->pioc_idx - 1];
> +
> +	/* The toplevel handler handles one bank of GPIOs, except
> +	 * on some SoC it can handles up to three...
> +	 * We only set up the handler for the first of the list.
> +	 */
> +	if (prev && prev->next == at91_gpio)
> +		return 0;
> +
> +	at91_gpio->pioc_virq = irq_create_mapping(irq_find_host(parent),
> +							at91_gpio->pioc_hwirq);
> +	irq_set_chip_data(at91_gpio->pioc_virq, at91_gpio);
> +	irq_set_chained_handler(at91_gpio->pioc_virq, gpio_irq_handler);
> +
> +	return 0;
> +}
> +#else
> +int __init at91_gpio_of_irq_setup(struct device_node *node,
> +				     struct device_node *parent)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
> +/*
>   * irqdomain initialization: pile up irqdomains on top of AIC range
>   */
>  static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
>  {
>  	int irq_base;
> -#if defined(CONFIG_OF)
> -	struct device_node *of_node = at91_gpio->chip.of_node;
> -#else
> -	struct device_node *of_node = NULL;
> -#endif
>  
>  	irq_base = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
>  	if (irq_base < 0)
>  		panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
>  			at91_gpio->pioc_idx, irq_base);
> -	at91_gpio->domain = irq_domain_add_legacy(of_node,
> -						  at91_gpio->chip.ngpio,
> +	at91_gpio->domain = irq_domain_add_legacy(NULL, at91_gpio->chip.ngpio,
>  						  irq_base, 0,
>  						  &irq_domain_simple_ops, NULL);
>  	if (!at91_gpio->domain)
> @@ -521,12 +588,6 @@ static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
>  }
>  
>  /*
> - * This lock class tells lockdep that GPIO irqs are in a different
> - * category than their parents, so it won't report false recursion.
> - */
> -static struct lock_class_key gpio_lock_class;
> -
> -/*
>   * Called from the processor-specific init to enable GPIO interrupt support.
>   */
>  void __init at91_gpio_irq_setup(void)
> @@ -538,8 +599,7 @@ void __init at91_gpio_irq_setup(void)
>  	for (pioc = 0, this = gpio_chip, prev = NULL;
>  			pioc++ < gpio_banks;
>  			prev = this, this++) {
> -		unsigned	pioc_hwirq = this->pioc_hwirq;
> -		int		offset;
> +		int offset;
>  
>  		__raw_writel(~0, this->regbase + PIO_IDR);
>  
> @@ -547,7 +607,7 @@ void __init at91_gpio_irq_setup(void)
>  		at91_gpio_irqdomain(this);
>  
>  		for (offset = 0; offset < this->chip.ngpio; offset++) {
> -			unsigned int virq = irq_find_mapping(this->domain, offset);
> +			unsigned int virq = irq_create_mapping(this->domain, offset);
>  			irq_set_lockdep_class(virq, &gpio_lock_class);
>  
>  			/*
> @@ -569,8 +629,9 @@ void __init at91_gpio_irq_setup(void)
>  		if (prev && prev->next == this)
>  			continue;
>  
> -		irq_set_chip_data(pioc_hwirq, this);
> -		irq_set_chained_handler(pioc_hwirq, gpio_irq_handler);
> +		this->pioc_virq = irq_create_mapping(NULL, this->pioc_hwirq);
> +		irq_set_chip_data(this->pioc_virq, this);
> +		irq_set_chained_handler(this->pioc_virq, gpio_irq_handler);
>  	}
>  	pr_info("AT91: %d gpio irqs in %d banks\n", gpio_irqnbr, gpio_banks);
>  }
> @@ -648,7 +709,12 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
>  static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset)
>  {
>  	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> -	int virq = irq_find_mapping(at91_gpio->domain, offset);
> +	int virq;
> +
> +	if (offset < chip->ngpio)
> +		virq = irq_create_mapping(at91_gpio->domain, offset);
> +	else
> +		virq = -ENXIO;
>  
>  	dev_dbg(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
>  				chip->label, offset + chip->base, virq);
> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
> index 46682fa..2e4ee09 100644
> --- a/arch/arm/mach-at91/irq.c
> +++ b/arch/arm/mach-at91/irq.c
> @@ -135,27 +135,72 @@ static struct irq_chip at91_aic_chip = {
>  	.irq_set_wake	= at91_aic_set_wake,
>  };
>  
> +static void __init at91_aic_hw_init(unsigned int spu_vector)
> +{
> +	int i;
> +
> +	/*
> +	 * Perform 8 End Of Interrupt Command to make sure AIC
> +	 * will not Lock out nIRQ
> +	 */
> +	for (i = 0; i < 8; i++)
> +		at91_aic_write(AT91_AIC_EOICR, 0);
> +
> +	/*
> +	 * Spurious Interrupt ID in Spurious Vector Register.
> +	 * When there is no current interrupt, the IRQ Vector Register
> +	 * reads the value stored in AIC_SPU
> +	 */
> +	at91_aic_write(AT91_AIC_SPU, spu_vector);
> +
> +	/* No debugging in AIC: Debug (Protect) Control Register */
> +	at91_aic_write(AT91_AIC_DCR, 0);
> +
> +	/* Disable and clear all interrupts initially */
> +	at91_aic_write(AT91_AIC_IDCR, 0xFFFFFFFF);
> +	at91_aic_write(AT91_AIC_ICCR, 0xFFFFFFFF);
> +}
> +
> +
> +
>  #if defined(CONFIG_OF)
> -static int __init __at91_aic_of_init(struct device_node *node,
> -				     struct device_node *parent)
> +static int at91_aic_irq_map(struct irq_domain *h, unsigned int virq,
> +							irq_hw_number_t hw)
>  {
> -	at91_aic_base = of_iomap(node, 0);
> -	at91_aic_np = node;
> +	/* Put virq number in Source Vector Register */
> +	at91_aic_write(AT91_AIC_SVR(hw), virq);
> +
> +	/* Active Low interrupt, without priority */
> +	at91_aic_write(AT91_AIC_SMR(hw), AT91_AIC_SRCTYPE_LOW);
> +
> +	irq_set_chip_and_handler(virq, &at91_aic_chip, handle_level_irq);
> +	set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
>  
>  	return 0;
>  }
>  
> -static const struct of_device_id aic_ids[] __initconst = {
> -	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
> -	{ /*sentinel*/ }
> +static struct irq_domain_ops at91_aic_irq_ops = {
> +	.map	= at91_aic_irq_map,
> +	.xlate	= irq_domain_xlate_twocell,
>  };
>  
> -static void __init at91_aic_of_init(void)
> +int __init at91_aic_of_init(struct device_node *node,
> +				     struct device_node *parent)
>  {
> -	of_irq_init(aic_ids);
> +	at91_aic_base = of_iomap(node, 0);
> +	at91_aic_np = node;
> +
> +	at91_aic_domain = irq_domain_add_linear(at91_aic_np, NR_AIC_IRQS,
> +						&at91_aic_irq_ops, NULL);
> +	if (!at91_aic_domain)
> +		panic("Unable to add AIC irq domain (DT)\n");
> +
> +	irq_set_default_host(at91_aic_domain);
> +
> +	at91_aic_hw_init(NR_AIC_IRQS);
> +
> +	return 0;
>  }
> -#else
> -static void __init at91_aic_of_init(void) {}
>  #endif
>  
>  /*
> @@ -166,11 +211,7 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>  	unsigned int i;
>  	int irq_base;
>  
> -	if (of_have_populated_dt())
> -		at91_aic_of_init();
> -	else
> -		at91_aic_base = ioremap(AT91_AIC, 512);
> -
> +	at91_aic_base = ioremap(AT91_AIC, 512);
>  	if (!at91_aic_base)
>  		panic("Unable to ioremap AIC registers\n");
>  
> @@ -187,6 +228,8 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>  	if (!at91_aic_domain)
>  		panic("Unable to add AIC irq domain\n");
>  
> +	irq_set_default_host(at91_aic_domain);
> +
>  	/*
>  	 * The IVR is used by macro get_irqnr_and_base to read and verify.
>  	 * The irq number is NR_AIC_IRQS when a spurious interrupt has occurred.
> @@ -199,22 +242,7 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>  
>  		irq_set_chip_and_handler(i, &at91_aic_chip, handle_level_irq);
>  		set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
> -
> -		/* Perform 8 End Of Interrupt Command to make sure AIC will not Lock out nIRQ */
> -		if (i < 8)
> -			at91_aic_write(AT91_AIC_EOICR, 0);
>  	}
>  
> -	/*
> -	 * Spurious Interrupt ID in Spurious Vector Register is NR_AIC_IRQS
> -	 * When there is no current interrupt, the IRQ Vector Register reads the value stored in AIC_SPU
> -	 */
> -	at91_aic_write(AT91_AIC_SPU, NR_AIC_IRQS);
> -
> -	/* No debugging in AIC: Debug (Protect) Control Register */
> -	at91_aic_write(AT91_AIC_DCR, 0);
> -
> -	/* Disable and clear all interrupts initially */
> -	at91_aic_write(AT91_AIC_IDCR, 0xFFFFFFFF);
> -	at91_aic_write(AT91_AIC_ICCR, 0xFFFFFFFF);
> +	at91_aic_hw_init(NR_AIC_IRQS);
>  }


-- 
Nicolas Ferre

Re: [PATCH] ARM: at91: AIC and GPIO IRQ device tree initilization

[Russell King - ARM Linux]

On Tue, Feb 21, 2012 at 11:06:13AM +0100, Nicolas Ferre wrote:
> Grant, Rob,
> 
> I would like to have some feedback from you so that we will be able to
> build our DT related work on top of this series.
> 
> If you agree with this work, we may need to send it through arm-soc git
> tree. This will require to establish a dependency with Grant's irqdomain
> branch: do you think that it is stable enough?

That depends whether Grant has fixed the x86 regression with
drivers/mfd/twl-core.c yet.

Re: [PATCH] ARM: at91: AIC and GPIO IRQ device tree initilization

[Rob Herring]

On 02/17/2012 03:27 AM, Nicolas Ferre wrote:
> Both AIC and GPIO controllers are now using the standard of_irq_init()
> function to initialize IRQs in case of DT use.
> The DT specific initialization functions are now separated from the
> non-DT case and are now using "linear" irq domains.
> The .map() irqdomain operation is responsible for positioning the IRQ
> handlers. In AIC case, the Linux IRQ number is directly programmed in
> the hardware to avoid an additional reverse mapping operation.
> The AIC position its irq domain as the "default" irq domain.
> 
> For DT case, the priority is not yet filled in the SMR. It will be the
> subject of another patch.
> 
> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> Hi,
> 
> This patch goes on top of previous series:
> "[PATCH 0/9] ARM: at91: irqdomain and device tree for AIC and GPIO"
> (which goes on top of Grant's "irq_domain generalization and rework").
> 
> It should address all advices by Rob Herring and I hope it is the best
> way to deal with irq domains in all DT/non-DT cases.
> 
> A simple concern is that the IRQ latency may be bigger because of the
> irq_create_mapping() call in .to_irq(). This is called in the hot path
> of GPIO IRQ handler...


Why are you not using irq_find_mapping?

> Best regards,
> 
> 
>  arch/arm/mach-at91/board-dt.c |   11 +++-
>  arch/arm/mach-at91/generic.h  |    6 ++
>  arch/arm/mach-at91/gpio.c     |  124 +++++++++++++++++++++++++++++++----------
>  arch/arm/mach-at91/irq.c      |   92 ++++++++++++++++++++-----------
>  4 files changed, 171 insertions(+), 62 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
> index 6120978..e0df4e5 100644
> --- a/arch/arm/mach-at91/board-dt.c
> +++ b/arch/arm/mach-at91/board-dt.c
> @@ -15,6 +15,8 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/gpio.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
>  #include <linux/of_platform.h>
>  
>  #include <mach/hardware.h>
> @@ -86,9 +88,16 @@ static void __init ek_add_device_nand(void)
>  	at91_add_device_nand(&ek_nand_data);
>  }
>  
> +static const struct of_device_id irq_of_match[] __initconst = {
> +
> +	{ .compatible = "atmel,at91rm9200-aic", .data = at91_aic_of_init },
> +	{ .compatible = "atmel,at91rm9200-gpio", .data = at91_gpio_of_irq_setup },
> +	{ /*sentinel*/ }
> +};
> +
>  static void __init at91_dt_init_irq(void)
>  {
> -	at91_init_irq_default();
> +	of_irq_init(irq_of_match);
>  }
>  
>  static void __init at91_dt_device_init(void)
> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> index 78cd605..c5d16e5 100644
> --- a/arch/arm/mach-at91/generic.h
> +++ b/arch/arm/mach-at91/generic.h
> @@ -9,6 +9,7 @@
>   */
>  
>  #include <linux/clkdev.h>
> +#include <linux/of.h>
>  
>   /* Map io */
>  extern void __init at91_map_io(void);
> @@ -25,6 +26,9 @@ extern void __init at91_init_irq_default(void);
>  extern void __init at91_init_interrupts(unsigned int priority[]);
>  extern void __init at91x40_init_interrupts(unsigned int priority[]);
>  extern void __init at91_aic_init(unsigned int priority[]);
> +extern int  __init at91_aic_of_init(struct device_node *node,
> +				    struct device_node *parent);
> +
>  
>   /* Timer */
>  struct sys_timer;
> @@ -75,5 +79,7 @@ struct at91_gpio_bank {
>  };
>  extern void __init at91_gpio_init(struct at91_gpio_bank *, int nr_banks);
>  extern void __init at91_gpio_irq_setup(void);
> +extern int  __init at91_gpio_of_irq_setup(struct device_node *node,
> +					  struct device_node *parent);
>  
>  extern int at91_extern_irq;
> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
> index fbcb47e..64395a5 100644
> --- a/arch/arm/mach-at91/gpio.c
> +++ b/arch/arm/mach-at91/gpio.c
> @@ -27,6 +27,7 @@
>  #include <linux/irqdomain.h>
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
> +#include <linux/of_gpio.h>
>  
>  #include <mach/hardware.h>
>  #include <mach/at91_pio.h>
> @@ -37,6 +38,7 @@ struct at91_gpio_chip {
>  	struct gpio_chip	chip;
>  	struct at91_gpio_chip	*next;		/* Bank sharing same clock */
>  	int			pioc_hwirq;	/* PIO bank interrupt identifier on AIC */
> +	int			pioc_virq;	/* PIO bank Linux virtual interrupt */
>  	int			pioc_idx;	/* PIO bank index */
>  	void __iomem		*regbase;	/* PIO bank virtual address */
>  	struct clk		*clock;		/* associated clock */
> @@ -295,7 +297,7 @@ static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
>  	else
>  		wakeups[bank] &= ~mask;
>  
> -	irq_set_irq_wake(gpio_chip[bank].pioc_hwirq, state);
> +	irq_set_irq_wake(at91_gpio->pioc_virq, state);
>  
>  	return 0;
>  }
> @@ -397,12 +399,12 @@ static struct irq_chip gpio_irqchip = {
>  
>  static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>  {
> -	unsigned	virq;
>  	struct irq_data *idata = irq_desc_get_irq_data(desc);
>  	struct irq_chip *chip = irq_data_get_irq_chip(idata);
>  	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(idata);
>  	void __iomem	*pio = at91_gpio->regbase;
> -	u32		isr;
> +	unsigned long	isr;
> +	int		n;
>  
>  	/* temporarily mask (level sensitive) parent IRQ */
>  	chip->irq_ack(idata);
> @@ -420,13 +422,10 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>  			continue;
>  		}
>  
> -		virq = gpio_to_irq(at91_gpio->chip.base);
> -
> -		while (isr) {
> -			if (isr & 1)
> -				generic_handle_irq(virq);
> -			virq++;
> -			isr >>= 1;
> +		n = find_first_bit(&isr, BITS_PER_LONG);
> +		while (n < BITS_PER_LONG) {
> +			generic_handle_irq(at91_gpiolib_to_irq(&at91_gpio->chip, n));
> +			n = find_next_bit(&isr, BITS_PER_LONG, n + 1);
>  		}
>  	}
>  	chip->irq_unmask(idata);
> @@ -496,23 +495,91 @@ postcore_initcall(at91_gpio_debugfs_init);
>  /*--------------------------------------------------------------------------*/
>  
>  /*
> + * This lock class tells lockdep that GPIO irqs are in a different
> + * category than their parents, so it won't report false recursion.
> + */
> +static struct lock_class_key gpio_lock_class;
> +
> +#if defined(CONFIG_OF)
> +static int at91_gpio_irq_map(struct irq_domain *h, unsigned int virq,
> +							irq_hw_number_t hw)
> +{
> +	struct at91_gpio_chip	*at91_gpio = h->host_data;
> +
> +	irq_set_lockdep_class(virq, &gpio_lock_class);
> +
> +	/*
> +	 * Can use the "simple" and not "edge" handler since it's
> +	 * shorter, and the AIC handles interrupts sanely.
> +	 */
> +	irq_set_chip_and_handler(virq, &gpio_irqchip,
> +				 handle_simple_irq);
> +	set_irq_flags(virq, IRQF_VALID);
> +	irq_set_chip_data(virq, at91_gpio);
> +
> +	return 0;
> +}
> +
> +static struct irq_domain_ops at91_gpio_ops = {
> +	.map	= at91_gpio_irq_map,
> +	.xlate	= irq_domain_xlate_twocell,
> +};
> +
> +int __init at91_gpio_of_irq_setup(struct device_node *node,
> +				     struct device_node *parent)
> +{
> +	struct at91_gpio_chip	*prev = NULL;
> +	int			alias_idx = of_alias_get_id(node, "gpio");
> +	struct at91_gpio_chip	*at91_gpio = &gpio_chip[alias_idx];
> +
> +	/* Disable irqs of this PIO controller */
> +	__raw_writel(~0, at91_gpio->regbase + PIO_IDR);
> +
> +	/* Setup irq domain */
> +	at91_gpio->domain = irq_domain_add_linear(node, at91_gpio->chip.ngpio,
> +						&at91_gpio_ops, at91_gpio);
> +	if (!at91_gpio->domain)
> +		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
> +			at91_gpio->pioc_idx);
> +
> +	/* Setup chained handler */
> +	if (at91_gpio->pioc_idx)
> +		prev = &gpio_chip[at91_gpio->pioc_idx - 1];
> +
> +	/* The toplevel handler handles one bank of GPIOs, except
> +	 * on some SoC it can handles up to three...
> +	 * We only set up the handler for the first of the list.
> +	 */
> +	if (prev && prev->next == at91_gpio)
> +		return 0;
> +
> +	at91_gpio->pioc_virq = irq_create_mapping(irq_find_host(parent),
> +							at91_gpio->pioc_hwirq);
> +	irq_set_chip_data(at91_gpio->pioc_virq, at91_gpio);
> +	irq_set_chained_handler(at91_gpio->pioc_virq, gpio_irq_handler);
> +
> +	return 0;
> +}
> +#else
> +int __init at91_gpio_of_irq_setup(struct device_node *node,
> +				     struct device_node *parent)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
> +/*
>   * irqdomain initialization: pile up irqdomains on top of AIC range
>   */
>  static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
>  {
>  	int irq_base;
> -#if defined(CONFIG_OF)
> -	struct device_node *of_node = at91_gpio->chip.of_node;
> -#else
> -	struct device_node *of_node = NULL;
> -#endif
>  
>  	irq_base = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
>  	if (irq_base < 0)
>  		panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
>  			at91_gpio->pioc_idx, irq_base);
> -	at91_gpio->domain = irq_domain_add_legacy(of_node,
> -						  at91_gpio->chip.ngpio,
> +	at91_gpio->domain = irq_domain_add_legacy(NULL, at91_gpio->chip.ngpio,
>  						  irq_base, 0,
>  						  &irq_domain_simple_ops, NULL);
>  	if (!at91_gpio->domain)
> @@ -521,12 +588,6 @@ static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
>  }
>  
>  /*
> - * This lock class tells lockdep that GPIO irqs are in a different
> - * category than their parents, so it won't report false recursion.
> - */
> -static struct lock_class_key gpio_lock_class;
> -
> -/*
>   * Called from the processor-specific init to enable GPIO interrupt support.
>   */
>  void __init at91_gpio_irq_setup(void)
> @@ -538,8 +599,7 @@ void __init at91_gpio_irq_setup(void)
>  	for (pioc = 0, this = gpio_chip, prev = NULL;
>  			pioc++ < gpio_banks;
>  			prev = this, this++) {
> -		unsigned	pioc_hwirq = this->pioc_hwirq;
> -		int		offset;
> +		int offset;
>  
>  		__raw_writel(~0, this->regbase + PIO_IDR);
>  
> @@ -547,7 +607,7 @@ void __init at91_gpio_irq_setup(void)
>  		at91_gpio_irqdomain(this);
>  
>  		for (offset = 0; offset < this->chip.ngpio; offset++) {
> -			unsigned int virq = irq_find_mapping(this->domain, offset);
> +			unsigned int virq = irq_create_mapping(this->domain, offset);

This should get done by irq_domain_add_legacy.


>  			irq_set_lockdep_class(virq, &gpio_lock_class);
>  
>  			/*
> @@ -569,8 +629,9 @@ void __init at91_gpio_irq_setup(void)
>  		if (prev && prev->next == this)
>  			continue;
>  
> -		irq_set_chip_data(pioc_hwirq, this);
> -		irq_set_chained_handler(pioc_hwirq, gpio_irq_handler);
> +		this->pioc_virq = irq_create_mapping(NULL, this->pioc_hwirq);
> +		irq_set_chip_data(this->pioc_virq, this);
> +		irq_set_chained_handler(this->pioc_virq, gpio_irq_handler);
>  	}
>  	pr_info("AT91: %d gpio irqs in %d banks\n", gpio_irqnbr, gpio_banks);
>  }
> @@ -648,7 +709,12 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
>  static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset)
>  {
>  	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
> -	int virq = irq_find_mapping(at91_gpio->domain, offset);
> +	int virq;
> +
> +	if (offset < chip->ngpio)
> +		virq = irq_create_mapping(at91_gpio->domain, offset);
> +	else
> +		virq = -ENXIO;
>  
>  	dev_dbg(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
>  				chip->label, offset + chip->base, virq);
> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
> index 46682fa..2e4ee09 100644
> --- a/arch/arm/mach-at91/irq.c
> +++ b/arch/arm/mach-at91/irq.c
> @@ -135,27 +135,72 @@ static struct irq_chip at91_aic_chip = {
>  	.irq_set_wake	= at91_aic_set_wake,
>  };
>  
> +static void __init at91_aic_hw_init(unsigned int spu_vector)
> +{
> +	int i;
> +
> +	/*
> +	 * Perform 8 End Of Interrupt Command to make sure AIC
> +	 * will not Lock out nIRQ
> +	 */
> +	for (i = 0; i < 8; i++)
> +		at91_aic_write(AT91_AIC_EOICR, 0);
> +
> +	/*
> +	 * Spurious Interrupt ID in Spurious Vector Register.
> +	 * When there is no current interrupt, the IRQ Vector Register
> +	 * reads the value stored in AIC_SPU
> +	 */
> +	at91_aic_write(AT91_AIC_SPU, spu_vector);
> +
> +	/* No debugging in AIC: Debug (Protect) Control Register */
> +	at91_aic_write(AT91_AIC_DCR, 0);
> +
> +	/* Disable and clear all interrupts initially */
> +	at91_aic_write(AT91_AIC_IDCR, 0xFFFFFFFF);
> +	at91_aic_write(AT91_AIC_ICCR, 0xFFFFFFFF);
> +}
> +
> +
> +

Remove extra blank lines.

>  #if defined(CONFIG_OF)
> -static int __init __at91_aic_of_init(struct device_node *node,
> -				     struct device_node *parent)
> +static int at91_aic_irq_map(struct irq_domain *h, unsigned int virq,
> +							irq_hw_number_t hw)
>  {
> -	at91_aic_base = of_iomap(node, 0);
> -	at91_aic_np = node;
> +	/* Put virq number in Source Vector Register */
> +	at91_aic_write(AT91_AIC_SVR(hw), virq);
> +
> +	/* Active Low interrupt, without priority */
> +	at91_aic_write(AT91_AIC_SMR(hw), AT91_AIC_SRCTYPE_LOW);
> +
> +	irq_set_chip_and_handler(virq, &at91_aic_chip, handle_level_irq);
> +	set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
>  
>  	return 0;
>  }
>  
> -static const struct of_device_id aic_ids[] __initconst = {
> -	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
> -	{ /*sentinel*/ }
> +static struct irq_domain_ops at91_aic_irq_ops = {
> +	.map	= at91_aic_irq_map,
> +	.xlate	= irq_domain_xlate_twocell,
>  };
>  
> -static void __init at91_aic_of_init(void)
> +int __init at91_aic_of_init(struct device_node *node,
> +				     struct device_node *parent)
>  {
> -	of_irq_init(aic_ids);
> +	at91_aic_base = of_iomap(node, 0);
> +	at91_aic_np = node;
> +
> +	at91_aic_domain = irq_domain_add_linear(at91_aic_np, NR_AIC_IRQS,
> +						&at91_aic_irq_ops, NULL);
> +	if (!at91_aic_domain)
> +		panic("Unable to add AIC irq domain (DT)\n");
> +
> +	irq_set_default_host(at91_aic_domain);

Is this really necessary? I don't think it should be.

> +
> +	at91_aic_hw_init(NR_AIC_IRQS);
> +
> +	return 0;
>  }
> -#else
> -static void __init at91_aic_of_init(void) {}
>  #endif
>  
>  /*
> @@ -166,11 +211,7 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>  	unsigned int i;
>  	int irq_base;
>  
> -	if (of_have_populated_dt())
> -		at91_aic_of_init();
> -	else
> -		at91_aic_base = ioremap(AT91_AIC, 512);
> -
> +	at91_aic_base = ioremap(AT91_AIC, 512);
>  	if (!at91_aic_base)
>  		panic("Unable to ioremap AIC registers\n");
>  
> @@ -187,6 +228,8 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>  	if (!at91_aic_domain)
>  		panic("Unable to add AIC irq domain\n");
>  
> +	irq_set_default_host(at91_aic_domain);
> +
>  	/*
>  	 * The IVR is used by macro get_irqnr_and_base to read and verify.
>  	 * The irq number is NR_AIC_IRQS when a spurious interrupt has occurred.
> @@ -199,22 +242,7 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>  
>  		irq_set_chip_and_handler(i, &at91_aic_chip, handle_level_irq);
>  		set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
> -
> -		/* Perform 8 End Of Interrupt Command to make sure AIC will not Lock out nIRQ */
> -		if (i < 8)
> -			at91_aic_write(AT91_AIC_EOICR, 0);
>  	}
>  
> -	/*
> -	 * Spurious Interrupt ID in Spurious Vector Register is NR_AIC_IRQS
> -	 * When there is no current interrupt, the IRQ Vector Register reads the value stored in AIC_SPU
> -	 */
> -	at91_aic_write(AT91_AIC_SPU, NR_AIC_IRQS);
> -
> -	/* No debugging in AIC: Debug (Protect) Control Register */
> -	at91_aic_write(AT91_AIC_DCR, 0);
> -
> -	/* Disable and clear all interrupts initially */
> -	at91_aic_write(AT91_AIC_IDCR, 0xFFFFFFFF);
> -	at91_aic_write(AT91_AIC_ICCR, 0xFFFFFFFF);
> +	at91_aic_hw_init(NR_AIC_IRQS);
>  }

Re: [PATCH] ARM: at91: AIC and GPIO IRQ device tree initilization

[Nicolas Ferre]

On 02/21/2012 03:07 PM, Rob Herring :
> On 02/17/2012 03:27 AM, Nicolas Ferre wrote:
>> Both AIC and GPIO controllers are now using the standard of_irq_init()
>> function to initialize IRQs in case of DT use.
>> The DT specific initialization functions are now separated from the
>> non-DT case and are now using "linear" irq domains.
>> The .map() irqdomain operation is responsible for positioning the IRQ
>> handlers. In AIC case, the Linux IRQ number is directly programmed in
>> the hardware to avoid an additional reverse mapping operation.
>> The AIC position its irq domain as the "default" irq domain.
>>
>> For DT case, the priority is not yet filled in the SMR. It will be the
>> subject of another patch.
>>
>> Signed-off-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> ---
>> Hi,
>>
>> This patch goes on top of previous series:
>> "[PATCH 0/9] ARM: at91: irqdomain and device tree for AIC and GPIO"
>> (which goes on top of Grant's "irq_domain generalization and rework").
>>
>> It should address all advices by Rob Herring and I hope it is the best
>> way to deal with irq domains in all DT/non-DT cases.
>>
>> A simple concern is that the IRQ latency may be bigger because of the
>> irq_create_mapping() call in .to_irq(). This is called in the hot path
>> of GPIO IRQ handler...
> 
> 
> Why are you not using irq_find_mapping?

Hehe, good question ;-)
It should have been experimented at some point and forgotten... But you
are absolutely right, it is the way to go.

>> Best regards,
>>
>>
>>  arch/arm/mach-at91/board-dt.c |   11 +++-
>>  arch/arm/mach-at91/generic.h  |    6 ++
>>  arch/arm/mach-at91/gpio.c     |  124 +++++++++++++++++++++++++++++++----------
>>  arch/arm/mach-at91/irq.c      |   92 ++++++++++++++++++++-----------
>>  4 files changed, 171 insertions(+), 62 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
>> index 6120978..e0df4e5 100644
>> --- a/arch/arm/mach-at91/board-dt.c
>> +++ b/arch/arm/mach-at91/board-dt.c
>> @@ -15,6 +15,8 @@
>>  #include <linux/init.h>
>>  #include <linux/module.h>
>>  #include <linux/gpio.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>>  #include <linux/of_platform.h>
>>  
>>  #include <mach/hardware.h>
>> @@ -86,9 +88,16 @@ static void __init ek_add_device_nand(void)
>>  	at91_add_device_nand(&ek_nand_data);
>>  }
>>  
>> +static const struct of_device_id irq_of_match[] __initconst = {
>> +
>> +	{ .compatible = "atmel,at91rm9200-aic", .data = at91_aic_of_init },
>> +	{ .compatible = "atmel,at91rm9200-gpio", .data = at91_gpio_of_irq_setup },
>> +	{ /*sentinel*/ }
>> +};
>> +
>>  static void __init at91_dt_init_irq(void)
>>  {
>> -	at91_init_irq_default();
>> +	of_irq_init(irq_of_match);
>>  }
>>  
>>  static void __init at91_dt_device_init(void)
>> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
>> index 78cd605..c5d16e5 100644
>> --- a/arch/arm/mach-at91/generic.h
>> +++ b/arch/arm/mach-at91/generic.h
>> @@ -9,6 +9,7 @@
>>   */
>>  
>>  #include <linux/clkdev.h>
>> +#include <linux/of.h>
>>  
>>   /* Map io */
>>  extern void __init at91_map_io(void);
>> @@ -25,6 +26,9 @@ extern void __init at91_init_irq_default(void);
>>  extern void __init at91_init_interrupts(unsigned int priority[]);
>>  extern void __init at91x40_init_interrupts(unsigned int priority[]);
>>  extern void __init at91_aic_init(unsigned int priority[]);
>> +extern int  __init at91_aic_of_init(struct device_node *node,
>> +				    struct device_node *parent);
>> +
>>  
>>   /* Timer */
>>  struct sys_timer;
>> @@ -75,5 +79,7 @@ struct at91_gpio_bank {
>>  };
>>  extern void __init at91_gpio_init(struct at91_gpio_bank *, int nr_banks);
>>  extern void __init at91_gpio_irq_setup(void);
>> +extern int  __init at91_gpio_of_irq_setup(struct device_node *node,
>> +					  struct device_node *parent);
>>  
>>  extern int at91_extern_irq;
>> diff --git a/arch/arm/mach-at91/gpio.c b/arch/arm/mach-at91/gpio.c
>> index fbcb47e..64395a5 100644
>> --- a/arch/arm/mach-at91/gpio.c
>> +++ b/arch/arm/mach-at91/gpio.c
>> @@ -27,6 +27,7 @@
>>  #include <linux/irqdomain.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_irq.h>
>> +#include <linux/of_gpio.h>
>>  
>>  #include <mach/hardware.h>
>>  #include <mach/at91_pio.h>
>> @@ -37,6 +38,7 @@ struct at91_gpio_chip {
>>  	struct gpio_chip	chip;
>>  	struct at91_gpio_chip	*next;		/* Bank sharing same clock */
>>  	int			pioc_hwirq;	/* PIO bank interrupt identifier on AIC */
>> +	int			pioc_virq;	/* PIO bank Linux virtual interrupt */
>>  	int			pioc_idx;	/* PIO bank index */
>>  	void __iomem		*regbase;	/* PIO bank virtual address */
>>  	struct clk		*clock;		/* associated clock */
>> @@ -295,7 +297,7 @@ static int gpio_irq_set_wake(struct irq_data *d, unsigned state)
>>  	else
>>  		wakeups[bank] &= ~mask;
>>  
>> -	irq_set_irq_wake(gpio_chip[bank].pioc_hwirq, state);
>> +	irq_set_irq_wake(at91_gpio->pioc_virq, state);
>>  
>>  	return 0;
>>  }
>> @@ -397,12 +399,12 @@ static struct irq_chip gpio_irqchip = {
>>  
>>  static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>>  {
>> -	unsigned	virq;
>>  	struct irq_data *idata = irq_desc_get_irq_data(desc);
>>  	struct irq_chip *chip = irq_data_get_irq_chip(idata);
>>  	struct at91_gpio_chip *at91_gpio = irq_data_get_irq_chip_data(idata);
>>  	void __iomem	*pio = at91_gpio->regbase;
>> -	u32		isr;
>> +	unsigned long	isr;
>> +	int		n;
>>  
>>  	/* temporarily mask (level sensitive) parent IRQ */
>>  	chip->irq_ack(idata);
>> @@ -420,13 +422,10 @@ static void gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>>  			continue;
>>  		}
>>  
>> -		virq = gpio_to_irq(at91_gpio->chip.base);
>> -
>> -		while (isr) {
>> -			if (isr & 1)
>> -				generic_handle_irq(virq);
>> -			virq++;
>> -			isr >>= 1;
>> +		n = find_first_bit(&isr, BITS_PER_LONG);
>> +		while (n < BITS_PER_LONG) {
>> +			generic_handle_irq(at91_gpiolib_to_irq(&at91_gpio->chip, n));
>> +			n = find_next_bit(&isr, BITS_PER_LONG, n + 1);
>>  		}
>>  	}
>>  	chip->irq_unmask(idata);
>> @@ -496,23 +495,91 @@ postcore_initcall(at91_gpio_debugfs_init);
>>  /*--------------------------------------------------------------------------*/
>>  
>>  /*
>> + * This lock class tells lockdep that GPIO irqs are in a different
>> + * category than their parents, so it won't report false recursion.
>> + */
>> +static struct lock_class_key gpio_lock_class;
>> +
>> +#if defined(CONFIG_OF)
>> +static int at91_gpio_irq_map(struct irq_domain *h, unsigned int virq,
>> +							irq_hw_number_t hw)
>> +{
>> +	struct at91_gpio_chip	*at91_gpio = h->host_data;
>> +
>> +	irq_set_lockdep_class(virq, &gpio_lock_class);
>> +
>> +	/*
>> +	 * Can use the "simple" and not "edge" handler since it's
>> +	 * shorter, and the AIC handles interrupts sanely.
>> +	 */
>> +	irq_set_chip_and_handler(virq, &gpio_irqchip,
>> +				 handle_simple_irq);
>> +	set_irq_flags(virq, IRQF_VALID);
>> +	irq_set_chip_data(virq, at91_gpio);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct irq_domain_ops at91_gpio_ops = {
>> +	.map	= at91_gpio_irq_map,
>> +	.xlate	= irq_domain_xlate_twocell,
>> +};
>> +
>> +int __init at91_gpio_of_irq_setup(struct device_node *node,
>> +				     struct device_node *parent)
>> +{
>> +	struct at91_gpio_chip	*prev = NULL;
>> +	int			alias_idx = of_alias_get_id(node, "gpio");
>> +	struct at91_gpio_chip	*at91_gpio = &gpio_chip[alias_idx];
>> +
>> +	/* Disable irqs of this PIO controller */
>> +	__raw_writel(~0, at91_gpio->regbase + PIO_IDR);
>> +
>> +	/* Setup irq domain */
>> +	at91_gpio->domain = irq_domain_add_linear(node, at91_gpio->chip.ngpio,
>> +						&at91_gpio_ops, at91_gpio);
>> +	if (!at91_gpio->domain)
>> +		panic("at91_gpio.%d: couldn't allocate irq domain (DT).\n",
>> +			at91_gpio->pioc_idx);
>> +
>> +	/* Setup chained handler */
>> +	if (at91_gpio->pioc_idx)
>> +		prev = &gpio_chip[at91_gpio->pioc_idx - 1];
>> +
>> +	/* The toplevel handler handles one bank of GPIOs, except
>> +	 * on some SoC it can handles up to three...
>> +	 * We only set up the handler for the first of the list.
>> +	 */
>> +	if (prev && prev->next == at91_gpio)
>> +		return 0;
>> +
>> +	at91_gpio->pioc_virq = irq_create_mapping(irq_find_host(parent),
>> +							at91_gpio->pioc_hwirq);
>> +	irq_set_chip_data(at91_gpio->pioc_virq, at91_gpio);
>> +	irq_set_chained_handler(at91_gpio->pioc_virq, gpio_irq_handler);
>> +
>> +	return 0;
>> +}
>> +#else
>> +int __init at91_gpio_of_irq_setup(struct device_node *node,
>> +				     struct device_node *parent)
>> +{
>> +	return -EINVAL;
>> +}
>> +#endif
>> +
>> +/*
>>   * irqdomain initialization: pile up irqdomains on top of AIC range
>>   */
>>  static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
>>  {
>>  	int irq_base;
>> -#if defined(CONFIG_OF)
>> -	struct device_node *of_node = at91_gpio->chip.of_node;
>> -#else
>> -	struct device_node *of_node = NULL;
>> -#endif
>>  
>>  	irq_base = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
>>  	if (irq_base < 0)
>>  		panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
>>  			at91_gpio->pioc_idx, irq_base);
>> -	at91_gpio->domain = irq_domain_add_legacy(of_node,
>> -						  at91_gpio->chip.ngpio,
>> +	at91_gpio->domain = irq_domain_add_legacy(NULL, at91_gpio->chip.ngpio,
>>  						  irq_base, 0,
>>  						  &irq_domain_simple_ops, NULL);
>>  	if (!at91_gpio->domain)
>> @@ -521,12 +588,6 @@ static void __init at91_gpio_irqdomain(struct at91_gpio_chip *at91_gpio)
>>  }
>>  
>>  /*
>> - * This lock class tells lockdep that GPIO irqs are in a different
>> - * category than their parents, so it won't report false recursion.
>> - */
>> -static struct lock_class_key gpio_lock_class;
>> -
>> -/*
>>   * Called from the processor-specific init to enable GPIO interrupt support.
>>   */
>>  void __init at91_gpio_irq_setup(void)
>> @@ -538,8 +599,7 @@ void __init at91_gpio_irq_setup(void)
>>  	for (pioc = 0, this = gpio_chip, prev = NULL;
>>  			pioc++ < gpio_banks;
>>  			prev = this, this++) {
>> -		unsigned	pioc_hwirq = this->pioc_hwirq;
>> -		int		offset;
>> +		int offset;
>>  
>>  		__raw_writel(~0, this->regbase + PIO_IDR);
>>  
>> @@ -547,7 +607,7 @@ void __init at91_gpio_irq_setup(void)
>>  		at91_gpio_irqdomain(this);
>>  
>>  		for (offset = 0; offset < this->chip.ngpio; offset++) {
>> -			unsigned int virq = irq_find_mapping(this->domain, offset);
>> +			unsigned int virq = irq_create_mapping(this->domain, offset);
> 
> This should get done by irq_domain_add_legacy.

Yes, I revert this to irq_find_mapping().


>>  			irq_set_lockdep_class(virq, &gpio_lock_class);
>>  
>>  			/*
>> @@ -569,8 +629,9 @@ void __init at91_gpio_irq_setup(void)
>>  		if (prev && prev->next == this)
>>  			continue;
>>  
>> -		irq_set_chip_data(pioc_hwirq, this);
>> -		irq_set_chained_handler(pioc_hwirq, gpio_irq_handler);
>> +		this->pioc_virq = irq_create_mapping(NULL, this->pioc_hwirq);
>> +		irq_set_chip_data(this->pioc_virq, this);
>> +		irq_set_chained_handler(this->pioc_virq, gpio_irq_handler);
>>  	}
>>  	pr_info("AT91: %d gpio irqs in %d banks\n", gpio_irqnbr, gpio_banks);
>>  }
>> @@ -648,7 +709,12 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
>>  static int at91_gpiolib_to_irq(struct gpio_chip *chip, unsigned offset)
>>  {
>>  	struct at91_gpio_chip *at91_gpio = to_at91_gpio_chip(chip);
>> -	int virq = irq_find_mapping(at91_gpio->domain, offset);
>> +	int virq;
>> +
>> +	if (offset < chip->ngpio)
>> +		virq = irq_create_mapping(at91_gpio->domain, offset);
>> +	else
>> +		virq = -ENXIO;
>>  
>>  	dev_dbg(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
>>  				chip->label, offset + chip->base, virq);
>> diff --git a/arch/arm/mach-at91/irq.c b/arch/arm/mach-at91/irq.c
>> index 46682fa..2e4ee09 100644
>> --- a/arch/arm/mach-at91/irq.c
>> +++ b/arch/arm/mach-at91/irq.c
>> @@ -135,27 +135,72 @@ static struct irq_chip at91_aic_chip = {
>>  	.irq_set_wake	= at91_aic_set_wake,
>>  };
>>  
>> +static void __init at91_aic_hw_init(unsigned int spu_vector)
>> +{
>> +	int i;
>> +
>> +	/*
>> +	 * Perform 8 End Of Interrupt Command to make sure AIC
>> +	 * will not Lock out nIRQ
>> +	 */
>> +	for (i = 0; i < 8; i++)
>> +		at91_aic_write(AT91_AIC_EOICR, 0);
>> +
>> +	/*
>> +	 * Spurious Interrupt ID in Spurious Vector Register.
>> +	 * When there is no current interrupt, the IRQ Vector Register
>> +	 * reads the value stored in AIC_SPU
>> +	 */
>> +	at91_aic_write(AT91_AIC_SPU, spu_vector);
>> +
>> +	/* No debugging in AIC: Debug (Protect) Control Register */
>> +	at91_aic_write(AT91_AIC_DCR, 0);
>> +
>> +	/* Disable and clear all interrupts initially */
>> +	at91_aic_write(AT91_AIC_IDCR, 0xFFFFFFFF);
>> +	at91_aic_write(AT91_AIC_ICCR, 0xFFFFFFFF);
>> +}
>> +
>> +
>> +
> 
> Remove extra blank lines.

Sure, done.

>>  #if defined(CONFIG_OF)
>> -static int __init __at91_aic_of_init(struct device_node *node,
>> -				     struct device_node *parent)
>> +static int at91_aic_irq_map(struct irq_domain *h, unsigned int virq,
>> +							irq_hw_number_t hw)
>>  {
>> -	at91_aic_base = of_iomap(node, 0);
>> -	at91_aic_np = node;
>> +	/* Put virq number in Source Vector Register */
>> +	at91_aic_write(AT91_AIC_SVR(hw), virq);
>> +
>> +	/* Active Low interrupt, without priority */
>> +	at91_aic_write(AT91_AIC_SMR(hw), AT91_AIC_SRCTYPE_LOW);
>> +
>> +	irq_set_chip_and_handler(virq, &at91_aic_chip, handle_level_irq);
>> +	set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
>>  
>>  	return 0;
>>  }
>>  
>> -static const struct of_device_id aic_ids[] __initconst = {
>> -	{ .compatible = "atmel,at91rm9200-aic", .data = __at91_aic_of_init },
>> -	{ /*sentinel*/ }
>> +static struct irq_domain_ops at91_aic_irq_ops = {
>> +	.map	= at91_aic_irq_map,
>> +	.xlate	= irq_domain_xlate_twocell,
>>  };
>>  
>> -static void __init at91_aic_of_init(void)
>> +int __init at91_aic_of_init(struct device_node *node,
>> +				     struct device_node *parent)
>>  {
>> -	of_irq_init(aic_ids);
>> +	at91_aic_base = of_iomap(node, 0);
>> +	at91_aic_np = node;
>> +
>> +	at91_aic_domain = irq_domain_add_linear(at91_aic_np, NR_AIC_IRQS,
>> +						&at91_aic_irq_ops, NULL);
>> +	if (!at91_aic_domain)
>> +		panic("Unable to add AIC irq domain (DT)\n");
>> +
>> +	irq_set_default_host(at91_aic_domain);
> 
> Is this really necessary? I don't think it should be.

Well, it allows me to retreive the IRQ controller domain from the gpio
driver.
Cf. at91_gpio_irq_setup() with the call to
irq_create_mapping(NULL, xxx) just above.

Is there a better way to do this?
(it seems that irq_find_host() is dedicated to the device tree enabled
configuration).

>> +	at91_aic_hw_init(NR_AIC_IRQS);
>> +
>> +	return 0;
>>  }
>> -#else
>> -static void __init at91_aic_of_init(void) {}
>>  #endif
>>  
>>  /*
>> @@ -166,11 +211,7 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>>  	unsigned int i;
>>  	int irq_base;
>>  
>> -	if (of_have_populated_dt())
>> -		at91_aic_of_init();
>> -	else
>> -		at91_aic_base = ioremap(AT91_AIC, 512);
>> -
>> +	at91_aic_base = ioremap(AT91_AIC, 512);
>>  	if (!at91_aic_base)
>>  		panic("Unable to ioremap AIC registers\n");
>>  
>> @@ -187,6 +228,8 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>>  	if (!at91_aic_domain)
>>  		panic("Unable to add AIC irq domain\n");
>>  
>> +	irq_set_default_host(at91_aic_domain);
>> +
>>  	/*
>>  	 * The IVR is used by macro get_irqnr_and_base to read and verify.
>>  	 * The irq number is NR_AIC_IRQS when a spurious interrupt has occurred.
>> @@ -199,22 +242,7 @@ void __init at91_aic_init(unsigned int priority[NR_AIC_IRQS])
>>  
>>  		irq_set_chip_and_handler(i, &at91_aic_chip, handle_level_irq);
>>  		set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
>> -
>> -		/* Perform 8 End Of Interrupt Command to make sure AIC will not Lock out nIRQ */
>> -		if (i < 8)
>> -			at91_aic_write(AT91_AIC_EOICR, 0);
>>  	}
>>  
>> -	/*
>> -	 * Spurious Interrupt ID in Spurious Vector Register is NR_AIC_IRQS
>> -	 * When there is no current interrupt, the IRQ Vector Register reads the value stored in AIC_SPU
>> -	 */
>> -	at91_aic_write(AT91_AIC_SPU, NR_AIC_IRQS);
>> -
>> -	/* No debugging in AIC: Debug (Protect) Control Register */
>> -	at91_aic_write(AT91_AIC_DCR, 0);
>> -
>> -	/* Disable and clear all interrupts initially */
>> -	at91_aic_write(AT91_AIC_IDCR, 0xFFFFFFFF);
>> -	at91_aic_write(AT91_AIC_ICCR, 0xFFFFFFFF);
>> +	at91_aic_hw_init(NR_AIC_IRQS);
>>  }


Best regards,
-- 
Nicolas Ferre