Patchwork Handling of IRQ in MPC8xx GPIO

login
register
mail settings
Submitter LEROY Christophe
Date Feb. 21, 2013, 4:32 p.m.
Message ID <201302211632.r1LGWaRr025445@localhost.localdomain>
Download mbox | patch
Permalink /patch/222377/
State Changes Requested
Delegated to: Kumar Gala
Headers show

Comments

LEROY Christophe - Feb. 21, 2013, 4:32 p.m.
This patch allows the use IRQ to notify the change of GPIO status on the MPC8xx
CPM IO ports. This then allows to associate IRQs to GPIOs in the Device Tree. Ex:
	CPM1_PIO_C: gpio-controller@960 {
		#gpio-cells = <2>;
		compatible = "fsl,cpm1-pario-bank-c";
		reg = <0x960 0x10>;
		interrupts = <255 255 255 255 1 2 6 9 10 11 14 15 23 24 26 31>;
		interrupt-parent = <&CPM_PIC>;
		gpio-controller;
	};

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Scott Wood - June 4, 2013, 9:59 p.m.
On Thu, Feb 21, 2013 at 06:32:36AM -0000, LEROY Christophe wrote:
> This patch allows the use IRQ to notify the change of GPIO status on the MPC8xx
> CPM IO ports. This then allows to associate IRQs to GPIOs in the Device Tree. Ex:
> 	CPM1_PIO_C: gpio-controller@960 {
> 		#gpio-cells = <2>;
> 		compatible = "fsl,cpm1-pario-bank-c";
> 		reg = <0x960 0x10>;
> 		interrupts = <255 255 255 255 1 2 6 9 10 11 14 15 23 24 26 31>;
> 		interrupt-parent = <&CPM_PIC>;
> 		gpio-controller;
> 	};
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
[snip]
> @@ -581,6 +588,30 @@
>  	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
>  }
>  
> +static int __cpm1_gpio16_to_irq(struct of_mm_gpio_chip *mm_gc,
> +		unsigned int gpio)
> +{
> +	struct cpm1_gpio16_chip *cpm1_gc = to_cpm1_gpio16_chip(mm_gc);
> +
> +	return cpm1_gc->irq[gpio] ? cpm1_gc->irq[gpio] : -ENXIO;
> +}

If it's an internal function, why not just pass in cpm1_gc?

Or just open-code it, as it's a one-liner and you don't use it anywhere
else.

Or fix the gpio layer to use 0 to mean "no irq" rather than -ENXIO... :-)

> diff -ur linux-3.7.9/kernel/irq/irqdomain.c linux/kernel/irq/irqdomain.c
> --- linux-3.7.9/kernel/irq/irqdomain.c	2013-02-17 19:53:32.000000000 +0100
> +++ linux/kernel/irq/irqdomain.c	2012-12-13 19:52:38.000000000 +0100
> @@ -763,7 +763,8 @@
>  	BUG_ON(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR);
>  
>  	/* Check revmap bounds; complain if exceeded */
> -	if (WARN_ON(hwirq >= domain->revmap_data.linear.size))
> +	/* 255 is a trick to allow UNDEF value in DTS */
> +	if (hwirq == 255 || WARN_ON(hwirq >= domain->revmap_data.linear.size))
>  		return 0;

NACK.  Besides the hackishness of it, 255 is valid for some interrupt
controllers.

If you need for a way to leave holes in an "interrupts" property, propose
something non-hacky on devicetree-discuss@lists.ozlabs.org.

Or, do what some other devices do, and have a different property that
indicates which pins are connected, and only include those in the
"interrupts" property.  See
Documentation/devicetree/bindings/powerpc/fsl/mpic-msgr.txt and
mpic-msgr-receive-mask as an example.

-Scott

Patch

diff -ur linux-3.7.9/arch/powerpc/include/asm/cpm1.h linux/arch/powerpc/include/asm/cpm1.h
--- linux-3.7.9/arch/powerpc/include/asm/cpm1.h	2013-02-17 19:53:32.000000000 +0100
+++ linux/arch/powerpc/include/asm/cpm1.h	2012-11-03 03:18:35.000000000 +0100
@@ -560,6 +560,8 @@ 
 #define CPM_PIN_SECONDARY 2
 #define CPM_PIN_GPIO      4
 #define CPM_PIN_OPENDRAIN 8
+#define CPM_PIN_FALLEDGE  16
+#define CPM_PIN_ANYEDGE   0
 
 enum cpm_port {
 	CPM_PORTA,
diff -ur linux-3.7.9/arch/powerpc/sysdev/cpm1.c linux/arch/powerpc/sysdev/cpm1.c
--- linux-3.7.9/arch/powerpc/sysdev/cpm1.c	2013-02-17 19:53:32.000000000 +0100
+++ linux/arch/powerpc/sysdev/cpm1.c	2013-02-21 15:52:51.000000000 +0100
@@ -375,6 +375,10 @@ 
 			setbits16(&iop->odr_sor, pin);
 		else
 			clrbits16(&iop->odr_sor, pin);
+		if (flags & CPM_PIN_FALLEDGE)
+			setbits16(&iop->intr, pin);
+		else
+			clrbits16(&iop->intr, pin);
 	}
 }
 
@@ -526,6 +530,9 @@ 
 
 	/* shadowed data register to clear/set bits safely */
 	u16 cpdata;
+
+	/* IRQ associated with Pins when relevant */
+	int irq[16];
 };
 
 static inline struct cpm1_gpio16_chip *
@@ -581,6 +588,30 @@ 
 	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
 }
 
+static int __cpm1_gpio16_to_irq(struct of_mm_gpio_chip *mm_gc,
+		unsigned int gpio)
+{
+	struct cpm1_gpio16_chip *cpm1_gc = to_cpm1_gpio16_chip(mm_gc);
+
+	return cpm1_gc->irq[gpio] ? cpm1_gc->irq[gpio] : -ENXIO;
+}
+
+static int cpm1_gpio16_to_irq(struct gpio_chip *gc, unsigned int gpio)
+{
+	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+	struct cpm1_gpio16_chip *cpm1_gc = to_cpm1_gpio16_chip(mm_gc);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&cpm1_gc->lock, flags);
+
+	ret = __cpm1_gpio16_to_irq(mm_gc, gpio);
+
+	spin_unlock_irqrestore(&cpm1_gc->lock, flags);
+
+	return ret;
+}
+
 static int cpm1_gpio16_dir_out(struct gpio_chip *gc, unsigned int gpio, int val)
 {
 	struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
@@ -621,6 +652,7 @@ 
 	struct cpm1_gpio16_chip *cpm1_gc;
 	struct of_mm_gpio_chip *mm_gc;
 	struct gpio_chip *gc;
+	int i;
 
 	cpm1_gc = kzalloc(sizeof(*cpm1_gc), GFP_KERNEL);
 	if (!cpm1_gc)
@@ -628,6 +660,9 @@ 
 
 	spin_lock_init(&cpm1_gc->lock);
 
+	for (i = 0; i < 16; i++)
+		cpm1_gc->irq[i] = irq_of_parse_and_map(np, i);
+
 	mm_gc = &cpm1_gc->mm_gc;
 	gc = &mm_gc->gc;
 
@@ -637,6 +672,7 @@ 
 	gc->direction_output = cpm1_gpio16_dir_out;
 	gc->get = cpm1_gpio16_get;
 	gc->set = cpm1_gpio16_set;
+	gc->to_irq = cpm1_gpio16_to_irq;
 
 	return of_mm_gpiochip_add(np, mm_gc);
 }
diff -ur linux-3.7.9/kernel/irq/irqdomain.c linux/kernel/irq/irqdomain.c
--- linux-3.7.9/kernel/irq/irqdomain.c	2013-02-17 19:53:32.000000000 +0100
+++ linux/kernel/irq/irqdomain.c	2012-12-13 19:52:38.000000000 +0100
@@ -763,7 +763,8 @@ 
 	BUG_ON(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR);
 
 	/* Check revmap bounds; complain if exceeded */
-	if (WARN_ON(hwirq >= domain->revmap_data.linear.size))
+	/* 255 is a trick to allow UNDEF value in DTS */
+	if (hwirq == 255 || WARN_ON(hwirq >= domain->revmap_data.linear.size))
 		return 0;
 
 	return domain->revmap_data.linear.revmap[hwirq];