ARM: at91: irqdomain and device tree for AIC and GPIO

Add an irqdomain for the AIC interrupt controller. The device tree support is mapping the registers and is using the irq_domain_simple_ops 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 <[email protected]>

---
This patch should go on top of Jamie's patch:
"irqdomain: export irq_domain_simple_ops for !CONFIG_OF"
https://lkml.org/lkml/2011/12/1/109

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                 |   16 ++--
 arch/arm/boot/dts/at91sam9g45.dtsi                 |   14 ++--
 arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
 arch/arm/mach-at91/irq.c                           |   91 +++++++++++++++-----
 6 files changed, 126 insertions(+), 37 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt

Documentation/devicetree/bindings/arm/atmel-aic.txt new file +38 -0

new file mode 100644
index 0000000..ade2761
@@ -0,0 +1,38 @@
+* Advanced Interrupt Controller (AIC)
+
+Required properties:
+- compatible: Should be "atmel,<chip>-dma"
+- 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 at 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 at ffffec00 {
+		compatible = "atmel,at91sam9g45-dma";
+		reg = <0xffffec00 0x200>;
+		interrupts = <21 4>;
+	};

arch/arm/Kconfig +1 -0

index e084b7e..ad13928 100644
@@ -318,6 +318,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 and AT91CAP9 processors.

arch/arm/boot/dts/at91sam9g20.dtsi +8 -8

index aeef042..0782f80 100644
@@ -47,7 +47,7 @@
 			ranges;
 
 			aic: interrupt-controller at fffff000 {
-				#interrupt-cells = <1>;
+				#interrupt-cells = <2>;
 				compatible = "atmel,at91rm9200-aic";
 				interrupt-controller;
 				interrupt-parent;

@@ -57,14 +57,14 @@
 			dbgu: serial at fffff200 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffff200 0x200>;
-				interrupts = <1>;
+				interrupts = <1 4>;
 				status = "disabled";
 			};
 
 			usart0: serial at 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 at 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 at 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 at 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 at 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 at fffd8000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffd8000 0x200>;
-				interrupts = <25>;
+				interrupts = <25 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";

arch/arm/boot/dts/at91sam9g45.dtsi +7 -7

index db6a452..e89b1d7 100644
@@ -46,7 +46,7 @@
 			ranges;
 
 			aic: interrupt-controller at fffff000 {
-				#interrupt-cells = <1>;
+				#interrupt-cells = <2>;
 				compatible = "atmel,at91rm9200-aic";
 				interrupt-controller;
 				interrupt-parent;

@@ -56,20 +56,20 @@
 			dma: dma-controller at ffffec00 {
 				compatible = "atmel,at91sam9g45-dma";
 				reg = <0xffffec00 0x200>;
-				interrupts = <21>;
+				interrupts = <21 4>;
 			};
 
 			dbgu: serial at ffffee00 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xffffee00 0x200>;
-				interrupts = <1>;
+				interrupts = <1 4>;
 				status = "disabled";
 			};
 
 			usart0: serial at 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 at 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 at 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 at fff98000 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfff98000 0x200>;
-				interrupts = <10>;
+				interrupts = <10 4>;
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
 				status = "disabled";

arch/arm/mach-at91/include/mach/irqs.h +2 -1

index ac8b7df..e3ee0fc 100644
@@ -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

arch/arm/mach-at91/irq.c +4 -0

index be6b639..3e3d521 100644
@@ -24,6 +24,10 @@
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/types.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>

arch/arm/mach-at91/irq.c +7 -7

 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 << aic_irq(d));
 }
 
 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 << aic_irq(d));
 }
 
 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 +73,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 ((aic_irq(d) == AT91_ID_FIQ) || is_extern_irq(aic_irq(d)))		/* 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 ((aic_irq(d) == AT91_ID_FIQ) || is_extern_irq(aic_irq(d)))		/* only supported on external interrupts */
 			srctype = AT91_AIC_SRCTYPE_FALLING;
 		else
 			return -EINVAL;

@@ -78,8 +88,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(aic_irq(d))) & ~AT91_AIC_SRCTYPE;
+	at91_aic_write(AT91_AIC_SMR(aic_irq(d)), smr | srctype);
 	return 0;
 }
 

arch/arm/mach-at91/irq.c +32 -4

 		return -EINVAL;
 
 	if (value)
-		wakeups |= (1 << d->irq);
+		wakeups |= (1 << aic_irq(d));
 	else
-		wakeups &= ~(1 << d->irq);
+		wakeups &= ~(1 << aic_irq(d));
 
 	return 0;
 }

@@ -127,36 +137,75 @@ 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_domain.of_node = of_node_get(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 hwirq, irq;
 
-	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");
+

arch/arm/mach-at91/irq.c +8 -8

 		/* Put irq number in Source Vector Register: */
-		at91_aic_write(AT91_AIC_SVR(i), i);
+		at91_aic_write(AT91_AIC_SVR(hwirq), irq);
 		/* Active Low interrupt, with the specified priority */
-		at91_aic_write(AT91_AIC_SMR(i), AT91_AIC_SRCTYPE_LOW | priority[i]);
+		at91_aic_write(AT91_AIC_SMR(hwirq), AT91_AIC_SRCTYPE_LOW | priority[hwirq]);
 
-		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);
+		irq_set_chip_and_handler(irq, &at91_aic_chip, handle_level_irq);
+		set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
 	}
 
+	/* 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 is NR_AIC_IRQS
 	 * When there is no current interrupt, the IRQ Vector Register reads the value stored in AIC_SPU

Signature · 1 line

1.7.5.4

Quoted 16 lines

arch/arm/mach-at91/irq.c +4 -0

@@ -24,6 +24,10 @@
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/types.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 +38,28 @@
 #include <asm/mach/map.h>
 

arch/arm/mach-at91/irq.c line 46

Quoted 7 lines

arch/arm/mach-at91/irq.c +6 -0

 void __iomem *at91_aic_base;
+static struct irq_domain at91_aic_domain;
+
+static inline unsigned int aic_irq(struct irq_data *d)
+{
+	return d->hwirq;
+}

This is a useless macro. Drop it.

arch/arm/mach-at91/irq.c static u32 backups; · line 103

Quoted 6 lines

arch/arm/mach-at91/irq.c +1 -1

@@ -90,13 +100,13 @@ static u32 backups;
 
 static int at91_aic_set_wake(struct irq_data *d, unsigned value)
 {
-	if (unlikely(d->irq >= 32))
+	if (unlikely(aic_irq(d) >= at91_aic_domain.nr_irq))

I don't think you need to derefernce the irq_domain.nr_irq here. Just keep the test for >= 32.

Quoted 47 lines

arch/arm/mach-at91/irq.c +36 -2

+#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_domain.of_node = of_node_get(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 hwirq, irq;
 
-	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 */
+	at91_aic_domain.nr_irq = NR_AIC_IRQS;
+	at91_aic_domain.irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ,
+						at91_aic_domain.nr_irq, 0);
+	if (IS_ERR_VALUE(at91_aic_domain.irq_base)) {
+		WARN(1, "Cannot allocate irq_descs, assuming pre-allocated\n");

arch/arm/mach-at91/irq.c static struct irq_chip at91_aic_chip = { · line 188

Quoted 4 lines

arch/arm/mach-at91/irq.c +4 -0

+		at91_aic_domain.irq_base = AIC_BASE_IRQ;
+	}
+	at91_aic_domain.ops = &irq_domain_simple_ops;
+	irq_domain_add(&at91_aic_domain);

Nothing special is being done with the irq domain. Please use irq_domain_add_simple(). It will make things easier for the next round of irq_domain patches that I'm getting ready to post.

arch/arm/mach-at91/irq.c static struct irq_chip at91_aic_chip = { · line 195

Quoted 7 lines

arch/arm/mach-at91/irq.c +2 -1

 	/*
 	 * 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++) {
+	irq_domain_for_each_irq((&at91_aic_domain), hwirq, irq) {
+

Same here; the driver is written for a fixed number of irqs (NR_AIC_IRQs). You don't need to reference the irq_domain for that.

Otherwise;

Acked-by Grant Likely <[email protected]>

Hi Grant,

Thanks for your review.

Some answer following...

On 01/04/2012 08:40 PM, Grant Likely :

[..]

Quoted 16 lines

Quoted 16 lines

arch/arm/mach-at91/irq.c +4 -0

@@ -24,6 +24,10 @@
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <linux/types.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 +38,28 @@
 #include <asm/mach/map.h>
 

Quoted 9 lines

arch/arm/mach-at91/irq.c line 46

Quoted 7 lines

arch/arm/mach-at91/irq.c +6 -0

 void __iomem *at91_aic_base;
+static struct irq_domain at91_aic_domain;
+
+static inline unsigned int aic_irq(struct irq_data *d)
+{
+	return d->hwirq;
+}

This is a useless macro. Drop it.

Ok. That was overkill ;-)

[..]

Quoted 8 lines

arch/arm/mach-at91/irq.c static u32 backups; · line 103

Quoted 6 lines

arch/arm/mach-at91/irq.c +1 -1

@@ -90,13 +100,13 @@ static u32 backups;
 
 static int at91_aic_set_wake(struct irq_data *d, unsigned value)
 {
-	if (unlikely(d->irq >= 32))
+	if (unlikely(aic_irq(d) >= at91_aic_domain.nr_irq))

I don't think you need to derefernce the irq_domain.nr_irq here. Just keep the test for >= 32.

Well I would like to keep it referring to this value as we may increase the number of irq handled by this controller in the future...

[..]

Quoted 47 lines

Quoted 47 lines

arch/arm/mach-at91/irq.c +36 -2

+#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_domain.of_node = of_node_get(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 hwirq, irq;
 
-	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 */
+	at91_aic_domain.nr_irq = NR_AIC_IRQS;
+	at91_aic_domain.irq_base = irq_alloc_descs(-1, AIC_BASE_IRQ,
+						at91_aic_domain.nr_irq, 0);
+	if (IS_ERR_VALUE(at91_aic_domain.irq_base)) {
+		WARN(1, "Cannot allocate irq_descs, assuming pre-allocated\n");

Quoted 8 lines

arch/arm/mach-at91/irq.c static struct irq_chip at91_aic_chip = { · line 188

Quoted 4 lines

arch/arm/mach-at91/irq.c +4 -0

+		at91_aic_domain.irq_base = AIC_BASE_IRQ;
+	}
+	at91_aic_domain.ops = &irq_domain_simple_ops;
+	irq_domain_add(&at91_aic_domain);

Nothing special is being done with the irq domain. Please use irq_domain_add_simple(). It will make things easier for the next round of irq_domain patches that I'm getting ready to post.

Fair enough, but I have one question: how do I specify "nr_irq" in the domain created by irq_domain_add_simple()? Maybe it is not needed?

Signed-off-by Nicolas Ferre <[email protected]>

Acked-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

---
 arch/arm/mach-at91/gpio.c |   34 +++++++++++++++++++++++++++++-----
 1 files changed, 29 insertions(+), 5 deletions(-)

arch/arm/mach-at91/gpio.c +29 -5

index 74d6783..7ffb893 100644
@@ -20,6 +20,7 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/io.h>
+#include <linux/irqdomain.h>
 
 #include <mach/hardware.h>
 #include <mach/at91_pio.h>

@@ -32,6 +33,7 @@ struct at91_gpio_chip {
 	int			id;		/* ID of register bank */
 	void __iomem		*regbase;	/* Base of register bank */
 	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)

@@ -483,6 +485,24 @@ 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)
+{
+	struct irq_domain	*gpio_irq_d = &at91_gpio->domain;
+	int			ret;
+
+	ret = irq_alloc_descs(-1, 0, at91_gpio->chip.ngpio, 0);
+	if (ret < 0)
+		panic("at91_gpio.%d: error %d: couldn't allocate IRQ numbers.\n",
+			at91_gpio->id, ret);
+	gpio_irq_d->irq_base = ret;
+	gpio_irq_d->nr_irq = at91_gpio->chip.ngpio;
+	gpio_irq_d->ops = &irq_domain_simple_ops;
+	irq_domain_add(gpio_irq_d);
+}
+
+/*
  * This lock class tells lockdep that GPIO irqs are in a different
  * category than their parents, so it won't report false recursion.
  */

@@ -493,19 +513,22 @@ 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			irq = 0;
 	struct at91_gpio_chip	*this, *prev;
 
 	for (pioc = 0, this = gpio_chip, prev = NULL;
 			pioc++ < gpio_banks;
 			prev = this, this++) {
 		unsigned	id = this->id;
-		unsigned	i;
+		int		hwirq;
 
 		__raw_writel(~0, this->regbase + PIO_IDR);
 
-		for (i = 0, irq = gpio_to_irq(this->chip.base); i < 32;
-		     i++, irq++) {
+		/* setup irq domain for this GPIO controller */
+		at91_gpio_irqdomain(this);
+
+		irq_domain_for_each_irq((&this->domain), hwirq, irq) {
 			irq_set_lockdep_class(irq, &gpio_lock_class);
 
 			/*

@@ -527,7 +550,8 @@ void __init at91_gpio_irq_setup(void)
 		irq_set_chip_data(id, this);
 		irq_set_chained_handler(id, 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",
+		irq - irq_domain_to_irq(&gpio_chip[0].domain, 0), gpio_banks);
 }
 
 /* gpiolib support */

Signature · 1 line

1.7.5.4

From: Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Signed-off-by Nicolas Ferre <[email protected]>

---

arch/arm/mach-at91/gpio.c +22 -0

index 7ffb893..edb453a 100644
@@ -21,6 +21,7 @@
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/irqdomain.h>
+#include <linux/of_address.h>
 
 #include <mach/hardware.h>
 #include <mach/at91_pio.h>

@@ -624,40 +625,122 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	}
 }
 
+#ifdef CONFIG_OF_GPIO
+static void __init of_at91_gpio_init_one(struct device_node *np)
+{
+	int alias_id;
+	struct at91_gpio_chip *at91_gpio;
+	const unsigned int *intspec;
+
+	if (!np)
+		return;
+
+	alias_id = of_alias_get_id(np, "gpio");
+	if (alias_id >= MAX_GPIO_BANKS) {
+		pr_err("at91_gpio, failed alias id(%d) > MAX_GPIO_BANKS(%d), ignoring.\n",
+								alias_id, MAX_GPIO_BANKS);
+		return;
+	}
+
+	at91_gpio = &gpio_chip[alias_id];
+	at91_gpio->chip.base = alias_id * at91_gpio->chip.ngpio;
+
+	at91_gpio->regbase = of_iomap(np, 0);

arch/arm/mach-at91/gpio.c +8 -0

+
+	at91_gpio->chip.of_node = np;
+	gpio_banks = max(gpio_banks, alias_id + 1);
+}
+
+static int __init of_at91_gpio_init(void)
+{
+	struct device_node *np = NULL;

arch/arm/mach-at91/gpio.c +21 -0

+
+	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 i, u32 regbase, int id)
+{
+	struct at91_gpio_chip *at91_gpio = &gpio_chip[i];
+
+	at91_gpio->chip.base = i * at91_gpio->chip.ngpio;
+	at91_gpio->id = id;
+
+	at91_gpio->regbase = ioremap(regbase, 512);
+	if (!at91_gpio->regbase) {
+		pr_err("at91_gpio.%d, failed to map registers, ignoring.\n", i);
+		return;

arch/arm/mach-at91/gpio.c +14 -22

+	gpio_banks = max(gpio_banks, i + 1);
+}
+
 /*
  * 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->id = data[i].id;
-		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);
-
-		/* AT91SAM9263_ID_PIOCDE groups PIOC, PIOD, PIOE */
+		/*
+		 * GPIO controller are grouped on some SoC:
+		 * PIOC, PIOD and PIOE can share the same IRQ line
+		 */
 		if (last && last->id == at91_gpio->id)
 			last->next = at91_gpio;
 		last = at91_gpio;

Signature · 1 line

1.7.5.4

Hi Nicolas, Jean-Christophe,

This looks pretty good to me, but a couple of minor comments inline.

Jamie

Quoted 5 lines · Nicolas Ferre

From: Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Signed-off-by Nicolas Ferre <[email protected]>

---

[...]

Quoted 37 lines

arch/arm/mach-at91/gpio.c +22 -0

index 7ffb893..edb453a 100644
@@ -21,6 +21,7 @@
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/irqdomain.h>
+#include <linux/of_address.h>
 
 #include <mach/hardware.h>
 #include <mach/at91_pio.h>

@@ -624,40 +625,122 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	}
 }
 
+#ifdef CONFIG_OF_GPIO
+static void __init of_at91_gpio_init_one(struct device_node *np)
+{
+	int alias_id;
+	struct at91_gpio_chip *at91_gpio;
+	const unsigned int *intspec;
+
+	if (!np)
+		return;
+
+	alias_id = of_alias_get_id(np, "gpio");
+	if (alias_id >= MAX_GPIO_BANKS) {
+		pr_err("at91_gpio, failed alias id(%d) > MAX_GPIO_BANKS(%d), ignoring.\n",
+								alias_id, MAX_GPIO_BANKS);
+		return;
+	}
+
+	at91_gpio = &gpio_chip[alias_id];
+	at91_gpio->chip.base = alias_id * at91_gpio->chip.ngpio;
+
+	at91_gpio->regbase = of_iomap(np, 0);

arch/arm/mach-at91/gpio.c static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip) · line 658

Quoted 10 lines

arch/arm/mach-at91/gpio.c +10 -0

+	if (!at91_gpio->regbase) {
+		pr_err("at91_gpio.%d, failed to map registers, ignoring.\n",
+								alias_id);
+		return;
+	}
+
+	/* Get the interrupts property */
+	intspec = of_get_property(np, "interrupts", NULL);
+	BUG_ON(!intspec);
+	at91_gpio->id = be32_to_cpup(intspec);

Use of_property_read_u32()? Also, BUG_ON() seems a bit harsh, better to print a warning and return an error.

arch/arm/mach-at91/gpio.c static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip) · line 664

Quoted 6 lines

arch/arm/mach-at91/gpio.c +6 -0

+
+	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",
+								alias_id);
+		return;

Perhaps unmap the regs here?

arch/arm/mach-at91/gpio.c static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip) · line 668

Quoted 4 lines

arch/arm/mach-at91/gpio.c +4 -0

+	}
+
+	/* enable PIO controller's clock */
+	clk_enable(at91_gpio->clock);

Missing return value check.

Quoted 8 lines

arch/arm/mach-at91/gpio.c +8 -0

+
+	at91_gpio->chip.of_node = np;
+	gpio_banks = max(gpio_banks, alias_id + 1);
+}
+
+static int __init of_at91_gpio_init(void)
+{
+	struct device_node *np = NULL;

arch/arm/mach-at91/gpio.c static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip) · line 686

Quoted 10 lines

arch/arm/mach-at91/gpio.c +10 -0

+
+	/*
+	 * This isn't ideal, but it gets things hooked up until this
+	 * driver is converted into a platform_device
+	 */
+	do {
+		np = of_find_compatible_node(np, NULL, "atmel,at91rm9200-gpio");
+
+		of_at91_gpio_init_one(np);
+	} while (np);

for_each_compatible_node9)?

Quoted 21 lines

arch/arm/mach-at91/gpio.c +21 -0

+
+	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 i, u32 regbase, int id)
+{
+	struct at91_gpio_chip *at91_gpio = &gpio_chip[i];
+
+	at91_gpio->chip.base = i * at91_gpio->chip.ngpio;
+	at91_gpio->id = id;
+
+	at91_gpio->regbase = ioremap(regbase, 512);
+	if (!at91_gpio->regbase) {
+		pr_err("at91_gpio.%d, failed to map registers, ignoring.\n", i);
+		return;

arch/arm/mach-at91/gpio.c static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip) · line 717

Quoted 10 lines

arch/arm/mach-at91/gpio.c +10 -0

+	}
+
+	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);
+		return;
+	}
+
+	/* enable PIO controller's clock */
+	clk_enable(at91_gpio->clock);

Missing return value check?

Quoted 44 lines

arch/arm/mach-at91/gpio.c +10 -22

+	gpio_banks = max(gpio_banks, i + 1);
+}
+
 /*
  * 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->id = data[i].id;
-		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);
-
-		/* AT91SAM9263_ID_PIOCDE groups PIOC, PIOD, PIOE */

arch/arm/mach-at91/gpio.c static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip) · line 746

Quoted 8 lines

arch/arm/mach-at91/gpio.c +4 -0

+		/*
+		 * GPIO controller are grouped on some SoC:
+		 * PIOC, PIOD and PIOE can share the same IRQ line
+		 */
 		if (last && last->id == at91_gpio->id)
 			last->next = at91_gpio;
 		last = at91_gpio;

From: Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

[nicolas.ferre at atmel.com: modify DT initialization]

Signed-off-by Nicolas Ferre <[email protected]>

---
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_at91.txt         |   20 +++
 arch/arm/boot/dts/at91sam9g20.dtsi                 |   28 ++++
 arch/arm/boot/dts/at91sam9g45.dtsi                 |   45 ++++++
 arch/arm/mach-at91/gpio.c                          |  144 +++++++++++++++++---
 4 files changed, 215 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_at91.txt

Documentation/devicetree/bindings/gpio/gpio_at91.txt new file +20 -0

new file mode 100644
index 0000000..a7bcaec
@@ -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 at fffff200 {
+		compatible = "atmel,at91rm9200-gpio";
+		reg = <0xfffff200 0x100>;
+		interrupts = <2 4>;
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
+

arch/arm/boot/dts/at91sam9g20.dtsi +28 -0

index 0782f80..ea942b5 100644
@@ -23,6 +23,9 @@
 		serial4 = &usart3;
 		serial5 = &usart4;
 		serial6 = &usart5;
+		gpio0 = &pioA;
+		gpio1 = &pioB;
+		gpio2 = &pioC;
 	};
 	cpus {
 		cpu at 0 {

@@ -54,6 +57,31 @@
 				reg = <0xfffff000 0x200>;
 			};
 
+
+			pioA: gpio at fffff400 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff400 0x100>;
+				interrupts = <2 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+
+			pioB: gpio at fffff600 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff600 0x100>;
+				interrupts = <3 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+
+			pioC: gpio at fffff800 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff800 0x100>;
+				interrupts = <4 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+
 			dbgu: serial at fffff200 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xfffff200 0x200>;

arch/arm/boot/dts/at91sam9g45.dtsi +45 -0

index e89b1d7..ebc9617 100644
@@ -22,6 +22,11 @@
 		serial2 = &usart1;
 		serial3 = &usart2;
 		serial4 = &usart3;
+		gpio0 = &pioA;
+		gpio1 = &pioB;
+		gpio2 = &pioC;
+		gpio3 = &pioD;
+		gpio4 = &pioE;
 	};
 	cpus {
 		cpu at 0 {

@@ -59,6 +64,46 @@
 				interrupts = <21 4>;
 			};
 
+			pioA: gpio at fffff200 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff200 0x100>;
+				interrupts = <2 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+
+			pioB: gpio at fffff400 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff400 0x100>;
+				interrupts = <3 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+
+			pioC: gpio at fffff600 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff600 0x100>;
+				interrupts = <4 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+
+			pioD: gpio at fffff800 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffff800 0x100>;
+				interrupts = <5 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+
+			pioE: gpio at fffffa00 {
+				compatible = "atmel,at91rm9200-gpio";
+				reg = <0xfffffa00 0x100>;
+				interrupts = <5 4>;
+				#gpio-cells = <2>;
+				gpio-controller;
+			};
+
 			dbgu: serial at ffffee00 {
 				compatible = "atmel,at91sam9260-usart";
 				reg = <0xffffee00 0x200>;

arch/arm/mach-at91/gpio.c +122 -22

index 7ffb893..4e1cab1 100644
@@ -21,6 +21,7 @@
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/irqdomain.h>
+#include <linux/of_address.h>
 
 #include <mach/hardware.h>
 #include <mach/at91_pio.h>

@@ -624,40 +625,139 @@ static void at91_gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	}
 }
 
+#ifdef CONFIG_OF_GPIO
+static void __init of_at91_gpio_init_one(struct device_node *np)
+{
+	int alias_id;
+	struct at91_gpio_chip *at91_gpio;
+
+	if (!np)
+		return;
+
+	alias_id = of_alias_get_id(np, "gpio");
+	if (alias_id >= MAX_GPIO_BANKS) {
+		pr_err("at91_gpio, failed alias id(%d) > MAX_GPIO_BANKS(%d), ignoring.\n",
+						alias_id, MAX_GPIO_BANKS);
+		return;
+	}
+
+	at91_gpio = &gpio_chip[alias_id];
+	at91_gpio->chip.base = alias_id * 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_id);
+		return;
+	}
+
+	/* Get the interrupts property */
+	if (of_property_read_u32(np, "interrupts", &at91_gpio->id)) {
+		pr_err("at91_gpio.%d, failed to get interrupts property, ignoring.\n",
+								alias_id);
+		goto ioremap_err;
+	}
+
+	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",
+								alias_id);
+		goto ioremap_err;
+	}
+
+	/* enable PIO controller's clock */
+	if(clk_enable(at91_gpio->clock)) {
+		pr_err("at91_gpio.%d, failed to enable clock, ignoring.\n",
+								alias_id);
+		goto clk_err;
+	}
+
+	at91_gpio->chip.of_node = np;
+	gpio_banks = max(gpio_banks, alias_id + 1);
+	return;
+
+clk_err:
+	clk_put(at91_gpio->clock);
+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 i, u32 regbase, int id)
+{
+	struct at91_gpio_chip *at91_gpio = &gpio_chip[i];
+
+	at91_gpio->chip.base = i * at91_gpio->chip.ngpio;
+	at91_gpio->id = id;
+
+	at91_gpio->regbase = ioremap(regbase, 512);
+	if (!at91_gpio->regbase) {
+		pr_err("at91_gpio.%d, failed to map registers, ignoring.\n", i);
+		return;
+	}
+
+	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);
+		goto ioremap_err;
+	}
+
+	if(clk_enable(at91_gpio->clock)) {
+		pr_err("at91_gpio.%d, failed to enable clock, ignoring.\n", i);
+		goto clk_err;
+	}
+
+	gpio_banks = max(gpio_banks, i + 1);
+	return;
+
+clk_err:
+	clk_put(at91_gpio->clock);
+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->id = data[i].id;
-		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);
-
-		/* AT91SAM9263_ID_PIOCDE groups PIOC, PIOD, PIOE */
+		/*
+		 * GPIO controller are grouped on some SoC:
+		 * PIOC, PIOD and PIOE can share the same IRQ line
+		 */
 		if (last && last->id == at91_gpio->id)
 			last->next = at91_gpio;
 		last = at91_gpio;

Signature · 6 lines

1.7.5.4

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

From: Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Signed-off-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Signed-off-by Nicolas Ferre <[email protected]>

Acked-by Grant Likely <[email protected]>

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

Replace the gpio_to_irq() macro by a plain gpiolib .to_irq() handler. The irq_to_gpio() macro is removed. A local replacement is created to fill the need of the gpio driver, internally.

Those calls are using the irqdomain to translate hardware to Linux IRQ numbers.

Signed-off-by Nicolas Ferre <[email protected]>

---
 arch/arm/mach-at91/gpio.c              |   23 +++++++++++++++++++++++
 arch/arm/mach-at91/include/mach/gpio.h |   12 ------------
 2 files changed, 23 insertions(+), 12 deletions(-)

arch/arm/mach-at91/gpio.c +23 -0

index edb453a..c390f71 100644
@@ -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>

@@ -46,6 +47,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)			\
 	{								\

@@ -57,6 +59,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,			\
 		},							\
 	}

@@ -86,6 +89,16 @@ static inline unsigned pin_to_mask(unsigned pin)
 }
 
 
+/*
+ * As gpio IRQs are stacked without holes, we can determine
+ * the gpio form an irq number comparing it with the first IRQ of first
+ * GPIO/IRQ domain.
+ */
+static inline unsigned irq_to_gpio(unsigned irq)
+{
+	return irq - irq_domain_to_irq(&gpio_chip[0].domain, 0);
+}
+
 /*--------------------------------------------------------------------------*/
 
 /* Not all hardware capabilities are exposed through these calls; they

@@ -625,6 +638,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 retirq = irq_domain_to_irq(&at91_gpio->domain, offset);
+
+	dev_dbg(chip->dev, "request IRQ for GPIO %d, return %d\n", offset,
+		retirq);
+	return retirq;
+}
+
 #ifdef CONFIG_OF_GPIO
 static void __init of_at91_gpio_init_one(struct device_node *np)
 {

arch/arm/mach-at91/include/mach/gpio.h +0 -12

index e3fd225..7cf009b 100644
@@ -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

Signature · 1 line

1.7.5.4

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

Signed-off-by Nicolas Ferre <[email protected]>

---
 arch/arm/mach-at91/gpio.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

arch/arm/mach-at91/gpio.c +6 -7

index c390f71..8ed0bc9 100644
@@ -49,7 +49,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,			\

@@ -58,18 +58,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;

Signature · 1 line

1.7.5.4

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

Signed-off-by Nicolas Ferre <[email protected]>

---
 arch/arm/mach-at91/board-dt.c |   15 +--------------
 1 files changed, 1 insertions(+), 14 deletions(-)

arch/arm/mach-at91/board-dt.c +1 -14

index bb6b434..119eda6 100644
@@ -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,17 +86,6 @@ 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();
-}
-
 static void __init at91_dt_device_init(void)
 {
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);

@@ -118,7 +105,7 @@ DT_MACHINE_START(at91sam_dt, "Atmel AT91SAM (Device Tree)")
 	.timer		= &at91sam926x_timer,
 	.map_io		= at91_map_io,
 	.init_early	= ek_init_early,
-	.init_irq	= at91_dt_init_irq,
+	.init_irq	= at91_init_irq_default,
 	.init_machine	= at91_dt_device_init,
 	.dt_compat	= at91_dt_board_compat,
 MACHINE_END

Signature · 1 line

1.7.5.4

Hi,

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

The AIC part has already been sent some time ago but has been reworked to address Rob's comments. This reworked patch is marked with a "v4" tag.

The series can go on top of Jamie's patch
"irqdomain: export irq_domain_simple_ops for !CONFIG_OF" and has been tested on top of 'v3.2-rc5' + 'arm-soc/at91/ioremap' and 'arm-soc/at91/gpio'

Jean-Christophe PLAGNIOL-VILLARD (1):
ARM: at91/gpio: add DT support

Nicolas Ferre (5):
ARM: at91/aic: add irq domain and device tree support
ARM: at91/gpio: add irqdomain to gpio interrupts
ARM: at91/gpio: add .to_irq gpio_chip handler and rework irq_to_gpio
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_at91.txt         |   20 ++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/at91sam9g20.dtsi                 |   44 ++++-
 arch/arm/boot/dts/at91sam9g45.dtsi                 |   59 +++++-
 arch/arm/mach-at91/board-dt.c                      |   15 +--
 arch/arm/mach-at91/gpio.c                          |  197 ++++++++++++++++----
 arch/arm/mach-at91/include/mach/gpio.h             |   12 --
 arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
 arch/arm/mach-at91/irq.c                           |   91 +++++++--
 10 files changed, 383 insertions(+), 97 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_at91.txt

Nicolas,

Quoted 24 lines · Nicolas Ferre

Hi,

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

The AIC part has already been sent some time ago but has been reworked to address Rob's comments. This reworked patch is marked with a "v4" tag.

The series can go on top of Jamie's patch
"irqdomain: export irq_domain_simple_ops for !CONFIG_OF" and has been tested on top of 'v3.2-rc5' + 'arm-soc/at91/ioremap' and 'arm-soc/at91/gpio'

Jean-Christophe PLAGNIOL-VILLARD (1):
ARM: at91/gpio: add DT support

Nicolas Ferre (5):
ARM: at91/aic: add irq domain and device tree support
ARM: at91/gpio: add irqdomain to gpio interrupts
ARM: at91/gpio: add .to_irq gpio_chip handler and rework irq_to_gpio
ARM: at91/gpio: remove the static specification of gpio_chip.base
ARM: at91/board-dt: remove AIC irq domain from board file

Quoted 13 lines

 .../devicetree/bindings/arm/atmel-aic.txt          |   38 ++++
 .../devicetree/bindings/gpio/gpio_at91.txt         |   20 ++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/boot/dts/at91sam9g20.dtsi                 |   44 ++++-
 arch/arm/boot/dts/at91sam9g45.dtsi                 |   59 +++++-
 arch/arm/mach-at91/board-dt.c                      |   15 +--
 arch/arm/mach-at91/gpio.c                          |  197 ++++++++++++++++----
 arch/arm/mach-at91/include/mach/gpio.h             |   12 --
 arch/arm/mach-at91/include/mach/irqs.h             |    3 +-
 arch/arm/mach-at91/irq.c                           |   91 +++++++--
 10 files changed, 383 insertions(+), 97 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/atmel-aic.txt
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_at91.txt

What about using generic irq chip as I previously mentioned? It looks to me like both the AIC and gpio controller could use it. I've added irq domain support to generic irq chip, so it should simplify your changes. The latest branch is here:

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