Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/9] Cleaning up some of the irqdomain features
@ 2021-04-06  9:35 Marc Zyngier
  2021-04-06  9:35 ` [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping() Marc Zyngier
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

The irqdomain subsystem has grown quite a lot over the years, and some
of its features are either oddly used or just pretty useless. Some
other helpers expose internals that are likely to change soon.

Here are the bits that I'm trying to get rid of:

- irq_linear_revmap exposes the internals of the domains, and only
  works for linear domains. The supposed speed improvement really
  isn't an argument, as it gets in the way of more significant
  optimisations. Reimplemented in terms of irq_find_mapping, which
  always works, and will eventually go at some point.

- irq_create_strict_mappings is just a way to constraint the
  allocation of irqdescs into a given range, which is better served by
  creating a legacy irqdomain, and shows that the platform really
  needs to catch up with the 21st century.

- irq_create_identity mapping is just a variation on the above, with
  irq==hwirq, although the way it is used is a gross hack in the SH
  code that needs to go.

- irq_domain_add_legacy_isa is, as the names shows, a variation on
  irq_domain_add_legacy with a reservation of 16 interrupts. This is
  only used in the PPC code.

The patches address all of the above, touching some of the ARM, PPC,
and SH code that is affected. Another couple of patches address a MIPS
platform that could use the generic code, and clean some of the
comments in the irqdomain code.

Unless anyone shouts, I'd like to take this into 5.13, as it is the
basis of some further (and deeper) changes in the way irqdomains work.

	M.

Marc Zyngier (9):
  irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping()
  ARM: PXA: Kill use of irq_create_strict_mappings()
  irqchip/jcore-aic: Kill use of irq_create_strict_mappings()
  sh: intc: Drop the use of irq_create_identity_mapping()
  irqdomain: Kill
    irq_create_strict_mappings()/irq_create_identity_mapping()
  mips: netlogic: Use irq_domain_simple_ops for XLP PIC
  irqdomain: Drop references to recusive irqdomain setup
  powerpc: Convert irq_domain_add_legacy_isa use to
    irq_domain_add_legacy
  irqdomain: Kill irq_domain_add_legacy_isa

 Documentation/core-api/irq/irq-domain.rst |  1 -
 arch/arm/mach-pxa/pxa_cplds_irqs.c        | 24 +++++------
 arch/mips/netlogic/common/irq.c           |  6 +--
 arch/powerpc/include/asm/irq.h            |  4 +-
 arch/powerpc/platforms/ps3/interrupt.c    |  4 +-
 arch/powerpc/sysdev/i8259.c               |  3 +-
 arch/powerpc/sysdev/mpic.c                |  2 +-
 arch/powerpc/sysdev/tsi108_pci.c          |  3 +-
 arch/powerpc/sysdev/xics/xics-common.c    |  2 +-
 drivers/irqchip/irq-jcore-aic.c           |  4 +-
 drivers/sh/intc/core.c                    | 50 ++++++++++-------------
 include/linux/irqdomain.h                 | 42 ++++---------------
 kernel/irq/irqdomain.c                    | 49 +++-------------------
 13 files changed, 59 insertions(+), 135 deletions(-)

-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping()
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  2021-04-06 11:21   ` Christophe Leroy
  2021-04-06  9:35 ` [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings() Marc Zyngier
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

irq_linear_revmap() is supposed to be a fast path for domain
lookups, but it only exposes low-level details of the irqdomain
implementation, details which are better kept private.

The *overhead* between the two is only a function call and
a couple of tests, so it is likely that noone can show any
meaningful difference compared to the cost of taking an
interrupt.

Reimplement irq_linear_revmap() with irq_find_mapping()
in order to preserve source code compatibility, and
rename the internal field for a measure.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/irqdomain.h | 22 +++++++++-------------
 kernel/irq/irqdomain.c    |  6 +++---
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 33cacc8af26d..b9600f24878a 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -154,9 +154,9 @@ struct irq_domain_chip_generic;
  * Revmap data, used internally by irq_domain
  * @revmap_direct_max_irq: The largest hwirq that can be set for controllers that
  *                         support direct mapping
- * @revmap_size: Size of the linear map table @linear_revmap[]
+ * @revmap_size: Size of the linear map table @revmap[]
  * @revmap_tree: Radix map tree for hwirqs that don't fit in the linear map
- * @linear_revmap: Linear table of hwirq->virq reverse mappings
+ * @revmap: Linear table of hwirq->virq reverse mappings
  */
 struct irq_domain {
 	struct list_head link;
@@ -180,7 +180,7 @@ struct irq_domain {
 	unsigned int revmap_size;
 	struct radix_tree_root revmap_tree;
 	struct mutex revmap_tree_mutex;
-	unsigned int linear_revmap[];
+	unsigned int revmap[];
 };
 
 /* Irq domain flags */
@@ -396,24 +396,20 @@ static inline unsigned int irq_create_mapping(struct irq_domain *host,
 	return irq_create_mapping_affinity(host, hwirq, NULL);
 }
 
-
 /**
- * irq_linear_revmap() - Find a linux irq from a hw irq number.
+ * irq_find_mapping() - 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 alternative to irq_find_mapping() that can be
- * called directly by irq controller code to save a handful of
- * instructions. It is always safe to call, but won't find irqs mapped
- * using the radix tree.
  */
+extern unsigned int irq_find_mapping(struct irq_domain *host,
+				     irq_hw_number_t hwirq);
+
 static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
 					     irq_hw_number_t hwirq)
 {
-	return hwirq < domain->revmap_size ? domain->linear_revmap[hwirq] : 0;
+	return irq_find_mapping(domain, hwirq);
 }
-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,
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d10ab1d689d5..dfa716305ea9 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -486,7 +486,7 @@ static void irq_domain_clear_mapping(struct irq_domain *domain,
 				     irq_hw_number_t hwirq)
 {
 	if (hwirq < domain->revmap_size) {
-		domain->linear_revmap[hwirq] = 0;
+		domain->revmap[hwirq] = 0;
 	} else {
 		mutex_lock(&domain->revmap_tree_mutex);
 		radix_tree_delete(&domain->revmap_tree, hwirq);
@@ -499,7 +499,7 @@ static void irq_domain_set_mapping(struct irq_domain *domain,
 				   struct irq_data *irq_data)
 {
 	if (hwirq < domain->revmap_size) {
-		domain->linear_revmap[hwirq] = irq_data->irq;
+		domain->revmap[hwirq] = irq_data->irq;
 	} else {
 		mutex_lock(&domain->revmap_tree_mutex);
 		radix_tree_insert(&domain->revmap_tree, hwirq, irq_data);
@@ -920,7 +920,7 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
 
 	/* Check if the hwirq is in the linear revmap. */
 	if (hwirq < domain->revmap_size)
-		return domain->linear_revmap[hwirq];
+		return domain->revmap[hwirq];
 
 	rcu_read_lock();
 	data = radix_tree_lookup(&domain->revmap_tree, hwirq);
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings()
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
  2021-04-06  9:35 ` [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping() Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  2021-04-26 22:39   ` Guenter Roeck
  2021-04-06  9:35 ` [PATCH 3/9] irqchip/jcore-aic: " Marc Zyngier
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

irq_create_strict_mappings() is a poor way to allow the use of
a linear IRQ domain as a legacy one. Let's be upfront about
it and use a legacy domain when appropriate.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm/mach-pxa/pxa_cplds_irqs.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c b/arch/arm/mach-pxa/pxa_cplds_irqs.c
index 45c19ca96f7a..ec0d9b094744 100644
--- a/arch/arm/mach-pxa/pxa_cplds_irqs.c
+++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c
@@ -147,22 +147,20 @@ static int cplds_probe(struct platform_device *pdev)
 	}
 
 	irq_set_irq_wake(fpga->irq, 1);
-	fpga->irqdomain = irq_domain_add_linear(pdev->dev.of_node,
-					       CPLDS_NB_IRQ,
-					       &cplds_irq_domain_ops, fpga);
+	if (base_irq)
+		fpga->irqdomain = irq_domain_add_legacy(pdev->dev.of_node,
+							CPLDS_NB_IRQ,
+							base_irq, 0,
+							&cplds_irq_domain_ops,
+							fpga);
+	else
+		fpga->irqdomain = irq_domain_add_linear(pdev->dev.of_node,
+							CPLDS_NB_IRQ,
+							&cplds_irq_domain_ops,
+							fpga);
 	if (!fpga->irqdomain)
 		return -ENODEV;
 
-	if (base_irq) {
-		ret = irq_create_strict_mappings(fpga->irqdomain, base_irq, 0,
-						 CPLDS_NB_IRQ);
-		if (ret) {
-			dev_err(&pdev->dev, "couldn't create the irq mapping %d..%d\n",
-				base_irq, base_irq + CPLDS_NB_IRQ);
-			return ret;
-		}
-	}
-
 	return 0;
 }
 
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 3/9] irqchip/jcore-aic: Kill use of irq_create_strict_mappings()
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
  2021-04-06  9:35 ` [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping() Marc Zyngier
  2021-04-06  9:35 ` [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings() Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  2021-04-06  9:35 ` [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping() Marc Zyngier
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

irq_create_strict_mappings() is a poor way to allow the use of
a linear IRQ domain as a legacy one. Let's be upfront about it.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-jcore-aic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-jcore-aic.c b/drivers/irqchip/irq-jcore-aic.c
index 033bccb41455..5f47d8ee4ae3 100644
--- a/drivers/irqchip/irq-jcore-aic.c
+++ b/drivers/irqchip/irq-jcore-aic.c
@@ -100,11 +100,11 @@ static int __init aic_irq_of_init(struct device_node *node,
 	jcore_aic.irq_unmask = noop;
 	jcore_aic.name = "AIC";
 
-	domain = irq_domain_add_linear(node, dom_sz, &jcore_aic_irqdomain_ops,
+	domain = irq_domain_add_legacy(node, dom_sz - min_irq, min_irq, min_irq,
+				       &jcore_aic_irqdomain_ops,
 				       &jcore_aic);
 	if (!domain)
 		return -ENOMEM;
-	irq_create_strict_mappings(domain, min_irq, min_irq, dom_sz - min_irq);
 
 	return 0;
 }
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping()
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
                   ` (2 preceding siblings ...)
  2021-04-06  9:35 ` [PATCH 3/9] irqchip/jcore-aic: " Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  2021-04-06 10:32   ` Geert Uytterhoeven
  2021-04-06  9:35 ` [PATCH 5/9] irqdomain: Kill irq_create_strict_mappings()/irq_create_identity_mapping() Marc Zyngier
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

Instead of playing games with using irq_create_identity_mapping()
and irq_domain_associate(), drop the use of the former and only
use the latter, together with the allocation of the irq_desc
as needed.

It doesn't make the code less awful, but at least the intent
is clearer.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/sh/intc/core.c | 50 ++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
index a14684ffe4c1..6c57ee1ce6c4 100644
--- a/drivers/sh/intc/core.c
+++ b/drivers/sh/intc/core.c
@@ -179,6 +179,23 @@ static unsigned int __init save_reg(struct intc_desc_int *d,
 	return 0;
 }
 
+static bool __init intc_map(struct irq_domain *domain, int irq)
+{
+	int res;
+
+	if (!irq_to_desc(irq) && irq_alloc_desc_at(irq, NUMA_NO_NODE) != irq) {
+		pr_err("uname to allocate IRQ %d\n", irq);
+		return false;
+	}
+
+	if (irq_domain_associate(domain, irq, irq)) {
+		pr_err("domain association failure\n");
+		return false;
+	}
+
+	return true;
+}
+
 int __init register_intc_controller(struct intc_desc *desc)
 {
 	unsigned int i, k, smp;
@@ -316,19 +333,8 @@ int __init register_intc_controller(struct intc_desc *desc)
 		if (!vect->enum_id)
 			continue;
 
-		res = irq_create_identity_mapping(d->domain, irq);
-		if (unlikely(res)) {
-			if (res == -EEXIST) {
-				res = irq_domain_associate(d->domain, irq, irq);
-				if (unlikely(res)) {
-					pr_err("domain association failure\n");
-					continue;
-				}
-			} else {
-				pr_err("can't identity map IRQ %d\n", irq);
-				continue;
-			}
-		}
+		if (!intc_map(d->domain, irq))
+			continue;
 
 		intc_irq_xlate_set(irq, vect->enum_id, d);
 		intc_register_irq(desc, d, vect->enum_id, irq);
@@ -345,22 +351,8 @@ int __init register_intc_controller(struct intc_desc *desc)
 			 * IRQ support, each vector still needs to have
 			 * its own backing irq_desc.
 			 */
-			res = irq_create_identity_mapping(d->domain, irq2);
-			if (unlikely(res)) {
-				if (res == -EEXIST) {
-					res = irq_domain_associate(d->domain,
-								   irq2, irq2);
-					if (unlikely(res)) {
-						pr_err("domain association "
-						       "failure\n");
-						continue;
-					}
-				} else {
-					pr_err("can't identity map IRQ %d\n",
-					       irq);
-					continue;
-				}
-			}
+			if (!intc_map(d->domain, irq2))
+				continue;
 
 			vect2->enum_id = 0;
 
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 5/9] irqdomain: Kill irq_create_strict_mappings()/irq_create_identity_mapping()
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
                   ` (3 preceding siblings ...)
  2021-04-06  9:35 ` [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping() Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  2021-04-06  9:35 ` [PATCH 6/9] mips: netlogic: Use irq_domain_simple_ops for XLP PIC Marc Zyngier
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

No user of these APIs are left, remove them.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/irqdomain.h |  9 ---------
 kernel/irq/irqdomain.c    | 35 -----------------------------------
 2 files changed, 44 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index b9600f24878a..3997ed9e4d7d 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -411,15 +411,6 @@ static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
 }
 
 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 const struct irq_domain_ops irq_domain_simple_ops;
 
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index dfa716305ea9..0ba761e6b0a8 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -703,41 +703,6 @@ unsigned int irq_create_mapping_affinity(struct irq_domain *domain,
 }
 EXPORT_SYMBOL_GPL(irq_create_mapping_affinity);
 
-/**
- * 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)
-{
-	struct device_node *of_node;
-	int ret;
-
-	of_node = irq_domain_get_of_node(domain);
-	ret = irq_alloc_descs(irq_base, irq_base, count,
-			      of_node_to_nid(of_node));
-	if (unlikely(ret < 0))
-		return ret;
-
-	irq_domain_associate_many(domain, irq_base, hwirq_base, count);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(irq_create_strict_mappings);
-
 static int irq_domain_translate(struct irq_domain *d,
 				struct irq_fwspec *fwspec,
 				irq_hw_number_t *hwirq, unsigned int *type)
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 6/9] mips: netlogic: Use irq_domain_simple_ops for XLP PIC
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
                   ` (4 preceding siblings ...)
  2021-04-06  9:35 ` [PATCH 5/9] irqdomain: Kill irq_create_strict_mappings()/irq_create_identity_mapping() Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  2021-04-06 11:36   ` Thomas Bogendoerfer
  2021-04-06  9:35 ` [PATCH 7/9] irqdomain: Drop references to recusive irqdomain setup Marc Zyngier
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

Use the generic irq_domain_simple_ops structure instead of
a home-grown one.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/mips/netlogic/common/irq.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/mips/netlogic/common/irq.c b/arch/mips/netlogic/common/irq.c
index cf33dd8a487e..c25a2ce5e29f 100644
--- a/arch/mips/netlogic/common/irq.c
+++ b/arch/mips/netlogic/common/irq.c
@@ -276,10 +276,6 @@ asmlinkage void plat_irq_dispatch(void)
 }
 
 #ifdef CONFIG_CPU_XLP
-static const struct irq_domain_ops xlp_pic_irq_domain_ops = {
-	.xlate = irq_domain_xlate_onetwocell,
-};
-
 static int __init xlp_of_pic_init(struct device_node *node,
 					struct device_node *parent)
 {
@@ -324,7 +320,7 @@ static int __init xlp_of_pic_init(struct device_node *node,
 
 	xlp_pic_domain = irq_domain_add_legacy(node, n_picirqs,
 		nlm_irq_to_xirq(socid, PIC_IRQ_BASE), PIC_IRQ_BASE,
-		&xlp_pic_irq_domain_ops, NULL);
+		&irq_domain_simple_ops, NULL);
 	if (xlp_pic_domain == NULL) {
 		pr_err("PIC %pOFn: Creating legacy domain failed!\n", node);
 		return -EINVAL;
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 7/9] irqdomain: Drop references to recusive irqdomain setup
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
                   ` (5 preceding siblings ...)
  2021-04-06  9:35 ` [PATCH 6/9] mips: netlogic: Use irq_domain_simple_ops for XLP PIC Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  2021-04-06  9:35 ` [PATCH 8/9] powerpc: Convert irq_domain_add_legacy_isa use to irq_domain_add_legacy Marc Zyngier
  2021-04-06  9:35 ` [PATCH 9/9] irqdomain: Kill irq_domain_add_legacy_isa Marc Zyngier
  8 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

It was never completely implemented, and was removed a long time
ago. Adjust the documentation to reflect this.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 kernel/irq/irqdomain.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 0ba761e6b0a8..89474aa88220 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1659,12 +1659,10 @@ void irq_domain_free_irqs(unsigned int virq, unsigned int nr_irqs)
 
 /**
  * irq_domain_alloc_irqs_parent - Allocate interrupts from parent domain
+ * @domain:	Domain below which interrupts must be allocated
  * @irq_base:	Base IRQ number
  * @nr_irqs:	Number of IRQs to allocate
  * @arg:	Allocation data (arch/domain specific)
- *
- * Check whether the domain has been setup recursive. If not allocate
- * through the parent domain.
  */
 int irq_domain_alloc_irqs_parent(struct irq_domain *domain,
 				 unsigned int irq_base, unsigned int nr_irqs,
@@ -1680,11 +1678,9 @@ EXPORT_SYMBOL_GPL(irq_domain_alloc_irqs_parent);
 
 /**
  * irq_domain_free_irqs_parent - Free interrupts from parent domain
+ * @domain:	Domain below which interrupts must be freed
  * @irq_base:	Base IRQ number
  * @nr_irqs:	Number of IRQs to free
- *
- * Check whether the domain has been setup recursive. If not free
- * through the parent domain.
  */
 void irq_domain_free_irqs_parent(struct irq_domain *domain,
 				 unsigned int irq_base, unsigned int nr_irqs)
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 8/9] powerpc: Convert irq_domain_add_legacy_isa use to irq_domain_add_legacy
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
                   ` (6 preceding siblings ...)
  2021-04-06  9:35 ` [PATCH 7/9] irqdomain: Drop references to recusive irqdomain setup Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  2021-04-06  9:35 ` [PATCH 9/9] irqdomain: Kill irq_domain_add_legacy_isa Marc Zyngier
  8 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

irq_domain_add_legacy_isa is a pain. It only exists for the benefit of
two PPC-specific drivers, and creates an ugly dependency between asm/irq.h
and linux/irqdomain.h

Instead, let's convert these two drivers to irq_domain_add_legacy(),
stop using NUM_ISA_INTERRUPTS by directly setting NR_IRQS_LEGACY.

The dependency cannot be broken yet as there is a lot of PPC-related
code that depends on it, but that's the first step towards it.

A followup patch will remove irq_domain_add_legacy_isa.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/powerpc/include/asm/irq.h         | 4 ++--
 arch/powerpc/platforms/ps3/interrupt.c | 4 ++--
 arch/powerpc/sysdev/i8259.c            | 3 ++-
 arch/powerpc/sysdev/mpic.c             | 2 +-
 arch/powerpc/sysdev/tsi108_pci.c       | 3 ++-
 arch/powerpc/sysdev/xics/xics-common.c | 2 +-
 6 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index f3f264e441a7..aeb209144c68 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -23,8 +23,8 @@ extern atomic_t ppc_n_lost_interrupts;
 /* Total number of virq in the platform */
 #define NR_IRQS		CONFIG_NR_IRQS
 
-/* Same thing, used by the generic IRQ code */
-#define NR_IRQS_LEGACY		NUM_ISA_INTERRUPTS
+/* Number of irqs reserved for a legacy isa controller */
+#define NR_IRQS_LEGACY		16
 
 extern irq_hw_number_t virq_to_hw(unsigned int virq);
 
diff --git a/arch/powerpc/platforms/ps3/interrupt.c b/arch/powerpc/platforms/ps3/interrupt.c
index 78f2339ed5cb..93e367a00452 100644
--- a/arch/powerpc/platforms/ps3/interrupt.c
+++ b/arch/powerpc/platforms/ps3/interrupt.c
@@ -45,7 +45,7 @@
  * implementation equates HV plug value to Linux virq value, constrains each
  * interrupt to have a system wide unique plug number, and limits the range
  * of the plug values to map into the first dword of the bitmaps.  This
- * gives a usable range of plug values of  {NUM_ISA_INTERRUPTS..63}.  Note
+ * gives a usable range of plug values of  {NR_IRQS_LEGACY..63}.  Note
  * that there is no constraint on how many in this set an individual thread
  * can acquire.
  *
@@ -721,7 +721,7 @@ static unsigned int ps3_get_irq(void)
 	}
 
 #if defined(DEBUG)
-	if (unlikely(plug < NUM_ISA_INTERRUPTS || plug > PS3_PLUG_MAX)) {
+	if (unlikely(plug < NR_IRQS_LEGACY || plug > PS3_PLUG_MAX)) {
 		dump_bmp(&per_cpu(ps3_private, 0));
 		dump_bmp(&per_cpu(ps3_private, 1));
 		BUG();
diff --git a/arch/powerpc/sysdev/i8259.c b/arch/powerpc/sysdev/i8259.c
index c1d76c344351..dc1a151c63d7 100644
--- a/arch/powerpc/sysdev/i8259.c
+++ b/arch/powerpc/sysdev/i8259.c
@@ -260,7 +260,8 @@ void i8259_init(struct device_node *node, unsigned long intack_addr)
 	raw_spin_unlock_irqrestore(&i8259_lock, flags);
 
 	/* create a legacy host */
-	i8259_host = irq_domain_add_legacy_isa(node, &i8259_host_ops, NULL);
+	i8259_host = irq_domain_add_legacy(node, NR_IRQS_LEGACY, 0, 0,
+					   &i8259_host_ops, NULL);
 	if (i8259_host == NULL) {
 		printk(KERN_ERR "i8259: failed to allocate irq host !\n");
 		return;
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index b0426f28946a..995fb2ada507 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -602,7 +602,7 @@ static void __init mpic_scan_ht_pics(struct mpic *mpic)
 /* Find an mpic associated with a given linux interrupt */
 static struct mpic *mpic_find(unsigned int irq)
 {
-	if (irq < NUM_ISA_INTERRUPTS)
+	if (irq < NR_IRQS_LEGACY)
 		return NULL;
 
 	return irq_get_chip_data(irq);
diff --git a/arch/powerpc/sysdev/tsi108_pci.c b/arch/powerpc/sysdev/tsi108_pci.c
index 49f9541954f8..042bb38fa5c2 100644
--- a/arch/powerpc/sysdev/tsi108_pci.c
+++ b/arch/powerpc/sysdev/tsi108_pci.c
@@ -404,7 +404,8 @@ void __init tsi108_pci_int_init(struct device_node *node)
 {
 	DBG("Tsi108_pci_int_init: initializing PCI interrupts\n");
 
-	pci_irq_host = irq_domain_add_legacy_isa(node, &pci_irq_domain_ops, NULL);
+	pci_irq_host = irq_domain_add_legacy(node, NR_IRQS_LEGACY, 0, 0,
+					     &pci_irq_domain_ops, NULL);
 	if (pci_irq_host == NULL) {
 		printk(KERN_ERR "pci_irq_host: failed to allocate irq domain!\n");
 		return;
diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c
index 7e4305c01bac..fdf8db4444b6 100644
--- a/arch/powerpc/sysdev/xics/xics-common.c
+++ b/arch/powerpc/sysdev/xics/xics-common.c
@@ -201,7 +201,7 @@ void xics_migrate_irqs_away(void)
 		struct ics *ics;
 
 		/* We can't set affinity on ISA interrupts */
-		if (virq < NUM_ISA_INTERRUPTS)
+		if (virq < NR_IRQS_LEGACY)
 			continue;
 		/* We only need to migrate enabled IRQS */
 		if (!desc->action)
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 9/9] irqdomain: Kill irq_domain_add_legacy_isa
  2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
                   ` (7 preceding siblings ...)
  2021-04-06  9:35 ` [PATCH 8/9] powerpc: Convert irq_domain_add_legacy_isa use to irq_domain_add_legacy Marc Zyngier
@ 2021-04-06  9:35 ` Marc Zyngier
  8 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev, linux-sh
  Cc: Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

This helper doesn't have a user anymore, let's remove it.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 Documentation/core-api/irq/irq-domain.rst |  1 -
 include/linux/irqdomain.h                 | 11 -----------
 2 files changed, 12 deletions(-)

diff --git a/Documentation/core-api/irq/irq-domain.rst b/Documentation/core-api/irq/irq-domain.rst
index a77c24c27f7b..84e561db468f 100644
--- a/Documentation/core-api/irq/irq-domain.rst
+++ b/Documentation/core-api/irq/irq-domain.rst
@@ -146,7 +146,6 @@ Legacy
 
 	irq_domain_add_simple()
 	irq_domain_add_legacy()
-	irq_domain_add_legacy_isa()
 	irq_domain_create_legacy()
 
 The Legacy mapping is a special case for drivers that already have a
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 3997ed9e4d7d..2a7ecf08d56e 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -45,9 +45,6 @@ struct cpumask;
 struct seq_file;
 struct irq_affinity_desc;
 
-/* Number of irqs reserved for a legacy isa controller */
-#define NUM_ISA_INTERRUPTS	16
-
 #define IRQ_DOMAIN_IRQ_SPEC_PARAMS 16
 
 /**
@@ -346,14 +343,6 @@ static inline struct irq_domain *irq_domain_add_nomap(struct device_node *of_nod
 {
 	return __irq_domain_add(of_node_to_fwnode(of_node), 0, max_irq, max_irq, ops, host_data);
 }
-static inline struct irq_domain *irq_domain_add_legacy_isa(
-				struct device_node *of_node,
-				const struct irq_domain_ops *ops,
-				void *host_data)
-{
-	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)
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping()
  2021-04-06  9:35 ` [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping() Marc Zyngier
@ 2021-04-06 10:32   ` Geert Uytterhoeven
  2021-04-06 13:02     ` Marc Zyngier
  0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2021-04-06 10:32 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Linux ARM, Linux Kernel Mailing List,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Linux-sh list,
	Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

Hi Marc,

On Tue, Apr 6, 2021 at 11:44 AM Marc Zyngier <maz@kernel.org> wrote:
> Instead of playing games with using irq_create_identity_mapping()
> and irq_domain_associate(), drop the use of the former and only
> use the latter, together with the allocation of the irq_desc
> as needed.
>
> It doesn't make the code less awful, but at least the intent
> is clearer.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Thanks for your patch!

> --- a/drivers/sh/intc/core.c
> +++ b/drivers/sh/intc/core.c
> @@ -179,6 +179,23 @@ static unsigned int __init save_reg(struct intc_desc_int *d,
>         return 0;
>  }
>
> +static bool __init intc_map(struct irq_domain *domain, int irq)
> +{
> +       int res;

warning: unused variable ‘res’ [-Wunused-variable]

> +
> +       if (!irq_to_desc(irq) && irq_alloc_desc_at(irq, NUMA_NO_NODE) != irq) {
> +               pr_err("uname to allocate IRQ %d\n", irq);
> +               return false;
> +       }
> +
> +       if (irq_domain_associate(domain, irq, irq)) {
> +               pr_err("domain association failure\n");
> +               return false;
> +       }
> +
> +       return true;
> +}
> +
>  int __init register_intc_controller(struct intc_desc *desc)
>  {
>         unsigned int i, k, smp;
> @@ -316,19 +333,8 @@ int __init register_intc_controller(struct intc_desc *desc)

warning: unused variable ‘res’ [-Wunused-variable]

>                 if (!vect->enum_id)
>                         continue;
>
> -               res = irq_create_identity_mapping(d->domain, irq);


> -               if (unlikely(res)) {
> -                       if (res == -EEXIST) {
> -                               res = irq_domain_associate(d->domain, irq, irq);
> -                               if (unlikely(res)) {
> -                                       pr_err("domain association failure\n");
> -                                       continue;
> -                               }
> -                       } else {
> -                               pr_err("can't identity map IRQ %d\n", irq);
> -                               continue;
> -                       }
> -               }
> +               if (!intc_map(d->domain, irq))
> +                       continue;
>
>                 intc_irq_xlate_set(irq, vect->enum_id, d);
>                 intc_register_irq(desc, d, vect->enum_id, irq);

Otherwise this seems to work fine on real hardware (landisk) and qemu
(rts7751r2d).  I did verify that the new function intc_map() is called.

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping()
  2021-04-06  9:35 ` [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping() Marc Zyngier
@ 2021-04-06 11:21   ` Christophe Leroy
  2021-04-06 12:12     ` Marc Zyngier
  0 siblings, 1 reply; 18+ messages in thread
From: Christophe Leroy @ 2021-04-06 11:21 UTC (permalink / raw)
  To: Marc Zyngier, linux-arm-kernel, linux-kernel, linux-mips,
	linuxppc-dev, linux-sh
  Cc: Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang, Rich Felker,
	Thomas Gleixner, Robert Jarzmik, Daniel Mack



Le 06/04/2021 à 11:35, Marc Zyngier a écrit :
> irq_linear_revmap() is supposed to be a fast path for domain
> lookups, but it only exposes low-level details of the irqdomain
> implementation, details which are better kept private.

Can you elaborate with more details ?

> 
> The *overhead* between the two is only a function call and
> a couple of tests, so it is likely that noone can show any
> meaningful difference compared to the cost of taking an
> interrupt.

Do you have any measurement ?

Can you make the "likely" a certitude ?

> 
> Reimplement irq_linear_revmap() with irq_find_mapping()
> in order to preserve source code compatibility, and
> rename the internal field for a measure.

This is in complete contradiction with commit https://github.com/torvalds/linux/commit/d3dcb436

At that time, irq_linear_revmap() was less complex than what irq_find_mapping() is today, and 
nevertheless it was considered worth restoring in as a fast path. What has changed since then ?

Can you also explain the reason for the renaming of "linear_revmap" into "revmap" ? What is that 
"measure" ?

> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>   include/linux/irqdomain.h | 22 +++++++++-------------
>   kernel/irq/irqdomain.c    |  6 +++---
>   2 files changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index 33cacc8af26d..b9600f24878a 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -154,9 +154,9 @@ struct irq_domain_chip_generic;
>    * Revmap data, used internally by irq_domain
>    * @revmap_direct_max_irq: The largest hwirq that can be set for controllers that
>    *                         support direct mapping
> - * @revmap_size: Size of the linear map table @linear_revmap[]
> + * @revmap_size: Size of the linear map table @revmap[]
>    * @revmap_tree: Radix map tree for hwirqs that don't fit in the linear map
> - * @linear_revmap: Linear table of hwirq->virq reverse mappings
> + * @revmap: Linear table of hwirq->virq reverse mappings
>    */
>   struct irq_domain {
>   	struct list_head link;
> @@ -180,7 +180,7 @@ struct irq_domain {
>   	unsigned int revmap_size;
>   	struct radix_tree_root revmap_tree;
>   	struct mutex revmap_tree_mutex;
> -	unsigned int linear_revmap[];
> +	unsigned int revmap[];
>   };
>   
>   /* Irq domain flags */
> @@ -396,24 +396,20 @@ static inline unsigned int irq_create_mapping(struct irq_domain *host,
>   	return irq_create_mapping_affinity(host, hwirq, NULL);
>   }
>   
> -
>   /**
> - * irq_linear_revmap() - Find a linux irq from a hw irq number.
> + * irq_find_mapping() - 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 alternative to irq_find_mapping() that can be
> - * called directly by irq controller code to save a handful of
> - * instructions. It is always safe to call, but won't find irqs mapped
> - * using the radix tree.
>    */
> +extern unsigned int irq_find_mapping(struct irq_domain *host,
> +				     irq_hw_number_t hwirq);
> +
>   static inline unsigned int irq_linear_revmap(struct irq_domain *domain,
>   					     irq_hw_number_t hwirq)
>   {
> -	return hwirq < domain->revmap_size ? domain->linear_revmap[hwirq] : 0;
> +	return irq_find_mapping(domain, hwirq);
>   }
> -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,
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index d10ab1d689d5..dfa716305ea9 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -486,7 +486,7 @@ static void irq_domain_clear_mapping(struct irq_domain *domain,
>   				     irq_hw_number_t hwirq)
>   {
>   	if (hwirq < domain->revmap_size) {
> -		domain->linear_revmap[hwirq] = 0;
> +		domain->revmap[hwirq] = 0;
>   	} else {
>   		mutex_lock(&domain->revmap_tree_mutex);
>   		radix_tree_delete(&domain->revmap_tree, hwirq);
> @@ -499,7 +499,7 @@ static void irq_domain_set_mapping(struct irq_domain *domain,
>   				   struct irq_data *irq_data)
>   {
>   	if (hwirq < domain->revmap_size) {
> -		domain->linear_revmap[hwirq] = irq_data->irq;
> +		domain->revmap[hwirq] = irq_data->irq;
>   	} else {
>   		mutex_lock(&domain->revmap_tree_mutex);
>   		radix_tree_insert(&domain->revmap_tree, hwirq, irq_data);
> @@ -920,7 +920,7 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
>   
>   	/* Check if the hwirq is in the linear revmap. */
>   	if (hwirq < domain->revmap_size)
> -		return domain->linear_revmap[hwirq];
> +		return domain->revmap[hwirq];
>   
>   	rcu_read_lock();
>   	data = radix_tree_lookup(&domain->revmap_tree, hwirq);
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 6/9] mips: netlogic: Use irq_domain_simple_ops for XLP PIC
  2021-04-06  9:35 ` [PATCH 6/9] mips: netlogic: Use irq_domain_simple_ops for XLP PIC Marc Zyngier
@ 2021-04-06 11:36   ` Thomas Bogendoerfer
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Bogendoerfer @ 2021-04-06 11:36 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh, Daniel Mack, Robert Jarzmik, Haojian Zhuang,
	Yoshinori Sato, Rich Felker, Michael Ellerman, Thomas Gleixner

On Tue, Apr 06, 2021 at 10:35:54AM +0100, Marc Zyngier wrote:
> Use the generic irq_domain_simple_ops structure instead of
> a home-grown one.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/mips/netlogic/common/irq.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping()
  2021-04-06 11:21   ` Christophe Leroy
@ 2021-04-06 12:12     ` Marc Zyngier
  0 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06 12:12 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh, Thomas Bogendoerfer, Yoshinori Sato, Haojian Zhuang,
	Rich Felker, Thomas Gleixner, Robert Jarzmik, Daniel Mack

Christophe,

On Tue, 06 Apr 2021 12:21:33 +0100,
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
> 
> 
> Le 06/04/2021 à 11:35, Marc Zyngier a écrit :
> > irq_linear_revmap() is supposed to be a fast path for domain
> > lookups, but it only exposes low-level details of the irqdomain
> > implementation, details which are better kept private.
> 
> Can you elaborate with more details ?

Things like directly picking into the revmap are positively awful, and
doesn't work if the domain has been constructed using the radix
tree. Which on its own is totally broken, because things like
irq_domain_create_hierarchy() will pick an implementation or the
other.

> 
> > 
> > The *overhead* between the two is only a function call and
> > a couple of tests, so it is likely that noone can show any
> > meaningful difference compared to the cost of taking an
> > interrupt.
> 
> Do you have any measurement ?

I did measure things on arm64, and couldn't come up with any
difference other than noise.

> Can you make the "likely" a certitude ?

Of course not. You can always come up with an artificial CPU
implementation that has a very small exception entry overhead, and a
ridiculously slow memory subsystem. Do I care about these? No.

If you can come up with realistic platforms that show a regression
with this patch, I'm all ears.

> 
> > 
> > Reimplement irq_linear_revmap() with irq_find_mapping()
> > in order to preserve source code compatibility, and
> > rename the internal field for a measure.
> 
> This is in complete contradiction with commit https://github.com/torvalds/linux/commit/d3dcb436
> 
> At that time, irq_linear_revmap() was less complex than what
> irq_find_mapping() is today, and nevertheless it was considered worth
> restoring in as a fast path. What has changed since then ?

Over 8 years? Plenty. The use of irqdomains has been generalised, we
have domain hierarchies, and if anything, this commit introduces the
buggy behaviour I was mentioning above. I also don't see any mention
of actual performance in that commit.

And if we're worried about a fast path, being able to directly cache
the irq_data in the revmap, hence skipping the irq_desc lookup that
inevitable follows, is a much more interesting prospect than the "get
useless data quick" that irq_linear_revmap() implements.

This latter optimisation is what I am after.

> Can you also explain the reason for the renaming of "linear_revmap"
> into "revmap" ? What is that "measure" ?

To catch the potential direct use of the reverse map field.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping()
  2021-04-06 10:32   ` Geert Uytterhoeven
@ 2021-04-06 13:02     ` Marc Zyngier
  0 siblings, 0 replies; 18+ messages in thread
From: Marc Zyngier @ 2021-04-06 13:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux ARM, Linux Kernel Mailing List,
	open list:BROADCOM NVRAM DRIVER, linuxppc-dev, Linux-sh list,
	Daniel Mack, Robert Jarzmik, Haojian Zhuang, Yoshinori Sato,
	Rich Felker, Thomas Bogendoerfer, Michael Ellerman,
	Thomas Gleixner

On Tue, 06 Apr 2021 11:32:13 +0100,
Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> 
> Hi Marc,
> 
> On Tue, Apr 6, 2021 at 11:44 AM Marc Zyngier <maz@kernel.org> wrote:
> > Instead of playing games with using irq_create_identity_mapping()
> > and irq_domain_associate(), drop the use of the former and only
> > use the latter, together with the allocation of the irq_desc
> > as needed.
> >
> > It doesn't make the code less awful, but at least the intent
> > is clearer.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> 
> Thanks for your patch!
> 
> > --- a/drivers/sh/intc/core.c
> > +++ b/drivers/sh/intc/core.c
> > @@ -179,6 +179,23 @@ static unsigned int __init save_reg(struct intc_desc_int *d,
> >         return 0;
> >  }
> >
> > +static bool __init intc_map(struct irq_domain *domain, int irq)
> > +{
> > +       int res;
> 
> warning: unused variable ‘res’ [-Wunused-variable]
> 
> > +
> > +       if (!irq_to_desc(irq) && irq_alloc_desc_at(irq, NUMA_NO_NODE) != irq) {
> > +               pr_err("uname to allocate IRQ %d\n", irq);
> > +               return false;
> > +       }
> > +
> > +       if (irq_domain_associate(domain, irq, irq)) {
> > +               pr_err("domain association failure\n");
> > +               return false;
> > +       }
> > +
> > +       return true;
> > +}
> > +
> >  int __init register_intc_controller(struct intc_desc *desc)
> >  {
> >         unsigned int i, k, smp;
> > @@ -316,19 +333,8 @@ int __init register_intc_controller(struct intc_desc *desc)
> 
> warning: unused variable ‘res’ [-Wunused-variable]

Ah, thanks for spotting these.

> 
> >                 if (!vect->enum_id)
> >                         continue;
> >
> > -               res = irq_create_identity_mapping(d->domain, irq);
> 
> 
> > -               if (unlikely(res)) {
> > -                       if (res == -EEXIST) {
> > -                               res = irq_domain_associate(d->domain, irq, irq);
> > -                               if (unlikely(res)) {
> > -                                       pr_err("domain association failure\n");
> > -                                       continue;
> > -                               }
> > -                       } else {
> > -                               pr_err("can't identity map IRQ %d\n", irq);
> > -                               continue;
> > -                       }
> > -               }
> > +               if (!intc_map(d->domain, irq))
> > +                       continue;
> >
> >                 intc_irq_xlate_set(irq, vect->enum_id, d);
> >                 intc_register_irq(desc, d, vect->enum_id, irq);
> 
> Otherwise this seems to work fine on real hardware (landisk) and qemu
> (rts7751r2d).  I did verify that the new function intc_map() is called.
> 
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Awesome, thanks Geert.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings()
  2021-04-06  9:35 ` [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings() Marc Zyngier
@ 2021-04-26 22:39   ` Guenter Roeck
  2021-04-27  8:30     ` Marc Zyngier
  0 siblings, 1 reply; 18+ messages in thread
From: Guenter Roeck @ 2021-04-26 22:39 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh, Daniel Mack, Robert Jarzmik, Haojian Zhuang,
	Yoshinori Sato, Rich Felker, Thomas Bogendoerfer,
	Michael Ellerman, Thomas Gleixner

On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote:
> irq_create_strict_mappings() is a poor way to allow the use of
> a linear IRQ domain as a legacy one. Let's be upfront about
> it and use a legacy domain when appropriate.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---

When running the "mainstone" qemu emulation, this patch results
in many (32, actually) runtime warnings such as the following.

[    0.528272] ------------[ cut here ]------------
[    0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550 irq_domain_associate+0x194/0x1f0
[    0.528315] error: virq335 is not allocated
[    0.528325] Modules linked in:
[    0.528351] CPU: 0 PID: 1 Comm: swapper Tainted: G        W         5.12.0-rc8-next-20210423 #1
[    0.528372] Hardware name: Intel HCDDBBVA0 Development Platform (aka Mainstone)
[    0.528387] Backtrace:
[    0.528406] [<c06bd188>] (dump_backtrace) from [<c06bd468>] (show_stack+0x20/0x24)
[    0.528441]  r7:00000226 r6:c00796e8 r5:00000009 r4:c088d2a0
[    0.528454] [<c06bd448>] (show_stack) from [<c06c11dc>] (dump_stack+0x28/0x30)
[    0.528479] [<c06c11b4>] (dump_stack) from [<c002a2b8>] (__warn+0xe8/0x110)
[    0.528507]  r5:00000009 r4:c0872698
[    0.528520] [<c002a1d0>] (__warn) from [<c06bdbc0>] (warn_slowpath_fmt+0xa0/0xe0)
[    0.528551]  r7:c00796e8 r6:00000226 r5:c0872698 r4:c0872700
[    0.528564] [<c06bdb24>] (warn_slowpath_fmt) from [<c00796e8>] (irq_domain_associate+0x194/0x1f0)
[    0.528597]  r8:00000130 r7:0000014f r6:0000001f r5:00000000 r4:c11bd780
[    0.528610] [<c0079554>] (irq_domain_associate) from [<c00797a4>] (irq_domain_associate_many+0x60/0xa4)
[    0.528642]  r8:00000130 r7:c11bd780 r6:fffffed0 r5:00000150 r4:00000150
[    0.528655] [<c0079744>] (irq_domain_associate_many) from [<c0079e5c>] (irq_domain_create_legacy+0x5c/0x68)
[    0.528687]  r8:00000130 r7:00000130 r6:00000020 r5:00000000 r4:c11bd780
[    0.528699] [<c0079e00>] (irq_domain_create_legacy) from [<c0079e9c>] (irq_domain_add_legacy+0x34/0x3c)
[    0.528730]  r7:c09b1370 r6:c09b1360 r5:c11bd3a0 r4:00000000
[    0.528743] [<c0079e68>] (irq_domain_add_legacy) from [<c0024f28>] (cplds_probe+0x170/0x1ac)
[    0.528768] [<c0024db8>] (cplds_probe) from [<c0432cec>] (platform_probe+0x50/0xb0)
[    0.528800]  r8:c09d2c94 r7:c0aa4f88 r6:c09d2c94 r5:c09b1370 r4:00000000
[    0.528814] [<c0432c9c>] (platform_probe) from [<c042fc70>] (really_probe+0x100/0x4d4)
[    0.528844]  r7:c0aa4f88 r6:00000000 r5:00000000 r4:c09b1370
[    0.528858] [<c042fb70>] (really_probe) from [<c04300cc>] (driver_probe_device+0x88/0x20c)
[    0.528892]  r10:c0974830 r9:c0a70000 r8:c093b224 r7:c0a31de8 r6:c09d2c94 r5:c09d2c94
[    0.528907]  r4:c09b1370
[    0.528919] [<c0430044>] (driver_probe_device) from [<c04306cc>] (device_driver_attach+0x68/0x70)
[    0.528953]  r9:c0a70000 r8:c093b224 r7:c0a31de8 r6:c09d2c94 r5:00000000 r4:c09b1370
[    0.528969] [<c0430664>] (device_driver_attach) from [<c0430794>] (__driver_attach+0xc0/0x164)
[    0.528997]  r7:c0a31de8 r6:c09b1370 r5:c09d2c94 r4:00000000
[    0.529009] [<c04306d4>] (__driver_attach) from [<c042d8d0>] (bus_for_each_dev+0x84/0xcc)
[    0.529039]  r7:c0a31de8 r6:c04306d4 r5:c09d2c94 r4:00000000
[    0.529052] [<c042d84c>] (bus_for_each_dev) from [<c042f494>] (driver_attach+0x28/0x30)
[    0.529082]  r6:00000000 r5:c11bd200 r4:c09d2c94
[    0.529095] [<c042f46c>] (driver_attach) from [<c042edac>] (bus_add_driver+0x168/0x210)
[    0.529122] [<c042ec44>] (bus_add_driver) from [<c0431304>] (driver_register+0x88/0x120)
[    0.529152]  r7:c0a5c7e0 r6:00000000 r5:ffffe000 r4:c09d2c94
[    0.529165] [<c043127c>] (driver_register) from [<c04329a0>] (__platform_driver_register+0x2c/0x34)
[    0.529191]  r5:ffffe000 r4:c094ba64
[    0.529204] [<c0432974>] (__platform_driver_register) from [<c094ba84>] (cplds_driver_init+0x20/0x28)
[    0.529230] [<c094ba64>] (cplds_driver_init) from [<c000a2a8>] (do_one_initcall+0x60/0x27c)
[    0.529255] [<c000a248>] (do_one_initcall) from [<c093f244>] (kernel_init_freeable+0x158/0x1e4)
[    0.529284]  r7:c0974850 r6:00000007 r5:c0c0f720 r4:c09a02fc
[    0.529297] [<c093f0ec>] (kernel_init_freeable) from [<c06c5274>] (kernel_init+0x18/0x110)
[    0.529328]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c06c525c
[    0.529343]  r4:00000000
[    0.529354] [<c06c525c>] (kernel_init) from [<c0008348>] (ret_from_fork+0x14/0x2c)
[    0.529387] Exception stack(0xc0bdffb0 to 0xc0bdfff8)
[    0.529467] ffa0:                                     00000000 00000000 00000000 00000000
[    0.529587] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[    0.529684] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    0.529726]  r5:c06c525c r4:00000000
[    0.529752] ---[ end trace d199929d2b87e077 ]---

Bisect log attached.

Guenter

---
# bad: [e3d35712f85ac84fb06234848f6c043ab418cf8b] Add linux-next specific files for 20210423
# good: [bf05bf16c76bb44ab5156223e1e58e26dfe30a88] Linux 5.12-rc8
git bisect start 'HEAD' 'v5.12-rc8'
# good: [d4b5d9d94679a18bfa4ccdafd19876d58777911e] Merge remote-tracking branch 'crypto/master'
git bisect good d4b5d9d94679a18bfa4ccdafd19876d58777911e
# good: [27628e42fe59a698e66b671bf1e1f01f6a3fe765] Merge remote-tracking branch 'tip/auto-latest'
git bisect good 27628e42fe59a698e66b671bf1e1f01f6a3fe765
# bad: [bc6c3ae4f662fc719d0bf144f150f72cab8912d4] Merge remote-tracking branch 'vfio/next'
git bisect bad bc6c3ae4f662fc719d0bf144f150f72cab8912d4
# bad: [5ff5b00609c64a043ccd5bc92273c132b33f7f9a] Merge remote-tracking branch 'driver-core/driver-core-next'
git bisect bad 5ff5b00609c64a043ccd5bc92273c132b33f7f9a
# bad: [c878be9c883153797d5749620e58f180cc429e88] Merge remote-tracking branch 'kvm/next'
git bisect bad c878be9c883153797d5749620e58f180cc429e88
# good: [52acd22faa1af8a0514ccd075a6978ac97986425] KVM: Boost vCPU candidate in user mode which is delivering interrupt
git bisect good 52acd22faa1af8a0514ccd075a6978ac97986425
# good: [988aab640a6c46ab9552e65c2c3a8d577a4e30f3] rcu: Make rcu_gp_cleanup() be noinline for tracing
git bisect good 988aab640a6c46ab9552e65c2c3a8d577a4e30f3
# bad: [6603c2d8bd6cc7fa591fd3b4232bf25b65a0ea8f] Merge remote-tracking branch 'ftrace/for-next'
git bisect bad 6603c2d8bd6cc7fa591fd3b4232bf25b65a0ea8f
# good: [c658797f1a70561205a224be0c8be64977ed64e8] tracing: Add method for recording "func_repeats" events
git bisect good c658797f1a70561205a224be0c8be64977ed64e8
# good: [46135d6f878ab00261d4a2082d620bfb41019aab] irqchip/gic-v4.1: Disable vSGI upon (GIC CPUIF < v4.1) detection
git bisect good 46135d6f878ab00261d4a2082d620bfb41019aab
# bad: [05d7bf817019890e4d049e0b851940c596adbd9b] dt-bindings: interrupt-controller: Add IDT 79RC3243x Interrupt Controller
git bisect bad 05d7bf817019890e4d049e0b851940c596adbd9b
# bad: [1a0b05e435544cd53cd3936bdab425d88784b71a] irqdomain: Get rid of irq_create_strict_mappings()
git bisect bad 1a0b05e435544cd53cd3936bdab425d88784b71a
# bad: [5f8b938bd790cff6542c7fe3c1495c71f89fef1b] irqchip/jcore-aic: Kill use of irq_create_strict_mappings()
git bisect bad 5f8b938bd790cff6542c7fe3c1495c71f89fef1b
# bad: [b68761da01114a64b9c521975c3bca6d10eeb950] ARM: PXA: Kill use of irq_create_strict_mappings()
git bisect bad b68761da01114a64b9c521975c3bca6d10eeb950
# first bad commit: [b68761da01114a64b9c521975c3bca6d10eeb950] ARM: PXA: Kill use of irq_create_strict_mappings()

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings()
  2021-04-26 22:39   ` Guenter Roeck
@ 2021-04-27  8:30     ` Marc Zyngier
  2021-04-27 12:56       ` Guenter Roeck
  0 siblings, 1 reply; 18+ messages in thread
From: Marc Zyngier @ 2021-04-27  8:30 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh, Daniel Mack, Robert Jarzmik, Haojian Zhuang,
	Yoshinori Sato, Rich Felker, Thomas Bogendoerfer,
	Michael Ellerman, Thomas Gleixner

Hi Guenter,

Thanks for the heads up.

On Mon, 26 Apr 2021 23:39:42 +0100,
Guenter Roeck <linux@roeck-us.net> wrote:
> 
> On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote:
> > irq_create_strict_mappings() is a poor way to allow the use of
> > a linear IRQ domain as a legacy one. Let's be upfront about
> > it and use a legacy domain when appropriate.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> 
> When running the "mainstone" qemu emulation, this patch results
> in many (32, actually) runtime warnings such as the following.
> 
> [    0.528272] ------------[ cut here ]------------
> [    0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550 irq_domain_associate+0x194/0x1f0
> [    0.528315] error: virq335 is not allocated

[...]

This looks like a case of CONFIG_SPARSE_IRQ, combined with a lack of
brain engagement. I've come up with the following patch, which lets
the kernel boot in QEMU without screaming (other than the lack of a
rootfs...).

Please let me know if this helps.

Thanks,

	M.

From 4d7f6ddbbfdff1c9f029bafca79020d3294dc32c Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Tue, 27 Apr 2021 09:00:28 +0100
Subject: [PATCH] ARM: PXA: Fix cplds irqdesc allocation when using legacy mode

The Mainstone PXA platform uses CONFIG_SPARSE_IRQ, and thus we
cannot rely on the irq descriptors to be readilly allocated
before creating the irqdomain in legacy mode. The kernel then
complains loudly about not being able to associate the interrupt
in the domain -- can't blame it.

Fix it by allocating the irqdescs upfront in the legacy case.

Fixes: b68761da0111 ("ARM: PXA: Kill use of irq_create_strict_mappings()")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm/mach-pxa/pxa_cplds_irqs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c b/arch/arm/mach-pxa/pxa_cplds_irqs.c
index ec0d9b094744..bddfc7cd5d40 100644
--- a/arch/arm/mach-pxa/pxa_cplds_irqs.c
+++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c
@@ -121,8 +121,13 @@ static int cplds_probe(struct platform_device *pdev)
 		return fpga->irq;
 
 	base_irq = platform_get_irq(pdev, 1);
-	if (base_irq < 0)
+	if (base_irq < 0) {
 		base_irq = 0;
+	} else {
+		ret = devm_irq_alloc_descs(&pdev->dev, base_irq, base_irq, CPLDS_NB_IRQ, 0);
+		if (ret < 0)
+			return ret;
+	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	fpga->base = devm_ioremap_resource(&pdev->dev, res);
-- 
2.30.2


-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings()
  2021-04-27  8:30     ` Marc Zyngier
@ 2021-04-27 12:56       ` Guenter Roeck
  0 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2021-04-27 12:56 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-arm-kernel, linux-kernel, linux-mips, linuxppc-dev,
	linux-sh, Daniel Mack, Robert Jarzmik, Haojian Zhuang,
	Yoshinori Sato, Rich Felker, Thomas Bogendoerfer,
	Michael Ellerman, Thomas Gleixner

On 4/27/21 1:30 AM, Marc Zyngier wrote:
> Hi Guenter,
> 
> Thanks for the heads up.
> 
> On Mon, 26 Apr 2021 23:39:42 +0100,
> Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Tue, Apr 06, 2021 at 10:35:50AM +0100, Marc Zyngier wrote:
>>> irq_create_strict_mappings() is a poor way to allow the use of
>>> a linear IRQ domain as a legacy one. Let's be upfront about
>>> it and use a legacy domain when appropriate.
>>>
>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>> ---
>>
>> When running the "mainstone" qemu emulation, this patch results
>> in many (32, actually) runtime warnings such as the following.
>>
>> [    0.528272] ------------[ cut here ]------------
>> [    0.528285] WARNING: CPU: 0 PID: 1 at kernel/irq/irqdomain.c:550 irq_domain_associate+0x194/0x1f0
>> [    0.528315] error: virq335 is not allocated
> 
> [...]
> 
> This looks like a case of CONFIG_SPARSE_IRQ, combined with a lack of
> brain engagement. I've come up with the following patch, which lets
> the kernel boot in QEMU without screaming (other than the lack of a
> rootfs...).
> 
> Please let me know if this helps.
> 

It does.

Tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter

> Thanks,
> 
> 	M.
> 
> From 4d7f6ddbbfdff1c9f029bafca79020d3294dc32c Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Tue, 27 Apr 2021 09:00:28 +0100
> Subject: [PATCH] ARM: PXA: Fix cplds irqdesc allocation when using legacy mode
> 
> The Mainstone PXA platform uses CONFIG_SPARSE_IRQ, and thus we
> cannot rely on the irq descriptors to be readilly allocated
> before creating the irqdomain in legacy mode. The kernel then
> complains loudly about not being able to associate the interrupt
> in the domain -- can't blame it.
> 
> Fix it by allocating the irqdescs upfront in the legacy case.
> 
> Fixes: b68761da0111 ("ARM: PXA: Kill use of irq_create_strict_mappings()")
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm/mach-pxa/pxa_cplds_irqs.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-pxa/pxa_cplds_irqs.c b/arch/arm/mach-pxa/pxa_cplds_irqs.c
> index ec0d9b094744..bddfc7cd5d40 100644
> --- a/arch/arm/mach-pxa/pxa_cplds_irqs.c
> +++ b/arch/arm/mach-pxa/pxa_cplds_irqs.c
> @@ -121,8 +121,13 @@ static int cplds_probe(struct platform_device *pdev)
>  		return fpga->irq;
>  
>  	base_irq = platform_get_irq(pdev, 1);
> -	if (base_irq < 0)
> +	if (base_irq < 0) {
>  		base_irq = 0;
> +	} else {
> +		ret = devm_irq_alloc_descs(&pdev->dev, base_irq, base_irq, CPLDS_NB_IRQ, 0);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	fpga->base = devm_ioremap_resource(&pdev->dev, res);
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-06  9:35 [PATCH 0/9] Cleaning up some of the irqdomain features Marc Zyngier
2021-04-06  9:35 ` [PATCH 1/9] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping() Marc Zyngier
2021-04-06 11:21   ` Christophe Leroy
2021-04-06 12:12     ` Marc Zyngier
2021-04-06  9:35 ` [PATCH 2/9] ARM: PXA: Kill use of irq_create_strict_mappings() Marc Zyngier
2021-04-26 22:39   ` Guenter Roeck
2021-04-27  8:30     ` Marc Zyngier
2021-04-27 12:56       ` Guenter Roeck
2021-04-06  9:35 ` [PATCH 3/9] irqchip/jcore-aic: " Marc Zyngier
2021-04-06  9:35 ` [PATCH 4/9] sh: intc: Drop the use of irq_create_identity_mapping() Marc Zyngier
2021-04-06 10:32   ` Geert Uytterhoeven
2021-04-06 13:02     ` Marc Zyngier
2021-04-06  9:35 ` [PATCH 5/9] irqdomain: Kill irq_create_strict_mappings()/irq_create_identity_mapping() Marc Zyngier
2021-04-06  9:35 ` [PATCH 6/9] mips: netlogic: Use irq_domain_simple_ops for XLP PIC Marc Zyngier
2021-04-06 11:36   ` Thomas Bogendoerfer
2021-04-06  9:35 ` [PATCH 7/9] irqdomain: Drop references to recusive irqdomain setup Marc Zyngier
2021-04-06  9:35 ` [PATCH 8/9] powerpc: Convert irq_domain_add_legacy_isa use to irq_domain_add_legacy Marc Zyngier
2021-04-06  9:35 ` [PATCH 9/9] irqdomain: Kill irq_domain_add_legacy_isa Marc Zyngier

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git