[PATCH 0/2] irq/of: Enchance irq_domain support.

[PATCH 0/2] irq/of: Enchance irq_domain support.

[ddaney.cavm at gmail.com]

From: David Daney <david.daney@cavium.com>

This is the first cut at hooking up my Octeon port to the irq_domain things.

The Octeon specific patches are part of a larger set, and will need to
be applied with that set, the first patch is stand-alone.

The basic problem being solved taken from one of my other e-mails:

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

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


David Daney (2):
  irq/of/ARM: Enhance irq iteration capability of irq_domain code.
  MIPS: Octeon: Add irq_create_of_mapping() and GPIO interrupts.

 arch/arm/common/gic.c                |   32 +++--
 arch/mips/Kconfig                    |    1 +
 arch/mips/cavium-octeon/octeon-irq.c |  279 +++++++++++++++++++++++++++++++++-
 include/linux/irqdomain.h            |   29 +++-
 kernel/irq/irqdomain.c               |   97 +++++++++---
 5 files changed, 390 insertions(+), 48 deletions(-)

-- 
1.7.2.3

[PATCH 1/2] irq/of/ARM: Enhance irq iteration capability of irq_domain code.

[ddaney.cavm at gmail.com]

From: David Daney <david.daney@cavium.com>

Not all irq_domain have linear sequences of irq numbers, so add hooks
for domain specific iteration of the irqs in the domain.

There are two new optional functions in irq_domain_ops that iterate
over the irqs in the domain calling a callback function on each one.
If these are not present, the new helper functions
irq_domain_each_irq() and irq_domain_each_hwirq() iterate using the
existing semantics.

arch/arm/common/gic.c had to be modified to use the new iteration
method as part of the change.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 arch/arm/common/gic.c     |   32 +++++++++------
 include/linux/irqdomain.h |   29 +++++++++----
 kernel/irq/irqdomain.c    |   97 +++++++++++++++++++++++++++++++++------------
 3 files changed, 111 insertions(+), 47 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 0e6ae47..64095c9 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -264,6 +264,24 @@ void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 	irq_set_chained_handler(irq, gic_handle_cascade_irq);
 }
 
+static int __init gic_dist_init_domain_cb(struct irq_domain *domain,
+					  unsigned int irq,
+					  unsigned int hwirq)
+{
+	if (hwirq < 32) {
+		irq_set_percpu_devid(irq);
+		irq_set_chip_and_handler(irq, &gic_chip,
+					 handle_percpu_devid_irq);
+		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
+	} else {
+		irq_set_chip_and_handler(irq, &gic_chip,
+					 handle_fasteoi_irq);
+		set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
+	}
+	irq_set_chip_data(irq, gic);
+	return 0;
+}
+
 static void __init gic_dist_init(struct gic_chip_data *gic)
 {
 	unsigned int i, irq;
@@ -311,19 +329,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
 	/*
 	 * Setup the Linux IRQ subsystem.
 	 */
-	irq_domain_for_each_irq(domain, i, irq) {
-		if (i < 32) {
-			irq_set_percpu_devid(irq);
-			irq_set_chip_and_handler(irq, &gic_chip,
-						 handle_percpu_devid_irq);
-			set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
-		} else {
-			irq_set_chip_and_handler(irq, &gic_chip,
-						 handle_fasteoi_irq);
-			set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
-		}
-		irq_set_chip_data(irq, gic);
-	}
+	irq_domain_each_irq(domain, gic_dist_init_domain_cb);
 
 	writel_relaxed(1, base + GIC_DIST_CTRL);
 }
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 99834e58..f8dceb6 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -27,16 +27,26 @@ struct irq_domain;
  * @to_irq: (optional) given a local hardware irq number, return the linux
  *          irq number.  If to_irq is not implemented, then the irq_domain
  *          will use this translation: irq = (domain->irq_base + hwirq)
+ * @each_irq: (optional) call the cb function for each irq that is a
+ *            member of the domain.
+ * @each_hwirq: (optional) call the cb function for each hwirq that is a
+ *              member of the domain.
  * @dt_translate: Given a device tree node and interrupt specifier, decode
  *                the hardware irq number and linux irq type value.
  */
 struct irq_domain_ops {
-	unsigned int (*to_irq)(struct irq_domain *d, unsigned long hwirq);
+	unsigned int (*to_irq)(struct irq_domain *d, unsigned int hwirq);
+	int (*each_irq)(struct irq_domain *d,
+			int (*cb)(struct irq_domain *d,
+				  unsigned int irq, unsigned int hwirq));
+	int (*each_hwirq)(struct irq_domain *d,
+			  int (*cb)(struct irq_domain *d,
+				    unsigned int hwirq));
 
 #ifdef CONFIG_OF
 	int (*dt_translate)(struct irq_domain *d, struct device_node *node,
 			    const u32 *intspec, unsigned int intsize,
-			    unsigned long *out_hwirq, unsigned int *out_type);
+			    unsigned int *out_hwirq, unsigned int *out_type);
 #endif /* CONFIG_OF */
 };
 
@@ -72,7 +82,7 @@ struct irq_domain {
  * be overridden if the irq_domain implements a .to_irq() hook.
  */
 static inline unsigned int irq_domain_to_irq(struct irq_domain *d,
-					     unsigned long hwirq)
+					     unsigned int hwirq)
 {
 	if (d->ops->to_irq)
 		return d->ops->to_irq(d, hwirq);
@@ -81,13 +91,14 @@ static inline unsigned int irq_domain_to_irq(struct irq_domain *d,
 	return d->irq_base + hwirq - d->hwirq_base;
 }
 
-#define irq_domain_for_each_hwirq(d, hw) \
-	for (hw = d->hwirq_base; hw < d->hwirq_base + d->nr_irq; hw++)
+extern int irq_domain_each_hwirq(struct irq_domain *d,
+				 int (*cb)(struct irq_domain *d,
+					   unsigned int hwirq));
 
-#define irq_domain_for_each_irq(d, hw, irq) \
-	for (hw = d->hwirq_base, irq = irq_domain_to_irq(d, hw); \
-	     hw < d->hwirq_base + d->nr_irq; \
-	     hw++, irq = irq_domain_to_irq(d, hw))
+extern int irq_domain_each_irq(struct irq_domain *d,
+			       int (*cb)(struct irq_domain *d,
+					 unsigned int irq,
+					 unsigned int hwirq));
 
 extern void irq_domain_add(struct irq_domain *domain);
 extern void irq_domain_del(struct irq_domain *domain);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 200ce83..528450e 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -9,6 +9,25 @@
 static LIST_HEAD(irq_domain_list);
 static DEFINE_MUTEX(irq_domain_mutex);
 
+static int irq_domain_add_per_irq(struct irq_domain *domain, unsigned int irq,
+				  unsigned int hwirq)
+{
+	struct irq_data *d = irq_get_irq_data(irq);
+
+	if (!d) {
+		WARN(1, "error: assigning domain to non existant irq_desc");
+		return 1;
+	}
+	if (d->domain) {
+		/* things are broken; just report, don't clean up */
+		WARN(1, "error: irq_desc already assigned to a domain");
+		return 1;
+	}
+	d->domain = domain;
+	d->hwirq = hwirq;
+	return 0;
+}
+
 /**
  * irq_domain_add() - Register an irq_domain
  * @domain: ptr to initialized irq_domain structure
@@ -19,52 +38,77 @@ static DEFINE_MUTEX(irq_domain_mutex);
  */
 void irq_domain_add(struct irq_domain *domain)
 {
-	struct irq_data *d;
-	int hwirq, irq;
-
 	/*
 	 * This assumes that the irq_domain owner has already allocated
 	 * the irq_descs.  This block will be removed when support for dynamic
 	 * allocation of irq_descs is added to irq_domain.
 	 */
-	irq_domain_for_each_irq(domain, hwirq, irq) {
-		d = irq_get_irq_data(irq);
-		if (!d) {
-			WARN(1, "error: assigning domain to non existant irq_desc");
-			return;
-		}
-		if (d->domain) {
-			/* things are broken; just report, don't clean up */
-			WARN(1, "error: irq_desc already assigned to a domain");
-			return;
-		}
-		d->domain = domain;
-		d->hwirq = hwirq;
-	}
+	irq_domain_each_irq(domain, irq_domain_add_per_irq);
 
 	mutex_lock(&irq_domain_mutex);
 	list_add(&domain->list, &irq_domain_list);
 	mutex_unlock(&irq_domain_mutex);
 }
 
+static int irq_domain_del_per_irq(struct irq_domain *domain, unsigned int irq,
+				  unsigned int hwirq)
+{
+	struct irq_data *d = irq_get_irq_data(irq);
+
+	d->domain = NULL;
+	return 0;
+}
+
 /**
  * irq_domain_del() - Unregister an irq_domain
  * @domain: ptr to registered irq_domain.
  */
 void irq_domain_del(struct irq_domain *domain)
 {
-	struct irq_data *d;
-	int hwirq, irq;
-
 	mutex_lock(&irq_domain_mutex);
 	list_del(&domain->list);
 	mutex_unlock(&irq_domain_mutex);
 
 	/* Clear the irq_domain assignments */
-	irq_domain_for_each_irq(domain, hwirq, irq) {
-		d = irq_get_irq_data(irq);
-		d->domain = NULL;
+	irq_domain_each_irq(domain, irq_domain_del_per_irq);
+}
+
+int irq_domain_each_hwirq(struct irq_domain *d,
+			  int (*cb)(struct irq_domain *d, unsigned int hwirq))
+{
+	unsigned int hw;
+	int ret = 0;
+
+	if (d->ops->each_hwirq)
+		return d->ops->each_hwirq(d, cb);
+
+	for (hw = d->hwirq_base; hw < d->hwirq_base + d->nr_irq; hw++) {
+		ret = cb(d, hw);
+		if (ret)
+			break;
+	}
+	return ret;
+}
+
+int irq_domain_each_irq(struct irq_domain *d,
+			int (*cb)(struct irq_domain *d,	unsigned int irq,
+				  unsigned int hwirq))
+{
+	unsigned int hw, irq;
+	int ret = 0;
+
+	if (d->ops->each_irq)
+		return d->ops->each_irq(d, cb);
+
+	for (hw = d->hwirq_base, irq = irq_domain_to_irq(d, hw);
+	     hw < d->hwirq_base + d->nr_irq;
+	     hw++, irq = irq_domain_to_irq(d, hw)) {
+		ret = cb(d, irq, hw);
+		if (ret)
+			break;
 	}
+
+	return 0;
 }
 
 #if defined(CONFIG_OF_IRQ)
@@ -82,13 +126,16 @@ unsigned int irq_create_of_mapping(struct device_node *controller,
 				   const u32 *intspec, unsigned int intsize)
 {
 	struct irq_domain *domain;
-	unsigned long hwirq;
+	unsigned int hwirq;
 	unsigned int irq, type;
 	int rc = -EINVAL;
 
 	/* Find a domain which can translate the irq spec */
 	mutex_lock(&irq_domain_mutex);
 	list_for_each_entry(domain, &irq_domain_list, list) {
+		if (domain->of_node != controller)
+			continue;
+
 		if (!domain->ops->dt_translate)
 			continue;
 		rc = domain->ops->dt_translate(domain, controller,
@@ -129,7 +176,7 @@ EXPORT_SYMBOL_GPL(irq_dispose_mapping);
 int irq_domain_simple_dt_translate(struct irq_domain *d,
 			    struct device_node *controller,
 			    const u32 *intspec, unsigned int intsize,
-			    unsigned long *out_hwirq, unsigned int *out_type)
+			    unsigned int *out_hwirq, unsigned int *out_type)
 {
 	if (d->of_node != controller)
 		return -EINVAL;
-- 
1.7.2.3

[PATCH 2/2] MIPS: Octeon: Add irq_create_of_mapping() and GPIO interrupts.

[ddaney.cavm at gmail.com]

From: David Daney <david.daney@cavium.com>

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@cavium.com>
---
 arch/mips/Kconfig                    |    1 +
 arch/mips/cavium-octeon/octeon-irq.c |  279 +++++++++++++++++++++++++++++++++-
 2 files changed, 279 insertions(+), 1 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index d46f1da..97c2cf2 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1404,6 +1404,7 @@ config CPU_CAVIUM_OCTEON
 	select WEAK_ORDERING
 	select CPU_SUPPORTS_HIGHMEM
 	select CPU_SUPPORTS_HUGEPAGES
+	select IRQ_DOMAIN
 	help
 	  The Cavium Octeon processor is a highly integrated chip containing
 	  many ethernet hardware widgets for networking tasks. The processor
diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
index ffd4ae6..5e08590 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -7,12 +7,16 @@
  */
 
 #include <linux/interrupt.h>
+#include <linux/irqdomain.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 +62,178 @@ static void __init octeon_irq_set_ciu_mapping(int irq, int line, int bit,
 	octeon_irq_ciu_to_irq[line][bit] = irq;
 }
 
+static int octeon_irq_gpio_dt_translate(struct irq_domain *d,
+				       struct device_node *node,
+				       const u32 *intspec,
+				       unsigned int intsize,
+				       unsigned int *out_hwirq,
+				       unsigned int *out_type)
+{
+	unsigned int irq;
+	unsigned int type;
+	unsigned int ciu = 0, bit = 0;
+	unsigned int pin;
+	unsigned int trigger;
+	bool set_edge_handler = false;
+
+	if (intsize < 2)
+		return -EINVAL;
+
+	pin = be32_to_cpup(intspec);
+	if (pin >= 16)
+		return -EINVAL;
+
+	trigger = be32_to_cpup(intspec + 1);
+
+	switch (trigger) {
+	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:
+		WARN(1, "hello");
+		pr_err("Error: (%s) Invalid irq trigger specification: %x\n",
+		       node->name,
+		       trigger);
+		type = IRQ_TYPE_LEVEL_LOW;
+		break;
+	}
+	*out_type = type;
+	*out_hwirq = d->hwirq_base + pin;
+
+	ciu = *out_hwirq >> 6;
+	bit = *out_hwirq & 0x3f;
+
+	irq = octeon_irq_ciu_to_irq[ciu][bit];
+
+	if (set_edge_handler)
+		__irq_set_handler(irq, handle_edge_irq, 0, NULL);
+
+
+	return 0;
+}
+
+/*
+ * octeon_irq_ciu_dt_translate - 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.
+ */
+static int octeon_irq_ciu_dt_translate(struct irq_domain *d,
+				       struct device_node *node,
+				       const u32 *intspec,
+				       unsigned int intsize,
+				       unsigned int *out_hwirq,
+				       unsigned int *out_type)
+{
+	unsigned int ciu, bit;
+
+	ciu = be32_to_cpup(intspec);
+	bit = be32_to_cpup(intspec + 1);
+
+	if (ciu > 1 || bit > 63)
+		return -EINVAL;
+
+	if (octeon_irq_ciu_to_irq[ciu][bit] == 0)
+		return -EINVAL;
+
+	*out_hwirq = (ciu << 6) | bit;
+	*out_type = 0;
+
+	return 0;
+}
+
+static unsigned int octeon_irq_ciu_to_irqf(struct irq_domain *d,
+					   unsigned int hwirq)
+{
+	unsigned int ciu, bit;
+
+	ciu = (hwirq >> 6) & 1;
+	bit = hwirq & 0x3f;
+	return octeon_irq_ciu_to_irq[ciu][bit];
+}
+
+static bool octeon_irq_ciu_in_domain(unsigned int irq)
+{
+	return (irq != 0) &&
+		!(irq >= OCTEON_IRQ_GPIO0 && irq < OCTEON_IRQ_GPIO0 + 16);
+}
+
+static int octeon_irq_ciu_each_irq(struct irq_domain *d,
+				   int (*cb)(struct irq_domain *d,
+					     unsigned int irq,
+					     unsigned int hwirq))
+{
+	int ciu, bit;
+	int ret = 0;
+	unsigned int irq, hwirq;
+
+	for (ciu = 0; ciu <=1; ciu++)
+		for (bit = 0; bit <= 63; bit++) {
+			irq = octeon_irq_ciu_to_irq[ciu][bit];
+			hwirq = (ciu << 6) | bit;
+			if (octeon_irq_ciu_in_domain(irq)) {
+				ret = cb(d, irq, hwirq);
+				if (ret)
+					return ret;
+			}
+		}
+	return 0;
+}
+
+static int octeon_irq_ciu_each_hwirq(struct irq_domain *d,
+				     int (*cb)(struct irq_domain *d,
+					       unsigned int hwirq))
+{
+	int ciu, bit;
+	int ret = 0;
+	unsigned int irq, hwirq;
+
+	for (ciu = 0; ciu <=1; ciu++)
+		for (bit = 0; bit <= 63; bit++) {
+			irq = octeon_irq_ciu_to_irq[ciu][bit];
+			hwirq = (ciu << 6) | bit;
+			if (octeon_irq_ciu_in_domain(irq)) {
+				ret = cb(d, hwirq);
+				if (ret)
+					return ret;
+			}
+		}
+	return 0;
+}
+
+static const struct irq_domain_ops octeon_irq_ciu_domain_ops = {
+	.to_irq = octeon_irq_ciu_to_irqf,
+	.each_irq = octeon_irq_ciu_each_irq,
+	.each_hwirq = octeon_irq_ciu_each_hwirq,
+	.dt_translate = octeon_irq_ciu_dt_translate,
+};
+
+static const struct irq_domain_ops octeon_irq_gpio_domain_ops = {
+	.to_irq = octeon_irq_ciu_to_irqf,
+	.dt_translate = octeon_irq_gpio_dt_translate,
+};
+
+static struct irq_domain octeon_irq_ciu_domain  = {
+	.ops = &octeon_irq_ciu_domain_ops
+};
+
+static struct irq_domain octeon_irq_gpio_domain  = {
+	.irq_base = OCTEON_IRQ_GPIO0,
+	.nr_irq = 16,
+	.ops = &octeon_irq_gpio_domain_ops
+};
+
 static int octeon_coreid_for_cpu(int cpu)
 {
 #ifdef CONFIG_SMP
@@ -505,6 +681,72 @@ static void octeon_irq_ciu_enable_all_v2(struct irq_data *data)
 	}
 }
 
+static void octeon_irq_gpio_setup(struct irq_data *data)
+{
+	union cvmx_gpio_bit_cfgx cfg;
+	int bit = data->irq - OCTEON_IRQ_GPIO0;
+	u32 t = irqd_get_trigger_type(data);
+
+	cfg.u64 = 0;
+	cfg.s.int_en = 1;
+	cfg.s.int_type = (t & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING)) != 0;
+	cfg.s.rx_xor = (t & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING)) != 0;
+
+	/* 1 uS glitch filter*/
+	cfg.s.fil_cnt = 7;
+	cfg.s.fil_sel = 3;
+
+	cvmx_write_csr(CVMX_GPIO_BIT_CFGX(bit), cfg.u64);
+}
+
+static void octeon_irq_ciu_enable_gpio_v2(struct irq_data *data)
+{
+	octeon_irq_gpio_setup(data);
+	octeon_irq_ciu_enable_v2(data);
+}
+
+static void octeon_irq_ciu_enable_gpio(struct irq_data *data)
+{
+	octeon_irq_gpio_setup(data);
+	octeon_irq_ciu_enable(data);
+}
+
+static int octeon_irq_ciu_gpio_set_type(struct irq_data *data, unsigned int t)
+{
+	u32 current_type = irqd_get_trigger_type(data);
+
+	/* If the type has been set, don't change it */
+	if (current_type && current_type != t)
+		return -EINVAL;
+
+	irqd_set_trigger_type(data, t);
+	return IRQ_SET_MASK_OK;
+}
+
+static void octeon_irq_ciu_disable_gpio_v2(struct irq_data *data)
+{
+	int bit = data->irq - OCTEON_IRQ_GPIO0;
+	cvmx_write_csr(CVMX_GPIO_BIT_CFGX(bit), 0);
+
+	octeon_irq_ciu_disable_all_v2(data);
+}
+
+static void octeon_irq_ciu_disable_gpio(struct irq_data *data)
+{
+	int bit = data->irq - OCTEON_IRQ_GPIO0;
+	cvmx_write_csr(CVMX_GPIO_BIT_CFGX(bit), 0);
+
+	octeon_irq_ciu_disable_all(data);
+}
+
+static void octeon_irq_ciu_gpio_ack(struct irq_data *data)
+{
+	int bit = data->irq - OCTEON_IRQ_GPIO0;
+	u64 mask = 1ull << bit;
+
+	cvmx_write_csr(CVMX_GPIO_INT_CLR, mask);
+}
+
 #ifdef CONFIG_SMP
 
 static void octeon_irq_cpu_offline_ciu(struct irq_data *data)
@@ -717,6 +959,31 @@ static struct irq_chip octeon_irq_chip_ciu_mbox = {
 	.flags = IRQCHIP_ONOFFLINE_ENABLED,
 };
 
+static struct irq_chip octeon_irq_chip_ciu_gpio_v2 = {
+	.name = "CIU-GPIO",
+	.irq_enable = octeon_irq_ciu_enable_gpio_v2,
+	.irq_disable = octeon_irq_ciu_disable_gpio_v2,
+	.irq_ack = octeon_irq_ciu_gpio_ack,
+	.irq_mask = octeon_irq_ciu_disable_local_v2,
+	.irq_unmask = octeon_irq_ciu_enable_v2,
+	.irq_set_type = octeon_irq_ciu_gpio_set_type,
+#ifdef CONFIG_SMP
+	.irq_set_affinity = octeon_irq_ciu_set_affinity_v2,
+#endif
+};
+
+static struct irq_chip octeon_irq_chip_ciu_gpio = {
+	.name = "CIU-GPIO",
+	.irq_enable = octeon_irq_ciu_enable_gpio,
+	.irq_disable = octeon_irq_ciu_disable_gpio,
+	.irq_mask = octeon_irq_dummy_mask,
+	.irq_ack = octeon_irq_ciu_gpio_ack,
+	.irq_set_type = octeon_irq_ciu_gpio_set_type,
+#ifdef CONFIG_SMP
+	.irq_set_affinity = octeon_irq_ciu_set_affinity,
+#endif
+};
+
 /*
  * Watchdog interrupts are special.  They are associated with a single
  * core, so we hardwire the affinity to that core.
@@ -890,6 +1157,7 @@ static void __init octeon_irq_init_ciu(void)
 	struct irq_chip *chip_edge;
 	struct irq_chip *chip_mbox;
 	struct irq_chip *chip_wd;
+	struct irq_chip *chip_gpio;
 
 	octeon_irq_init_ciu_percpu();
 	octeon_irq_setup_secondary = octeon_irq_setup_secondary_ciu;
@@ -904,6 +1172,7 @@ static void __init octeon_irq_init_ciu(void)
 		chip_edge = &octeon_irq_chip_ciu_edge_v2;
 		chip_mbox = &octeon_irq_chip_ciu_mbox_v2;
 		chip_wd = &octeon_irq_chip_ciu_wd_v2;
+		chip_gpio = &octeon_irq_chip_ciu_gpio_v2;
 	} else {
 		octeon_irq_ip2 = octeon_irq_ip2_v1;
 		octeon_irq_ip3 = octeon_irq_ip3_v1;
@@ -911,6 +1180,7 @@ static void __init octeon_irq_init_ciu(void)
 		chip_edge = &octeon_irq_chip_ciu_edge;
 		chip_mbox = &octeon_irq_chip_ciu_mbox;
 		chip_wd = &octeon_irq_chip_ciu_wd;
+		chip_gpio = &octeon_irq_chip_ciu_gpio;
 	}
 	octeon_irq_ip4 = octeon_irq_ip4_mask;
 
@@ -921,7 +1191,7 @@ static void __init octeon_irq_init_ciu(void)
 	for (i = 0; i < 16; i++)
 		octeon_irq_set_ciu_mapping(i + OCTEON_IRQ_WORKQ0, 0, i + 0, chip, handle_level_irq);
 	for (i = 0; i < 16; i++)
-		octeon_irq_set_ciu_mapping(i + OCTEON_IRQ_GPIO0, 0, i + 16, chip, handle_level_irq);
+		octeon_irq_set_ciu_mapping(i + OCTEON_IRQ_GPIO0, 0, i + 16, chip_gpio, handle_level_irq);
 
 	octeon_irq_set_ciu_mapping(OCTEON_IRQ_MBOX0, 0, 32, chip_mbox, handle_percpu_irq);
 	octeon_irq_set_ciu_mapping(OCTEON_IRQ_MBOX1, 0, 33, chip_mbox, handle_percpu_irq);
@@ -995,6 +1265,13 @@ static void __init octeon_irq_init_ciu(void)
 	octeon_irq_set_ciu_mapping(OCTEON_IRQ_DFM, 1, 56, chip, handle_level_irq);
 	octeon_irq_set_ciu_mapping(OCTEON_IRQ_RST, 1, 63, chip, handle_level_irq);
 
+	octeon_irq_ciu_domain.of_node = of_find_compatible_node(NULL, NULL, "cavium,octeon-3860-ciu");
+	irq_domain_add(&octeon_irq_ciu_domain);
+
+	octeon_irq_gpio_domain.of_node = of_find_compatible_node(NULL, NULL, "cavium,octeon-3860-gpio");
+	octeon_irq_gpio_domain.hwirq_base = ((0 << 6) | 16);
+	irq_domain_add(&octeon_irq_gpio_domain);
+
 	/* Enable the CIU lines */
 	set_c0_status(STATUSF_IP3 | STATUSF_IP2);
 	clear_c0_status(STATUSF_IP4);
-- 
1.7.2.3

[PATCH 0/2] irq/of: Enchance irq_domain support.

[Rob Herring]

On 11/11/2011 07:50 PM, ddaney.cavm at gmail.com wrote:
> From: David Daney <david.daney@cavium.com>
> 
> This is the first cut at hooking up my Octeon port to the irq_domain things.
> 
> The Octeon specific patches are part of a larger set, and will need to
> be applied with that set, the first patch is stand-alone.
> 
> The basic problem being solved taken from one of my other e-mails:
> 
>    Unfortunately, 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.
> 
>    2) It is assumed that the concepts of nr_irq, irq_base and
>    hwirq_base have easy to determine values and you can do iteration
>    over their ranges by adding indexes to the bases.
> 

I still think this is the wrong approach.

Are the gpio interrupts the source of your problem here? That's how I
read it. You have 16 GPIO irqs directly connected into lines on your
primary interrupt controller which has 128 lines. So for a Linux irq
number, you want to translate to a GPIO hwirq number and/or a CIU hwirq
number. Trying to have 2 hwirq mappings for 1 Linux irq number just
won't work. It seems to me you should use a chained handler here because
you need to process the interrupt at both the primary ctrlr and gpio
ctrlr levels.

Rob

> 
> David Daney (2):
>   irq/of/ARM: Enhance irq iteration capability of irq_domain code.
>   MIPS: Octeon: Add irq_create_of_mapping() and GPIO interrupts.
> 
>  arch/arm/common/gic.c                |   32 +++--
>  arch/mips/Kconfig                    |    1 +
>  arch/mips/cavium-octeon/octeon-irq.c |  279 +++++++++++++++++++++++++++++++++-
>  include/linux/irqdomain.h            |   29 +++-
>  kernel/irq/irqdomain.c               |   97 +++++++++---
>  5 files changed, 390 insertions(+), 48 deletions(-)
> 

[PATCH 0/2] irq/of: Enchance irq_domain support.

[David Daney]

On 11/11/2011 07:55 PM, Rob Herring wrote:
> On 11/11/2011 07:50 PM, ddaney.cavm at gmail.com wrote:
>> From: David Daney<david.daney@cavium.com>
>>
>> This is the first cut at hooking up my Octeon port to the irq_domain things.
>>
>> The Octeon specific patches are part of a larger set, and will need to
>> be applied with that set, the first patch is stand-alone.
>>
>> The basic problem being solved taken from one of my other e-mails:
>>
>>     Unfortunately, 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.
>>
>>     2) It is assumed that the concepts of nr_irq, irq_base and
>>     hwirq_base have easy to determine values and you can do iteration
>>     over their ranges by adding indexes to the bases.
>>
>
> I still think this is the wrong approach.
>
> Are the gpio interrupts the source of your problem here?

No.

> That's how I read it.

Take a look at Patch 2/2, since the GPIO irqs are contiguous over both 
irq and hwirq numbers, I use the existing infrastructure with no 
modifications.

> You have 16 GPIO irqs directly connected into lines on your
> primary interrupt controller which has 128 lines. So for a Linux irq
> number, you want to translate to a GPIO hwirq number and/or a CIU hwirq
> number. Trying to have 2 hwirq mappings for 1 Linux irq number just
> won't work. It seems to me you should use a chained handler here because
> you need to process the interrupt at both the primary ctrlr and gpio
> ctrlr levels.
>

All moot as it is based on the false predicate of GPIO irqs being the 
problem.


The root of the problem are all of the irqs that are not GPIO.  I have:

o irq numbers currently in the range [9..196], with holes for any given 
SOC/Board implementation.  SOCs currently in development will have 
additional irq numbers with even more holes.

o Two different interrupt controllers.  One with 128 lines, the other 
with  512 or more lines, both sparsely populated.  The mapping of hwirq 
to irq is done at boot time based on the hardware the kernel image is 
running on.  Note that the second type of irq controller support is not 
in the kernel.org kernel, but it exists, and I intend on getting support 
for it merged ASAP.

At a minimum the loop in irq_domain_add() where we iterate over a linear 
range of irq numbers is not flexible enough.  You may not like my 
iterator functions in irq_domain_ops, but we need to provide something 
better than the irq_domain_for_each_irq() macro.

David Daney


> Rob
>
>>
>> David Daney (2):
>>    irq/of/ARM: Enhance irq iteration capability of irq_domain code.
>>    MIPS: Octeon: Add irq_create_of_mapping() and GPIO interrupts.
>>
>>   arch/arm/common/gic.c                |   32 +++--
>>   arch/mips/Kconfig                    |    1 +
>>   arch/mips/cavium-octeon/octeon-irq.c |  279 +++++++++++++++++++++++++++++++++-
>>   include/linux/irqdomain.h            |   29 +++-
>>   kernel/irq/irqdomain.c               |   97 +++++++++---
>>   5 files changed, 390 insertions(+), 48 deletions(-)
>>
>
>

[PATCH 0/2] irq/of: Enchance irq_domain support.

[Rob Herring]

On 11/14/2011 11:58 AM, David Daney wrote:
> On 11/11/2011 07:55 PM, Rob Herring wrote:
>> On 11/11/2011 07:50 PM, ddaney.cavm at gmail.com wrote:
>>> From: David Daney<david.daney@cavium.com>
>>>
>>> This is the first cut at hooking up my Octeon port to the irq_domain
>>> things.
>>>
>>> The Octeon specific patches are part of a larger set, and will need to
>>> be applied with that set, the first patch is stand-alone.
>>>
>>> The basic problem being solved taken from one of my other e-mails:
>>>
>>>     Unfortunately, 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.
>>>
>>>     2) It is assumed that the concepts of nr_irq, irq_base and
>>>     hwirq_base have easy to determine values and you can do iteration
>>>     over their ranges by adding indexes to the bases.
>>>
>>
>> I still think this is the wrong approach.
>>
>> Are the gpio interrupts the source of your problem here?
> 
> No.
> 
>> That's how I read it.
> 
> Take a look at Patch 2/2, since the GPIO irqs are contiguous over both
> irq and hwirq numbers, I use the existing infrastructure with no
> modifications.
> 

I did. You are adding GPIO support to the existing support. It would be
nice to separate that from add DT support.

>> You have 16 GPIO irqs directly connected into lines on your
>> primary interrupt controller which has 128 lines. So for a Linux irq
>> number, you want to translate to a GPIO hwirq number and/or a CIU hwirq
>> number. Trying to have 2 hwirq mappings for 1 Linux irq number just
>> won't work. It seems to me you should use a chained handler here because
>> you need to process the interrupt at both the primary ctrlr and gpio
>> ctrlr levels.
>>
> 
> All moot as it is based on the false predicate of GPIO irqs being the
> problem.
> 
Let me rephrase, if you completely ignore GPIO for a minute, what is the
issue. Just that irq_descs are sparsely allocated for the primary
controller. Then the GPIO interrupts are inserted into the middle of
that irq space. Handling sparse irqs is a potentially common problem, so
we should address that in the core irqdomain code.

> 
> The root of the problem are all of the irqs that are not GPIO.  I have:
> 
> o irq numbers currently in the range [9..196], with holes for any given
> SOC/Board implementation.  SOCs currently in development will have
> additional irq numbers with even more holes.
> 
> o Two different interrupt controllers.  One with 128 lines, the other
> with  512 or more lines, both sparsely populated.  The mapping of hwirq
> to irq is done at boot time based on the hardware the kernel image is
> running on.  Note that the second type of irq controller support is not
> in the kernel.org kernel, but it exists, and I intend on getting support
> for it merged ASAP.

To be clear, those are not holes in hwirq's, but many lines don't have
connections so you are not allocating Linux irqs for them. hwirq should
reflect interrupt numbers from the controller's perspective and be
directly usable to index to the correct register and bit mask. There's
no storage associated with a hwirq, so the only cost is iterating over
them which is not done frequently.

There is not a clean separation of the primary interrupt controller's
hwirq numbers and Linux irq numbers in your patch. Then you are
overlaying the GPIO interrupts into the Linux irq space.

> At a minimum the loop in irq_domain_add() where we iterate over a linear
> range of irq numbers is not flexible enough.  You may not like my
> iterator functions in irq_domain_ops, but we need to provide something
> better than the irq_domain_for_each_irq() macro.

I'm just trying to back-up some and understand the problem.

How about if .to_irq returns 0, the loop can just continue and skip over
that hwirq without error.

I don't think .each_hwirq is being used, so you can delete that.

Rob

[PATCH 0/2] irq/of: Enchance irq_domain support.

[David Daney]

On 11/14/2011 02:41 PM, Rob Herring wrote:
> On 11/14/2011 11:58 AM, David Daney wrote:
>> On 11/11/2011 07:55 PM, Rob Herring wrote:
>>> On 11/11/2011 07:50 PM, ddaney.cavm at gmail.com wrote:
>>>> From: David Daney<david.daney@cavium.com>
>>>>
>>>> This is the first cut at hooking up my Octeon port to the irq_domain
>>>> things.
>>>>
>>>> The Octeon specific patches are part of a larger set, and will need to
>>>> be applied with that set, the first patch is stand-alone.
>>>>
>>>> The basic problem being solved taken from one of my other e-mails:
>>>>
>>>>      Unfortunately, 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.
>>>>
>>>>      2) It is assumed that the concepts of nr_irq, irq_base and
>>>>      hwirq_base have easy to determine values and you can do iteration
>>>>      over their ranges by adding indexes to the bases.
>>>>
>>>
>>> I still think this is the wrong approach.
>>>
>>> Are the gpio interrupts the source of your problem here?
>>
>> No.
>>
>>> That's how I read it.
>>
>> Take a look at Patch 2/2, since the GPIO irqs are contiguous over both
>> irq and hwirq numbers, I use the existing infrastructure with no
>> modifications.
>>
>
> I did. You are adding GPIO support to the existing support.

No, I am adding DT support.  For that I need a working 
irq_create_of_mapping() for non-GPIO interrupts.  I am also adding GPIO, 
but that is not the part giving us problems.

You may recall that in an earlier e-mail you asked me to use irq_domain 
support for my irq_create_of_mapping() rather than my previous ad hoc 
approach.  I agree that this is a good idea, and now we are here.

> It would be
> nice to separate that from add DT support.

Granted, the non-GPIO support for irq_create_of_mapping() and GPIO 
support could be separated into two patches, but that is somewhat of an 
orthogonal issue.  Since that part is isolated in arch/mips, Ralf and I 
can discuss doing that.  For now I would like to concentrate on figuring 
out if it is advantageous to adjust the irq_domain core implementation.

>
>>> You have 16 GPIO irqs directly connected into lines on your
>>> primary interrupt controller which has 128 lines. So for a Linux irq
>>> number, you want to translate to a GPIO hwirq number and/or a CIU hwirq
>>> number. Trying to have 2 hwirq mappings for 1 Linux irq number just
>>> won't work. It seems to me you should use a chained handler here because
>>> you need to process the interrupt at both the primary ctrlr and gpio
>>> ctrlr levels.
>>>
>>
>> All moot as it is based on the false predicate of GPIO irqs being the
>> problem.
>>
> Let me rephrase, if you completely ignore GPIO for a minute, what is the
> issue.

Yes, let's do that.  It is clouding the issue.

> Just that irq_descs are sparsely allocated for the primary
> controller. Then the GPIO interrupts are inserted into the middle of
> that irq space. Handling sparse irqs is a potentially common problem, so
> we should address that in the core irqdomain code.

This is precisely the issue, and what patch 1/2 is doing.

My Patch 2/2 was just included to show how I would be using the changes.

>
>>
>> The root of the problem are all of the irqs that are not GPIO.  I have:
>>
>> o irq numbers currently in the range [9..196], with holes for any given
>> SOC/Board implementation.  SOCs currently in development will have
>> additional irq numbers with even more holes.
>>
>> o Two different interrupt controllers.  One with 128 lines, the other
>> with  512 or more lines, both sparsely populated.  The mapping of hwirq
>> to irq is done at boot time based on the hardware the kernel image is
>> running on.  Note that the second type of irq controller support is not
>> in the kernel.org kernel, but it exists, and I intend on getting support
>> for it merged ASAP.
>
> To be clear, those are not holes in hwirq's, but many lines don't have
> connections so you are not allocating Linux irqs for them. hwirq should
> reflect interrupt numbers from the controller's perspective and be
> directly usable to index to the correct register and bit mask. There's
> no storage associated with a hwirq, so the only cost is iterating over
> them which is not done frequently.

Yes, this is essentially correct.

There are two iterators defined:

1) irq_domain_each_hwirq()

2) irq_domain_for_each_irq()

Yet struct irq_domain has only nr_irq, this clearly cannot be used as 
the range of both iterators.  Furthermore I think the concept of a 
linear iteration through irq values is fundamentally broken.

>
> There is not a clean separation of the primary interrupt controller's
> hwirq numbers and Linux irq numbers in your patch.

?? I don't get what you mean.  They are clearly separate concepts, my 
interrupt controller code even goes as far as having a map to translate 
between the two.

> Then you are
> overlaying the GPIO interrupts into the Linux irq space.
>

Indeed I am, but its only effect is to introduce an additional hole 
(among many others) in the irq number space of the primary controller. 
It is only tangentially related to the real issues of this discussion.


>> At a minimum the loop in irq_domain_add() where we iterate over a linear
>> range of irq numbers is not flexible enough.  You may not like my
>> iterator functions in irq_domain_ops, but we need to provide something
>> better than the irq_domain_for_each_irq() macro.
>
> I'm just trying to back-up some and understand the problem.
>
> How about if .to_irq returns 0, the loop can just continue and skip over
> that hwirq without error.
>

Ok, now we are getting somewhere.

However I am not sufficiently clever to design an iteration macro like 
irq_domain_for_each_irq(), that would use the return value of zero from 
.to_irq to determine if we should continue to the next hwirq value 
without entering the body of the loop.

I am left with my callback iterator thing that is in the patch 1/2.

> I don't think .each_hwirq is being used, so you can delete that.

Yes, I noticed that.  There are two options:

1) Remove it as you suggest, along with irq_domain_for_each_hwirq() and 
irq_domain_each_hwirq().

2) Leave it on the grounds that irq_domain_for_each_hwirq() is there for 
a reason.

David Daney