public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] irqdomain cleanup and refactoring
@ 2012-06-16  5:01 Grant Likely
  2012-06-16  5:01 ` [PATCH 01/12] irqdomain: Split disassociating code into separate function Grant Likely
                   ` (12 more replies)
  0 siblings, 13 replies; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel; +Cc: Milton Miller

Hey folks,

This is some of the refactoring that I've been wanting to do for
irqdomains.  Lots of patches here, but the major changes are:
- eliminate the legacy mapping by replacing it with a pre-populated linear map,
- Get rid of the slow-path setup for mapping, and
- to merge the linear and tree mapping into a single domain type.

The last change still needs some investigation and review, I'm not as
confident that this is correct and it hasn't been tested much.

Milton, the last version of these patches that I posted broke one of
your powerpc machines.  Can you please retest?  I've pushed this
series out into my irqdomain/test branch (see below)

For further work, I'd like to find a way to eliminate the nomap revmap
too so I can get rid of the irqdomain type entirely and simplify the
code even further.  I've not applied many brain cells towards doing
this yet though.  This is an area for further research.

Also, Paul tells me that there are some platforms that want to use
something like a linear mapping, but there are large holes in the
ranges of hwirq numbers which is a little wasteful of memory.  Another
area of investigation is to figure out how to be more memory-efficient
here without slowing down the linear revmap path.

g.

The following changes since commit cfaf025112d3856637ff34a767ef785ef5cf2ca9:

  Linux 3.5-rc2 (2012-06-08 18:40:09 -0700)

are available in the git repository at:

  git://git.secretlab.ca/git/linux-2.6 irqdomain/test

for you to fetch changes up to ca2f744b40214645e0274e8681023ee6d2387f9d:

  irqdomain: merge linear and tree reverse mappings. (2012-06-15 22:36:24 -0600)

Grant Likely (13):
      devicetree: add helper inline for retrieving a node's full name
      irqdomain: Remove unnecessary test for IRQ_DOMAIN_MAP_LEGACY
      irqdomain: Split disassociating code into separate function
      irqdomain: Always update revmap when setting up a virq
      irqdomain: Eliminate dedicated radix lookup functions
      irqdomain: Fix irq_create_direct_mapping() to test irq_domain type.
      irqdomain: eliminate slow-path revmap lookups
      irqdomain: Make ops->map hook optional
      irqdomain: Replace LEGACY mapping with LINEAR
      irqdomain: Reserve IRQs for legacy domain
      irqdomain: Add debugging message
      irqdomain: reorganize revmap data.
      irqdomain: merge linear and tree reverse mappings.

Paul Mundt (2):
      irqdomain: Simple NUMA awareness.
      irqdomain: Support for static IRQ mapping and association.

 arch/arm/plat-versatile/fpga-irq.c     |    2 +-
 arch/microblaze/pci/pci-common.c       |    6 +-
 arch/powerpc/kernel/pci-common.c       |    6 +-
 arch/powerpc/kernel/vio.c              |    5 +-
 arch/powerpc/platforms/cell/iommu.c    |    3 +-
 arch/powerpc/platforms/pseries/iommu.c |    2 +-
 arch/powerpc/sysdev/xics/icp-hv.c      |    2 +-
 arch/powerpc/sysdev/xics/icp-native.c  |    2 +-
 arch/powerpc/sysdev/xics/xics-common.c |    3 -
 arch/sparc/kernel/of_device_64.c       |    2 +-
 drivers/of/base.c                      |    2 +-
 drivers/of/irq.c                       |    2 +-
 drivers/pinctrl/pinctrl-nomadik.c      |    4 +-
 include/linux/irqdomain.h              |   55 ++--
 include/linux/of.h                     |   23 +-
 kernel/irq/irqdomain.c                 |  472 +++++++++++++-------------------
 16 files changed, 251 insertions(+), 340 deletions(-)



^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/12] irqdomain: Split disassociating code into separate function
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  5:57   ` Benjamin Herrenschmidt
  2012-06-16  5:01 ` [PATCH 02/12] irqdomain: Always update revmap when setting up a virq Grant Likely
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

This patch moves the irq disassociation code out into a separate
function in preparation to extend irq_setup_virq to handle multiple
irqs and rename it for use by interrupt controller drivers.  The new
function will be used by irq_setup_virq() in its error path.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 kernel/irq/irqdomain.c |   75 ++++++++++++++++++++++++++++++------------------
 1 file changed, 47 insertions(+), 28 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index b1f774c..4161d2a 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -333,6 +333,52 @@ void irq_set_default_host(struct irq_domain *domain)
 }
 EXPORT_SYMBOL_GPL(irq_set_default_host);
 
+static void irq_domain_disassociate_many(struct irq_domain *domain,
+					 unsigned int irq_base, int count)
+{
+	/*
+	 * disassociate in reverse order;
+	 * not strictly necessary, but nice for unwinding
+	 */
+	while (count--) {
+		int irq = irq_base + count;
+		struct irq_data *irq_data = irq_get_irq_data(irq);
+		irq_hw_number_t hwirq = irq_data->hwirq;
+
+		if (WARN_ON(!irq_data || irq_data->domain != domain))
+			continue;
+
+		irq_set_status_flags(irq, IRQ_NOREQUEST);
+
+		/* remove chip and handler */
+		irq_set_chip_and_handler(irq, NULL, NULL);
+
+		/* Make sure it's completed */
+		synchronize_irq(irq);
+
+		/* Tell the PIC about it */
+		if (domain->ops->unmap)
+			domain->ops->unmap(domain, irq);
+		smp_mb();
+
+		irq_data->domain = NULL;
+		irq_data->hwirq = 0;
+
+		/* Clear reverse map */
+		switch(domain->revmap_type) {
+		case IRQ_DOMAIN_MAP_LINEAR:
+			if (hwirq < domain->revmap_data.linear.size)
+				domain->revmap_data.linear.revmap[hwirq] = 0;
+			break;
+		case IRQ_DOMAIN_MAP_TREE:
+			mutex_lock(&revmap_trees_mutex);
+			radix_tree_delete(&domain->revmap_data.tree, hwirq);
+			mutex_unlock(&revmap_trees_mutex);
+			break;
+		}
+	}
+}
+
 static int irq_setup_virq(struct irq_domain *domain, unsigned int virq,
 			    irq_hw_number_t hwirq)
 {
@@ -513,7 +559,6 @@ void irq_dispose_mapping(unsigned int virq)
 {
 	struct irq_data *irq_data = irq_get_irq_data(virq);
 	struct irq_domain *domain;
-	irq_hw_number_t hwirq;
 
 	if (!virq || !irq_data)
 		return;
@@ -526,33 +571,7 @@ void irq_dispose_mapping(unsigned int virq)
 	if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
 		return;
 
-	irq_set_status_flags(virq, IRQ_NOREQUEST);
-
-	/* remove chip and handler */
-	irq_set_chip_and_handler(virq, NULL, NULL);
-
-	/* Make sure it's completed */
-	synchronize_irq(virq);
-
-	/* Tell the PIC about it */
-	if (domain->ops->unmap)
-		domain->ops->unmap(domain, virq);
-	smp_mb();
-
-	/* Clear reverse map */
-	hwirq = irq_data->hwirq;
-	switch(domain->revmap_type) {
-	case IRQ_DOMAIN_MAP_LINEAR:
-		if (hwirq < domain->revmap_data.linear.size)
-			domain->revmap_data.linear.revmap[hwirq] = 0;
-		break;
-	case IRQ_DOMAIN_MAP_TREE:
-		mutex_lock(&revmap_trees_mutex);
-		radix_tree_delete(&domain->revmap_data.tree, hwirq);
-		mutex_unlock(&revmap_trees_mutex);
-		break;
-	}
-
+	irq_domain_disassociate_many(domain, virq, 1);
 	irq_free_desc(virq);
 }
 EXPORT_SYMBOL_GPL(irq_dispose_mapping);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/12] irqdomain: Always update revmap when setting up a virq
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
  2012-06-16  5:01 ` [PATCH 01/12] irqdomain: Split disassociating code into separate function Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  5:57   ` Benjamin Herrenschmidt
  2012-06-16  5:01 ` [PATCH 03/12] irqdomain: Support for static IRQ mapping and association Grant Likely
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

At irq_setup_virq() time all of the data needed to update the reverse
map is available, but the current code ignores it and relies upon the
slow path to insert revmap records.  This patch adds revmap updating
to the setup path so the slow path will no longer be necessary.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 kernel/irq/irqdomain.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 4161d2a..5c36722 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -393,6 +393,16 @@ static int irq_setup_virq(struct irq_domain *domain, unsigned int virq,
 		return -1;
 	}
 
+	switch (domain->revmap_type) {
+	case IRQ_DOMAIN_MAP_LINEAR:
+		if (hwirq < domain->revmap_data.linear.size)
+			domain->revmap_data.linear.revmap[hwirq] = virq;
+		break;
+	case IRQ_DOMAIN_MAP_TREE:
+		irq_radix_revmap_insert(domain, virq, hwirq);
+		break;
+	}
+
 	irq_clear_status_flags(virq, IRQ_NOREQUEST);
 
 	return 0;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/12] irqdomain: Support for static IRQ mapping and association.
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
  2012-06-16  5:01 ` [PATCH 01/12] irqdomain: Split disassociating code into separate function Grant Likely
  2012-06-16  5:01 ` [PATCH 02/12] irqdomain: Always update revmap when setting up a virq Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  5:58   ` Benjamin Herrenschmidt
  2012-06-16  5:01 ` [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions Grant Likely
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Paul Mundt, Grant Likely, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

From: Paul Mundt <lethal@linux-sh.org>

This adds a new strict mapping API for supporting creation of linux IRQs
at existing positions within the domain. The new routines are as follows:

For dynamic allocation and insertion to specified ranges:

	- irq_create_identity_mapping()
	- irq_create_strict_mappings()

These will allocate and associate a range of linux IRQs at the specified
location. This can be used by controllers that have their own static linux IRQ
definitions to map a hwirq range to, as well as for platforms that wish to
establish 1:1 identity mapping between linux and hwirq space.

For insertion to specified ranges by platforms that do their own irq_desc
management:

	- irq_domain_associate()
	- irq_domain_associate_many()

These in turn call back in to the domain's ->map() routine, for further
processing by the platform. Disassociation of IRQs get handled through
irq_dispose_mapping() as normal.

With these in place it should be possible to begin migration of legacy IRQ
domains to linear ones, without requiring special handling for static vs
dynamic IRQ definitions in DT vs non-DT paths. This also makes it possible
for domains with static mappings to adopt whichever tree model best fits
their needs, rather than simply restricting them to linear revmaps.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>
[grant.likely: Reorganized irq_domain_associate{,_many} to have all logic in one place]
[grant.likely: Add error checking for unallocated irq_descs at associate time]
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 include/linux/irqdomain.h |   19 ++++++++
 kernel/irq/irqdomain.c    |  106 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 102 insertions(+), 23 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 5abb533..3425631 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -144,12 +144,31 @@ static inline struct irq_domain *irq_domain_add_legacy_isa(
 
 extern void irq_domain_remove(struct irq_domain *host);
 
+extern int irq_domain_associate_many(struct irq_domain *domain,
+				     unsigned int irq_base,
+				     irq_hw_number_t hwirq_base, int count);
+static inline int irq_domain_associate(struct irq_domain *domain, unsigned int irq,
+					irq_hw_number_t hwirq)
+{
+	return irq_domain_associate_many(domain, irq, hwirq, 1);
+}
+
 extern unsigned int irq_create_mapping(struct irq_domain *host,
 				       irq_hw_number_t hwirq);
 extern void irq_dispose_mapping(unsigned int virq);
 extern unsigned int irq_find_mapping(struct irq_domain *host,
 				     irq_hw_number_t hwirq);
 extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
+extern int irq_create_strict_mappings(struct irq_domain *domain,
+				      unsigned int irq_base,
+				      irq_hw_number_t hwirq_base, int count);
+
+static inline int irq_create_identity_mapping(struct irq_domain *host,
+					      irq_hw_number_t hwirq)
+{
+	return irq_create_strict_mappings(host, hwirq, hwirq, 1);
+}
+
 extern void irq_radix_revmap_insert(struct irq_domain *host, unsigned int virq,
 				    irq_hw_number_t hwirq);
 extern unsigned int irq_radix_revmap_lookup(struct irq_domain *host,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 5c36722..1a8f3d2 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -379,34 +379,56 @@ static void irq_domain_disassociate_many(struct irq_domain *domain,
 	}
 }
 
-static int irq_setup_virq(struct irq_domain *domain, unsigned int virq,
-			    irq_hw_number_t hwirq)
+int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
+			      irq_hw_number_t hwirq_base, int count)
 {
-	struct irq_data *irq_data = irq_get_irq_data(virq);
+	unsigned int virq = irq_base;
+	irq_hw_number_t hwirq = hwirq_base;
+	int i;
 
-	irq_data->hwirq = hwirq;
-	irq_data->domain = domain;
-	if (domain->ops->map(domain, virq, hwirq)) {
-		pr_debug("irq-%i==>hwirq-0x%lx mapping failed\n", virq, hwirq);
-		irq_data->domain = NULL;
-		irq_data->hwirq = 0;
-		return -1;
-	}
+	for (i = 0; i < count; i++) {
+		struct irq_data *irq_data = irq_get_irq_data(virq + i);
 
-	switch (domain->revmap_type) {
-	case IRQ_DOMAIN_MAP_LINEAR:
-		if (hwirq < domain->revmap_data.linear.size)
-			domain->revmap_data.linear.revmap[hwirq] = virq;
-		break;
-	case IRQ_DOMAIN_MAP_TREE:
-		irq_radix_revmap_insert(domain, virq, hwirq);
-		break;
-	}
+		if (WARN(!irq_data, "error: irq_desc not allocated; "
+			 "irq=%i hwirq=0x%x\n", virq + i, (int)hwirq + i))
+			return -EINVAL;
+		if (WARN(irq_data->domain, "error: irq_desc already associated; "
+			 "irq=%i hwirq=0x%x\n", virq + i, (int)hwirq + i))
+			return -EINVAL;
+	};
 
-	irq_clear_status_flags(virq, IRQ_NOREQUEST);
+	for (i = 0; i < count; i++, virq++, hwirq++) {
+		struct irq_data *irq_data = irq_get_irq_data(virq);
+
+		irq_data->hwirq = hwirq;
+		irq_data->domain = domain;
+		if (domain->ops->map(domain, virq, hwirq)) {
+			pr_debug("irq-%i==>hwirq-0x%lx mapping failed\n", virq, hwirq);
+			irq_data->domain = NULL;
+			irq_data->hwirq = 0;
+			goto err_unmap;
+		}
+
+		switch (domain->revmap_type) {
+		case IRQ_DOMAIN_MAP_LINEAR:
+			if (hwirq < domain->revmap_data.linear.size)
+				domain->revmap_data.linear.revmap[hwirq] = virq;
+			break;
+		case IRQ_DOMAIN_MAP_TREE:
+			irq_radix_revmap_insert(domain, virq, hwirq);
+			break;
+		}
+
+		irq_clear_status_flags(virq, IRQ_NOREQUEST);
+	}
 
 	return 0;
+
+ err_unmap:
+	irq_domain_disassociate_many(domain, irq_base, i);
+	return -EINVAL;
 }
+EXPORT_SYMBOL_GPL(irq_domain_associate_many);
 
 /**
  * irq_create_direct_mapping() - Allocate an irq for direct mapping
@@ -439,7 +461,7 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
 	}
 	pr_debug("create_direct obtained virq %d\n", virq);
 
-	if (irq_setup_virq(domain, virq, virq)) {
+	if (irq_domain_associate(domain, virq, virq)) {
 		irq_free_desc(virq);
 		return 0;
 	}
@@ -500,7 +522,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 		return 0;
 	}
 
-	if (irq_setup_virq(domain, virq, hwirq)) {
+	if (irq_domain_associate(domain, virq, hwirq)) {
 		irq_free_desc(virq);
 		return 0;
 	}
@@ -512,6 +534,44 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 }
 EXPORT_SYMBOL_GPL(irq_create_mapping);
 
+/**
+ * irq_create_strict_mappings() - Map a range of hw irqs to fixed linux irqs
+ * @domain: domain owning the interrupt range
+ * @irq_base: beginning of linux IRQ range
+ * @hwirq_base: beginning of hardware IRQ range
+ * @count: Number of interrupts to map
+ *
+ * This routine is used for allocating and mapping a range of hardware
+ * irqs to linux irqs where the linux irq numbers are at pre-defined
+ * locations. For use by controllers that already have static mappings
+ * to insert in to the domain.
+ *
+ * Non-linear users can use irq_create_identity_mapping() for IRQ-at-a-time
+ * domain insertion.
+ *
+ * 0 is returned upon success, while any failure to establish a static
+ * mapping is treated as an error.
+ */
+int irq_create_strict_mappings(struct irq_domain *domain, unsigned int irq_base,
+			       irq_hw_number_t hwirq_base, int count)
+{
+	int ret;
+
+	ret = irq_alloc_descs(irq_base, irq_base, count,
+			      of_node_to_nid(domain->of_node));
+	if (unlikely(ret < 0))
+		return ret;
+
+	ret = irq_domain_associate_many(domain, irq_base, hwirq_base, count);
+	if (unlikely(ret < 0)) {
+		irq_free_descs(irq_base, count);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(irq_create_strict_mappings);
+
 unsigned int irq_create_of_mapping(struct device_node *controller,
 				   const u32 *intspec, unsigned int intsize)
 {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (2 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 03/12] irqdomain: Support for static IRQ mapping and association Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  5:56   ` Benjamin Herrenschmidt
  2012-06-16  5:01 ` [PATCH 05/12] irqdomain: Fix irq_create_direct_mapping() to test irq_domain type Grant Likely
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

In preparation to remove the slow revmap path, eliminate the public
radix revmap lookup functions.  This simplifies the code and makes the
slowpath removal patch a lot simpler.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/powerpc/sysdev/xics/icp-hv.c      |    2 +-
 arch/powerpc/sysdev/xics/icp-native.c  |    2 +-
 arch/powerpc/sysdev/xics/xics-common.c |    3 --
 include/linux/irqdomain.h              |    4 ---
 kernel/irq/irqdomain.c                 |   62 ++------------------------------
 5 files changed, 5 insertions(+), 68 deletions(-)

diff --git a/arch/powerpc/sysdev/xics/icp-hv.c b/arch/powerpc/sysdev/xics/icp-hv.c
index 253dce9..14469cf 100644
--- a/arch/powerpc/sysdev/xics/icp-hv.c
+++ b/arch/powerpc/sysdev/xics/icp-hv.c
@@ -111,7 +111,7 @@ static unsigned int icp_hv_get_irq(void)
 	if (vec == XICS_IRQ_SPURIOUS)
 		return NO_IRQ;
 
-	irq = irq_radix_revmap_lookup(xics_host, vec);
+	irq = irq_find_mapping(xics_host, vec);
 	if (likely(irq != NO_IRQ)) {
 		xics_push_cppr(vec);
 		return irq;
diff --git a/arch/powerpc/sysdev/xics/icp-native.c b/arch/powerpc/sysdev/xics/icp-native.c
index 4c79b6f..48861d3 100644
--- a/arch/powerpc/sysdev/xics/icp-native.c
+++ b/arch/powerpc/sysdev/xics/icp-native.c
@@ -119,7 +119,7 @@ static unsigned int icp_native_get_irq(void)
 	if (vec == XICS_IRQ_SPURIOUS)
 		return NO_IRQ;
 
-	irq = irq_radix_revmap_lookup(xics_host, vec);
+	irq = irq_find_mapping(xics_host, vec);
 	if (likely(irq != NO_IRQ)) {
 		xics_push_cppr(vec);
 		return irq;
diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c
index cd1d18d..9049d9f 100644
--- a/arch/powerpc/sysdev/xics/xics-common.c
+++ b/arch/powerpc/sysdev/xics/xics-common.c
@@ -329,9 +329,6 @@ static int xics_host_map(struct irq_domain *h, unsigned int virq,
 
 	pr_devel("xics: map virq %d, hwirq 0x%lx\n", virq, hw);
 
-	/* Insert the interrupt mapping into the radix tree for fast lookup */
-	irq_radix_revmap_insert(xics_host, virq, hw);
-
 	/* They aren't all level sensitive but we just don't really know */
 	irq_set_status_flags(virq, IRQ_LEVEL);
 
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 3425631..d8b88c5 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -169,10 +169,6 @@ static inline int irq_create_identity_mapping(struct irq_domain *host,
 	return irq_create_strict_mappings(host, hwirq, hwirq, 1);
 }
 
-extern void irq_radix_revmap_insert(struct irq_domain *host, unsigned int virq,
-				    irq_hw_number_t hwirq);
-extern unsigned int irq_radix_revmap_lookup(struct irq_domain *host,
-					    irq_hw_number_t hwirq);
 extern unsigned int irq_linear_revmap(struct irq_domain *host,
 				      irq_hw_number_t hwirq);
 
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 1a8f3d2..79a7711 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -415,7 +415,9 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
 				domain->revmap_data.linear.revmap[hwirq] = virq;
 			break;
 		case IRQ_DOMAIN_MAP_TREE:
-			irq_radix_revmap_insert(domain, virq, hwirq);
+			mutex_lock(&revmap_trees_mutex);
+			radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
+			mutex_unlock(&revmap_trees_mutex);
 			break;
 		}
 
@@ -688,64 +690,6 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
 EXPORT_SYMBOL_GPL(irq_find_mapping);
 
 /**
- * irq_radix_revmap_lookup() - Find a linux irq from a hw irq number.
- * @domain: domain owning this hardware interrupt
- * @hwirq: hardware irq number in that domain space
- *
- * This is a fast path, for use by irq controller code that uses radix tree
- * revmaps
- */
-unsigned int irq_radix_revmap_lookup(struct irq_domain *domain,
-				     irq_hw_number_t hwirq)
-{
-	struct irq_data *irq_data;
-
-	if (WARN_ON_ONCE(domain->revmap_type != IRQ_DOMAIN_MAP_TREE))
-		return irq_find_mapping(domain, hwirq);
-
-	/*
-	 * Freeing an irq can delete nodes along the path to
-	 * do the lookup via call_rcu.
-	 */
-	rcu_read_lock();
-	irq_data = radix_tree_lookup(&domain->revmap_data.tree, hwirq);
-	rcu_read_unlock();
-
-	/*
-	 * If found in radix tree, then fine.
-	 * Else fallback to linear lookup - this should not happen in practice
-	 * as it means that we failed to insert the node in the radix tree.
-	 */
-	return irq_data ? irq_data->irq : irq_find_mapping(domain, hwirq);
-}
-EXPORT_SYMBOL_GPL(irq_radix_revmap_lookup);
-
-/**
- * irq_radix_revmap_insert() - Insert a hw irq to linux irq number mapping.
- * @domain: domain owning this hardware interrupt
- * @virq: linux irq number
- * @hwirq: hardware irq number in that domain space
- *
- * This is for use by irq controllers that use a radix tree reverse
- * mapping for fast lookup.
- */
-void irq_radix_revmap_insert(struct irq_domain *domain, unsigned int virq,
-			     irq_hw_number_t hwirq)
-{
-	struct irq_data *irq_data = irq_get_irq_data(virq);
-
-	if (WARN_ON(domain->revmap_type != IRQ_DOMAIN_MAP_TREE))
-		return;
-
-	if (virq) {
-		mutex_lock(&revmap_trees_mutex);
-		radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
-		mutex_unlock(&revmap_trees_mutex);
-	}
-}
-EXPORT_SYMBOL_GPL(irq_radix_revmap_insert);
-
-/**
  * irq_linear_revmap() - Find a linux irq from a hw irq number.
  * @domain: domain owning this hardware interrupt
  * @hwirq: hardware irq number in that domain space
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/12] irqdomain: Fix irq_create_direct_mapping() to test irq_domain type.
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (3 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  5:01 ` [PATCH 06/12] irqdomain: eliminate slow-path revmap lookups Grant Likely
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

irq_create_direct_mapping can only be used with the NOMAP type.  Make
the function test to ensure it is passed the correct type of
irq_domain.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 kernel/irq/irqdomain.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 79a7711..0483e27 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -447,8 +447,8 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
 	if (domain == NULL)
 		domain = irq_default_domain;
 
-	BUG_ON(domain == NULL);
-	WARN_ON(domain->revmap_type != IRQ_DOMAIN_MAP_NOMAP);
+	if (WARN_ON(!domain || domain->revmap_type != IRQ_DOMAIN_MAP_NOMAP))
+		return 0;
 
 	virq = irq_alloc_desc_from(1, of_node_to_nid(domain->of_node));
 	if (!virq) {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/12] irqdomain: eliminate slow-path revmap lookups
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (4 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 05/12] irqdomain: Fix irq_create_direct_mapping() to test irq_domain type Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  5:01 ` [PATCH 07/12] irqdomain: Make ops->map hook optional Grant Likely
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Benjamin Herrenschmidt,
	Thomas Gleixner, Paul Mundt, Rob Herring

With the current state of irq_domain, the reverse map is always updated
when new IRQs get mapped.  This means that the irq_find_mapping() function
can be simplified to execute the revmap lookup functions unconditionally

This patch adds lookup functions for the revmaps that don't yet have one
and removes the slow path lookup code path.

v8: Broke out unrelated changes into separate patches.  Rebased on Paul's irq
    association patches.
v7: Rebased to irqdomain/next for v3.4 and applied before the removal of 'hint'
v6: Remove the slow path entirely.  The only place where the slow path
    could get called is for a linear mapping if the hwirq number is larger
    than the linear revmap size.  There shouldn't be any interrupt
    controllers that do that.
v5: rewrite to not use a ->revmap() callback.  It is simpler, smaller,
    safer and faster to open code each of the revmap lookups directly into
    irq_find_mapping() via a switch statement.
v4: Fix build failure on incorrect variable reference.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Milton Miller <miltonm@bga.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 kernel/irq/irqdomain.c |   67 +++++++++++++++++++-----------------------------
 1 file changed, 27 insertions(+), 40 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 0483e27..00c383b 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -652,16 +652,11 @@ EXPORT_SYMBOL_GPL(irq_dispose_mapping);
  * irq_find_mapping() - Find a linux irq from an hw irq number.
  * @domain: domain owning this hardware interrupt
  * @hwirq: hardware irq number in that domain space
- *
- * This is a slow path, for use by generic code. It's expected that an
- * irq controller implementation directly calls the appropriate low level
- * mapping function.
  */
 unsigned int irq_find_mapping(struct irq_domain *domain,
 			      irq_hw_number_t hwirq)
 {
-	unsigned int i;
-	unsigned int hint = hwirq % nr_irqs;
+	struct irq_data *data;
 
 	/* Look for default domain if nececssary */
 	if (domain == NULL)
@@ -669,22 +664,27 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
 	if (domain == NULL)
 		return 0;
 
-	/* legacy -> bail early */
-	if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
+	switch (domain->revmap_type) {
+	case IRQ_DOMAIN_MAP_LEGACY:
 		return irq_domain_legacy_revmap(domain, hwirq);
-
-	/* Slow path does a linear search of the map */
-	if (hint == 0)
-		hint = 1;
-	i = hint;
-	do {
-		struct irq_data *data = irq_get_irq_data(i);
+	case IRQ_DOMAIN_MAP_LINEAR:
+		return irq_linear_revmap(domain, hwirq);
+	case IRQ_DOMAIN_MAP_TREE:
+		rcu_read_lock();
+		data = radix_tree_lookup(&domain->revmap_data.tree, hwirq);
+		rcu_read_unlock();
+		if (data)
+			return data->irq;
+		break;
+	case IRQ_DOMAIN_MAP_NOMAP:
+		data = irq_get_irq_data(hwirq);
 		if (data && (data->domain == domain) && (data->hwirq == hwirq))
-			return i;
-		i++;
-		if (i >= nr_irqs)
-			i = 1;
-	} while(i != hint);
+			return hwirq;
+		break;
+	}
+
+	WARN(1, "ERROR: irq revmap went horribly wrong. revmap_type=%i\n",
+		domain->revmap_type);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(irq_find_mapping);
@@ -694,32 +694,19 @@ EXPORT_SYMBOL_GPL(irq_find_mapping);
  * @domain: domain owning this hardware interrupt
  * @hwirq: hardware irq number in that domain space
  *
- * This is a fast path, for use by irq controller code that uses linear
- * revmaps. It does fallback to the slow path if the revmap doesn't exist
- * yet and will create the revmap entry with appropriate locking
+ * This is a fast path that can be called directly by irq controller code to
+ * save a handful of instructions.
  */
 unsigned int irq_linear_revmap(struct irq_domain *domain,
 			       irq_hw_number_t hwirq)
 {
-	unsigned int *revmap;
+	BUG_ON(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR);
 
-	if (WARN_ON_ONCE(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR))
-		return irq_find_mapping(domain, hwirq);
-
-	/* Check revmap bounds */
-	if (unlikely(hwirq >= domain->revmap_data.linear.size))
-		return irq_find_mapping(domain, hwirq);
-
-	/* Check if revmap was allocated */
-	revmap = domain->revmap_data.linear.revmap;
-	if (unlikely(revmap == NULL))
-		return irq_find_mapping(domain, hwirq);
-
-	/* Fill up revmap with slow path if no mapping found */
-	if (unlikely(!revmap[hwirq]))
-		revmap[hwirq] = irq_find_mapping(domain, hwirq);
+	/* Check revmap bounds; complain if exceeded */
+	if (WARN_ON(hwirq >= domain->revmap_data.linear.size))
+		return 0;
 
-	return revmap[hwirq];
+	return domain->revmap_data.linear.revmap[hwirq];
 }
 EXPORT_SYMBOL_GPL(irq_linear_revmap);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/12] irqdomain: Make ops->map hook optional
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (5 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 06/12] irqdomain: eliminate slow-path revmap lookups Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  5:59   ` Benjamin Herrenschmidt
  2012-06-16  5:01 ` [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR Grant Likely
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

There isn't a really compelling reason to force ->map to be populated,
so allow it to be left unset.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 kernel/irq/irqdomain.c |   14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 00c383b..e88a7b0 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -205,7 +205,8 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
 		 * one can then use irq_create_mapping() to
 		 * explicitly change them
 		 */
-		ops->map(domain, irq, hwirq);
+		if (ops->map)
+			ops->map(domain, irq, hwirq);
 
 		/* Clear norequest flags */
 		irq_clear_status_flags(irq, IRQ_NOREQUEST);
@@ -402,8 +403,8 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
 
 		irq_data->hwirq = hwirq;
 		irq_data->domain = domain;
-		if (domain->ops->map(domain, virq, hwirq)) {
-			pr_debug("irq-%i==>hwirq-0x%lx mapping failed\n", virq, hwirq);
+		if (domain->ops->map && domain->ops->map(domain, virq, hwirq)) {
+			pr_err("irq-%i==>hwirq-0x%lx mapping failed\n", virq, hwirq);
 			irq_data->domain = NULL;
 			irq_data->hwirq = 0;
 			goto err_unmap;
@@ -783,12 +784,6 @@ static int __init irq_debugfs_init(void)
 __initcall(irq_debugfs_init);
 #endif /* CONFIG_IRQ_DOMAIN_DEBUG */
 
-static int irq_domain_simple_map(struct irq_domain *d, unsigned int irq,
-				 irq_hw_number_t hwirq)
-{
-	return 0;
-}
-
 /**
  * irq_domain_xlate_onecell() - Generic xlate for direct one cell bindings
  *
@@ -851,7 +846,6 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d,
 EXPORT_SYMBOL_GPL(irq_domain_xlate_onetwocell);
 
 const struct irq_domain_ops irq_domain_simple_ops = {
-	.map = irq_domain_simple_map,
 	.xlate = irq_domain_xlate_onetwocell,
 };
 EXPORT_SYMBOL_GPL(irq_domain_simple_ops);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (6 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 07/12] irqdomain: Make ops->map hook optional Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  6:01   ` Benjamin Herrenschmidt
  2012-06-16  5:01 ` [PATCH 09/12] irqdomain: Reserve IRQs for legacy domain Grant Likely
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

The LEGACY mapping unnecessarily complicates the irqdomain code and
can easily be implemented with a linear mapping.  By ripping it out
and replacing it with the LINEAR mapping the object size of
irqdomain.c shrinks by about 330 bytes (ARMv7) which offsets the
additional allocation required by the linear map.  It also makes it
possible for current LEGACY map users to pre-allocate irq_descs for a
subset of the hwirqs and dynamically allocate the rest as needed.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/plat-versatile/fpga-irq.c |    2 +-
 include/linux/irqdomain.h          |    5 ---
 kernel/irq/irqdomain.c             |   82 +++---------------------------------
 3 files changed, 7 insertions(+), 82 deletions(-)

diff --git a/arch/arm/plat-versatile/fpga-irq.c b/arch/arm/plat-versatile/fpga-irq.c
index 6e70d03..05d029a 100644
--- a/arch/arm/plat-versatile/fpga-irq.c
+++ b/arch/arm/plat-versatile/fpga-irq.c
@@ -111,7 +111,7 @@ static int fpga_irqdomain_map(struct irq_domain *d, unsigned int irq,
 
 	/* Skip invalid IRQs, only register handlers for the real ones */
 	if (!(f->valid & (1 << hwirq)))
-		return -ENOTSUPP;
+		return 0;
 	irq_set_chip_data(irq, f);
 	irq_set_chip_and_handler(irq, &f->chip,
 				handle_level_irq);
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index d8b88c5..b46a551 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -91,11 +91,6 @@ struct irq_domain {
 	union {
 		struct {
 			unsigned int size;
-			unsigned int first_irq;
-			irq_hw_number_t first_hwirq;
-		} legacy;
-		struct {
-			unsigned int size;
 			unsigned int *revmap;
 		} linear;
 		struct {
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index e88a7b0..d6d0de0 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -16,8 +16,6 @@
 #include <linux/smp.h>
 #include <linux/fs.h>
 
-#define IRQ_DOMAIN_MAP_LEGACY 0 /* driver allocated fixed range of irqs.
-				 * ie. legacy 8259, gets irqs 1..15 */
 #define IRQ_DOMAIN_MAP_NOMAP 1 /* no fast reverse mapping */
 #define IRQ_DOMAIN_MAP_LINEAR 2 /* linear map of interrupts */
 #define IRQ_DOMAIN_MAP_TREE 3 /* radix tree */
@@ -88,13 +86,6 @@ void irq_domain_remove(struct irq_domain *domain)
 	mutex_lock(&irq_domain_mutex);
 
 	switch (domain->revmap_type) {
-	case IRQ_DOMAIN_MAP_LEGACY:
-		/*
-		 * Legacy domains don't manage their own irq_desc
-		 * allocations, we expect the caller to handle irq_desc
-		 * freeing on their own.
-		 */
-		break;
 	case IRQ_DOMAIN_MAP_TREE:
 		/*
 		 * radix_tree_delete() takes care of destroying the root
@@ -128,17 +119,6 @@ void irq_domain_remove(struct irq_domain *domain)
 }
 EXPORT_SYMBOL_GPL(irq_domain_remove);
 
-static unsigned int irq_domain_legacy_revmap(struct irq_domain *domain,
-					     irq_hw_number_t hwirq)
-{
-	irq_hw_number_t first_hwirq = domain->revmap_data.legacy.first_hwirq;
-	int size = domain->revmap_data.legacy.size;
-
-	if (WARN_ON(hwirq < first_hwirq || hwirq >= first_hwirq + size))
-		return 0;
-	return hwirq - first_hwirq + domain->revmap_data.legacy.first_irq;
-}
-
 /**
  * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
  * @of_node: pointer to interrupt controller's device tree node.
@@ -162,57 +142,17 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
 					 void *host_data)
 {
 	struct irq_domain *domain;
-	unsigned int i;
 
-	domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LEGACY, ops, host_data);
+	pr_debug("Setting up legacy domain virq[%i:%i] ==> hwirq[%i:%i]\n",
+		 first_irq, first_irq + size - 1,
+		 (int)first_hwirq, (int)first_hwirq + size -1);
+
+	domain = irq_domain_add_linear(of_node, first_hwirq + size, ops, host_data);
 	if (!domain)
 		return NULL;
 
-	domain->revmap_data.legacy.first_irq = first_irq;
-	domain->revmap_data.legacy.first_hwirq = first_hwirq;
-	domain->revmap_data.legacy.size = size;
-
-	mutex_lock(&irq_domain_mutex);
-	/* Verify that all the irqs are available */
-	for (i = 0; i < size; i++) {
-		int irq = first_irq + i;
-		struct irq_data *irq_data = irq_get_irq_data(irq);
-
-		if (WARN_ON(!irq_data || irq_data->domain)) {
-			mutex_unlock(&irq_domain_mutex);
-			irq_domain_free(domain);
-			return NULL;
-		}
-	}
-
-	/* Claim all of the irqs before registering a legacy domain */
-	for (i = 0; i < size; i++) {
-		struct irq_data *irq_data = irq_get_irq_data(first_irq + i);
-		irq_data->hwirq = first_hwirq + i;
-		irq_data->domain = domain;
-	}
-	mutex_unlock(&irq_domain_mutex);
-
-	for (i = 0; i < size; i++) {
-		int irq = first_irq + i;
-		int hwirq = first_hwirq + i;
-
-		/* IRQ0 gets ignored */
-		if (!irq)
-			continue;
+	WARN_ON(irq_domain_associate_many(domain, first_irq, first_hwirq, size));
 
-		/* Legacy flags are left to default at this point,
-		 * one can then use irq_create_mapping() to
-		 * explicitly change them
-		 */
-		if (ops->map)
-			ops->map(domain, irq, hwirq);
-
-		/* Clear norequest flags */
-		irq_clear_status_flags(irq, IRQ_NOREQUEST);
-	}
-
-	irq_domain_add(domain);
 	return domain;
 }
 EXPORT_SYMBOL_GPL(irq_domain_add_legacy);
@@ -509,10 +449,6 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 		return virq;
 	}
 
-	/* Get a virtual interrupt number */
-	if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
-		return irq_domain_legacy_revmap(domain, hwirq);
-
 	/* Allocate a virtual interrupt number */
 	hint = hwirq % nr_irqs;
 	if (hint == 0)
@@ -640,10 +576,6 @@ void irq_dispose_mapping(unsigned int virq)
 	if (WARN_ON(domain == NULL))
 		return;
 
-	/* Never unmap legacy interrupts */
-	if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
-		return;
-
 	irq_domain_disassociate_many(domain, virq, 1);
 	irq_free_desc(virq);
 }
@@ -666,8 +598,6 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
 		return 0;
 
 	switch (domain->revmap_type) {
-	case IRQ_DOMAIN_MAP_LEGACY:
-		return irq_domain_legacy_revmap(domain, hwirq);
 	case IRQ_DOMAIN_MAP_LINEAR:
 		return irq_linear_revmap(domain, hwirq);
 	case IRQ_DOMAIN_MAP_TREE:
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/12] irqdomain: Reserve IRQs for legacy domain
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (7 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  5:01 ` [PATCH 10/12] irqdomain: Add debugging message Grant Likely
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel; +Cc: Milton Miller, Grant Likely

It is really important to make sure irqs used by the legacy domain are
still reserved in the irq_desc subsystem so they don't get allocated
for some other purpose.  Without SPARSE_IRQ, this generally isn't a
problem because it is assumed that all irqs have static assignments
(which isn't actually true, and is part of the reason why SPARSE_IRQ
should really be turned on these days).  However, when SPARSE_IRQs is
turned on, the irq range passed to irq_domain_add_legacy() is not
guaranteed to be reserved.

This patch fixes the problem by unconditionally reserving any irqs
passed into irq_domain_add_legacy().  The irqs may have already been
reserved, so the return code on irq_reserve_irqs() may report a
failure, but it should still be safe.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
 kernel/irq/irqdomain.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d6d0de0..c0a00de 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -151,6 +151,13 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
 	if (!domain)
 		return NULL;
 
+	/*
+	 * Do our best to reserve the irq descs; but this overlaps with the nr_irqs setting
+	 * from the platform code
+	 */
+	if (irq_reserve_irqs(first_irq, size))
+		pr_info("IRQs %i..%i already reserved, overlapping with nr_irqs?\n",
+			first_irq, first_irq + size - 1);
 	WARN_ON(irq_domain_associate_many(domain, first_irq, first_hwirq, size));
 
 	return domain;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/12] irqdomain: Add debugging message
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (8 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 09/12] irqdomain: Reserve IRQs for legacy domain Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  6:02   ` Benjamin Herrenschmidt
  2012-06-16  5:01 ` [PATCH 11/12] irqdomain: reorganize revmap data Grant Likely
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

Trivial patch to beef up debugging messages on irqdomain.c

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 kernel/irq/irqdomain.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index c0a00de..0e02a1d 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -334,6 +334,9 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
 	irq_hw_number_t hwirq = hwirq_base;
 	int i;
 
+	pr_debug("%s(%s, irqbase=%i, hwbase=%i, count=%i)\n", __func__,
+		of_node_full_name(domain->of_node), irq_base, (int)hwirq_base, count);
+
 	for (i = 0; i < count; i++) {
 		struct irq_data *irq_data = irq_get_irq_data(virq + i);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 11/12] irqdomain: reorganize revmap data.
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (9 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 10/12] irqdomain: Add debugging message Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-16  6:06   ` Benjamin Herrenschmidt
  2012-06-16  5:01 ` [PATCH 12/12] irqdomain: merge linear and tree reverse mappings Grant Likely
  2012-06-18 12:28 ` [PATCH 00/12] irqdomain cleanup and refactoring Mark Brown
  12 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

In struct irq_domain, reorganize the revmap data in preparation for
unifying the linear and radix mappings.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 drivers/pinctrl/pinctrl-nomadik.c |    4 +--
 include/linux/irqdomain.h         |   17 ++++-------
 kernel/irq/irqdomain.c            |   58 ++++++++++++++-----------------------
 3 files changed, 29 insertions(+), 50 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index b26395d..22fee45 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -826,16 +826,14 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
 {
 	struct nmk_gpio_chip *nmk_chip;
 	struct irq_chip *host_chip = irq_get_chip(irq);
-	unsigned int first_irq;
 
 	chained_irq_enter(host_chip, desc);
 
 	nmk_chip = irq_get_handler_data(irq);
-	first_irq = nmk_chip->domain->revmap_data.legacy.first_irq;
 	while (status) {
 		int bit = __ffs(status);
 
-		generic_handle_irq(first_irq + bit);
+		generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
 		status &= ~BIT(bit);
 	}
 
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index b46a551..ee90765 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -71,7 +71,6 @@ struct irq_domain_ops {
  * @link: Element in global irq_domain list.
  * @revmap_type: Method used for reverse mapping hwirq numbers to linux irq. This
  *               will be one of the IRQ_DOMAIN_MAP_* values.
- * @revmap_data: Revmap method specific data.
  * @ops: pointer to irq_domain methods
  * @host_data: private data pointer for use by owner.  Not touched by irq_domain
  *             core code.
@@ -88,22 +87,18 @@ struct irq_domain {
 
 	/* type of reverse mapping_technique */
 	unsigned int revmap_type;
-	union {
-		struct {
-			unsigned int size;
-			unsigned int *revmap;
-		} linear;
-		struct {
-			unsigned int max_irq;
-		} nomap;
-		struct radix_tree_root tree;
-	} revmap_data;
 	const struct irq_domain_ops *ops;
 	void *host_data;
 	irq_hw_number_t inval_irq;
 
 	/* Optional device node pointer */
 	struct device_node *of_node;
+
+	/* Reverse mapping data */
+	unsigned int nomap_max_irq;
+	struct radix_tree_root radix_tree;
+	unsigned int linear_size;
+	unsigned int linear_revmap[];
 };
 
 #ifdef CONFIG_IRQ_DOMAIN
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 0e02a1d..433971a 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -38,14 +38,14 @@ static struct irq_domain *irq_default_domain;
  * to IRQ domain, or NULL on failure.
  */
 static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
-					   unsigned int revmap_type,
+					   unsigned int revmap_type, int size,
 					   const struct irq_domain_ops *ops,
 					   void *host_data)
 {
 	struct irq_domain *domain;
 
-	domain = kzalloc_node(sizeof(*domain), GFP_KERNEL,
-			      of_node_to_nid(of_node));
+	domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
+			      GFP_KERNEL, of_node_to_nid(of_node));
 	if (WARN_ON(!domain))
 		return NULL;
 
@@ -54,6 +54,7 @@ static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
 	domain->ops = ops;
 	domain->host_data = host_data;
 	domain->of_node = of_node_get(of_node);
+	domain->linear_size = size;
 
 	return domain;
 }
@@ -92,13 +93,7 @@ void irq_domain_remove(struct irq_domain *domain)
 		 * node when all entries are removed. Shout if there are
 		 * any mappings left.
 		 */
-		WARN_ON(domain->revmap_data.tree.height);
-		break;
-	case IRQ_DOMAIN_MAP_LINEAR:
-		kfree(domain->revmap_data.linear.revmap);
-		domain->revmap_data.linear.size = 0;
-		break;
-	case IRQ_DOMAIN_MAP_NOMAP:
+		WARN_ON(domain->radix_tree.height);
 		break;
 	}
 
@@ -177,20 +172,11 @@ struct irq_domain *irq_domain_add_linear(struct device_node *of_node,
 					 void *host_data)
 {
 	struct irq_domain *domain;
-	unsigned int *revmap;
 
-	revmap = kzalloc_node(sizeof(*revmap) * size, GFP_KERNEL,
-			      of_node_to_nid(of_node));
-	if (WARN_ON(!revmap))
+	domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LINEAR, size, ops, host_data);
+	if (!domain)
 		return NULL;
 
-	domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LINEAR, ops, host_data);
-	if (!domain) {
-		kfree(revmap);
-		return NULL;
-	}
-	domain->revmap_data.linear.size = size;
-	domain->revmap_data.linear.revmap = revmap;
 	irq_domain_add(domain);
 	return domain;
 }
@@ -202,9 +188,9 @@ struct irq_domain *irq_domain_add_nomap(struct device_node *of_node,
 					 void *host_data)
 {
 	struct irq_domain *domain = irq_domain_alloc(of_node,
-					IRQ_DOMAIN_MAP_NOMAP, ops, host_data);
+					IRQ_DOMAIN_MAP_NOMAP, 0, ops, host_data);
 	if (domain) {
-		domain->revmap_data.nomap.max_irq = max_irq ? max_irq : ~0;
+		domain->nomap_max_irq = max_irq ? max_irq : ~0;
 		irq_domain_add(domain);
 	}
 	return domain;
@@ -224,9 +210,9 @@ struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
 					 void *host_data)
 {
 	struct irq_domain *domain = irq_domain_alloc(of_node,
-					IRQ_DOMAIN_MAP_TREE, ops, host_data);
+					IRQ_DOMAIN_MAP_TREE, 0, ops, host_data);
 	if (domain) {
-		INIT_RADIX_TREE(&domain->revmap_data.tree, GFP_KERNEL);
+		INIT_RADIX_TREE(&domain->radix_tree, GFP_KERNEL);
 		irq_domain_add(domain);
 	}
 	return domain;
@@ -315,12 +301,12 @@ static void irq_domain_disassociate_many(struct irq_domain *domain,
 		/* Clear reverse map */
 		switch(domain->revmap_type) {
 		case IRQ_DOMAIN_MAP_LINEAR:
-			if (hwirq < domain->revmap_data.linear.size)
-				domain->revmap_data.linear.revmap[hwirq] = 0;
+			if (hwirq < domain->linear_size)
+				domain->linear_revmap[hwirq] = 0;
 			break;
 		case IRQ_DOMAIN_MAP_TREE:
 			mutex_lock(&revmap_trees_mutex);
-			radix_tree_delete(&domain->revmap_data.tree, hwirq);
+			radix_tree_delete(&domain->radix_tree, hwirq);
 			mutex_unlock(&revmap_trees_mutex);
 			break;
 		}
@@ -362,12 +348,12 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
 
 		switch (domain->revmap_type) {
 		case IRQ_DOMAIN_MAP_LINEAR:
-			if (hwirq < domain->revmap_data.linear.size)
-				domain->revmap_data.linear.revmap[hwirq] = virq;
+			if (hwirq < domain->linear_size)
+				domain->linear_revmap[hwirq] = virq;
 			break;
 		case IRQ_DOMAIN_MAP_TREE:
 			mutex_lock(&revmap_trees_mutex);
-			radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
+			radix_tree_insert(&domain->radix_tree, hwirq, irq_data);
 			mutex_unlock(&revmap_trees_mutex);
 			break;
 		}
@@ -406,9 +392,9 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
 		pr_debug("create_direct virq allocation failed\n");
 		return 0;
 	}
-	if (virq >= domain->revmap_data.nomap.max_irq) {
+	if (virq >= domain->nomap_max_irq) {
 		pr_err("ERROR: no free irqs available below %i maximum\n",
-			domain->revmap_data.nomap.max_irq);
+			domain->nomap_max_irq);
 		irq_free_desc(virq);
 		return 0;
 	}
@@ -612,7 +598,7 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
 		return irq_linear_revmap(domain, hwirq);
 	case IRQ_DOMAIN_MAP_TREE:
 		rcu_read_lock();
-		data = radix_tree_lookup(&domain->revmap_data.tree, hwirq);
+		data = radix_tree_lookup(&domain->radix_tree, hwirq);
 		rcu_read_unlock();
 		if (data)
 			return data->irq;
@@ -644,10 +630,10 @@ unsigned int irq_linear_revmap(struct irq_domain *domain,
 	BUG_ON(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR);
 
 	/* Check revmap bounds; complain if exceeded */
-	if (WARN_ON(hwirq >= domain->revmap_data.linear.size))
+	if (WARN_ON(hwirq >= domain->linear_size))
 		return 0;
 
-	return domain->revmap_data.linear.revmap[hwirq];
+	return domain->linear_revmap[hwirq];
 }
 EXPORT_SYMBOL_GPL(irq_linear_revmap);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 12/12] irqdomain: merge linear and tree reverse mappings.
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (10 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 11/12] irqdomain: reorganize revmap data Grant Likely
@ 2012-06-16  5:01 ` Grant Likely
  2012-06-18 12:28 ` [PATCH 00/12] irqdomain cleanup and refactoring Mark Brown
  12 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2012-06-16  5:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Milton Miller, Grant Likely, Paul Mundt, Benjamin Herrenschmidt,
	Thomas Gleixner, Rob Herring

Keeping them separate adds a lot of code.  Merging them simplifies the
whole scheme.  This change makes it so both the tree and linear
methods can be used by the same irq_domain instance.  If the hwirq is
less than the ->linear_size, then the linear map is used to reverse
map the hwirq.  Otherwise the radix tree is used.

It also means that complex interrupt controllers can use both the
linear map and a tree in the same domain.  This may be useful for an
interrupt controller with a base set of core irqs and a large number
of GPIOs which might be used as irqs.  The linear map could cover the
core irqs, and the tree used for thas irqs.  The linear map could
cover the core irqs, and the tree used for the gpios.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 include/linux/irqdomain.h |   10 ++++---
 kernel/irq/irqdomain.c    |   72 ++++++++++-----------------------------------
 2 files changed, 21 insertions(+), 61 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index ee90765..775765d 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -116,10 +116,6 @@ struct irq_domain *irq_domain_add_nomap(struct device_node *of_node,
 					 unsigned int max_irq,
 					 const struct irq_domain_ops *ops,
 					 void *host_data);
-struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
-					 const struct irq_domain_ops *ops,
-					 void *host_data);
-
 extern struct irq_domain *irq_find_host(struct device_node *node);
 extern void irq_set_default_host(struct irq_domain *host);
 
@@ -131,6 +127,12 @@ static inline struct irq_domain *irq_domain_add_legacy_isa(
 	return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS, 0, 0, ops,
 				     host_data);
 }
+static inline struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
+					 const struct irq_domain_ops *ops,
+					 void *host_data)
+{
+	return irq_domain_add_linear(of_node, 0, ops, host_data);
+}
 
 extern void irq_domain_remove(struct irq_domain *host);
 
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 433971a..8db53b3 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -18,7 +18,6 @@
 
 #define IRQ_DOMAIN_MAP_NOMAP 1 /* no fast reverse mapping */
 #define IRQ_DOMAIN_MAP_LINEAR 2 /* linear map of interrupts */
-#define IRQ_DOMAIN_MAP_TREE 3 /* radix tree */
 
 static LIST_HEAD(irq_domain_list);
 static DEFINE_MUTEX(irq_domain_mutex);
@@ -50,6 +49,7 @@ static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
 		return NULL;
 
 	/* Fill structure */
+	INIT_RADIX_TREE(&domain->radix_tree, GFP_KERNEL);
 	domain->revmap_type = revmap_type;
 	domain->ops = ops;
 	domain->host_data = host_data;
@@ -85,17 +85,7 @@ static void irq_domain_add(struct irq_domain *domain)
 void irq_domain_remove(struct irq_domain *domain)
 {
 	mutex_lock(&irq_domain_mutex);
-
-	switch (domain->revmap_type) {
-	case IRQ_DOMAIN_MAP_TREE:
-		/*
-		 * radix_tree_delete() takes care of destroying the root
-		 * node when all entries are removed. Shout if there are
-		 * any mappings left.
-		 */
-		WARN_ON(domain->radix_tree.height);
-		break;
-	}
+	WARN_ON(domain->radix_tree.height);
 
 	list_del(&domain->link);
 
@@ -198,28 +188,6 @@ struct irq_domain *irq_domain_add_nomap(struct device_node *of_node,
 EXPORT_SYMBOL_GPL(irq_domain_add_nomap);
 
 /**
- * irq_domain_add_tree()
- * @of_node: pointer to interrupt controller's device tree node.
- * @ops: map/unmap domain callbacks
- *
- * Note: The radix tree will be allocated later during boot automatically
- * (the reverse mapping will use the slow path until that happens).
- */
-struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
-					 const struct irq_domain_ops *ops,
-					 void *host_data)
-{
-	struct irq_domain *domain = irq_domain_alloc(of_node,
-					IRQ_DOMAIN_MAP_TREE, 0, ops, host_data);
-	if (domain) {
-		INIT_RADIX_TREE(&domain->radix_tree, GFP_KERNEL);
-		irq_domain_add(domain);
-	}
-	return domain;
-}
-EXPORT_SYMBOL_GPL(irq_domain_add_tree);
-
-/**
  * irq_find_host() - Locates a domain for a given device node
  * @node: device-tree node of the interrupt controller
  */
@@ -299,16 +267,12 @@ static void irq_domain_disassociate_many(struct irq_domain *domain,
 		irq_data->hwirq = 0;
 
 		/* Clear reverse map */
-		switch(domain->revmap_type) {
-		case IRQ_DOMAIN_MAP_LINEAR:
-			if (hwirq < domain->linear_size)
-				domain->linear_revmap[hwirq] = 0;
-			break;
-		case IRQ_DOMAIN_MAP_TREE:
+		if (hwirq < domain->linear_size)
+			domain->linear_revmap[hwirq] = 0;
+		else {
 			mutex_lock(&revmap_trees_mutex);
 			radix_tree_delete(&domain->radix_tree, hwirq);
 			mutex_unlock(&revmap_trees_mutex);
-			break;
 		}
 	}
 }
@@ -346,16 +310,12 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
 			goto err_unmap;
 		}
 
-		switch (domain->revmap_type) {
-		case IRQ_DOMAIN_MAP_LINEAR:
-			if (hwirq < domain->linear_size)
-				domain->linear_revmap[hwirq] = virq;
-			break;
-		case IRQ_DOMAIN_MAP_TREE:
+		if (hwirq < domain->linear_size)
+			domain->linear_revmap[hwirq] = virq;
+		else {
 			mutex_lock(&revmap_trees_mutex);
 			radix_tree_insert(&domain->radix_tree, hwirq, irq_data);
 			mutex_unlock(&revmap_trees_mutex);
-			break;
 		}
 
 		irq_clear_status_flags(virq, IRQ_NOREQUEST);
@@ -596,13 +556,6 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
 	switch (domain->revmap_type) {
 	case IRQ_DOMAIN_MAP_LINEAR:
 		return irq_linear_revmap(domain, hwirq);
-	case IRQ_DOMAIN_MAP_TREE:
-		rcu_read_lock();
-		data = radix_tree_lookup(&domain->radix_tree, hwirq);
-		rcu_read_unlock();
-		if (data)
-			return data->irq;
-		break;
 	case IRQ_DOMAIN_MAP_NOMAP:
 		data = irq_get_irq_data(hwirq);
 		if (data && (data->domain == domain) && (data->hwirq == hwirq))
@@ -627,11 +580,16 @@ EXPORT_SYMBOL_GPL(irq_find_mapping);
 unsigned int irq_linear_revmap(struct irq_domain *domain,
 			       irq_hw_number_t hwirq)
 {
+	struct irq_data *data;
 	BUG_ON(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR);
 
 	/* Check revmap bounds; complain if exceeded */
-	if (WARN_ON(hwirq >= domain->linear_size))
-		return 0;
+	if (hwirq >= domain->linear_size) {
+		rcu_read_lock();
+		data = radix_tree_lookup(&domain->radix_tree, hwirq);
+		rcu_read_unlock();
+		return data ? data->irq : 0;
+	}
 
 	return domain->linear_revmap[hwirq];
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions
  2012-06-16  5:01 ` [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions Grant Likely
@ 2012-06-16  5:56   ` Benjamin Herrenschmidt
  2012-06-16  6:12     ` Grant Likely
  0 siblings, 1 reply; 29+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-16  5:56 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> In preparation to remove the slow revmap path, eliminate the public
> radix revmap lookup functions.  This simplifies the code and makes the
> slowpath removal patch a lot simpler.

The idea here was to avoid a test (performance in the fast path) since
the controller knows the type of revmap it uses. You could just remove
the fallback to the slow path if there's a mismatch and keep the WARN_ON
no ? No biggie, it's just a switch/case with fairly predictable outcome
I suppose ...

A few other nits:

> 	return irq;
> diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c
> index cd1d18d..9049d9f 100644
> --- a/arch/powerpc/sysdev/xics/xics-common.c
> +++ b/arch/powerpc/sysdev/xics/xics-common.c
> @@ -329,9 +329,6 @@ static int xics_host_map(struct irq_domain *h, unsigned int virq,
>  
>  	pr_devel("xics: map virq %d, hwirq 0x%lx\n", virq, hw);
>  
> -	/* Insert the interrupt mapping into the radix tree for fast lookup */
> -	irq_radix_revmap_insert(xics_host, virq, hw);
> -
>  	/* They aren't all level sensitive but we just don't really know */
>  	irq_set_status_flags(virq, IRQ_LEVEL);

This looks like it belongs in a different patch, possibly "[02/12]
irqdomain: Always update revmap when setting up a virq" no ? 

> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 3425631..d8b88c5 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -169,10 +169,6 @@ static inline int irq_create_identity_mapping(struct irq_domain *host,
>  	return irq_create_strict_mappings(host, hwirq, hwirq, 1);
>  }
>  
> -extern void irq_radix_revmap_insert(struct irq_domain *host, unsigned int virq,
> -				    irq_hw_number_t hwirq);
> -extern unsigned int irq_radix_revmap_lookup(struct irq_domain *host,
> -					    irq_hw_number_t hwirq);
>  extern unsigned int irq_linear_revmap(struct irq_domain *host,
>  				      irq_hw_number_t hwirq);
>  
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 1a8f3d2..79a7711 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -415,7 +415,9 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
>  				domain->revmap_data.linear.revmap[hwirq] = virq;
>  			break;
>  		case IRQ_DOMAIN_MAP_TREE:
> -			irq_radix_revmap_insert(domain, virq, hwirq);
> +			mutex_lock(&revmap_trees_mutex);
> +			radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
> +			mutex_unlock(&revmap_trees_mutex);
>  			break;
>  		}

That too looks like it belongs in another patch.

> @@ -688,64 +690,6 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>  EXPORT_SYMBOL_GPL(irq_find_mapping);
>  
>  /**
> - * irq_radix_revmap_lookup() - Find a linux irq from a hw irq number.
> - * @domain: domain owning this hardware interrupt
> - * @hwirq: hardware irq number in that domain space
> - *
> - * This is a fast path, for use by irq controller code that uses radix tree
> - * revmaps
> - */
> -unsigned int irq_radix_revmap_lookup(struct irq_domain *domain,
> -				     irq_hw_number_t hwirq)
> -{
> -	struct irq_data *irq_data;
> -
> -	if (WARN_ON_ONCE(domain->revmap_type != IRQ_DOMAIN_MAP_TREE))
> -		return irq_find_mapping(domain, hwirq);
> -
> -	/*
> -	 * Freeing an irq can delete nodes along the path to
> -	 * do the lookup via call_rcu.
> -	 */
> -	rcu_read_lock();
> -	irq_data = radix_tree_lookup(&domain->revmap_data.tree, hwirq);
> -	rcu_read_unlock();
> -
> -	/*
> -	 * If found in radix tree, then fine.
> -	 * Else fallback to linear lookup - this should not happen in practice
> -	 * as it means that we failed to insert the node in the radix tree.
> -	 */
> -	return irq_data ? irq_data->irq : irq_find_mapping(domain, hwirq);
> -}
> -EXPORT_SYMBOL_GPL(irq_radix_revmap_lookup);
> -
> -/**
> - * irq_radix_revmap_insert() - Insert a hw irq to linux irq number mapping.
> - * @domain: domain owning this hardware interrupt
> - * @virq: linux irq number
> - * @hwirq: hardware irq number in that domain space
> - *
> - * This is for use by irq controllers that use a radix tree reverse
> - * mapping for fast lookup.
> - */
> -void irq_radix_revmap_insert(struct irq_domain *domain, unsigned int virq,
> -			     irq_hw_number_t hwirq)
> -{
> -	struct irq_data *irq_data = irq_get_irq_data(virq);
> -
> -	if (WARN_ON(domain->revmap_type != IRQ_DOMAIN_MAP_TREE))
> -		return;
> -
> -	if (virq) {
> -		mutex_lock(&revmap_trees_mutex);
> -		radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
> -		mutex_unlock(&revmap_trees_mutex);
> -	}
> -}
> -EXPORT_SYMBOL_GPL(irq_radix_revmap_insert);
> -
> -/**
>   * irq_linear_revmap() - Find a linux irq from a hw irq number.
>   * @domain: domain owning this hardware interrupt
>   * @hwirq: hardware irq number in that domain space

Cheers,
Ben.



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/12] irqdomain: Split disassociating code into separate function
  2012-06-16  5:01 ` [PATCH 01/12] irqdomain: Split disassociating code into separate function Grant Likely
@ 2012-06-16  5:57   ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 29+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-16  5:57 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> This patch moves the irq disassociation code out into a separate
> function in preparation to extend irq_setup_virq to handle multiple
> irqs and rename it for use by interrupt controller drivers.  The new
> function will be used by irq_setup_virq() in its error path.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Paul Mundt <lethal@linux-sh.org>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  kernel/irq/irqdomain.c |   75 ++++++++++++++++++++++++++++++------------------
>  1 file changed, 47 insertions(+), 28 deletions(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index b1f774c..4161d2a 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -333,6 +333,52 @@ void irq_set_default_host(struct irq_domain *domain)
>  }
>  EXPORT_SYMBOL_GPL(irq_set_default_host);
>  
> +static void irq_domain_disassociate_many(struct irq_domain *domain,
> +					 unsigned int irq_base, int count)
> +{
> +	/*
> +	 * disassociate in reverse order;
> +	 * not strictly necessary, but nice for unwinding
> +	 */
> +	while (count--) {
> +		int irq = irq_base + count;
> +		struct irq_data *irq_data = irq_get_irq_data(irq);
> +		irq_hw_number_t hwirq = irq_data->hwirq;
> +
> +		if (WARN_ON(!irq_data || irq_data->domain != domain))
> +			continue;
> +
> +		irq_set_status_flags(irq, IRQ_NOREQUEST);
> +
> +		/* remove chip and handler */
> +		irq_set_chip_and_handler(irq, NULL, NULL);
> +
> +		/* Make sure it's completed */
> +		synchronize_irq(irq);
> +
> +		/* Tell the PIC about it */
> +		if (domain->ops->unmap)
> +			domain->ops->unmap(domain, irq);
> +		smp_mb();
> +
> +		irq_data->domain = NULL;
> +		irq_data->hwirq = 0;
> +
> +		/* Clear reverse map */
> +		switch(domain->revmap_type) {
> +		case IRQ_DOMAIN_MAP_LINEAR:
> +			if (hwirq < domain->revmap_data.linear.size)
> +				domain->revmap_data.linear.revmap[hwirq] = 0;
> +			break;
> +		case IRQ_DOMAIN_MAP_TREE:
> +			mutex_lock(&revmap_trees_mutex);
> +			radix_tree_delete(&domain->revmap_data.tree, hwirq);
> +			mutex_unlock(&revmap_trees_mutex);
> +			break;
> +		}
> +	}
> +}
> +
>  static int irq_setup_virq(struct irq_domain *domain, unsigned int virq,
>  			    irq_hw_number_t hwirq)
>  {
> @@ -513,7 +559,6 @@ void irq_dispose_mapping(unsigned int virq)
>  {
>  	struct irq_data *irq_data = irq_get_irq_data(virq);
>  	struct irq_domain *domain;
> -	irq_hw_number_t hwirq;
>  
>  	if (!virq || !irq_data)
>  		return;
> @@ -526,33 +571,7 @@ void irq_dispose_mapping(unsigned int virq)
>  	if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
>  		return;
>  
> -	irq_set_status_flags(virq, IRQ_NOREQUEST);
> -
> -	/* remove chip and handler */
> -	irq_set_chip_and_handler(virq, NULL, NULL);
> -
> -	/* Make sure it's completed */
> -	synchronize_irq(virq);
> -
> -	/* Tell the PIC about it */
> -	if (domain->ops->unmap)
> -		domain->ops->unmap(domain, virq);
> -	smp_mb();
> -
> -	/* Clear reverse map */
> -	hwirq = irq_data->hwirq;
> -	switch(domain->revmap_type) {
> -	case IRQ_DOMAIN_MAP_LINEAR:
> -		if (hwirq < domain->revmap_data.linear.size)
> -			domain->revmap_data.linear.revmap[hwirq] = 0;
> -		break;
> -	case IRQ_DOMAIN_MAP_TREE:
> -		mutex_lock(&revmap_trees_mutex);
> -		radix_tree_delete(&domain->revmap_data.tree, hwirq);
> -		mutex_unlock(&revmap_trees_mutex);
> -		break;
> -	}
> -
> +	irq_domain_disassociate_many(domain, virq, 1);
>  	irq_free_desc(virq);
>  }
>  EXPORT_SYMBOL_GPL(irq_dispose_mapping);



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 02/12] irqdomain: Always update revmap when setting up a virq
  2012-06-16  5:01 ` [PATCH 02/12] irqdomain: Always update revmap when setting up a virq Grant Likely
@ 2012-06-16  5:57   ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 29+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-16  5:57 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> At irq_setup_virq() time all of the data needed to update the reverse
> map is available, but the current code ignores it and relies upon the
> slow path to insert revmap records.  This patch adds revmap updating
> to the setup path so the slow path will no longer be necessary.

See my comments in patch 4...

Cheers,
Ben.

> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  kernel/irq/irqdomain.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 4161d2a..5c36722 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -393,6 +393,16 @@ static int irq_setup_virq(struct irq_domain *domain, unsigned int virq,
>  		return -1;
>  	}
>  
> +	switch (domain->revmap_type) {
> +	case IRQ_DOMAIN_MAP_LINEAR:
> +		if (hwirq < domain->revmap_data.linear.size)
> +			domain->revmap_data.linear.revmap[hwirq] = virq;
> +		break;
> +	case IRQ_DOMAIN_MAP_TREE:
> +		irq_radix_revmap_insert(domain, virq, hwirq);
> +		break;
> +	}
> +
>  	irq_clear_status_flags(virq, IRQ_NOREQUEST);
>  
>  	return 0;



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/12] irqdomain: Support for static IRQ mapping and association.
  2012-06-16  5:01 ` [PATCH 03/12] irqdomain: Support for static IRQ mapping and association Grant Likely
@ 2012-06-16  5:58   ` Benjamin Herrenschmidt
  2012-06-17 22:16     ` Grant Likely
  0 siblings, 1 reply; 29+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-16  5:58 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> From: Paul Mundt <lethal@linux-sh.org>
> 
> This adds a new strict mapping API for supporting creation of linux IRQs
> at existing positions within the domain. The new routines are as follows:
> 
> For dynamic allocation and insertion to specified ranges:
> 
> 	- irq_create_identity_mapping()
> 	- irq_create_strict_mappings()

How does that differ from NOMAP ? Any reason to add that rather than use
NOMAP and some offset built into the PIC driver ?

Cheers,
Ben.

> These will allocate and associate a range of linux IRQs at the specified
> location. This can be used by controllers that have their own static linux IRQ
> definitions to map a hwirq range to, as well as for platforms that wish to
> establish 1:1 identity mapping between linux and hwirq space.
> 
> For insertion to specified ranges by platforms that do their own irq_desc
> management:
> 
> 	- irq_domain_associate()
> 	- irq_domain_associate_many()
> 
> These in turn call back in to the domain's ->map() routine, for further
> processing by the platform. Disassociation of IRQs get handled through
> irq_dispose_mapping() as normal.
> 
> With these in place it should be possible to begin migration of legacy IRQ
> domains to linear ones, without requiring special handling for static vs
> dynamic IRQ definitions in DT vs non-DT paths. This also makes it possible
> for domains with static mappings to adopt whichever tree model best fits
> their needs, rather than simply restricting them to linear revmaps.
> 
> Signed-off-by: Paul Mundt <lethal@linux-sh.org>
> [grant.likely: Reorganized irq_domain_associate{,_many} to have all logic in one place]
> [grant.likely: Add error checking for unallocated irq_descs at associate time]
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  include/linux/irqdomain.h |   19 ++++++++
>  kernel/irq/irqdomain.c    |  106 +++++++++++++++++++++++++++++++++++----------
>  2 files changed, 102 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 5abb533..3425631 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -144,12 +144,31 @@ static inline struct irq_domain *irq_domain_add_legacy_isa(
>  
>  extern void irq_domain_remove(struct irq_domain *host);
>  
> +extern int irq_domain_associate_many(struct irq_domain *domain,
> +				     unsigned int irq_base,
> +				     irq_hw_number_t hwirq_base, int count);
> +static inline int irq_domain_associate(struct irq_domain *domain, unsigned int irq,
> +					irq_hw_number_t hwirq)
> +{
> +	return irq_domain_associate_many(domain, irq, hwirq, 1);
> +}
> +
>  extern unsigned int irq_create_mapping(struct irq_domain *host,
>  				       irq_hw_number_t hwirq);
>  extern void irq_dispose_mapping(unsigned int virq);
>  extern unsigned int irq_find_mapping(struct irq_domain *host,
>  				     irq_hw_number_t hwirq);
>  extern unsigned int irq_create_direct_mapping(struct irq_domain *host);
> +extern int irq_create_strict_mappings(struct irq_domain *domain,
> +				      unsigned int irq_base,
> +				      irq_hw_number_t hwirq_base, int count);
> +
> +static inline int irq_create_identity_mapping(struct irq_domain *host,
> +					      irq_hw_number_t hwirq)
> +{
> +	return irq_create_strict_mappings(host, hwirq, hwirq, 1);
> +}
> +
>  extern void irq_radix_revmap_insert(struct irq_domain *host, unsigned int virq,
>  				    irq_hw_number_t hwirq);
>  extern unsigned int irq_radix_revmap_lookup(struct irq_domain *host,
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 5c36722..1a8f3d2 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -379,34 +379,56 @@ static void irq_domain_disassociate_many(struct irq_domain *domain,
>  	}
>  }
>  
> -static int irq_setup_virq(struct irq_domain *domain, unsigned int virq,
> -			    irq_hw_number_t hwirq)
> +int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
> +			      irq_hw_number_t hwirq_base, int count)
>  {
> -	struct irq_data *irq_data = irq_get_irq_data(virq);
> +	unsigned int virq = irq_base;
> +	irq_hw_number_t hwirq = hwirq_base;
> +	int i;
>  
> -	irq_data->hwirq = hwirq;
> -	irq_data->domain = domain;
> -	if (domain->ops->map(domain, virq, hwirq)) {
> -		pr_debug("irq-%i==>hwirq-0x%lx mapping failed\n", virq, hwirq);
> -		irq_data->domain = NULL;
> -		irq_data->hwirq = 0;
> -		return -1;
> -	}
> +	for (i = 0; i < count; i++) {
> +		struct irq_data *irq_data = irq_get_irq_data(virq + i);
>  
> -	switch (domain->revmap_type) {
> -	case IRQ_DOMAIN_MAP_LINEAR:
> -		if (hwirq < domain->revmap_data.linear.size)
> -			domain->revmap_data.linear.revmap[hwirq] = virq;
> -		break;
> -	case IRQ_DOMAIN_MAP_TREE:
> -		irq_radix_revmap_insert(domain, virq, hwirq);
> -		break;
> -	}
> +		if (WARN(!irq_data, "error: irq_desc not allocated; "
> +			 "irq=%i hwirq=0x%x\n", virq + i, (int)hwirq + i))
> +			return -EINVAL;
> +		if (WARN(irq_data->domain, "error: irq_desc already associated; "
> +			 "irq=%i hwirq=0x%x\n", virq + i, (int)hwirq + i))
> +			return -EINVAL;
> +	};
>  
> -	irq_clear_status_flags(virq, IRQ_NOREQUEST);
> +	for (i = 0; i < count; i++, virq++, hwirq++) {
> +		struct irq_data *irq_data = irq_get_irq_data(virq);
> +
> +		irq_data->hwirq = hwirq;
> +		irq_data->domain = domain;
> +		if (domain->ops->map(domain, virq, hwirq)) {
> +			pr_debug("irq-%i==>hwirq-0x%lx mapping failed\n", virq, hwirq);
> +			irq_data->domain = NULL;
> +			irq_data->hwirq = 0;
> +			goto err_unmap;
> +		}
> +
> +		switch (domain->revmap_type) {
> +		case IRQ_DOMAIN_MAP_LINEAR:
> +			if (hwirq < domain->revmap_data.linear.size)
> +				domain->revmap_data.linear.revmap[hwirq] = virq;
> +			break;
> +		case IRQ_DOMAIN_MAP_TREE:
> +			irq_radix_revmap_insert(domain, virq, hwirq);
> +			break;
> +		}
> +
> +		irq_clear_status_flags(virq, IRQ_NOREQUEST);
> +	}
>  
>  	return 0;
> +
> + err_unmap:
> +	irq_domain_disassociate_many(domain, irq_base, i);
> +	return -EINVAL;
>  }
> +EXPORT_SYMBOL_GPL(irq_domain_associate_many);
>  
>  /**
>   * irq_create_direct_mapping() - Allocate an irq for direct mapping
> @@ -439,7 +461,7 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
>  	}
>  	pr_debug("create_direct obtained virq %d\n", virq);
>  
> -	if (irq_setup_virq(domain, virq, virq)) {
> +	if (irq_domain_associate(domain, virq, virq)) {
>  		irq_free_desc(virq);
>  		return 0;
>  	}
> @@ -500,7 +522,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
>  		return 0;
>  	}
>  
> -	if (irq_setup_virq(domain, virq, hwirq)) {
> +	if (irq_domain_associate(domain, virq, hwirq)) {
>  		irq_free_desc(virq);
>  		return 0;
>  	}
> @@ -512,6 +534,44 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
>  }
>  EXPORT_SYMBOL_GPL(irq_create_mapping);
>  
> +/**
> + * irq_create_strict_mappings() - Map a range of hw irqs to fixed linux irqs
> + * @domain: domain owning the interrupt range
> + * @irq_base: beginning of linux IRQ range
> + * @hwirq_base: beginning of hardware IRQ range
> + * @count: Number of interrupts to map
> + *
> + * This routine is used for allocating and mapping a range of hardware
> + * irqs to linux irqs where the linux irq numbers are at pre-defined
> + * locations. For use by controllers that already have static mappings
> + * to insert in to the domain.
> + *
> + * Non-linear users can use irq_create_identity_mapping() for IRQ-at-a-time
> + * domain insertion.
> + *
> + * 0 is returned upon success, while any failure to establish a static
> + * mapping is treated as an error.
> + */
> +int irq_create_strict_mappings(struct irq_domain *domain, unsigned int irq_base,
> +			       irq_hw_number_t hwirq_base, int count)
> +{
> +	int ret;
> +
> +	ret = irq_alloc_descs(irq_base, irq_base, count,
> +			      of_node_to_nid(domain->of_node));
> +	if (unlikely(ret < 0))
> +		return ret;
> +
> +	ret = irq_domain_associate_many(domain, irq_base, hwirq_base, count);
> +	if (unlikely(ret < 0)) {
> +		irq_free_descs(irq_base, count);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(irq_create_strict_mappings);
> +
>  unsigned int irq_create_of_mapping(struct device_node *controller,
>  				   const u32 *intspec, unsigned int intsize)
>  {



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 07/12] irqdomain: Make ops->map hook optional
  2012-06-16  5:01 ` [PATCH 07/12] irqdomain: Make ops->map hook optional Grant Likely
@ 2012-06-16  5:59   ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 29+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-16  5:59 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> There isn't a really compelling reason to force ->map to be populated,
> so allow it to be left unset.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Paul Mundt <lethal@linux-sh.org>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  kernel/irq/irqdomain.c |   14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 00c383b..e88a7b0 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -205,7 +205,8 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
>  		 * one can then use irq_create_mapping() to
>  		 * explicitly change them
>  		 */
> -		ops->map(domain, irq, hwirq);
> +		if (ops->map)
> +			ops->map(domain, irq, hwirq);
>  
>  		/* Clear norequest flags */
>  		irq_clear_status_flags(irq, IRQ_NOREQUEST);
> @@ -402,8 +403,8 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
>  
>  		irq_data->hwirq = hwirq;
>  		irq_data->domain = domain;
> -		if (domain->ops->map(domain, virq, hwirq)) {
> -			pr_debug("irq-%i==>hwirq-0x%lx mapping failed\n", virq, hwirq);
> +		if (domain->ops->map && domain->ops->map(domain, virq, hwirq)) {
> +			pr_err("irq-%i==>hwirq-0x%lx mapping failed\n", virq, hwirq);
>  			irq_data->domain = NULL;
>  			irq_data->hwirq = 0;
>  			goto err_unmap;
> @@ -783,12 +784,6 @@ static int __init irq_debugfs_init(void)
>  __initcall(irq_debugfs_init);
>  #endif /* CONFIG_IRQ_DOMAIN_DEBUG */
>  
> -static int irq_domain_simple_map(struct irq_domain *d, unsigned int irq,
> -				 irq_hw_number_t hwirq)
> -{
> -	return 0;
> -}
> -
>  /**
>   * irq_domain_xlate_onecell() - Generic xlate for direct one cell bindings
>   *
> @@ -851,7 +846,6 @@ int irq_domain_xlate_onetwocell(struct irq_domain *d,
>  EXPORT_SYMBOL_GPL(irq_domain_xlate_onetwocell);
>  
>  const struct irq_domain_ops irq_domain_simple_ops = {
> -	.map = irq_domain_simple_map,
>  	.xlate = irq_domain_xlate_onetwocell,
>  };
>  EXPORT_SYMBOL_GPL(irq_domain_simple_ops);



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR
  2012-06-16  5:01 ` [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR Grant Likely
@ 2012-06-16  6:01   ` Benjamin Herrenschmidt
  2012-06-16  6:16     ` Grant Likely
  0 siblings, 1 reply; 29+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-16  6:01 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> The LEGACY mapping unnecessarily complicates the irqdomain code and
> can easily be implemented with a linear mapping.  By ripping it out
> and replacing it with the LINEAR mapping the object size of
> irqdomain.c shrinks by about 330 bytes (ARMv7) which offsets the
> additional allocation required by the linear map.  It also makes it
> possible for current LEGACY map users to pre-allocate irq_descs for a
> subset of the hwirqs and dynamically allocate the rest as needed.

The point of legacy was to reserve 0...15, they could only be handed out
to that controller, that guarantees that ancient crap x86 drivers with
hard coded irq numbers would fail ... unless you have an x86-style
chipset (PReP, some CHRP, ...).

I'd like to keep that functionality a way or another.

Cheers,
Ben.


> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  arch/arm/plat-versatile/fpga-irq.c |    2 +-
>  include/linux/irqdomain.h          |    5 ---
>  kernel/irq/irqdomain.c             |   82 +++---------------------------------
>  3 files changed, 7 insertions(+), 82 deletions(-)
> 
> diff --git a/arch/arm/plat-versatile/fpga-irq.c b/arch/arm/plat-versatile/fpga-irq.c
> index 6e70d03..05d029a 100644
> --- a/arch/arm/plat-versatile/fpga-irq.c
> +++ b/arch/arm/plat-versatile/fpga-irq.c
> @@ -111,7 +111,7 @@ static int fpga_irqdomain_map(struct irq_domain *d, unsigned int irq,
>  
>  	/* Skip invalid IRQs, only register handlers for the real ones */
>  	if (!(f->valid & (1 << hwirq)))
> -		return -ENOTSUPP;
> +		return 0;
>  	irq_set_chip_data(irq, f);
>  	irq_set_chip_and_handler(irq, &f->chip,
>  				handle_level_irq);
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index d8b88c5..b46a551 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -91,11 +91,6 @@ struct irq_domain {
>  	union {
>  		struct {
>  			unsigned int size;
> -			unsigned int first_irq;
> -			irq_hw_number_t first_hwirq;
> -		} legacy;
> -		struct {
> -			unsigned int size;
>  			unsigned int *revmap;
>  		} linear;
>  		struct {
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index e88a7b0..d6d0de0 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -16,8 +16,6 @@
>  #include <linux/smp.h>
>  #include <linux/fs.h>
>  
> -#define IRQ_DOMAIN_MAP_LEGACY 0 /* driver allocated fixed range of irqs.
> -				 * ie. legacy 8259, gets irqs 1..15 */
>  #define IRQ_DOMAIN_MAP_NOMAP 1 /* no fast reverse mapping */
>  #define IRQ_DOMAIN_MAP_LINEAR 2 /* linear map of interrupts */
>  #define IRQ_DOMAIN_MAP_TREE 3 /* radix tree */
> @@ -88,13 +86,6 @@ void irq_domain_remove(struct irq_domain *domain)
>  	mutex_lock(&irq_domain_mutex);
>  
>  	switch (domain->revmap_type) {
> -	case IRQ_DOMAIN_MAP_LEGACY:
> -		/*
> -		 * Legacy domains don't manage their own irq_desc
> -		 * allocations, we expect the caller to handle irq_desc
> -		 * freeing on their own.
> -		 */
> -		break;
>  	case IRQ_DOMAIN_MAP_TREE:
>  		/*
>  		 * radix_tree_delete() takes care of destroying the root
> @@ -128,17 +119,6 @@ void irq_domain_remove(struct irq_domain *domain)
>  }
>  EXPORT_SYMBOL_GPL(irq_domain_remove);
>  
> -static unsigned int irq_domain_legacy_revmap(struct irq_domain *domain,
> -					     irq_hw_number_t hwirq)
> -{
> -	irq_hw_number_t first_hwirq = domain->revmap_data.legacy.first_hwirq;
> -	int size = domain->revmap_data.legacy.size;
> -
> -	if (WARN_ON(hwirq < first_hwirq || hwirq >= first_hwirq + size))
> -		return 0;
> -	return hwirq - first_hwirq + domain->revmap_data.legacy.first_irq;
> -}
> -
>  /**
>   * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
>   * @of_node: pointer to interrupt controller's device tree node.
> @@ -162,57 +142,17 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
>  					 void *host_data)
>  {
>  	struct irq_domain *domain;
> -	unsigned int i;
>  
> -	domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LEGACY, ops, host_data);
> +	pr_debug("Setting up legacy domain virq[%i:%i] ==> hwirq[%i:%i]\n",
> +		 first_irq, first_irq + size - 1,
> +		 (int)first_hwirq, (int)first_hwirq + size -1);
> +
> +	domain = irq_domain_add_linear(of_node, first_hwirq + size, ops, host_data);
>  	if (!domain)
>  		return NULL;
>  
> -	domain->revmap_data.legacy.first_irq = first_irq;
> -	domain->revmap_data.legacy.first_hwirq = first_hwirq;
> -	domain->revmap_data.legacy.size = size;
> -
> -	mutex_lock(&irq_domain_mutex);
> -	/* Verify that all the irqs are available */
> -	for (i = 0; i < size; i++) {
> -		int irq = first_irq + i;
> -		struct irq_data *irq_data = irq_get_irq_data(irq);
> -
> -		if (WARN_ON(!irq_data || irq_data->domain)) {
> -			mutex_unlock(&irq_domain_mutex);
> -			irq_domain_free(domain);
> -			return NULL;
> -		}
> -	}
> -
> -	/* Claim all of the irqs before registering a legacy domain */
> -	for (i = 0; i < size; i++) {
> -		struct irq_data *irq_data = irq_get_irq_data(first_irq + i);
> -		irq_data->hwirq = first_hwirq + i;
> -		irq_data->domain = domain;
> -	}
> -	mutex_unlock(&irq_domain_mutex);
> -
> -	for (i = 0; i < size; i++) {
> -		int irq = first_irq + i;
> -		int hwirq = first_hwirq + i;
> -
> -		/* IRQ0 gets ignored */
> -		if (!irq)
> -			continue;
> +	WARN_ON(irq_domain_associate_many(domain, first_irq, first_hwirq, size));
>  
> -		/* Legacy flags are left to default at this point,
> -		 * one can then use irq_create_mapping() to
> -		 * explicitly change them
> -		 */
> -		if (ops->map)
> -			ops->map(domain, irq, hwirq);
> -
> -		/* Clear norequest flags */
> -		irq_clear_status_flags(irq, IRQ_NOREQUEST);
> -	}
> -
> -	irq_domain_add(domain);
>  	return domain;
>  }
>  EXPORT_SYMBOL_GPL(irq_domain_add_legacy);
> @@ -509,10 +449,6 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
>  		return virq;
>  	}
>  
> -	/* Get a virtual interrupt number */
> -	if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
> -		return irq_domain_legacy_revmap(domain, hwirq);
> -
>  	/* Allocate a virtual interrupt number */
>  	hint = hwirq % nr_irqs;
>  	if (hint == 0)
> @@ -640,10 +576,6 @@ void irq_dispose_mapping(unsigned int virq)
>  	if (WARN_ON(domain == NULL))
>  		return;
>  
> -	/* Never unmap legacy interrupts */
> -	if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
> -		return;
> -
>  	irq_domain_disassociate_many(domain, virq, 1);
>  	irq_free_desc(virq);
>  }
> @@ -666,8 +598,6 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>  		return 0;
>  
>  	switch (domain->revmap_type) {
> -	case IRQ_DOMAIN_MAP_LEGACY:
> -		return irq_domain_legacy_revmap(domain, hwirq);
>  	case IRQ_DOMAIN_MAP_LINEAR:
>  		return irq_linear_revmap(domain, hwirq);
>  	case IRQ_DOMAIN_MAP_TREE:



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 10/12] irqdomain: Add debugging message
  2012-06-16  5:01 ` [PATCH 10/12] irqdomain: Add debugging message Grant Likely
@ 2012-06-16  6:02   ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 29+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-16  6:02 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> Trivial patch to beef up debugging messages on irqdomain.c
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Paul Mundt <lethal@linux-sh.org>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  kernel/irq/irqdomain.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index c0a00de..0e02a1d 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -334,6 +334,9 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
>  	irq_hw_number_t hwirq = hwirq_base;
>  	int i;
>  
> +	pr_debug("%s(%s, irqbase=%i, hwbase=%i, count=%i)\n", __func__,
> +		of_node_full_name(domain->of_node), irq_base, (int)hwirq_base, count);
> +
>  	for (i = 0; i < count; i++) {
>  		struct irq_data *irq_data = irq_get_irq_data(virq + i);
>  



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 11/12] irqdomain: reorganize revmap data.
  2012-06-16  5:01 ` [PATCH 11/12] irqdomain: reorganize revmap data Grant Likely
@ 2012-06-16  6:06   ` Benjamin Herrenschmidt
  2012-06-16  6:19     ` Grant Likely
  0 siblings, 1 reply; 29+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-16  6:06 UTC (permalink / raw)
  To: Grant Likely
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> In struct irq_domain, reorganize the revmap data in preparation for
> unifying the linear and radix mappings.

I fail to understand the patch itself... ie:

> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  drivers/pinctrl/pinctrl-nomadik.c |    4 +--
>  include/linux/irqdomain.h         |   17 ++++-------
>  kernel/irq/irqdomain.c            |   58 ++++++++++++++-----------------------
>  3 files changed, 29 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> index b26395d..22fee45 100644
> --- a/drivers/pinctrl/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/pinctrl-nomadik.c
> @@ -826,16 +826,14 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
>  {
>  	struct nmk_gpio_chip *nmk_chip;
>  	struct irq_chip *host_chip = irq_get_chip(irq);
> -	unsigned int first_irq;
>  
>  	chained_irq_enter(host_chip, desc);
>  
>  	nmk_chip = irq_get_handler_data(irq);
> -	first_irq = nmk_chip->domain->revmap_data.legacy.first_irq;
>  	while (status) {
>  		int bit = __ffs(status);
>  
> -		generic_handle_irq(first_irq + bit);
> +		generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
>  		status &= ~BIT(bit);
>  	}

That doesn't seem related to either the subject or the description
(though it looks like something valid to do per-se). Maybe belongs to a
separate patch ?
 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index b46a551..ee90765 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -71,7 +71,6 @@ struct irq_domain_ops {
>   * @link: Element in global irq_domain list.
>   * @revmap_type: Method used for reverse mapping hwirq numbers to linux irq. This
>   *               will be one of the IRQ_DOMAIN_MAP_* values.
> - * @revmap_data: Revmap method specific data.
>   * @ops: pointer to irq_domain methods
>   * @host_data: private data pointer for use by owner.  Not touched by irq_domain
>   *             core code.
> @@ -88,22 +87,18 @@ struct irq_domain {
>  
>  	/* type of reverse mapping_technique */
>  	unsigned int revmap_type;
> -	union {
> -		struct {
> -			unsigned int size;
> -			unsigned int *revmap;
> -		} linear;
> -		struct {
> -			unsigned int max_irq;
> -		} nomap;
> -		struct radix_tree_root tree;
> -	} revmap_data;
>  	const struct irq_domain_ops *ops;
>  	void *host_data;
>  	irq_hw_number_t inval_irq;
>  
>  	/* Optional device node pointer */
>  	struct device_node *of_node;
> +
> +	/* Reverse mapping data */
> +	unsigned int nomap_max_irq;
> +	struct radix_tree_root radix_tree;
> +	unsigned int linear_size;
> +	unsigned int linear_revmap[];
>  };

Hrm, I'm thinking maybe you should just merge those changes with patch
12/12 ... the moving around of the fields in a first patch doesn't seem
to clarify this from my perspective at least.

Cheers,
Ben.

>  #ifdef CONFIG_IRQ_DOMAIN
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 0e02a1d..433971a 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -38,14 +38,14 @@ static struct irq_domain *irq_default_domain;
>   * to IRQ domain, or NULL on failure.
>   */
>  static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
> -					   unsigned int revmap_type,
> +					   unsigned int revmap_type, int size,
>  					   const struct irq_domain_ops *ops,
>  					   void *host_data)
>  {
>  	struct irq_domain *domain;
>  
> -	domain = kzalloc_node(sizeof(*domain), GFP_KERNEL,
> -			      of_node_to_nid(of_node));
> +	domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
> +			      GFP_KERNEL, of_node_to_nid(of_node));
>  	if (WARN_ON(!domain))
>  		return NULL;
>  
> @@ -54,6 +54,7 @@ static struct irq_domain *irq_domain_alloc(struct device_node *of_node,
>  	domain->ops = ops;
>  	domain->host_data = host_data;
>  	domain->of_node = of_node_get(of_node);
> +	domain->linear_size = size;
>  
>  	return domain;
>  }
> @@ -92,13 +93,7 @@ void irq_domain_remove(struct irq_domain *domain)
>  		 * node when all entries are removed. Shout if there are
>  		 * any mappings left.
>  		 */
> -		WARN_ON(domain->revmap_data.tree.height);
> -		break;
> -	case IRQ_DOMAIN_MAP_LINEAR:
> -		kfree(domain->revmap_data.linear.revmap);
> -		domain->revmap_data.linear.size = 0;
> -		break;
> -	case IRQ_DOMAIN_MAP_NOMAP:
> +		WARN_ON(domain->radix_tree.height);
>  		break;
>  	}

> @@ -177,20 +172,11 @@ struct irq_domain *irq_domain_add_linear(struct device_node *of_node,
>  					 void *host_data)
>  {
>  	struct irq_domain *domain;
> -	unsigned int *revmap;
>  
> -	revmap = kzalloc_node(sizeof(*revmap) * size, GFP_KERNEL,
> -			      of_node_to_nid(of_node));
> -	if (WARN_ON(!revmap))
> +	domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LINEAR, size, ops, host_data);
> +	if (!domain)
>  		return NULL;
>  
> -	domain = irq_domain_alloc(of_node, IRQ_DOMAIN_MAP_LINEAR, ops, host_data);
> -	if (!domain) {
> -		kfree(revmap);
> -		return NULL;
> -	}
> -	domain->revmap_data.linear.size = size;
> -	domain->revmap_data.linear.revmap = revmap;
>  	irq_domain_add(domain);
>  	return domain;
>  }
> @@ -202,9 +188,9 @@ struct irq_domain *irq_domain_add_nomap(struct device_node *of_node,
>  					 void *host_data)
>  {
>  	struct irq_domain *domain = irq_domain_alloc(of_node,
> -					IRQ_DOMAIN_MAP_NOMAP, ops, host_data);
> +					IRQ_DOMAIN_MAP_NOMAP, 0, ops, host_data);
>  	if (domain) {
> -		domain->revmap_data.nomap.max_irq = max_irq ? max_irq : ~0;
> +		domain->nomap_max_irq = max_irq ? max_irq : ~0;
>  		irq_domain_add(domain);
>  	}
>  	return domain;
> @@ -224,9 +210,9 @@ struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
>  					 void *host_data)
>  {
>  	struct irq_domain *domain = irq_domain_alloc(of_node,
> -					IRQ_DOMAIN_MAP_TREE, ops, host_data);
> +					IRQ_DOMAIN_MAP_TREE, 0, ops, host_data);
>  	if (domain) {
> -		INIT_RADIX_TREE(&domain->revmap_data.tree, GFP_KERNEL);
> +		INIT_RADIX_TREE(&domain->radix_tree, GFP_KERNEL);
>  		irq_domain_add(domain);
>  	}
>  	return domain;
> @@ -315,12 +301,12 @@ static void irq_domain_disassociate_many(struct irq_domain *domain,
>  		/* Clear reverse map */
>  		switch(domain->revmap_type) {
>  		case IRQ_DOMAIN_MAP_LINEAR:
> -			if (hwirq < domain->revmap_data.linear.size)
> -				domain->revmap_data.linear.revmap[hwirq] = 0;
> +			if (hwirq < domain->linear_size)
> +				domain->linear_revmap[hwirq] = 0;
>  			break;
>  		case IRQ_DOMAIN_MAP_TREE:
>  			mutex_lock(&revmap_trees_mutex);
> -			radix_tree_delete(&domain->revmap_data.tree, hwirq);
> +			radix_tree_delete(&domain->radix_tree, hwirq);
>  			mutex_unlock(&revmap_trees_mutex);
>  			break;
>  		}
> @@ -362,12 +348,12 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
>  
>  		switch (domain->revmap_type) {
>  		case IRQ_DOMAIN_MAP_LINEAR:
> -			if (hwirq < domain->revmap_data.linear.size)
> -				domain->revmap_data.linear.revmap[hwirq] = virq;
> +			if (hwirq < domain->linear_size)
> +				domain->linear_revmap[hwirq] = virq;
>  			break;
>  		case IRQ_DOMAIN_MAP_TREE:
>  			mutex_lock(&revmap_trees_mutex);
> -			radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
> +			radix_tree_insert(&domain->radix_tree, hwirq, irq_data);
>  			mutex_unlock(&revmap_trees_mutex);
>  			break;
>  		}
> @@ -406,9 +392,9 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
>  		pr_debug("create_direct virq allocation failed\n");
>  		return 0;
>  	}
> -	if (virq >= domain->revmap_data.nomap.max_irq) {
> +	if (virq >= domain->nomap_max_irq) {
>  		pr_err("ERROR: no free irqs available below %i maximum\n",
> -			domain->revmap_data.nomap.max_irq);
> +			domain->nomap_max_irq);
>  		irq_free_desc(virq);
>  		return 0;
>  	}
> @@ -612,7 +598,7 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>  		return irq_linear_revmap(domain, hwirq);
>  	case IRQ_DOMAIN_MAP_TREE:
>  		rcu_read_lock();
> -		data = radix_tree_lookup(&domain->revmap_data.tree, hwirq);
> +		data = radix_tree_lookup(&domain->radix_tree, hwirq);
>  		rcu_read_unlock();
>  		if (data)
>  			return data->irq;
> @@ -644,10 +630,10 @@ unsigned int irq_linear_revmap(struct irq_domain *domain,
>  	BUG_ON(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR);
>  
>  	/* Check revmap bounds; complain if exceeded */
> -	if (WARN_ON(hwirq >= domain->revmap_data.linear.size))
> +	if (WARN_ON(hwirq >= domain->linear_size))
>  		return 0;
>  
> -	return domain->revmap_data.linear.revmap[hwirq];
> +	return domain->linear_revmap[hwirq];
>  }
>  EXPORT_SYMBOL_GPL(irq_linear_revmap);
>  



^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions
  2012-06-16  5:56   ` Benjamin Herrenschmidt
@ 2012-06-16  6:12     ` Grant Likely
  2012-06-17 21:58       ` Grant Likely
  0 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  6:12 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Fri, Jun 15, 2012 at 11:56 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
>> In preparation to remove the slow revmap path, eliminate the public
>> radix revmap lookup functions.  This simplifies the code and makes the
>> slowpath removal patch a lot simpler.
>
> The idea here was to avoid a test (performance in the fast path) since
> the controller knows the type of revmap it uses. You could just remove
> the fallback to the slow path if there's a mismatch and keep the WARN_ON
> no ? No biggie, it's just a switch/case with fairly predictable outcome
> I suppose ...

With some of the further refactoring, I'm hoping to reduce the number
of tests to even less than the original 'optimized' path did.  For
instance the linear lookup function still checked the domain type for
safety (although I think I probably added that test).  If there aren't
a whole bunch of different domain types, then the only test becomes
whether or not the hwirq is greater than the linear map size.  NOMAP
is still a wrench in the works though.

>
> A few other nits:
>
>>       return irq;
>> diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c
>> index cd1d18d..9049d9f 100644
>> --- a/arch/powerpc/sysdev/xics/xics-common.c
>> +++ b/arch/powerpc/sysdev/xics/xics-common.c
>> @@ -329,9 +329,6 @@ static int xics_host_map(struct irq_domain *h, unsigned int virq,
>>
>>       pr_devel("xics: map virq %d, hwirq 0x%lx\n", virq, hw);
>>
>> -     /* Insert the interrupt mapping into the radix tree for fast lookup */
>> -     irq_radix_revmap_insert(xics_host, virq, hw);
>> -
>>       /* They aren't all level sensitive but we just don't really know */
>>       irq_set_status_flags(virq, IRQ_LEVEL);
>
> This looks like it belongs in a different patch, possibly "[02/12]
> irqdomain: Always update revmap when setting up a virq" no ?
>
>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>> index 3425631..d8b88c5 100644
>> --- a/include/linux/irqdomain.h
>> +++ b/include/linux/irqdomain.h
>> @@ -169,10 +169,6 @@ static inline int irq_create_identity_mapping(struct irq_domain *host,
>>       return irq_create_strict_mappings(host, hwirq, hwirq, 1);
>>  }
>>
>> -extern void irq_radix_revmap_insert(struct irq_domain *host, unsigned int virq,
>> -                                 irq_hw_number_t hwirq);
>> -extern unsigned int irq_radix_revmap_lookup(struct irq_domain *host,
>> -                                         irq_hw_number_t hwirq);
>>  extern unsigned int irq_linear_revmap(struct irq_domain *host,
>>                                     irq_hw_number_t hwirq);
>>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index 1a8f3d2..79a7711 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -415,7 +415,9 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
>>                               domain->revmap_data.linear.revmap[hwirq] = virq;
>>                       break;
>>               case IRQ_DOMAIN_MAP_TREE:
>> -                     irq_radix_revmap_insert(domain, virq, hwirq);
>> +                     mutex_lock(&revmap_trees_mutex);
>> +                     radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
>> +                     mutex_unlock(&revmap_trees_mutex);
>>                       break;
>>               }
>
> That too looks like it belongs in another patch.

Okay, I'll take a look at those.

g.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR
  2012-06-16  6:01   ` Benjamin Herrenschmidt
@ 2012-06-16  6:16     ` Grant Likely
  2012-06-18 12:23       ` Mark Brown
  0 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  6:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Sat, Jun 16, 2012 at 12:01 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
>> The LEGACY mapping unnecessarily complicates the irqdomain code and
>> can easily be implemented with a linear mapping.  By ripping it out
>> and replacing it with the LINEAR mapping the object size of
>> irqdomain.c shrinks by about 330 bytes (ARMv7) which offsets the
>> additional allocation required by the linear map.  It also makes it
>> possible for current LEGACY map users to pre-allocate irq_descs for a
>> subset of the hwirqs and dynamically allocate the rest as needed.
>
> The point of legacy was to reserve 0...15, they could only be handed out
> to that controller, that guarantees that ancient crap x86 drivers with
> hard coded irq numbers would fail ... unless you have an x86-style
> chipset (PReP, some CHRP, ...).
>
> I'd like to keep that functionality a way or another.

The functionality is still entirely there; it's just implemented with
a linear map that gets populated with mappings to reserved irqs.
Everything *should* keep working correctly.

g.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 11/12] irqdomain: reorganize revmap data.
  2012-06-16  6:06   ` Benjamin Herrenschmidt
@ 2012-06-16  6:19     ` Grant Likely
  2012-06-16  6:20       ` Grant Likely
  0 siblings, 1 reply; 29+ messages in thread
From: Grant Likely @ 2012-06-16  6:19 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Sat, Jun 16, 2012 at 12:06 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
>> In struct irq_domain, reorganize the revmap data in preparation for
>> unifying the linear and radix mappings.
>
> I fail to understand the patch itself... ie:

The intention was for this patch to have zero functional change; just
move around where the data was (although it looks like I missed a
hunk), and the second be the real behaviour change.  I'm fine with
merging the patches too.

g.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 11/12] irqdomain: reorganize revmap data.
  2012-06-16  6:19     ` Grant Likely
@ 2012-06-16  6:20       ` Grant Likely
  0 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2012-06-16  6:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Sat, Jun 16, 2012 at 12:19 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Sat, Jun 16, 2012 at 12:06 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>> On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
>>> In struct irq_domain, reorganize the revmap data in preparation for
>>> unifying the linear and radix mappings.
>>
>> I fail to understand the patch itself... ie:
>
> The intention was for this patch to have zero functional change; just
> move around where the data was (although it looks like I missed a
> hunk), and the second be the real behaviour change.  I'm fine with
> merging the patches too.

Oh, and thanks for the quick review!  :-)

g.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions
  2012-06-16  6:12     ` Grant Likely
@ 2012-06-17 21:58       ` Grant Likely
  0 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2012-06-17 21:58 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]

On Sat, 16 Jun 2012 00:12:45 -0600, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Jun 15, 2012 at 11:56 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> >> @@ -329,9 +329,6 @@ static int xics_host_map(struct irq_domain *h, unsigned int virq,
> >>
> >>       pr_devel("xics: map virq %d, hwirq 0x%lx\n", virq, hw);
> >>
> >> -     /* Insert the interrupt mapping into the radix tree for fast lookup */
> >> -     irq_radix_revmap_insert(xics_host, virq, hw);
> >> -
> >>       /* They aren't all level sensitive but we just don't really know */
> >>       irq_set_status_flags(virq, IRQ_LEVEL);
> >
> > This looks like it belongs in a different patch, possibly "[02/12]
> > irqdomain: Always update revmap when setting up a virq" no ?
[...]
> >> @@ -415,7 +415,9 @@ int irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
> >>                               domain->revmap_data.linear.revmap[hwirq] = virq;
> >>                       break;
> >>               case IRQ_DOMAIN_MAP_TREE:
> >> -                     irq_radix_revmap_insert(domain, virq, hwirq);
> >> +                     mutex_lock(&revmap_trees_mutex);
> >> +                     radix_tree_insert(&domain->revmap_data.tree, hwirq, irq_data);
> >> +                     mutex_unlock(&revmap_trees_mutex);
> >>                       break;
> >>               }
> >
> > That too looks like it belongs in another patch.
> 
> Okay, I'll take a look at those.

Both moved to patch 2.

g.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/12] irqdomain: Support for static IRQ mapping and association.
  2012-06-16  5:58   ` Benjamin Herrenschmidt
@ 2012-06-17 22:16     ` Grant Likely
  0 siblings, 0 replies; 29+ messages in thread
From: Grant Likely @ 2012-06-17 22:16 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-kernel, Milton Miller, Paul Mundt, Thomas Gleixner,
	Rob Herring

On Sat, 16 Jun 2012 15:58:39 +1000, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Fri, 2012-06-15 at 23:01 -0600, Grant Likely wrote:
> > From: Paul Mundt <lethal@linux-sh.org>
> > 
> > This adds a new strict mapping API for supporting creation of linux IRQs
> > at existing positions within the domain. The new routines are as follows:
> > 
> > For dynamic allocation and insertion to specified ranges:
> > 
> > 	- irq_create_identity_mapping()
> > 	- irq_create_strict_mappings()
> 
> How does that differ from NOMAP ? Any reason to add that rather than use
> NOMAP and some offset built into the PIC driver ?

Ultimately it simplifies the code.  It allows the irq controller to
specify arbitrary ranges of hwirq->irq mappings without any special
processing.  Some of the irq controllers have multiple hwirq ranges
that need to be mapped, and this is a reasonable approach for doing so
regardless of the revmap type.

Ideally I'd rather not do any of this and have the virqs dynamically
assigned, but as long as there are still platforms relying on static
platform_data it will be required.

g.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR
  2012-06-16  6:16     ` Grant Likely
@ 2012-06-18 12:23       ` Mark Brown
  0 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-06-18 12:23 UTC (permalink / raw)
  To: Grant Likely
  Cc: Benjamin Herrenschmidt, linux-kernel, Milton Miller, Paul Mundt,
	Thomas Gleixner, Rob Herring

On Sat, Jun 16, 2012 at 12:16:32AM -0600, Grant Likely wrote:

> The functionality is still entirely there; it's just implemented with
> a linear map that gets populated with mappings to reserved irqs.
> Everything *should* keep working correctly.

I didn't see anywhere in the series where a user was updated to use this
mechanism - it might help make it clear what's going on if there was
sample code.  Since I added irqdomain support to some of my drivers I've
had several people say things like "Oh, that's actually really simple
now I see how to use it", this might be an example of that sort of
issue.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 00/12] irqdomain cleanup and refactoring
  2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
                   ` (11 preceding siblings ...)
  2012-06-16  5:01 ` [PATCH 12/12] irqdomain: merge linear and tree reverse mappings Grant Likely
@ 2012-06-18 12:28 ` Mark Brown
  12 siblings, 0 replies; 29+ messages in thread
From: Mark Brown @ 2012-06-18 12:28 UTC (permalink / raw)
  To: Grant Likely; +Cc: linux-kernel, Milton Miller

On Fri, Jun 15, 2012 at 11:01:25PM -0600, Grant Likely wrote:

> - eliminate the legacy mapping by replacing it with a pre-populated linear map,

Can we also merge the registration functions (or add something like the
irq_domain_register_simple() patch I posted) while we're at it?  With
this code I *think* users still need to implement the boiler plate to
decided if they need linear or legacy mode.

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2012-06-18 12:28 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-16  5:01 [PATCH 00/12] irqdomain cleanup and refactoring Grant Likely
2012-06-16  5:01 ` [PATCH 01/12] irqdomain: Split disassociating code into separate function Grant Likely
2012-06-16  5:57   ` Benjamin Herrenschmidt
2012-06-16  5:01 ` [PATCH 02/12] irqdomain: Always update revmap when setting up a virq Grant Likely
2012-06-16  5:57   ` Benjamin Herrenschmidt
2012-06-16  5:01 ` [PATCH 03/12] irqdomain: Support for static IRQ mapping and association Grant Likely
2012-06-16  5:58   ` Benjamin Herrenschmidt
2012-06-17 22:16     ` Grant Likely
2012-06-16  5:01 ` [PATCH 04/12] irqdomain: Eliminate dedicated radix lookup functions Grant Likely
2012-06-16  5:56   ` Benjamin Herrenschmidt
2012-06-16  6:12     ` Grant Likely
2012-06-17 21:58       ` Grant Likely
2012-06-16  5:01 ` [PATCH 05/12] irqdomain: Fix irq_create_direct_mapping() to test irq_domain type Grant Likely
2012-06-16  5:01 ` [PATCH 06/12] irqdomain: eliminate slow-path revmap lookups Grant Likely
2012-06-16  5:01 ` [PATCH 07/12] irqdomain: Make ops->map hook optional Grant Likely
2012-06-16  5:59   ` Benjamin Herrenschmidt
2012-06-16  5:01 ` [PATCH 08/12] irqdomain: Replace LEGACY mapping with LINEAR Grant Likely
2012-06-16  6:01   ` Benjamin Herrenschmidt
2012-06-16  6:16     ` Grant Likely
2012-06-18 12:23       ` Mark Brown
2012-06-16  5:01 ` [PATCH 09/12] irqdomain: Reserve IRQs for legacy domain Grant Likely
2012-06-16  5:01 ` [PATCH 10/12] irqdomain: Add debugging message Grant Likely
2012-06-16  6:02   ` Benjamin Herrenschmidt
2012-06-16  5:01 ` [PATCH 11/12] irqdomain: reorganize revmap data Grant Likely
2012-06-16  6:06   ` Benjamin Herrenschmidt
2012-06-16  6:19     ` Grant Likely
2012-06-16  6:20       ` Grant Likely
2012-06-16  5:01 ` [PATCH 12/12] irqdomain: merge linear and tree reverse mappings Grant Likely
2012-06-18 12:28 ` [PATCH 00/12] irqdomain cleanup and refactoring Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox