Re: [PATCH 3/8] MIPS: Octeon: Add irq_create_of_mapping() and GPIOinterrupts.

From: David Daney
Date: Fri Nov 11 2011 - 15:45:37 EST


On 11/11/2011 12:41 PM, Rob Herring wrote:
David,

On 11/11/2011 01:49 PM, David Daney wrote:
On 11/11/2011 08:08 AM, Rob Herring wrote:
On 11/10/2011 08:21 PM, ddaney.cavm@xxxxxxxxx wrote:
From: David Daney<david.daney@xxxxxxxxxx>

This is needed for Octeon to use the Device Tree.

The GPIO interrupts are configured based on Device Tree properties

Signed-off-by: David Daney<david.daney@xxxxxxxxxx>
---
arch/mips/cavium-octeon/octeon-irq.c | 188
+++++++++++++++++++++++++++++++++-
1 files changed, 187 insertions(+), 1 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-irq.c
b/arch/mips/cavium-octeon/octeon-irq.c
index ffd4ae6..bb10546 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -8,11 +8,14 @@

#include<linux/interrupt.h>
#include<linux/bitops.h>
+#include<linux/module.h>
#include<linux/percpu.h>
+#include<linux/of_irq.h>
#include<linux/irq.h>
#include<linux/smp.h>

#include<asm/octeon/octeon.h>
+#include<asm/octeon/cvmx-gpio-defs.h>

static DEFINE_RAW_SPINLOCK(octeon_irq_ciu0_lock);
static DEFINE_RAW_SPINLOCK(octeon_irq_ciu1_lock);
@@ -58,6 +61,95 @@ static void __init octeon_irq_set_ciu_mapping(int
irq, int line, int bit,
octeon_irq_ciu_to_irq[line][bit] = irq;
}

+static unsigned int octeon_irq_gpio_mapping(struct device_node
*controller,
+ const u32 *intspec,
+ unsigned int intsize)
+{
+ struct of_irq oirq;
+ int i;
+ unsigned int irq = 0;
+ unsigned int type;
+ unsigned int ciu = 0, bit = 0;
+ unsigned int pin = be32_to_cpup(intspec);
+ unsigned int trigger = be32_to_cpup(intspec + 1);
+ bool set_edge_handler = false;
+
+ if (pin>= 16)
+ goto err;
+ i = of_irq_map_one(controller, 0,&oirq);
+ if (i)
+ goto err;
+ if (oirq.size != 2)
+ goto err_put;
+
+ ciu = oirq.specifier[0];
+ bit = oirq.specifier[1] + pin;
+
+ if (ciu>= 8 || bit>= 64)
+ goto err_put;
+
+ irq = octeon_irq_ciu_to_irq[ciu][bit];
+ if (!irq)
+ goto err_put;
+
+ switch (trigger& 0xf) {
+ case 1:
+ type = IRQ_TYPE_EDGE_RISING;
+ set_edge_handler = true;
+ break;
+ case 2:
+ type = IRQ_TYPE_EDGE_FALLING;
+ set_edge_handler = true;
+ break;
+ case 4:
+ type = IRQ_TYPE_LEVEL_HIGH;
+ break;
+ case 8:
+ type = IRQ_TYPE_LEVEL_LOW;
+ break;
+ default:
+ pr_err("Error: Invalid irq trigger specification: %x\n",
+ trigger);
+ type = IRQ_TYPE_LEVEL_LOW;
+ break;
+ }
+
+ irq_set_irq_type(irq, type);
+
+ if (set_edge_handler)
+ __irq_set_handler(irq, handle_edge_irq, 0, NULL);
+
+err_put:
+ of_node_put(oirq.controller);
+err:
+ return irq;
+}
+
+/*
+ * irq_create_of_mapping - Hook to resolve OF irq specifier into a
Linux irq#
+ *
+ * Octeon irq maps are a pair of indexes. The first selects either
+ * ciu0 or ciu1, the second is the bit within the ciu register.
+ */
+unsigned int irq_create_of_mapping(struct device_node *controller,
+ const u32 *intspec, unsigned int intsize)
+{
+ unsigned int irq = 0;
+ unsigned int ciu, bit;
+
+ if (of_device_is_compatible(controller, "cavium,octeon-3860-gpio"))
+ return octeon_irq_gpio_mapping(controller, intspec, intsize);
+
+ ciu = be32_to_cpup(intspec);
+ bit = be32_to_cpup(intspec + 1);
+
+ if (ciu< 8&& bit< 64)
+ irq = octeon_irq_ciu_to_irq[ciu][bit];
+
+ return irq;
+}
+EXPORT_SYMBOL_GPL(irq_create_of_mapping);

Have you looked at irq_domains (kernel/irq/irqdomain.c)? That is what
you should be using for your (gpio) interrupt controller and then use
the common irq_create_of_mapping.


Unfortunatly, although a good idea, kernel/irq/irqdomain.c makes a bunch
of assumptions that don't hold for Octeon. We may be able to improve it
so that it flexible enough to suit us.


Here are the problems I see:

1) It is assumed that there is some sort of linear correspondence
between 'hwirq' and 'irq', and that the range of valid values is
contiguous.

That's not true if you implement .to_irq for your domain.

It is true. irq_domain_add() iterates.


2) It is assumed that the concepts of nr_irq, irq_base and hwirq_base
have easy to determin values and you can do iteration over their ranges
by adding indexes to the bases.

That's true for hwirq numbering, but not for Linux irq numbering.
irq_base is only used if you don't provide .to_irq. hwirq_base is just
to allow a linear range that doesn't start with 0.

I think we can fix this by adding iteration helper functions to struct
irq_domain. If these are present, we would just ignore the irq_base,
nr_irq and hwirq_base elements of the structure.
irq_domain_for_each_hwirq() and irq_domain_for_each_irq() would be
modified to use the iteration helper functions.

Expanding irqdomain is certainly the intention and right direction.

I have a patch. It should be ready in a couple of hours.


It seems to me you just need something different than an irq. controller
with a linear range of irq lines. Perhaps sparsely allocated hwirq's?
Can you describe the h/w?

Also, you could have a different domain per irqdesc if you really needed to.


Too ugly.

David Daney
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/