[PATCH 0/5] Making the generic ACPI GSI layer irqdomain aware

[PATCH 0/5] Making the generic ACPI GSI layer irqdomain aware

[Marc Zyngier]

The irqdomain code is not entierely really ACPI friendly, as it has
some built-in knowledge of the device-tree. Nothing too harmful, but
enough to scare the ARM ACPI developpers which end up with their own
version of the square wheel.

This small patch series adapt the irqdomain code to remove the hurdles
that prevent the full blown irqdomain subsystem to be used on ACPI,
creates an interface between the GSI layer and the irqdomain, and as
an example, convert the ARM GIC ACPI support to use irqdomains as
originally intended.

Overall, this gives us a way to use irqdomains on both DT and ACPI
enabled platforms, having very little changes made to the actual
drivers (other than the probing infrastructure). Because we keep the
flow of information between the various layers identical between ACPI
and DT, we immediately benefit from the existing infrastructure.

I'd really like to hear what people think of that approach, as it
looks to me a lot simpler than the other approaches currently put on
the list. The "convert the GSI information to be DT friendly" is
admitedly not very pretty, but I see it as a stepping stone towards
unifying the two structures.

This has been test-booted on Juno, and is based on my irq/ncpi-msi-2
branch.

Marc Zyngier (5):
  genirq: irqdomain: Use an accessor for the of_node field
  genirq: irqdomain: Remove irqdomain dependency on struct device_node
  genirq: irqdomain: Add irq_create_acpi_mappings
  acpi: gsi: Use acpi_gsi_descriptor to allocate interrupts
  irqchip: GIC: Switch ACPI support to stacked domains

 arch/mips/cavium-octeon/octeon-irq.c          |  4 +-
 arch/powerpc/platforms/cell/axon_msi.c        |  2 +-
 arch/powerpc/platforms/cell/spider-pic.c      |  9 ++-
 arch/powerpc/platforms/powernv/opal-irqchip.c |  2 +-
 arch/powerpc/sysdev/ehv_pic.c                 |  3 +-
 arch/powerpc/sysdev/fsl_msi.c                 |  2 +-
 arch/powerpc/sysdev/i8259.c                   |  3 +-
 arch/powerpc/sysdev/ipic.c                    |  3 +-
 arch/powerpc/sysdev/mpic.c                    |  3 +-
 arch/powerpc/sysdev/mpic_msi.c                |  2 +-
 arch/powerpc/sysdev/qe_lib/qe_ic.c            |  3 +-
 drivers/acpi/gsi.c                            | 54 ++++++++++-----
 drivers/gpio/gpio-sodaville.c                 |  2 +-
 drivers/irqchip/exynos-combiner.c             |  2 +-
 drivers/irqchip/irq-atmel-aic-common.c        |  2 +-
 drivers/irqchip/irq-crossbar.c                |  4 +-
 drivers/irqchip/irq-gic-v2m.c                 |  2 +-
 drivers/irqchip/irq-gic-v3-its.c              |  2 +-
 drivers/irqchip/irq-gic-v3.c                  |  2 +-
 drivers/irqchip/irq-gic.c                     | 17 ++---
 drivers/irqchip/irq-hip04.c                   |  2 +-
 drivers/irqchip/irq-mtk-sysirq.c              |  2 +-
 drivers/irqchip/irq-s3c24xx.c                 |  4 +-
 drivers/irqchip/irq-tegra.c                   |  4 +-
 drivers/irqchip/irq-vf610-mscm-ir.c           |  5 +-
 drivers/spmi/spmi-pmic-arb.c                  |  2 +-
 include/linux/acpi.h                          |  9 +++
 include/linux/irqchip/arm-gic.h               |  2 +-
 include/linux/irqdomain.h                     | 67 +++++++++++--------
 kernel/irq/irqdomain.c                        | 95 +++++++++++++++++++++------
 30 files changed, 206 insertions(+), 109 deletions(-)

-- 
2.1.4

[PATCH 1/5] genirq: irqdomain: Use an accessor for the of_node field

[Marc Zyngier]

The struct irq_domain contains a "struct device_node *" field
(of_node) that is almost the only link between the irqdomain
and the device tree infrastructure.

In order to prepare for the removal of that field, convert all
users outside of kernel/irq/irqdomain.c to use an accessor.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/mips/cavium-octeon/octeon-irq.c          | 4 ++--
 arch/powerpc/platforms/cell/axon_msi.c        | 2 +-
 arch/powerpc/platforms/cell/spider-pic.c      | 9 ++++++---
 arch/powerpc/platforms/powernv/opal-irqchip.c | 2 +-
 arch/powerpc/sysdev/ehv_pic.c                 | 3 ++-
 arch/powerpc/sysdev/fsl_msi.c                 | 2 +-
 arch/powerpc/sysdev/i8259.c                   | 3 ++-
 arch/powerpc/sysdev/ipic.c                    | 3 ++-
 arch/powerpc/sysdev/mpic.c                    | 3 ++-
 arch/powerpc/sysdev/mpic_msi.c                | 2 +-
 arch/powerpc/sysdev/qe_lib/qe_ic.c            | 3 ++-
 drivers/gpio/gpio-sodaville.c                 | 2 +-
 drivers/irqchip/exynos-combiner.c             | 2 +-
 drivers/irqchip/irq-atmel-aic-common.c        | 2 +-
 drivers/irqchip/irq-crossbar.c                | 4 ++--
 drivers/irqchip/irq-gic-v2m.c                 | 2 +-
 drivers/irqchip/irq-gic-v3-its.c              | 2 +-
 drivers/irqchip/irq-gic-v3.c                  | 2 +-
 drivers/irqchip/irq-gic.c                     | 2 +-
 drivers/irqchip/irq-hip04.c                   | 2 +-
 drivers/irqchip/irq-mtk-sysirq.c              | 2 +-
 drivers/irqchip/irq-s3c24xx.c                 | 4 ++--
 drivers/irqchip/irq-tegra.c                   | 4 ++--
 drivers/irqchip/irq-vf610-mscm-ir.c           | 5 +++--
 drivers/spmi/spmi-pmic-arb.c                  | 2 +-
 include/linux/irqdomain.h                     | 5 +++++
 26 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
index d8124a3..f1fbb1f 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -1097,7 +1097,7 @@ static int octeon_irq_gpio_xlat(struct irq_domain *d,
 	unsigned int pin;
 	unsigned int trigger;
 
-	if (d->of_node != node)
+	if (irq_domain_get_of_node(d) != node)
 		return -EINVAL;
 
 	if (intsize < 2)
@@ -2161,7 +2161,7 @@ static int octeon_irq_cib_map(struct irq_domain *d,
 
 	if (hw >= host_data->max_bits) {
 		pr_err("ERROR: %s mapping %u is to big!\n",
-		       d->of_node->name, (unsigned)hw);
+		       irq_domain_get_of_node(d)->name, (unsigned)hw);
 		return -EINVAL;
 	}
 
diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
index 306888a..bcbe341 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -327,7 +327,7 @@ static void axon_msi_shutdown(struct platform_device *device)
 	u32 tmp;
 
 	pr_devel("axon_msi: disabling %s\n",
-		  msic->irq_domain->of_node->full_name);
+		 irq_domain_get_of_node(msic->irq_domain)->full_name);
 	tmp  = dcr_read(msic->dcr_host, MSIC_CTRL_REG);
 	tmp &= ~MSIC_CTRL_ENABLE & ~MSIC_CTRL_IRQ_ENABLE;
 	msic_dcr_write(msic, MSIC_CTRL_REG, tmp);
diff --git a/arch/powerpc/platforms/cell/spider-pic.c b/arch/powerpc/platforms/cell/spider-pic.c
index 1f72f4a..078b633 100644
--- a/arch/powerpc/platforms/cell/spider-pic.c
+++ b/arch/powerpc/platforms/cell/spider-pic.c
@@ -231,20 +231,23 @@ static unsigned int __init spider_find_cascade_and_node(struct spider_pic *pic)
 	const u32 *imap, *tmp;
 	int imaplen, intsize, unit;
 	struct device_node *iic;
+	struct device_node *of_node;
+
+	of_node = irq_domain_get_of_node(pic->host);
 
 	/* First, we check whether we have a real "interrupts" in the device
 	 * tree in case the device-tree is ever fixed
 	 */
-	virq = irq_of_parse_and_map(pic->host->of_node, 0);
+	virq = irq_of_parse_and_map(of_node, 0);
 	if (virq)
 		return virq;
 
 	/* Now do the horrible hacks */
-	tmp = of_get_property(pic->host->of_node, "#interrupt-cells", NULL);
+	tmp = of_get_property(of_node, "#interrupt-cells", NULL);
 	if (tmp == NULL)
 		return NO_IRQ;
 	intsize = *tmp;
-	imap = of_get_property(pic->host->of_node, "interrupt-map", &imaplen);
+	imap = of_get_property(of_node, "interrupt-map", &imaplen);
 	if (imap == NULL || imaplen < (intsize + 1))
 		return NO_IRQ;
 	iic = of_find_node_by_phandle(imap[intsize]);
diff --git a/arch/powerpc/platforms/powernv/opal-irqchip.c b/arch/powerpc/platforms/powernv/opal-irqchip.c
index 2c91ee7..6ccfb6c 100644
--- a/arch/powerpc/platforms/powernv/opal-irqchip.c
+++ b/arch/powerpc/platforms/powernv/opal-irqchip.c
@@ -137,7 +137,7 @@ static void opal_handle_irq_work(struct irq_work *work)
 static int opal_event_match(struct irq_domain *h, struct device_node *node,
 			    enum irq_domain_bus_token bus_token)
 {
-	return h->of_node == node;
+	return irq_domain_get_of_node(h) == node;
 }
 
 static int opal_event_xlate(struct irq_domain *h, struct device_node *np,
diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
index eca0b00..bffcc7a 100644
--- a/arch/powerpc/sysdev/ehv_pic.c
+++ b/arch/powerpc/sysdev/ehv_pic.c
@@ -181,7 +181,8 @@ static int ehv_pic_host_match(struct irq_domain *h, struct device_node *node,
 			      enum irq_domain_bus_token bus_token)
 {
 	/* Exact match, unless ehv_pic node is NULL */
-	return h->of_node == NULL || h->of_node == node;
+	struct device_node *of_node = irq_domain_get_of_node(h);
+	return of_node == NULL || of_node == node;
 }
 
 static int ehv_pic_host_map(struct irq_domain *h, unsigned int virq,
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 5916da1..a9bdc4b 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -110,7 +110,7 @@ static int fsl_msi_init_allocator(struct fsl_msi *msi_data)
 	int rc, hwirq;
 
 	rc = msi_bitmap_alloc(&msi_data->bitmap, NR_MSI_IRQS_MAX,
-			      msi_data->irqhost->of_node);
+			      irq_domain_get_of_node(msi_data->irqhost));
 	if (rc)
 		return rc;
 
diff --git a/arch/powerpc/sysdev/i8259.c b/arch/powerpc/sysdev/i8259.c
index e1a9c2c..6f99ed3 100644
--- a/arch/powerpc/sysdev/i8259.c
+++ b/arch/powerpc/sysdev/i8259.c
@@ -165,7 +165,8 @@ static struct resource pic_edgectrl_iores = {
 static int i8259_host_match(struct irq_domain *h, struct device_node *node,
 			    enum irq_domain_bus_token bus_token)
 {
-	return h->of_node == NULL || h->of_node == node;
+	struct device_node *of_node = irq_domain_get_of_node(h);
+	return of_node == NULL || of_node == node;
 }
 
 static int i8259_host_map(struct irq_domain *h, unsigned int virq,
diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index 6b2b689..1fbae68 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -675,7 +675,8 @@ static int ipic_host_match(struct irq_domain *h, struct device_node *node,
 			   enum irq_domain_bus_token bus_token)
 {
 	/* Exact match, unless ipic node is NULL */
-	return h->of_node == NULL || h->of_node == node;
+	struct device_node *of_node = irq_domain_get_of_node(h);
+	return of_node == NULL || of_node == node;
 }
 
 static int ipic_host_map(struct irq_domain *h, unsigned int virq,
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 97a8ae8..aea54c2 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1011,7 +1011,8 @@ static int mpic_host_match(struct irq_domain *h, struct device_node *node,
 			   enum irq_domain_bus_token bus_token)
 {
 	/* Exact match, unless mpic node is NULL */
-	return h->of_node == NULL || h->of_node == node;
+	struct device_node *of_node = irq_domain_get_of_node(h);
+	return of_node == NULL || of_node == node;
 }
 
 static int mpic_host_map(struct irq_domain *h, unsigned int virq,
diff --git a/arch/powerpc/sysdev/mpic_msi.c b/arch/powerpc/sysdev/mpic_msi.c
index 7dc39f3..1d48a53 100644
--- a/arch/powerpc/sysdev/mpic_msi.c
+++ b/arch/powerpc/sysdev/mpic_msi.c
@@ -84,7 +84,7 @@ int mpic_msi_init_allocator(struct mpic *mpic)
 	int rc;
 
 	rc = msi_bitmap_alloc(&mpic->msi_bitmap, mpic->num_sources,
-			      mpic->irqhost->of_node);
+			      irq_domain_get_of_node(mpic->irqhost));
 	if (rc)
 		return rc;
 
diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index 47b352e..8378dd5 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -248,7 +248,8 @@ static int qe_ic_host_match(struct irq_domain *h, struct device_node *node,
 			    enum irq_domain_bus_token bus_token)
 {
 	/* Exact match, unless qe_ic node is NULL */
-	return h->of_node == NULL || h->of_node == node;
+	struct device_node *of_node = irq_domain_get_of_node(h);
+	return of_node == NULL || of_node == node;
 }
 
 static int qe_ic_host_map(struct irq_domain *h, unsigned int virq,
diff --git a/drivers/gpio/gpio-sodaville.c b/drivers/gpio/gpio-sodaville.c
index 65bc9f4..34b02b4 100644
--- a/drivers/gpio/gpio-sodaville.c
+++ b/drivers/gpio/gpio-sodaville.c
@@ -102,7 +102,7 @@ static int sdv_xlate(struct irq_domain *h, struct device_node *node,
 {
 	u32 line, type;
 
-	if (node != h->of_node)
+	if (node != irq_domain_get_of_node(h))
 		return -EINVAL;
 
 	if (intsize < 2)
diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index 5c82e3b..312cb86 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -146,7 +146,7 @@ static int combiner_irq_domain_xlate(struct irq_domain *d,
 				     unsigned long *out_hwirq,
 				     unsigned int *out_type)
 {
-	if (d->of_node != controller)
+	if (irq_domain_get_of_node(d) != controller)
 		return -EINVAL;
 
 	if (intsize < 2)
diff --git a/drivers/irqchip/irq-atmel-aic-common.c b/drivers/irqchip/irq-atmel-aic-common.c
index 63cd031..b12a5d5 100644
--- a/drivers/irqchip/irq-atmel-aic-common.c
+++ b/drivers/irqchip/irq-atmel-aic-common.c
@@ -114,7 +114,7 @@ int aic_common_irq_domain_xlate(struct irq_domain *d,
 
 static void __init aic_common_ext_irq_of_init(struct irq_domain *domain)
 {
-	struct device_node *node = domain->of_node;
+	struct device_node *node = irq_domain_get_of_node(domain);
 	struct irq_chip_generic *gc;
 	struct aic_chip_data *aic;
 	struct property *prop;
diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 692fe2b..1a3248b 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -93,7 +93,7 @@ static int allocate_gic_irq(struct irq_domain *domain, unsigned virq,
 	if (i < 0)
 		return -ENODEV;
 
-	args.np = domain->parent->of_node;
+	args.np = irq_domain_get_of_node(domain->parent);
 	args.args_count = 3;
 	args.args[0] = 0;	/* SPI */
 	args.args[1] = i;
@@ -171,7 +171,7 @@ static int crossbar_domain_xlate(struct irq_domain *d,
 				 unsigned long *out_hwirq,
 				 unsigned int *out_type)
 {
-	if (d->of_node != controller)
+	if (irq_domain_get_of_node(d) != controller)
 		return -EINVAL;	/* Shouldn't happen, really... */
 	if (intsize != 3)
 		return -EINVAL;	/* Not GIC compliant */
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index db04fc1..d0fcbf8 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -117,7 +117,7 @@ static int gicv2m_irq_gic_domain_alloc(struct irq_domain *domain,
 	struct irq_data *d;
 	int err;
 
-	args.np = domain->parent->of_node;
+	args.np = irq_domain_get_of_node(domain->parent);
 	args.args_count = 3;
 	args.args[0] = 0;
 	args.args[1] = hwirq - 32;
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 21b002f..e85a7e3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1201,7 +1201,7 @@ static int its_irq_gic_domain_alloc(struct irq_domain *domain,
 {
 	struct of_phandle_args args;
 
-	args.np = domain->parent->of_node;
+	args.np = irq_domain_get_of_node(domain->parent);
 	args.args_count = 3;
 	args.args[0] = GIC_IRQ_TYPE_LPI;
 	args.args[1] = hwirq;
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index c52f7ba..5492f4e 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -706,7 +706,7 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
 				const u32 *intspec, unsigned int intsize,
 				unsigned long *out_hwirq, unsigned int *out_type)
 {
-	if (d->of_node != controller)
+	if (irq_domain_get_of_node(d) != controller)
 		return -EINVAL;
 	if (intsize < 3)
 		return -EINVAL;
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4dd8826..b41ccf5 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -813,7 +813,7 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
 {
 	unsigned long ret = 0;
 
-	if (d->of_node != controller)
+	if (irq_domain_get_of_node(d) != controller)
 		return -EINVAL;
 	if (intsize < 3)
 		return -EINVAL;
diff --git a/drivers/irqchip/irq-hip04.c b/drivers/irqchip/irq-hip04.c
index 0cae45d..73e5263 100644
--- a/drivers/irqchip/irq-hip04.c
+++ b/drivers/irqchip/irq-hip04.c
@@ -323,7 +323,7 @@ static int hip04_irq_domain_xlate(struct irq_domain *d,
 {
 	unsigned long ret = 0;
 
-	if (d->of_node != controller)
+	if (irq_domain_get_of_node(d) != controller)
 		return -EINVAL;
 	if (intsize < 3)
 		return -EINVAL;
diff --git a/drivers/irqchip/irq-mtk-sysirq.c b/drivers/irqchip/irq-mtk-sysirq.c
index 15c1303..f3567b8 100644
--- a/drivers/irqchip/irq-mtk-sysirq.c
+++ b/drivers/irqchip/irq-mtk-sysirq.c
@@ -107,7 +107,7 @@ static int mtk_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 					      &mtk_sysirq_chip,
 					      domain->host_data);
 
-	gic_data.np = domain->parent->of_node;
+	gic_data.np = irq_domain_get_of_node(domain->parent);
 	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &gic_data);
 }
 
diff --git a/drivers/irqchip/irq-s3c24xx.c b/drivers/irqchip/irq-s3c24xx.c
index e96717f..85cccc2 100644
--- a/drivers/irqchip/irq-s3c24xx.c
+++ b/drivers/irqchip/irq-s3c24xx.c
@@ -314,7 +314,7 @@ static void s3c_irq_demux(unsigned int irq, struct irq_desc *desc)
 	 * and one big domain for the dt case where the subintc
 	 * starts at hwirq number 32.
 	 */
-	offset = (intc->domain->of_node) ? 32 : 0;
+	offset = irq_domain_get_of_node(intc->domain) ? 32 : 0;
 
 	chained_irq_enter(chip, desc);
 
@@ -345,7 +345,7 @@ static inline int s3c24xx_handle_intc(struct s3c_irq_intc *intc,
 		return false;
 
 	/* non-dt machines use individual domains */
-	if (!intc->domain->of_node)
+	if (!irq_domain_get_of_node(intc->domain))
 		intc_offset = 0;
 
 	/* We have a problem that the INTOFFSET register does not always
diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
index f67bbd8..0eef868 100644
--- a/drivers/irqchip/irq-tegra.c
+++ b/drivers/irqchip/irq-tegra.c
@@ -228,7 +228,7 @@ static int tegra_ictlr_domain_xlate(struct irq_domain *domain,
 				    unsigned long *out_hwirq,
 				    unsigned int *out_type)
 {
-	if (domain->of_node != controller)
+	if (irq_domain_get_of_node(domain) != controller)
 		return -EINVAL;	/* Shouldn't happen, really... */
 	if (intsize != 3)
 		return -EINVAL;	/* Not GIC compliant */
@@ -268,7 +268,7 @@ static int tegra_ictlr_domain_alloc(struct irq_domain *domain,
 	}
 
 	parent_args = *args;
-	parent_args.np = domain->parent->of_node;
+	parent_args.np = irq_domain_get_of_node(domain->parent);
 	return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &parent_args);
 }
 
diff --git a/drivers/irqchip/irq-vf610-mscm-ir.c b/drivers/irqchip/irq-vf610-mscm-ir.c
index f5c01cb..3f74676 100644
--- a/drivers/irqchip/irq-vf610-mscm-ir.c
+++ b/drivers/irqchip/irq-vf610-mscm-ir.c
@@ -143,7 +143,7 @@ static int vf610_mscm_ir_domain_alloc(struct irq_domain *domain, unsigned int vi
 					      &vf610_mscm_ir_irq_chip,
 					      domain->host_data);
 
-	gic_data.np = domain->parent->of_node;
+	gic_data.np = irq_domain_get_of_node(domain->parent);
 
 	if (mscm_ir_data->is_nvic) {
 		gic_data.args_count = 1;
@@ -206,7 +206,8 @@ static int __init vf610_mscm_ir_of_init(struct device_node *node,
 		goto out_unmap;
 	}
 
-	if (of_device_is_compatible(domain->parent->of_node, "arm,armv7m-nvic"))
+	if (of_device_is_compatible(irq_domain_get_of_node(domain->parent),
+				    "arm,armv7m-nvic"))
 		mscm_ir_data->is_nvic = true;
 
 	cpu_pm_register_notifier(&mscm_ir_notifier_block);
diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index d7119db..7f04635 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -640,7 +640,7 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
 		"intspec[0] 0x%1x intspec[1] 0x%02x intspec[2] 0x%02x\n",
 		intspec[0], intspec[1], intspec[2]);
 
-	if (d->of_node != controller)
+	if (irq_domain_get_of_node(d) != controller)
 		return -EINVAL;
 	if (intsize != 4)
 		return -EINVAL;
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index d3ca792..f644fdb 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -161,6 +161,11 @@ enum {
 	IRQ_DOMAIN_FLAG_NONCORE		= (1 << 16),
 };
 
+static inline struct device_node *irq_domain_get_of_node(struct irq_domain *d)
+{
+	return d->of_node;
+}
+
 #ifdef CONFIG_IRQ_DOMAIN
 struct irq_domain *__irq_domain_add(struct device_node *of_node, int size,
 				    irq_hw_number_t hwirq_max, int direct_max,
-- 
2.1.4

[PATCH 2/5] genirq: irqdomain: Remove irqdomain dependency on struct device_node

[Marc Zyngier]

struct device_node is very much DT specific, and the original authors
of the irqdomain subsystem recognized that tie, and went as far as
mentionning that this could be replaced by some "void *token",
should another firmware infrastructure be using it.

As we move ACPI on arm64 towards this model too, it makes a lot of sense
to perform that particular move.

We replace "struct device_node *of_node" with "void *domain_token", which
is a benign enough transformation. A non DT user of irqdomain can now
identify its domains using this pointer.

Also, in order to prevent the introduction of sideband type information,
only DT is allowed to store a valid kernel pointer in domain_token
(a pointer that passes the virt_addr_valid() test will be considered
as a valid device_node).

non-DT users that wish to store valid pointers in domain_token are
required to use another structure such as an IDR.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/irqdomain.h | 66 ++++++++++++++++++++++------------------
 kernel/irq/irqdomain.c    | 77 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 93 insertions(+), 50 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index f644fdb..7fd998d 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -17,16 +17,14 @@
  * model). It's the domain callbacks that are responsible for setting the
  * irq_chip on a given irq_desc after it's been mapped.
  *
- * The host code and data structures are agnostic to whether or not
- * we use an open firmware device-tree. We do have references to struct
- * device_node in two places: in irq_find_host() to find the host matching
- * a given interrupt controller node, and of course as an argument to its
- * counterpart domain->ops->match() callback. However, those are treated as
- * generic pointers by the core and the fact that it's actually a device-node
- * pointer is purely a convention between callers and implementation. This
- * code could thus be used on other architectures by replacing those two
- * by some sort of arch-specific void * "token" used to identify interrupt
- * controllers.
+ * The host code and data structures are agnostic to whether or not we
+ * use an open firmware device-tree. Domains can be assigned a
+ * "void *domain_token" identifier, which is assumed to represent a
+ * "struct device_node" if the pointer is a valid virtual address.
+ *
+ * Otherwise, the value is assumed to be an opaque identifier. Should
+ * a pointer to a non "struct device_node" be required, another data
+ * structure should be used to indirect it (an IDR for example).
  */
 
 #ifndef _LINUX_IRQDOMAIN_H
@@ -108,8 +106,8 @@ struct irq_domain_chip_generic;
  * @flags: host per irq_domain flags
  *
  * Optional elements
- * @of_node: Pointer to device tree nodes associated with the irq_domain. Used
- *           when decoding device tree interrupt specifiers.
+ * @domain_token: optional firmware-specific identifier for
+ *                the interrupt controller
  * @gc: Pointer to a list of generic chips. There is a helper function for
  *      setting up one or more generic chips for interrupt controllers
  *      drivers using the generic chip library which uses this pointer.
@@ -130,7 +128,7 @@ struct irq_domain {
 	unsigned int flags;
 
 	/* Optional data */
-	struct device_node *of_node;
+	void *domain_token;
 	enum irq_domain_bus_token bus_token;
 	struct irq_domain_chip_generic *gc;
 #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
@@ -161,70 +159,73 @@ enum {
 	IRQ_DOMAIN_FLAG_NONCORE		= (1 << 16),
 };
 
+#ifdef CONFIG_IRQ_DOMAIN
+extern struct device_node *irq_domain_token_to_of_node(void *domain_token);
+
 static inline struct device_node *irq_domain_get_of_node(struct irq_domain *d)
 {
-	return d->of_node;
+	return irq_domain_token_to_of_node(d->domain_token);
 }
 
-#ifdef CONFIG_IRQ_DOMAIN
-struct irq_domain *__irq_domain_add(struct device_node *of_node, int size,
+struct irq_domain *__irq_domain_add(void *domain_token, int size,
 				    irq_hw_number_t hwirq_max, int direct_max,
 				    const struct irq_domain_ops *ops,
 				    void *host_data);
-struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
+struct irq_domain *irq_domain_add_simple(void *domain_token,
 					 unsigned int size,
 					 unsigned int first_irq,
 					 const struct irq_domain_ops *ops,
 					 void *host_data);
-struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
+struct irq_domain *irq_domain_add_legacy(void *domain_token,
 					 unsigned int size,
 					 unsigned int first_irq,
 					 irq_hw_number_t first_hwirq,
 					 const struct irq_domain_ops *ops,
 					 void *host_data);
-extern struct irq_domain *irq_find_matching_host(struct device_node *node,
+extern struct irq_domain *irq_find_matching_host(void *domain_token,
 						 enum irq_domain_bus_token bus_token);
 extern void irq_set_default_host(struct irq_domain *host);
 
-static inline struct irq_domain *irq_find_host(struct device_node *node)
+static inline struct irq_domain *irq_find_host(void *domain_token)
 {
-	return irq_find_matching_host(node, DOMAIN_BUS_ANY);
+	return irq_find_matching_host(domain_token, DOMAIN_BUS_ANY);
 }
 
 /**
  * irq_domain_add_linear() - Allocate and register a linear revmap irq_domain.
- * @of_node: pointer to interrupt controller's device tree node.
+ * @domain_token: optional firmware-specific identifier for
+ *                the interrupt controller
  * @size: Number of interrupts in the domain.
  * @ops: map/unmap domain callbacks
  * @host_data: Controller private data pointer
  */
-static inline struct irq_domain *irq_domain_add_linear(struct device_node *of_node,
+static inline struct irq_domain *irq_domain_add_linear(void *domain_token,
 					 unsigned int size,
 					 const struct irq_domain_ops *ops,
 					 void *host_data)
 {
-	return __irq_domain_add(of_node, size, size, 0, ops, host_data);
+	return __irq_domain_add(domain_token, size, size, 0, ops, host_data);
 }
-static inline struct irq_domain *irq_domain_add_nomap(struct device_node *of_node,
+static inline struct irq_domain *irq_domain_add_nomap(void *domain_token,
 					 unsigned int max_irq,
 					 const struct irq_domain_ops *ops,
 					 void *host_data)
 {
-	return __irq_domain_add(of_node, 0, max_irq, max_irq, ops, host_data);
+	return __irq_domain_add(domain_token, 0, max_irq, max_irq, ops, host_data);
 }
 static inline struct irq_domain *irq_domain_add_legacy_isa(
-				struct device_node *of_node,
+				void *domain_token,
 				const struct irq_domain_ops *ops,
 				void *host_data)
 {
-	return irq_domain_add_legacy(of_node, NUM_ISA_INTERRUPTS, 0, 0, ops,
+	return irq_domain_add_legacy(domain_token, NUM_ISA_INTERRUPTS, 0, 0, ops,
 				     host_data);
 }
-static inline struct irq_domain *irq_domain_add_tree(struct device_node *of_node,
+static inline struct irq_domain *irq_domain_add_tree(void *domain_token,
 					 const struct irq_domain_ops *ops,
 					 void *host_data)
 {
-	return __irq_domain_add(of_node, 0, ~0, 0, ops, host_data);
+	return __irq_domain_add(domain_token, 0, ~0, 0, ops, host_data);
 }
 
 extern void irq_domain_remove(struct irq_domain *host);
@@ -347,6 +348,11 @@ static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
 #endif	/* CONFIG_IRQ_DOMAIN_HIERARCHY */
 
 #else /* CONFIG_IRQ_DOMAIN */
+static inline struct device_node *irq_domain_get_of_node(struct irq_domain *d)
+{
+	return d->domain_token;
+}
+
 static inline void irq_dispose_mapping(unsigned int virq) { }
 static inline void irq_domain_activate_irq(struct irq_data *data) { }
 static inline void irq_domain_deactivate_irq(struct irq_data *data) { }
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 995d217..27f4ec7 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -6,6 +6,7 @@
 #include <linux/irq.h>
 #include <linux/irqdesc.h>
 #include <linux/irqdomain.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
@@ -27,9 +28,24 @@ static int irq_domain_alloc_descs(int virq, unsigned int nr_irqs,
 				  irq_hw_number_t hwirq, int node);
 static void irq_domain_check_hierarchy(struct irq_domain *domain);
 
+struct device_node *irq_domain_token_to_of_node(void *domain_token)
+{
+	/*
+	 * Assume that anything represented by a valid kernel address
+	 * is a device_node. Anything else must be a "small integer",
+	 * and indirected by some other structure (an IDR, for
+	 * example) if a pointer is required.
+	 */
+	if (virt_addr_valid(domain_token))
+		return domain_token;
+
+	return NULL;
+}
+
 /**
  * __irq_domain_add() - Allocate a new irq_domain data structure
- * @of_node: optional device-tree node of the interrupt controller
+ * @domain_token: optional firmware-specific identifier for
+ *                the interrupt controller
  * @size: Size of linear map; 0 for radix mapping only
  * @hwirq_max: Maximum number of interrupts supported by controller
  * @direct_max: Maximum value of direct maps; Use ~0 for no limit; 0 for no
@@ -40,13 +56,15 @@ static void irq_domain_check_hierarchy(struct irq_domain *domain);
  * Allocates and initialize and irq_domain structure.
  * Returns pointer to IRQ domain, or NULL on failure.
  */
-struct irq_domain *__irq_domain_add(struct device_node *of_node, int size,
+struct irq_domain *__irq_domain_add(void *domain_token, int size,
 				    irq_hw_number_t hwirq_max, int direct_max,
 				    const struct irq_domain_ops *ops,
 				    void *host_data)
 {
+	struct device_node *of_node;
 	struct irq_domain *domain;
 
+	of_node = irq_domain_token_to_of_node(domain_token);
 	domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
 			      GFP_KERNEL, of_node_to_nid(of_node));
 	if (WARN_ON(!domain))
@@ -56,7 +74,7 @@ struct irq_domain *__irq_domain_add(struct device_node *of_node, int size,
 	INIT_RADIX_TREE(&domain->revmap_tree, GFP_KERNEL);
 	domain->ops = ops;
 	domain->host_data = host_data;
-	domain->of_node = of_node_get(of_node);
+	domain->domain_token = of_node_get(of_node) ?: domain_token;
 	domain->hwirq_max = hwirq_max;
 	domain->revmap_size = size;
 	domain->revmap_direct_max_irq = direct_max;
@@ -102,14 +120,15 @@ void irq_domain_remove(struct irq_domain *domain)
 
 	pr_debug("Removed domain %s\n", domain->name);
 
-	of_node_put(domain->of_node);
+	of_node_put(irq_domain_get_of_node(domain));
 	kfree(domain);
 }
 EXPORT_SYMBOL_GPL(irq_domain_remove);
 
 /**
  * irq_domain_add_simple() - Register an irq_domain and optionally map a range of irqs
- * @of_node: pointer to interrupt controller's device tree node.
+ * @domain_token: optional firmware-specific identifier for
+ *                the interrupt controller
  * @size: total number of irqs in mapping
  * @first_irq: first number of irq block assigned to the domain,
  *	pass zero to assign irqs on-the-fly. If first_irq is non-zero, then
@@ -125,18 +144,20 @@ EXPORT_SYMBOL_GPL(irq_domain_remove);
  * irqs get mapped dynamically on the fly. However, if the controller requires
  * static virq assignments (non-DT boot) then it will set that up correctly.
  */
-struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
+struct irq_domain *irq_domain_add_simple(void *domain_token,
 					 unsigned int size,
 					 unsigned int first_irq,
 					 const struct irq_domain_ops *ops,
 					 void *host_data)
 {
+	struct device_node *of_node;
 	struct irq_domain *domain;
 
-	domain = __irq_domain_add(of_node, size, size, 0, ops, host_data);
+	domain = __irq_domain_add(domain_token, size, size, 0, ops, host_data);
 	if (!domain)
 		return NULL;
 
+	of_node = irq_domain_token_to_of_node(domain_token);
 	if (first_irq > 0) {
 		if (IS_ENABLED(CONFIG_SPARSE_IRQ)) {
 			/* attempt to allocated irq_descs */
@@ -155,7 +176,8 @@ EXPORT_SYMBOL_GPL(irq_domain_add_simple);
 
 /**
  * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
- * @of_node: pointer to interrupt controller's device tree node.
+ * @domain_token: optional firmware-specific identifier for
+ *                the interrupt controller
  * @size: total number of irqs in legacy mapping
  * @first_irq: first number of irq block assigned to the domain
  * @first_hwirq: first hwirq number to use for the translation. Should normally
@@ -168,7 +190,7 @@ EXPORT_SYMBOL_GPL(irq_domain_add_simple);
  * for all legacy interrupts except 0 (which is always the invalid irq for
  * a legacy controller).
  */
-struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
+struct irq_domain *irq_domain_add_legacy(void *domain_token,
 					 unsigned int size,
 					 unsigned int first_irq,
 					 irq_hw_number_t first_hwirq,
@@ -177,7 +199,7 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
 {
 	struct irq_domain *domain;
 
-	domain = __irq_domain_add(of_node, first_hwirq + size,
+	domain = __irq_domain_add(domain_token, first_hwirq + size,
 				  first_hwirq + size, 0, ops, host_data);
 	if (domain)
 		irq_domain_associate_many(domain, first_irq, first_hwirq, size);
@@ -191,7 +213,7 @@ EXPORT_SYMBOL_GPL(irq_domain_add_legacy);
  * @node: device-tree node of the interrupt controller
  * @data: domain-specific data
  */
-struct irq_domain *irq_find_matching_host(struct device_node *node,
+struct irq_domain *irq_find_matching_host(void *domain_token,
 					  enum irq_domain_bus_token bus_token)
 {
 	struct irq_domain *h, *found = NULL;
@@ -208,12 +230,16 @@ struct irq_domain *irq_find_matching_host(struct device_node *node,
 	 */
 	mutex_lock(&irq_domain_mutex);
 	list_for_each_entry(h, &irq_domain_list, link) {
-		if (h->ops->match)
+		if (h->ops->match) {
+			struct device_node *node;
+			node = irq_domain_get_of_node(h);
 			rc = h->ops->match(h, node, bus_token);
-		else
-			rc = ((h->of_node != NULL) && (h->of_node == node) &&
+		} else {
+			rc = ((h->domain_token != NULL) &&
+			      (h->domain_token == domain_token) &&
 			      ((bus_token == DOMAIN_BUS_ANY) ||
 			       (h->bus_token == bus_token)));
+		}
 
 		if (rc) {
 			found = h;
@@ -336,10 +362,12 @@ EXPORT_SYMBOL_GPL(irq_domain_associate);
 void irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
 			       irq_hw_number_t hwirq_base, int count)
 {
+	struct device_node *of_node;
 	int i;
 
+	of_node = irq_domain_get_of_node(domain);
 	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);
+		of_node_full_name(of_node), irq_base, (int)hwirq_base, count);
 
 	for (i = 0; i < count; i++) {
 		irq_domain_associate(domain, irq_base + i, hwirq_base + i);
@@ -359,12 +387,14 @@ EXPORT_SYMBOL_GPL(irq_domain_associate_many);
  */
 unsigned int irq_create_direct_mapping(struct irq_domain *domain)
 {
+	struct device_node *of_node;
 	unsigned int virq;
 
 	if (domain == NULL)
 		domain = irq_default_domain;
 
-	virq = irq_alloc_desc_from(1, of_node_to_nid(domain->of_node));
+	of_node = irq_domain_get_of_node(domain);
+	virq = irq_alloc_desc_from(1, of_node_to_nid(of_node));
 	if (!virq) {
 		pr_debug("create_direct virq allocation failed\n");
 		return 0;
@@ -399,6 +429,7 @@ EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
 unsigned int irq_create_mapping(struct irq_domain *domain,
 				irq_hw_number_t hwirq)
 {
+	struct device_node *of_node;
 	int virq;
 
 	pr_debug("irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq);
@@ -419,9 +450,11 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 		return virq;
 	}
 
+	of_node = irq_domain_get_of_node(domain);
+
 	/* Allocate a virtual interrupt number */
 	virq = irq_domain_alloc_descs(-1, 1, hwirq,
-				      of_node_to_nid(domain->of_node));
+				      of_node_to_nid(of_node));
 	if (virq <= 0) {
 		pr_debug("-> virq allocation failed\n");
 		return 0;
@@ -433,7 +466,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 	}
 
 	pr_debug("irq %lu on domain %s mapped to virtual irq %u\n",
-		hwirq, of_node_full_name(domain->of_node), virq);
+		hwirq, of_node_full_name(of_node), virq);
 
 	return virq;
 }
@@ -460,10 +493,12 @@ EXPORT_SYMBOL_GPL(irq_create_mapping);
 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(domain->of_node));
+			      of_node_to_nid(of_node));
 	if (unlikely(ret < 0))
 		return ret;
 
@@ -590,14 +625,16 @@ static int virq_debug_show(struct seq_file *m, void *private)
 		   "name", "mapped", "linear-max", "direct-max", "devtree-node");
 	mutex_lock(&irq_domain_mutex);
 	list_for_each_entry(domain, &irq_domain_list, link) {
+		struct device_node *of_node;
 		int count = 0;
+		of_node = irq_domain_token_get_node(domain);
 		radix_tree_for_each_slot(slot, &domain->revmap_tree, &iter, 0)
 			count++;
 		seq_printf(m, "%c%-16s  %6u  %10u  %10u  %s\n",
 			   domain == irq_default_domain ? '*' : ' ', domain->name,
 			   domain->revmap_size + count, domain->revmap_size,
 			   domain->revmap_direct_max_irq,
-			   domain->of_node ? of_node_full_name(domain->of_node) : "");
+			   of_node ? of_node_full_name(of_node) : "");
 	}
 	mutex_unlock(&irq_domain_mutex);
 
-- 
2.1.4

[PATCH 3/5] genirq: irqdomain: Add irq_create_acpi_mappings

[Marc Zyngier]

In order to help ACPI on arm64 to make use of most of the irqdomain
goodies, add a new entry point (irq_create_acpi_mapping) which
mimics irq_create_of_mapping, except that it takes a new
struct acpi_gsi_descriptor, which is the pendent of of_phandle_args
in the OF world.

We assume that the way the acpi_gsi_descriptor is populated matches
of_phandle_args, as the latter is still the building block for
interrupt descriptor in the whole kernel.

Eventually, these two representations should be merged in a single
structure, but that's probably for another day.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/acpi.h   |  9 +++++++++
 kernel/irq/irqdomain.c | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d2445fa..2f23ab0 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -205,6 +205,15 @@ int acpi_register_gsi (struct device *dev, u32 gsi, int triggering, int polarity
 int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
 int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
 
+#define MAX_GSI_DESC_PARAMS 16
+struct acpi_gsi_descriptor {
+	int param_count;
+	u32 param[MAX_GSI_DESC_PARAMS];
+};
+
+unsigned int irq_create_acpi_mapping(struct irq_domain *d,
+				     struct acpi_gsi_descriptor *irq_data);
+
 #ifdef CONFIG_X86_IO_APIC
 extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
 #else
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 27f4ec7..542cda3 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1,5 +1,6 @@
 #define pr_fmt(fmt)  "irq: " fmt
 
+#include <linux/acpi.h>
 #include <linux/debugfs.h>
 #include <linux/hardirq.h>
 #include <linux/interrupt.h>
@@ -557,6 +558,23 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
 }
 EXPORT_SYMBOL_GPL(irq_create_of_mapping);
 
+#ifdef CONFIG_ACPI
+unsigned int irq_create_acpi_mapping(struct irq_domain *d,
+				     struct acpi_gsi_descriptor *irq_data)
+{
+	struct of_phandle_args args;
+	int i;
+
+	for (i = 0; i < min(irq_data->param_count, MAX_PHANDLE_ARGS); i++)
+		args.args[i] = irq_data->param[i];
+
+	args.np = d ? d->domain_token : NULL;
+	args.args_count = i;
+
+	return irq_create_of_mapping(&args);
+}
+#endif
+
 /**
  * irq_dispose_mapping() - Unmap an interrupt
  * @virq: linux irq number of the interrupt to unmap
-- 
2.1.4

[PATCH 4/5] acpi: gsi: Use acpi_gsi_descriptor to allocate interrupts

[Marc Zyngier]

Now that the irqdomain layer is a bit more ACPI friendly, add the
mapping code that allows irq_create_acpi_mapping to be called.

As we only support the GIC so far, support is pretty limited.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/acpi/gsi.c | 54 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index 38208f2..005d843 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -33,6 +33,12 @@ static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
 	}
 }
 
+static struct irq_domain *irq_find_acpi_domain(enum acpi_irq_model_id id,
+					       enum irq_domain_bus_token bus_token)
+{
+	return irq_find_matching_host((void *)id, bus_token);
+}
+
 /**
  * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
  * @gsi: GSI IRQ number to map
@@ -45,12 +51,10 @@ static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
  */
 int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
 {
-	/*
-	 * Only default domain is supported at present, always find
-	 * the mapping corresponding to default domain by passing NULL
-	 * as irq_domain parameter
-	 */
-	*irq = irq_find_mapping(NULL, gsi);
+	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
+						    DOMAIN_BUS_ANY);
+
+	*irq = irq_find_mapping(d, gsi);
 	/*
 	 * *irq == 0 means no mapping, that should
 	 * be reported as a failure
@@ -72,23 +76,35 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
 int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
 		      int polarity)
 {
-	unsigned int irq;
+	struct acpi_gsi_descriptor data;
 	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
+	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
+						    DOMAIN_BUS_ANY);
 
 	/*
-	 * There is no way at present to look-up the IRQ domain on ACPI,
-	 * hence always create mapping referring to the default domain
-	 * by passing NULL as irq_domain parameter
+	 * Populate the GSI descriptor in a way that matches the way
+	 * the driver expects its of_phandle_args.
 	 */
-	irq = irq_create_mapping(NULL, gsi);
-	if (!irq)
+	switch (acpi_irq_model) {
+	case ACPI_IRQ_MODEL_GIC:
+		if (gsi >= 32) {
+			data.param[0] = 0;
+			data.param[1] = gsi - 32;
+			data.param[2] = irq_type;
+		} else {
+			data.param[0] = 1;
+			data.param[1] = gsi - 16;
+			data.param[2] = 0xff << 4 | irq_type;
+		}
+
+		data.param_count = 3;
+		break;
+	default:
+		pr_warn("Unknown acpi_irq_model = %d\n", acpi_irq_model);
 		return -EINVAL;
+	}
 
-	/* Set irq type if specified and different than the current one */
-	if (irq_type != IRQ_TYPE_NONE &&
-		irq_type != irq_get_trigger_type(irq))
-		irq_set_irq_type(irq, irq_type);
-	return irq;
+	return irq_create_acpi_mapping(d, &data);
 }
 EXPORT_SYMBOL_GPL(acpi_register_gsi);
 
@@ -98,7 +114,9 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi);
  */
 void acpi_unregister_gsi(u32 gsi)
 {
-	int irq = irq_find_mapping(NULL, gsi);
+	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
+						    DOMAIN_BUS_ANY);
+	int irq = irq_find_mapping(d, gsi);
 
 	irq_dispose_mapping(irq);
 }
-- 
2.1.4

[PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Marc Zyngier]

Now that the basic ACPI GSI code is irq domain aware, make sure
that the ACPI support in the GIC doesn't pointlessly deviate from
the DT path.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic.c       | 17 ++++++-----------
 include/linux/irqchip/arm-gic.h |  2 +-
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index b41ccf5..f5d365d 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
 {
 	unsigned long ret = 0;
 
-	if (irq_domain_get_of_node(d) != controller)
-		return -EINVAL;
 	if (intsize < 3)
 		return -EINVAL;
 
@@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
 
 void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 			   void __iomem *dist_base, void __iomem *cpu_base,
-			   u32 percpu_offset, struct device_node *node)
+			   u32 percpu_offset, void *domain_token)
 {
 	irq_hw_number_t hwirq_base;
 	struct gic_chip_data *gic;
@@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 		gic_irqs = 1020;
 	gic->gic_irqs = gic_irqs;
 
-	if (node) {		/* DT case */
-		gic->domain = irq_domain_add_linear(node, gic_irqs,
+	if (domain_token) {		/* DT/ACPI case */
+		gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
 						    &gic_irq_domain_hierarchy_ops,
 						    gic);
 	} else {		/* Non-DT case */
@@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 			irq_base = irq_start;
 		}
 
-		gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
+		gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
 					hwirq_base, &gic_irq_domain_ops, gic);
 	}
 
@@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
 	}
 
 	/*
-	 * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
-	 * as default IRQ domain to allow for GSI registration and GSI to IRQ
-	 * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
+	 * Initialize zero GIC instance (no multi-GIC support).
 	 */
-	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
-	irq_set_default_host(gic_data[0].domain);
+	gic_init_bases(0, -1, dist_base, cpu_base, 0, (void *)ACPI_IRQ_MODEL_GIC);
 
 	acpi_irq_model = ACPI_IRQ_MODEL_GIC;
 	return 0;
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 9de976b..97799b7 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -97,7 +97,7 @@ struct device_node;
 
 void gic_set_irqchip_flags(unsigned long flags);
 void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
-		    u32 offset, struct device_node *);
+		    u32 offset, void *);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_cpu_if_down(void);
 
-- 
2.1.4

Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Graeme Gregory]

On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
> Now that the basic ACPI GSI code is irq domain aware, make sure
> that the ACPI support in the GIC doesn't pointlessly deviate from
> the DT path.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/irqchip/irq-gic.c       | 17 ++++++-----------
>  include/linux/irqchip/arm-gic.h |  2 +-
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index b41ccf5..f5d365d 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>  {
>  	unsigned long ret = 0;
>  
> -	if (irq_domain_get_of_node(d) != controller)
> -		return -EINVAL;
This change seems to have nothing to do with the description.

Graeme

>  	if (intsize < 3)
>  		return -EINVAL;
>  
> @@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
>  
>  void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  			   void __iomem *dist_base, void __iomem *cpu_base,
> -			   u32 percpu_offset, struct device_node *node)
> +			   u32 percpu_offset, void *domain_token)
>  {
>  	irq_hw_number_t hwirq_base;
>  	struct gic_chip_data *gic;
> @@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  		gic_irqs = 1020;
>  	gic->gic_irqs = gic_irqs;
>  
> -	if (node) {		/* DT case */
> -		gic->domain = irq_domain_add_linear(node, gic_irqs,
> +	if (domain_token) {		/* DT/ACPI case */
> +		gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
>  						    &gic_irq_domain_hierarchy_ops,
>  						    gic);
>  	} else {		/* Non-DT case */
> @@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  			irq_base = irq_start;
>  		}
>  
> -		gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
> +		gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
>  					hwirq_base, &gic_irq_domain_ops, gic);
>  	}
>  
> @@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
>  	}
>  
>  	/*
> -	 * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
> -	 * as default IRQ domain to allow for GSI registration and GSI to IRQ
> -	 * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
> +	 * Initialize zero GIC instance (no multi-GIC support).
>  	 */
> -	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
> -	irq_set_default_host(gic_data[0].domain);
> +	gic_init_bases(0, -1, dist_base, cpu_base, 0, (void *)ACPI_IRQ_MODEL_GIC);
>  
>  	acpi_irq_model = ACPI_IRQ_MODEL_GIC;
>  	return 0;
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 9de976b..97799b7 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -97,7 +97,7 @@ struct device_node;
>  
>  void gic_set_irqchip_flags(unsigned long flags);
>  void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
> -		    u32 offset, struct device_node *);
> +		    u32 offset, void *);
>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>  void gic_cpu_if_down(void);
>  
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/5] Making the generic ACPI GSI layer irqdomain aware

[Graeme Gregory]

On Tue, Jul 21, 2015 at 11:07:55AM +0100, Marc Zyngier wrote:
> The irqdomain code is not entierely really ACPI friendly, as it has
> some built-in knowledge of the device-tree. Nothing too harmful, but
> enough to scare the ARM ACPI developpers which end up with their own
> version of the square wheel.
> 
> This small patch series adapt the irqdomain code to remove the hurdles
> that prevent the full blown irqdomain subsystem to be used on ACPI,
> creates an interface between the GSI layer and the irqdomain, and as
> an example, convert the ARM GIC ACPI support to use irqdomains as
> originally intended.
> 
> Overall, this gives us a way to use irqdomains on both DT and ACPI
> enabled platforms, having very little changes made to the actual
> drivers (other than the probing infrastructure). Because we keep the
> flow of information between the various layers identical between ACPI
> and DT, we immediately benefit from the existing infrastructure.
> 
> I'd really like to hear what people think of that approach, as it
> looks to me a lot simpler than the other approaches currently put on
> the list. The "convert the GSI information to be DT friendly" is
> admitedly not very pretty, but I see it as a stepping stone towards
> unifying the two structures.
> 
> This has been test-booted on Juno, and is based on my irq/ncpi-msi-2
> branch.
> 

I have tested this series on Seattle and it looks excellent to me.

Graeme


> Marc Zyngier (5):
>   genirq: irqdomain: Use an accessor for the of_node field
>   genirq: irqdomain: Remove irqdomain dependency on struct device_node
>   genirq: irqdomain: Add irq_create_acpi_mappings
>   acpi: gsi: Use acpi_gsi_descriptor to allocate interrupts
>   irqchip: GIC: Switch ACPI support to stacked domains
> 
>  arch/mips/cavium-octeon/octeon-irq.c          |  4 +-
>  arch/powerpc/platforms/cell/axon_msi.c        |  2 +-
>  arch/powerpc/platforms/cell/spider-pic.c      |  9 ++-
>  arch/powerpc/platforms/powernv/opal-irqchip.c |  2 +-
>  arch/powerpc/sysdev/ehv_pic.c                 |  3 +-
>  arch/powerpc/sysdev/fsl_msi.c                 |  2 +-
>  arch/powerpc/sysdev/i8259.c                   |  3 +-
>  arch/powerpc/sysdev/ipic.c                    |  3 +-
>  arch/powerpc/sysdev/mpic.c                    |  3 +-
>  arch/powerpc/sysdev/mpic_msi.c                |  2 +-
>  arch/powerpc/sysdev/qe_lib/qe_ic.c            |  3 +-
>  drivers/acpi/gsi.c                            | 54 ++++++++++-----
>  drivers/gpio/gpio-sodaville.c                 |  2 +-
>  drivers/irqchip/exynos-combiner.c             |  2 +-
>  drivers/irqchip/irq-atmel-aic-common.c        |  2 +-
>  drivers/irqchip/irq-crossbar.c                |  4 +-
>  drivers/irqchip/irq-gic-v2m.c                 |  2 +-
>  drivers/irqchip/irq-gic-v3-its.c              |  2 +-
>  drivers/irqchip/irq-gic-v3.c                  |  2 +-
>  drivers/irqchip/irq-gic.c                     | 17 ++---
>  drivers/irqchip/irq-hip04.c                   |  2 +-
>  drivers/irqchip/irq-mtk-sysirq.c              |  2 +-
>  drivers/irqchip/irq-s3c24xx.c                 |  4 +-
>  drivers/irqchip/irq-tegra.c                   |  4 +-
>  drivers/irqchip/irq-vf610-mscm-ir.c           |  5 +-
>  drivers/spmi/spmi-pmic-arb.c                  |  2 +-
>  include/linux/acpi.h                          |  9 +++
>  include/linux/irqchip/arm-gic.h               |  2 +-
>  include/linux/irqdomain.h                     | 67 +++++++++++--------
>  kernel/irq/irqdomain.c                        | 95 +++++++++++++++++++++------
>  30 files changed, 206 insertions(+), 109 deletions(-)
> 
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Marc Zyngier]

On Tue, 21 Jul 2015 13:34:08 +0100
Graeme Gregory <graeme@xora.org.uk> wrote:

> On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
> > Now that the basic ACPI GSI code is irq domain aware, make sure
> > that the ACPI support in the GIC doesn't pointlessly deviate from
> > the DT path.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  drivers/irqchip/irq-gic.c       | 17 ++++++-----------
> >  include/linux/irqchip/arm-gic.h |  2 +-
> >  2 files changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> > index b41ccf5..f5d365d 100644
> > --- a/drivers/irqchip/irq-gic.c
> > +++ b/drivers/irqchip/irq-gic.c
> > @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
> >  {
> >  	unsigned long ret = 0;
> >  
> > -	if (irq_domain_get_of_node(d) != controller)
> > -		return -EINVAL;
> This change seems to have nothing to do with the description.

It has everything to do with making this function usable in the context
of ACPI ;-).

This is another ugly aspect of the irqdomain part, where "controller"
is actually the device_node extracted from of_phandle_args. This will
actually be the domain_token, and this comparison would fail with ACPI.
I may add another patch for that.

On DT, this is actually pretty useless, as we're always registering the
GIC domain with its device_node, so this is really guaranteed to match.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH 2/5] genirq: irqdomain: Remove irqdomain dependency on struct device_node

[Lorenzo Pieralisi]

On Tue, Jul 21, 2015 at 11:07:57AM +0100, Marc Zyngier wrote:
> struct device_node is very much DT specific, and the original authors
> of the irqdomain subsystem recognized that tie, and went as far as
> mentionning that this could be replaced by some "void *token",
> should another firmware infrastructure be using it.

Yes, that's similar to the problem Rafael solved with fwnode_handle,
I do not know if we can extend the fwnode_handle to manage these
generic tokens too, but the approach you have taken seem the right
one to me (and you are doing this for components that are not really
attached to struct device so I am not sure the fwnode_handle approach
can be shoehorned to solve it).

[...]

> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 995d217..27f4ec7 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -6,6 +6,7 @@
>  #include <linux/irq.h>
>  #include <linux/irqdesc.h>
>  #include <linux/irqdomain.h>
> +#include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of.h>
> @@ -27,9 +28,24 @@ static int irq_domain_alloc_descs(int virq, unsigned int nr_irqs,
>                                   irq_hw_number_t hwirq, int node);
>  static void irq_domain_check_hierarchy(struct irq_domain *domain);
> 
> +struct device_node *irq_domain_token_to_of_node(void *domain_token)
> +{
> +       /*
> +        * Assume that anything represented by a valid kernel address
> +        * is a device_node. Anything else must be a "small integer",
> +        * and indirected by some other structure (an IDR, for
> +        * example) if a pointer is required.
> +        */

Or we can convert the opaque token to a struct like fwnode_handle (or
even augment it for this purpose but in the ACPI irqdomain lookup
context it is not necessarily well defined - ie you are doing it for
components (GIC) that are not really attached to struct device).

The rest of the patch is a conversion of the existing code to the opaque
token approach, so once we agreed on the token format we agreed on
the patch, I am fine as it is (we might need an IDR though for other
use cases not covered by this set).

Thanks for putting it together.

Lorenzo

> +       if (virt_addr_valid(domain_token))
> +               return domain_token;
> +
> +       return NULL;
> +}
> +
>  /**
>   * __irq_domain_add() - Allocate a new irq_domain data structure
> - * @of_node: optional device-tree node of the interrupt controller
> + * @domain_token: optional firmware-specific identifier for
> + *                the interrupt controller
>   * @size: Size of linear map; 0 for radix mapping only
>   * @hwirq_max: Maximum number of interrupts supported by controller
>   * @direct_max: Maximum value of direct maps; Use ~0 for no limit; 0 for no
> @@ -40,13 +56,15 @@ static void irq_domain_check_hierarchy(struct irq_domain *domain);
>   * Allocates and initialize and irq_domain structure.
>   * Returns pointer to IRQ domain, or NULL on failure.
>   */
> -struct irq_domain *__irq_domain_add(struct device_node *of_node, int size,
> +struct irq_domain *__irq_domain_add(void *domain_token, int size,
>                                     irq_hw_number_t hwirq_max, int direct_max,
>                                     const struct irq_domain_ops *ops,
>                                     void *host_data)
>  {
> +       struct device_node *of_node;
>         struct irq_domain *domain;
> 
> +       of_node = irq_domain_token_to_of_node(domain_token);
>         domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
>                               GFP_KERNEL, of_node_to_nid(of_node));
>         if (WARN_ON(!domain))
> @@ -56,7 +74,7 @@ struct irq_domain *__irq_domain_add(struct device_node *of_node, int size,
>         INIT_RADIX_TREE(&domain->revmap_tree, GFP_KERNEL);
>         domain->ops = ops;
>         domain->host_data = host_data;
> -       domain->of_node = of_node_get(of_node);
> +       domain->domain_token = of_node_get(of_node) ?: domain_token;
>         domain->hwirq_max = hwirq_max;
>         domain->revmap_size = size;
>         domain->revmap_direct_max_irq = direct_max;
> @@ -102,14 +120,15 @@ void irq_domain_remove(struct irq_domain *domain)
> 
>         pr_debug("Removed domain %s\n", domain->name);
> 
> -       of_node_put(domain->of_node);
> +       of_node_put(irq_domain_get_of_node(domain));
>         kfree(domain);
>  }
>  EXPORT_SYMBOL_GPL(irq_domain_remove);
> 
>  /**
>   * irq_domain_add_simple() - Register an irq_domain and optionally map a range of irqs
> - * @of_node: pointer to interrupt controller's device tree node.
> + * @domain_token: optional firmware-specific identifier for
> + *                the interrupt controller
>   * @size: total number of irqs in mapping
>   * @first_irq: first number of irq block assigned to the domain,
>   *     pass zero to assign irqs on-the-fly. If first_irq is non-zero, then
> @@ -125,18 +144,20 @@ EXPORT_SYMBOL_GPL(irq_domain_remove);
>   * irqs get mapped dynamically on the fly. However, if the controller requires
>   * static virq assignments (non-DT boot) then it will set that up correctly.
>   */
> -struct irq_domain *irq_domain_add_simple(struct device_node *of_node,
> +struct irq_domain *irq_domain_add_simple(void *domain_token,
>                                          unsigned int size,
>                                          unsigned int first_irq,
>                                          const struct irq_domain_ops *ops,
>                                          void *host_data)
>  {
> +       struct device_node *of_node;
>         struct irq_domain *domain;
> 
> -       domain = __irq_domain_add(of_node, size, size, 0, ops, host_data);
> +       domain = __irq_domain_add(domain_token, size, size, 0, ops, host_data);
>         if (!domain)
>                 return NULL;
> 
> +       of_node = irq_domain_token_to_of_node(domain_token);
>         if (first_irq > 0) {
>                 if (IS_ENABLED(CONFIG_SPARSE_IRQ)) {
>                         /* attempt to allocated irq_descs */
> @@ -155,7 +176,8 @@ EXPORT_SYMBOL_GPL(irq_domain_add_simple);
> 
>  /**
>   * irq_domain_add_legacy() - Allocate and register a legacy revmap irq_domain.
> - * @of_node: pointer to interrupt controller's device tree node.
> + * @domain_token: optional firmware-specific identifier for
> + *                the interrupt controller
>   * @size: total number of irqs in legacy mapping
>   * @first_irq: first number of irq block assigned to the domain
>   * @first_hwirq: first hwirq number to use for the translation. Should normally
> @@ -168,7 +190,7 @@ EXPORT_SYMBOL_GPL(irq_domain_add_simple);
>   * for all legacy interrupts except 0 (which is always the invalid irq for
>   * a legacy controller).
>   */
> -struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
> +struct irq_domain *irq_domain_add_legacy(void *domain_token,
>                                          unsigned int size,
>                                          unsigned int first_irq,
>                                          irq_hw_number_t first_hwirq,
> @@ -177,7 +199,7 @@ struct irq_domain *irq_domain_add_legacy(struct device_node *of_node,
>  {
>         struct irq_domain *domain;
> 
> -       domain = __irq_domain_add(of_node, first_hwirq + size,
> +       domain = __irq_domain_add(domain_token, first_hwirq + size,
>                                   first_hwirq + size, 0, ops, host_data);
>         if (domain)
>                 irq_domain_associate_many(domain, first_irq, first_hwirq, size);
> @@ -191,7 +213,7 @@ EXPORT_SYMBOL_GPL(irq_domain_add_legacy);
>   * @node: device-tree node of the interrupt controller
>   * @data: domain-specific data
>   */
> -struct irq_domain *irq_find_matching_host(struct device_node *node,
> +struct irq_domain *irq_find_matching_host(void *domain_token,
>                                           enum irq_domain_bus_token bus_token)
>  {
>         struct irq_domain *h, *found = NULL;
> @@ -208,12 +230,16 @@ struct irq_domain *irq_find_matching_host(struct device_node *node,
>          */
>         mutex_lock(&irq_domain_mutex);
>         list_for_each_entry(h, &irq_domain_list, link) {
> -               if (h->ops->match)
> +               if (h->ops->match) {
> +                       struct device_node *node;
> +                       node = irq_domain_get_of_node(h);
>                         rc = h->ops->match(h, node, bus_token);
> -               else
> -                       rc = ((h->of_node != NULL) && (h->of_node == node) &&
> +               } else {
> +                       rc = ((h->domain_token != NULL) &&
> +                             (h->domain_token == domain_token) &&
>                               ((bus_token == DOMAIN_BUS_ANY) ||
>                                (h->bus_token == bus_token)));
> +               }
> 
>                 if (rc) {
>                         found = h;
> @@ -336,10 +362,12 @@ EXPORT_SYMBOL_GPL(irq_domain_associate);
>  void irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
>                                irq_hw_number_t hwirq_base, int count)
>  {
> +       struct device_node *of_node;
>         int i;
> 
> +       of_node = irq_domain_get_of_node(domain);
>         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);
> +               of_node_full_name(of_node), irq_base, (int)hwirq_base, count);
> 
>         for (i = 0; i < count; i++) {
>                 irq_domain_associate(domain, irq_base + i, hwirq_base + i);
> @@ -359,12 +387,14 @@ EXPORT_SYMBOL_GPL(irq_domain_associate_many);
>   */
>  unsigned int irq_create_direct_mapping(struct irq_domain *domain)
>  {
> +       struct device_node *of_node;
>         unsigned int virq;
> 
>         if (domain == NULL)
>                 domain = irq_default_domain;
> 
> -       virq = irq_alloc_desc_from(1, of_node_to_nid(domain->of_node));
> +       of_node = irq_domain_get_of_node(domain);
> +       virq = irq_alloc_desc_from(1, of_node_to_nid(of_node));
>         if (!virq) {
>                 pr_debug("create_direct virq allocation failed\n");
>                 return 0;
> @@ -399,6 +429,7 @@ EXPORT_SYMBOL_GPL(irq_create_direct_mapping);
>  unsigned int irq_create_mapping(struct irq_domain *domain,
>                                 irq_hw_number_t hwirq)
>  {
> +       struct device_node *of_node;
>         int virq;
> 
>         pr_debug("irq_create_mapping(0x%p, 0x%lx)\n", domain, hwirq);
> @@ -419,9 +450,11 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
>                 return virq;
>         }
> 
> +       of_node = irq_domain_get_of_node(domain);
> +
>         /* Allocate a virtual interrupt number */
>         virq = irq_domain_alloc_descs(-1, 1, hwirq,
> -                                     of_node_to_nid(domain->of_node));
> +                                     of_node_to_nid(of_node));
>         if (virq <= 0) {
>                 pr_debug("-> virq allocation failed\n");
>                 return 0;
> @@ -433,7 +466,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
>         }
> 
>         pr_debug("irq %lu on domain %s mapped to virtual irq %u\n",
> -               hwirq, of_node_full_name(domain->of_node), virq);
> +               hwirq, of_node_full_name(of_node), virq);
> 
>         return virq;
>  }
> @@ -460,10 +493,12 @@ EXPORT_SYMBOL_GPL(irq_create_mapping);
>  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(domain->of_node));
> +                             of_node_to_nid(of_node));
>         if (unlikely(ret < 0))
>                 return ret;
> 
> @@ -590,14 +625,16 @@ static int virq_debug_show(struct seq_file *m, void *private)
>                    "name", "mapped", "linear-max", "direct-max", "devtree-node");
>         mutex_lock(&irq_domain_mutex);
>         list_for_each_entry(domain, &irq_domain_list, link) {
> +               struct device_node *of_node;
>                 int count = 0;
> +               of_node = irq_domain_token_get_node(domain);
>                 radix_tree_for_each_slot(slot, &domain->revmap_tree, &iter, 0)
>                         count++;
>                 seq_printf(m, "%c%-16s  %6u  %10u  %10u  %s\n",
>                            domain == irq_default_domain ? '*' : ' ', domain->name,
>                            domain->revmap_size + count, domain->revmap_size,
>                            domain->revmap_direct_max_irq,
> -                          domain->of_node ? of_node_full_name(domain->of_node) : "");
> +                          of_node ? of_node_full_name(of_node) : "");
>         }
>         mutex_unlock(&irq_domain_mutex);
> 
> --
> 2.1.4
> 

Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Lorenzo Pieralisi]

On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
> Now that the basic ACPI GSI code is irq domain aware, make sure
> that the ACPI support in the GIC doesn't pointlessly deviate from
> the DT path.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/irqchip/irq-gic.c       | 17 ++++++-----------
>  include/linux/irqchip/arm-gic.h |  2 +-
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index b41ccf5..f5d365d 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>  {
>  	unsigned long ret = 0;
>  
> -	if (irq_domain_get_of_node(d) != controller)
> -		return -EINVAL;
>  	if (intsize < 3)
>  		return -EINVAL;
>  
> @@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
>  
>  void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  			   void __iomem *dist_base, void __iomem *cpu_base,
> -			   u32 percpu_offset, struct device_node *node)
> +			   u32 percpu_offset, void *domain_token)
>  {
>  	irq_hw_number_t hwirq_base;
>  	struct gic_chip_data *gic;
> @@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  		gic_irqs = 1020;
>  	gic->gic_irqs = gic_irqs;
>  
> -	if (node) {		/* DT case */
> -		gic->domain = irq_domain_add_linear(node, gic_irqs,
> +	if (domain_token) {		/* DT/ACPI case */
> +		gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
>  						    &gic_irq_domain_hierarchy_ops,
>  						    gic);
>  	} else {		/* Non-DT case */
> @@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  			irq_base = irq_start;
>  		}
>  
> -		gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
> +		gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
>  					hwirq_base, &gic_irq_domain_ops, gic);
>  	}
>  
> @@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
>  	}
>  
>  	/*
> -	 * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
> -	 * as default IRQ domain to allow for GSI registration and GSI to IRQ
> -	 * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
> +	 * Initialize zero GIC instance (no multi-GIC support).
>  	 */
> -	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
> -	irq_set_default_host(gic_data[0].domain);
> +	gic_init_bases(0, -1, dist_base, cpu_base, 0, (void *)ACPI_IRQ_MODEL_GIC);

Nit: the acpi_irq_model_id enum starts from 0, I do not think we will
use the IRQ domain look-up for the ACPI_IRQ_MODEL_PIC but we have
to be careful anyway.

Lorenzo

>  
>  	acpi_irq_model = ACPI_IRQ_MODEL_GIC;
>  	return 0;
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 9de976b..97799b7 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -97,7 +97,7 @@ struct device_node;
>  
>  void gic_set_irqchip_flags(unsigned long flags);
>  void gic_init_bases(unsigned int, int, void __iomem *, void __iomem *,
> -		    u32 offset, struct device_node *);
> +		    u32 offset, void *);
>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>  void gic_cpu_if_down(void);
>  
> -- 
> 2.1.4
> 

Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Marc Zyngier]

On 21/07/15 19:05, Lorenzo Pieralisi wrote:
> On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
>> Now that the basic ACPI GSI code is irq domain aware, make sure
>> that the ACPI support in the GIC doesn't pointlessly deviate from
>> the DT path.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/irqchip/irq-gic.c       | 17 ++++++-----------
>>  include/linux/irqchip/arm-gic.h |  2 +-
>>  2 files changed, 7 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index b41ccf5..f5d365d 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>>  {
>>  	unsigned long ret = 0;
>>  
>> -	if (irq_domain_get_of_node(d) != controller)
>> -		return -EINVAL;
>>  	if (intsize < 3)
>>  		return -EINVAL;
>>  
>> @@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
>>  
>>  void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>  			   void __iomem *dist_base, void __iomem *cpu_base,
>> -			   u32 percpu_offset, struct device_node *node)
>> +			   u32 percpu_offset, void *domain_token)
>>  {
>>  	irq_hw_number_t hwirq_base;
>>  	struct gic_chip_data *gic;
>> @@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>  		gic_irqs = 1020;
>>  	gic->gic_irqs = gic_irqs;
>>  
>> -	if (node) {		/* DT case */
>> -		gic->domain = irq_domain_add_linear(node, gic_irqs,
>> +	if (domain_token) {		/* DT/ACPI case */
>> +		gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
>>  						    &gic_irq_domain_hierarchy_ops,
>>  						    gic);
>>  	} else {		/* Non-DT case */
>> @@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>  			irq_base = irq_start;
>>  		}
>>  
>> -		gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
>> +		gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
>>  					hwirq_base, &gic_irq_domain_ops, gic);
>>  	}
>>  
>> @@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
>>  	}
>>  
>>  	/*
>> -	 * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
>> -	 * as default IRQ domain to allow for GSI registration and GSI to IRQ
>> -	 * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
>> +	 * Initialize zero GIC instance (no multi-GIC support).
>>  	 */
>> -	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>> -	irq_set_default_host(gic_data[0].domain);
>> +	gic_init_bases(0, -1, dist_base, cpu_base, 0, (void *)ACPI_IRQ_MODEL_GIC);
> 
> Nit: the acpi_irq_model_id enum starts from 0, I do not think we will
> use the IRQ domain look-up for the ACPI_IRQ_MODEL_PIC but we have
> to be careful anyway.

Yeah, I noticed that one too, but couldn't imagine the PIC being
migrated to that model just yet. It looks like it would be pretty
harmless to set ACPI_IRQ_MODEL_PIC to 1, and introduce
ACPI_IRQ_MODEL_ILLEGAL as zero.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 4/5] acpi: gsi: Use acpi_gsi_descriptor to allocate interrupts

[Lorenzo Pieralisi]

On Tue, Jul 21, 2015 at 11:07:59AM +0100, Marc Zyngier wrote:
> Now that the irqdomain layer is a bit more ACPI friendly, add the
> mapping code that allows irq_create_acpi_mapping to be called.
> 
> As we only support the GIC so far, support is pretty limited.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/acpi/gsi.c | 54 ++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
> index 38208f2..005d843 100644
> --- a/drivers/acpi/gsi.c
> +++ b/drivers/acpi/gsi.c
> @@ -33,6 +33,12 @@ static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
>  	}
>  }
>  
> +static struct irq_domain *irq_find_acpi_domain(enum acpi_irq_model_id id,
> +					       enum irq_domain_bus_token bus_token)
> +{
> +	return irq_find_matching_host((void *)id, bus_token);
> +}
> +
>  /**
>   * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
>   * @gsi: GSI IRQ number to map
> @@ -45,12 +51,10 @@ static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
>   */
>  int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
>  {
> -	/*
> -	 * Only default domain is supported at present, always find
> -	 * the mapping corresponding to default domain by passing NULL
> -	 * as irq_domain parameter
> -	 */
> -	*irq = irq_find_mapping(NULL, gsi);
> +	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
> +						    DOMAIN_BUS_ANY);
> +
> +	*irq = irq_find_mapping(d, gsi);
>  	/*
>  	 * *irq == 0 means no mapping, that should
>  	 * be reported as a failure
> @@ -72,23 +76,35 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>  int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
>  		      int polarity)
>  {
> -	unsigned int irq;
> +	struct acpi_gsi_descriptor data;
>  	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
> +	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
> +						    DOMAIN_BUS_ANY);
>  
>  	/*
> -	 * There is no way at present to look-up the IRQ domain on ACPI,
> -	 * hence always create mapping referring to the default domain
> -	 * by passing NULL as irq_domain parameter
> +	 * Populate the GSI descriptor in a way that matches the way
> +	 * the driver expects its of_phandle_args.
>  	 */
> -	irq = irq_create_mapping(NULL, gsi);
> -	if (!irq)
> +	switch (acpi_irq_model) {
> +	case ACPI_IRQ_MODEL_GIC:
> +		if (gsi >= 32) {
> +			data.param[0] = 0;
> +			data.param[1] = gsi - 32;
> +			data.param[2] = irq_type;
> +		} else {
> +			data.param[0] = 1;
> +			data.param[1] = gsi - 16;
> +			data.param[2] = 0xff << 4 | irq_type;
> +		}
> +
> +		data.param_count = 3;
> +		break;
> +	default:
> +		pr_warn("Unknown acpi_irq_model = %d\n", acpi_irq_model);
>  		return -EINVAL;
> +	}

Nit: you could move the switch in a function (in this file) so that
code that is irq model specific is self-contained and in a function
on its own, if we ever have to add other irq models later I think it
is easier to parse.

Lorenzo

>  
> -	/* Set irq type if specified and different than the current one */
> -	if (irq_type != IRQ_TYPE_NONE &&
> -		irq_type != irq_get_trigger_type(irq))
> -		irq_set_irq_type(irq, irq_type);
> -	return irq;
> +	return irq_create_acpi_mapping(d, &data);
>  }
>  EXPORT_SYMBOL_GPL(acpi_register_gsi);
>  
> @@ -98,7 +114,9 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi);
>   */
>  void acpi_unregister_gsi(u32 gsi)
>  {
> -	int irq = irq_find_mapping(NULL, gsi);
> +	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
> +						    DOMAIN_BUS_ANY);
> +	int irq = irq_find_mapping(d, gsi);
>  
>  	irq_dispose_mapping(irq);
>  }
> -- 
> 2.1.4
> 

Re: [PATCH 0/5] Making the generic ACPI GSI layer irqdomain aware

[Hanjun Guo]

Hi Marc,

On 07/21/2015 06:07 PM, Marc Zyngier wrote:
> The irqdomain code is not entierely really ACPI friendly, as it has
> some built-in knowledge of the device-tree. Nothing too harmful, but
> enough to scare the ARM ACPI developpers which end up with their own
> version of the square wheel.
>
> This small patch series adapt the irqdomain code to remove the hurdles
> that prevent the full blown irqdomain subsystem to be used on ACPI,
> creates an interface between the GSI layer and the irqdomain, and as
> an example, convert the ARM GIC ACPI support to use irqdomains as
> originally intended.
>
> Overall, this gives us a way to use irqdomains on both DT and ACPI
> enabled platforms, having very little changes made to the actual
> drivers (other than the probing infrastructure). Because we keep the
> flow of information between the various layers identical between ACPI
> and DT, we immediately benefit from the existing infrastructure.
>
> I'd really like to hear what people think of that approach, as it
> looks to me a lot simpler than the other approaches currently put on
> the list. The "convert the GSI information to be DT friendly" is
> admitedly not very pretty, but I see it as a stepping stone towards
> unifying the two structures.
>
> This has been test-booted on Juno, and is based on my irq/ncpi-msi-2
> branch.

Thank you very much for putting them together, really appreciated. I
will comment on each patch and test them.

Thanks
Hanjun

Re: [PATCH 1/5] genirq: irqdomain: Use an accessor for the of_node field

[Hanjun Guo]

On 07/21/2015 06:07 PM, Marc Zyngier wrote:
> The struct irq_domain contains a "struct device_node *" field
> (of_node) that is almost the only link between the irqdomain
> and the device tree infrastructure.
>
> In order to prepare for the removal of that field, convert all
> users outside of kernel/irq/irqdomain.c to use an accessor.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>   arch/mips/cavium-octeon/octeon-irq.c          | 4 ++--
>   arch/powerpc/platforms/cell/axon_msi.c        | 2 +-
>   arch/powerpc/platforms/cell/spider-pic.c      | 9 ++++++---
>   arch/powerpc/platforms/powernv/opal-irqchip.c | 2 +-
>   arch/powerpc/sysdev/ehv_pic.c                 | 3 ++-
>   arch/powerpc/sysdev/fsl_msi.c                 | 2 +-
>   arch/powerpc/sysdev/i8259.c                   | 3 ++-
>   arch/powerpc/sysdev/ipic.c                    | 3 ++-
>   arch/powerpc/sysdev/mpic.c                    | 3 ++-
>   arch/powerpc/sysdev/mpic_msi.c                | 2 +-
>   arch/powerpc/sysdev/qe_lib/qe_ic.c            | 3 ++-
>   drivers/gpio/gpio-sodaville.c                 | 2 +-
>   drivers/irqchip/exynos-combiner.c             | 2 +-
>   drivers/irqchip/irq-atmel-aic-common.c        | 2 +-
>   drivers/irqchip/irq-crossbar.c                | 4 ++--
>   drivers/irqchip/irq-gic-v2m.c                 | 2 +-
>   drivers/irqchip/irq-gic-v3-its.c              | 2 +-
>   drivers/irqchip/irq-gic-v3.c                  | 2 +-
>   drivers/irqchip/irq-gic.c                     | 2 +-
>   drivers/irqchip/irq-hip04.c                   | 2 +-
>   drivers/irqchip/irq-mtk-sysirq.c              | 2 +-
>   drivers/irqchip/irq-s3c24xx.c                 | 4 ++--
>   drivers/irqchip/irq-tegra.c                   | 4 ++--
>   drivers/irqchip/irq-vf610-mscm-ir.c           | 5 +++--
>   drivers/spmi/spmi-pmic-arb.c                  | 2 +-
>   include/linux/irqdomain.h                     | 5 +++++
>   26 files changed, 46 insertions(+), 32 deletions(-)

It seems to me that some other files missed,
in arch/powerpc/platforms/pasemi/msi.c:

         if (!mpic->irqhost->of_node ||
             !of_device_is_compatible(mpic->irqhost->of_node,
                                      "pasemi,pwrficient-openpic"))
                 return -ENODEV;

If you need more eyes to find all of them, please count me in :)

Thanks
Hanjun

Re: [PATCH 4/5] acpi: gsi: Use acpi_gsi_descriptor to allocate interrupts

[Marc Zyngier]

On 21/07/15 19:16, Lorenzo Pieralisi wrote:
> On Tue, Jul 21, 2015 at 11:07:59AM +0100, Marc Zyngier wrote:
>> Now that the irqdomain layer is a bit more ACPI friendly, add the
>> mapping code that allows irq_create_acpi_mapping to be called.
>>
>> As we only support the GIC so far, support is pretty limited.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  drivers/acpi/gsi.c | 54 ++++++++++++++++++++++++++++++++++++------------------
>>  1 file changed, 36 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
>> index 38208f2..005d843 100644
>> --- a/drivers/acpi/gsi.c
>> +++ b/drivers/acpi/gsi.c
>> @@ -33,6 +33,12 @@ static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
>>  	}
>>  }
>>  
>> +static struct irq_domain *irq_find_acpi_domain(enum acpi_irq_model_id id,
>> +					       enum irq_domain_bus_token bus_token)
>> +{
>> +	return irq_find_matching_host((void *)id, bus_token);
>> +}
>> +
>>  /**
>>   * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
>>   * @gsi: GSI IRQ number to map
>> @@ -45,12 +51,10 @@ static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
>>   */
>>  int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
>>  {
>> -	/*
>> -	 * Only default domain is supported at present, always find
>> -	 * the mapping corresponding to default domain by passing NULL
>> -	 * as irq_domain parameter
>> -	 */
>> -	*irq = irq_find_mapping(NULL, gsi);
>> +	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
>> +						    DOMAIN_BUS_ANY);
>> +
>> +	*irq = irq_find_mapping(d, gsi);
>>  	/*
>>  	 * *irq == 0 means no mapping, that should
>>  	 * be reported as a failure
>> @@ -72,23 +76,35 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>>  int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
>>  		      int polarity)
>>  {
>> -	unsigned int irq;
>> +	struct acpi_gsi_descriptor data;
>>  	unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
>> +	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
>> +						    DOMAIN_BUS_ANY);
>>  
>>  	/*
>> -	 * There is no way at present to look-up the IRQ domain on ACPI,
>> -	 * hence always create mapping referring to the default domain
>> -	 * by passing NULL as irq_domain parameter
>> +	 * Populate the GSI descriptor in a way that matches the way
>> +	 * the driver expects its of_phandle_args.
>>  	 */
>> -	irq = irq_create_mapping(NULL, gsi);
>> -	if (!irq)
>> +	switch (acpi_irq_model) {
>> +	case ACPI_IRQ_MODEL_GIC:
>> +		if (gsi >= 32) {
>> +			data.param[0] = 0;
>> +			data.param[1] = gsi - 32;
>> +			data.param[2] = irq_type;
>> +		} else {
>> +			data.param[0] = 1;
>> +			data.param[1] = gsi - 16;
>> +			data.param[2] = 0xff << 4 | irq_type;
>> +		}
>> +
>> +		data.param_count = 3;
>> +		break;
>> +	default:
>> +		pr_warn("Unknown acpi_irq_model = %d\n", acpi_irq_model);
>>  		return -EINVAL;
>> +	}
> 
> Nit: you could move the switch in a function (in this file) so that
> code that is irq model specific is self-contained and in a function
> on its own, if we ever have to add other irq models later I think it
> is easier to parse.

Yup, makes sense.

Actually, having given it a bit more thoughts, there is not reason why
this translation function should be located in this file. The interrupt
controller driver, instead of doing a

acpi_irq_model = ACPI_IRQ_MODEL_BLAH;

could as well be doing

acpi_set_irq_model(ACPI_IRQ_MODEL_BLAH, blah_acpi_gsi_desc_populate);

making the whole thing completely interrupt-controller agnostic.

Thanks,

	M.

> 
> Lorenzo
> 
>>  
>> -	/* Set irq type if specified and different than the current one */
>> -	if (irq_type != IRQ_TYPE_NONE &&
>> -		irq_type != irq_get_trigger_type(irq))
>> -		irq_set_irq_type(irq, irq_type);
>> -	return irq;
>> +	return irq_create_acpi_mapping(d, &data);
>>  }
>>  EXPORT_SYMBOL_GPL(acpi_register_gsi);
>>  
>> @@ -98,7 +114,9 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi);
>>   */
>>  void acpi_unregister_gsi(u32 gsi)
>>  {
>> -	int irq = irq_find_mapping(NULL, gsi);
>> +	struct irq_domain *d = irq_find_acpi_domain(acpi_irq_model,
>> +						    DOMAIN_BUS_ANY);
>> +	int irq = irq_find_mapping(d, gsi);
>>  
>>  	irq_dispose_mapping(irq);
>>  }
>> -- 
>> 2.1.4
>>
> 


-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/5] genirq: irqdomain: Use an accessor for the of_node field

[Marc Zyngier]

On 22/07/15 08:35, Hanjun Guo wrote:
> On 07/21/2015 06:07 PM, Marc Zyngier wrote:
>> The struct irq_domain contains a "struct device_node *" field
>> (of_node) that is almost the only link between the irqdomain
>> and the device tree infrastructure.
>>
>> In order to prepare for the removal of that field, convert all
>> users outside of kernel/irq/irqdomain.c to use an accessor.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>   arch/mips/cavium-octeon/octeon-irq.c          | 4 ++--
>>   arch/powerpc/platforms/cell/axon_msi.c        | 2 +-
>>   arch/powerpc/platforms/cell/spider-pic.c      | 9 ++++++---
>>   arch/powerpc/platforms/powernv/opal-irqchip.c | 2 +-
>>   arch/powerpc/sysdev/ehv_pic.c                 | 3 ++-
>>   arch/powerpc/sysdev/fsl_msi.c                 | 2 +-
>>   arch/powerpc/sysdev/i8259.c                   | 3 ++-
>>   arch/powerpc/sysdev/ipic.c                    | 3 ++-
>>   arch/powerpc/sysdev/mpic.c                    | 3 ++-
>>   arch/powerpc/sysdev/mpic_msi.c                | 2 +-
>>   arch/powerpc/sysdev/qe_lib/qe_ic.c            | 3 ++-
>>   drivers/gpio/gpio-sodaville.c                 | 2 +-
>>   drivers/irqchip/exynos-combiner.c             | 2 +-
>>   drivers/irqchip/irq-atmel-aic-common.c        | 2 +-
>>   drivers/irqchip/irq-crossbar.c                | 4 ++--
>>   drivers/irqchip/irq-gic-v2m.c                 | 2 +-
>>   drivers/irqchip/irq-gic-v3-its.c              | 2 +-
>>   drivers/irqchip/irq-gic-v3.c                  | 2 +-
>>   drivers/irqchip/irq-gic.c                     | 2 +-
>>   drivers/irqchip/irq-hip04.c                   | 2 +-
>>   drivers/irqchip/irq-mtk-sysirq.c              | 2 +-
>>   drivers/irqchip/irq-s3c24xx.c                 | 4 ++--
>>   drivers/irqchip/irq-tegra.c                   | 4 ++--
>>   drivers/irqchip/irq-vf610-mscm-ir.c           | 5 +++--
>>   drivers/spmi/spmi-pmic-arb.c                  | 2 +-
>>   include/linux/irqdomain.h                     | 5 +++++
>>   26 files changed, 46 insertions(+), 32 deletions(-)
> 
> It seems to me that some other files missed,
> in arch/powerpc/platforms/pasemi/msi.c:
> 
>          if (!mpic->irqhost->of_node ||
>              !of_device_is_compatible(mpic->irqhost->of_node,
>                                       "pasemi,pwrficient-openpic"))
>                  return -ENODEV;
> 
> If you need more eyes to find all of them, please count me in :)

Looks like my coccinelle foo is still lacking a bit... Oh well.

Thanks for the help!

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 1/5] genirq: irqdomain: Use an accessor for the of_node field

[Thomas Gleixner]

BOn Wed, 22 Jul 2015, Hanjun Guo wrote:

> On 07/21/2015 06:07 PM, Marc Zyngier wrote:
> > The struct irq_domain contains a "struct device_node *" field
> > (of_node) that is almost the only link between the irqdomain
> > and the device tree infrastructure.
> > 
>1;2802;0c > In order to prepare for the removal of that field, convert all
> > users outside of kernel/irq/irqdomain.c to use an accessor.
> > 
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >   arch/mips/cavium-octeon/octeon-irq.c          | 4 ++--
> >   arch/powerpc/platforms/cell/axon_msi.c        | 2 +-
> >   arch/powerpc/platforms/cell/spider-pic.c      | 9 ++++++---
> >   arch/powerpc/platforms/powernv/opal-irqchip.c | 2 +-
> >   arch/powerpc/sysdev/ehv_pic.c                 | 3 ++-
> >   arch/powerpc/sysdev/fsl_msi.c                 | 2 +-
> >   arch/powerpc/sysdev/i8259.c                   | 3 ++-
> >   arch/powerpc/sysdev/ipic.c                    | 3 ++-
> >   arch/powerpc/sysdev/mpic.c                    | 3 ++-
> >   arch/powerpc/sysdev/mpic_msi.c                | 2 +-
> >   arch/powerpc/sysdev/qe_lib/qe_ic.c            | 3 ++-
> >   drivers/gpio/gpio-sodaville.c                 | 2 +-
> >   drivers/irqchip/exynos-combiner.c             | 2 +-
> >   drivers/irqchip/irq-atmel-aic-common.c        | 2 +-
> >   drivers/irqchip/irq-crossbar.c                | 4 ++--
> >   drivers/irqchip/irq-gic-v2m.c                 | 2 +-
> >   drivers/irqchip/irq-gic-v3-its.c              | 2 +-
> >   drivers/irqchip/irq-gic-v3.c                  | 2 +-
> >   drivers/irqchip/irq-gic.c                     | 2 +-
> >   drivers/irqchip/irq-hip04.c                   | 2 +-
> >   drivers/irqchip/irq-mtk-sysirq.c              | 2 +-
> >   drivers/irqchip/irq-s3c24xx.c                 | 4 ++--
> >   drivers/irqchip/irq-tegra.c                   | 4 ++--
> >   drivers/irqchip/irq-vf610-mscm-ir.c           | 5 +++--
> >   drivers/spmi/spmi-pmic-arb.c                  | 2 +-
> >   include/linux/irqdomain.h                     | 5 +++++
> >   26 files changed, 46 insertions(+), 32 deletions(-)
> 
> It seems to me that some other files missed,
> in arch/powerpc/platforms/pasemi/msi.c:
> 
>         if (!mpic->irqhost->of_node ||
>             !of_device_is_compatible(mpic->irqhost->of_node,
>                                      "pasemi,pwrficient-openpic"))
>                 return -ENODEV;
> 
> If you need more eyes to find all of them, please count me in :)

The proper tool to find them is coccinelle, not eyes.

Thanks,

	tglx

Re: [PATCH 1/5] genirq: irqdomain: Use an accessor for the of_node field

[Thomas Gleixner]

On Wed, 22 Jul 2015, Marc Zyngier wrote:
> 
> Looks like my coccinelle foo is still lacking a bit... Oh well.

Please post your semantic patch and don't forget to Cc Julia Lawall :)

Re: [PATCH 2/5] genirq: irqdomain: Remove irqdomain dependency on struct device_node

[Hanjun Guo]

On 07/22/2015 01:56 AM, Lorenzo Pieralisi wrote:
> On Tue, Jul 21, 2015 at 11:07:57AM +0100, Marc Zyngier wrote:
>> struct device_node is very much DT specific, and the original authors
>> of the irqdomain subsystem recognized that tie, and went as far as
>> mentionning that this could be replaced by some "void *token",
>> should another firmware infrastructure be using it.
>
> Yes, that's similar to the problem Rafael solved with fwnode_handle,
> I do not know if we can extend the fwnode_handle to manage these
> generic tokens too, but the approach you have taken seem the right
> one to me (and you are doing this for components that are not really
> attached to struct device so I am not sure the fwnode_handle approach
> can be shoehorned to solve it).

If I understand correctly, fwnode_handle is not better , since
we need to update every caller of irqdomain functions as it's a new
type of pointer, that's will be the big change, void * is just fine.

Thanks
Hanjun

Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Hanjun Guo]

On 07/22/2015 02:12 AM, Marc Zyngier wrote:
> On 21/07/15 19:05, Lorenzo Pieralisi wrote:
>> On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
>>> Now that the basic ACPI GSI code is irq domain aware, make sure
>>> that the ACPI support in the GIC doesn't pointlessly deviate from
>>> the DT path.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>>   drivers/irqchip/irq-gic.c       | 17 ++++++-----------
>>>   include/linux/irqchip/arm-gic.h |  2 +-
>>>   2 files changed, 7 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>> index b41ccf5..f5d365d 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>>>   {
>>>   	unsigned long ret = 0;
>>>
>>> -	if (irq_domain_get_of_node(d) != controller)
>>> -		return -EINVAL;
>>>   	if (intsize < 3)
>>>   		return -EINVAL;
>>>
>>> @@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
>>>
>>>   void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>   			   void __iomem *dist_base, void __iomem *cpu_base,
>>> -			   u32 percpu_offset, struct device_node *node)
>>> +			   u32 percpu_offset, void *domain_token)
>>>   {
>>>   	irq_hw_number_t hwirq_base;
>>>   	struct gic_chip_data *gic;
>>> @@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>   		gic_irqs = 1020;
>>>   	gic->gic_irqs = gic_irqs;
>>>
>>> -	if (node) {		/* DT case */
>>> -		gic->domain = irq_domain_add_linear(node, gic_irqs,
>>> +	if (domain_token) {		/* DT/ACPI case */
>>> +		gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
>>>   						    &gic_irq_domain_hierarchy_ops,
>>>   						    gic);
>>>   	} else {		/* Non-DT case */
>>> @@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>   			irq_base = irq_start;
>>>   		}
>>>
>>> -		gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
>>> +		gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
>>>   					hwirq_base, &gic_irq_domain_ops, gic);
>>>   	}
>>>
>>> @@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
>>>   	}
>>>
>>>   	/*
>>> -	 * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
>>> -	 * as default IRQ domain to allow for GSI registration and GSI to IRQ
>>> -	 * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
>>> +	 * Initialize zero GIC instance (no multi-GIC support).
>>>   	 */
>>> -	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>>> -	irq_set_default_host(gic_data[0].domain);
>>> +	gic_init_bases(0, -1, dist_base, cpu_base, 0, (void *)ACPI_IRQ_MODEL_GIC);
>>
>> Nit: the acpi_irq_model_id enum starts from 0, I do not think we will
>> use the IRQ domain look-up for the ACPI_IRQ_MODEL_PIC but we have
>> to be careful anyway.
>
> Yeah, I noticed that one too, but couldn't imagine the PIC being
> migrated to that model just yet. It looks like it would be pretty
> harmless to set ACPI_IRQ_MODEL_PIC to 1, and introduce
> ACPI_IRQ_MODEL_ILLEGAL as zero.

I think this will be a problem, because acpi_irq_model_id enum actually
is defined by the ACPI spec, and the value is used to report to BIOS
the current interrupt model used by OS, see section 5.8.1 _PIC Method
in ACPI 6.0:

0 – PIC mode
1 – APIC mode
2 – SAPIC mode
Other values –Reserved

so we can't set ACPI_IRQ_MODEL_PIC to 1 as it may break the firmware,
also _PIC method actually is not needed for ARM platform at all, we
don't need to report to firmware the interrupt model used by OS on
ARM, it only used by legacy IA platforms, actually I'm planning to
remove acpi_irq_model_id on ARM64.

So to me acpi_irq_model_id is suitable for the token, can we use
another one as the token? how about the GIC ID in the MADT table?
and this also can be used for x86 (IOAPIC IDs) too.

Thanks
Hanjun

Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Hanjun Guo]

On 07/22/2015 04:35 PM, Hanjun Guo wrote:
> On 07/22/2015 02:12 AM, Marc Zyngier wrote:
>> On 21/07/15 19:05, Lorenzo Pieralisi wrote:
>>> On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
>>>> Now that the basic ACPI GSI code is irq domain aware, make sure
>>>> that the ACPI support in the GIC doesn't pointlessly deviate from
>>>> the DT path.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>>   drivers/irqchip/irq-gic.c       | 17 ++++++-----------
>>>>   include/linux/irqchip/arm-gic.h |  2 +-
>>>>   2 files changed, 7 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>>> index b41ccf5..f5d365d 100644
>>>> --- a/drivers/irqchip/irq-gic.c
>>>> +++ b/drivers/irqchip/irq-gic.c
>>>> @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct
>>>> irq_domain *d,
>>>>   {
>>>>       unsigned long ret = 0;
>>>>
>>>> -    if (irq_domain_get_of_node(d) != controller)
>>>> -        return -EINVAL;
>>>>       if (intsize < 3)
>>>>           return -EINVAL;
>>>>
>>>> @@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
>>>>
>>>>   void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>>                  void __iomem *dist_base, void __iomem *cpu_base,
>>>> -               u32 percpu_offset, struct device_node *node)
>>>> +               u32 percpu_offset, void *domain_token)
>>>>   {
>>>>       irq_hw_number_t hwirq_base;
>>>>       struct gic_chip_data *gic;
>>>> @@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr,
>>>> int irq_start,
>>>>           gic_irqs = 1020;
>>>>       gic->gic_irqs = gic_irqs;
>>>>
>>>> -    if (node) {        /* DT case */
>>>> -        gic->domain = irq_domain_add_linear(node, gic_irqs,
>>>> +    if (domain_token) {        /* DT/ACPI case */
>>>> +        gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
>>>>                               &gic_irq_domain_hierarchy_ops,
>>>>                               gic);
>>>>       } else {        /* Non-DT case */
>>>> @@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr,
>>>> int irq_start,
>>>>               irq_base = irq_start;
>>>>           }
>>>>
>>>> -        gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
>>>> +        gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
>>>>                       hwirq_base, &gic_irq_domain_ops, gic);
>>>>       }
>>>>
>>>> @@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header
>>>> *table)
>>>>       }
>>>>
>>>>       /*
>>>> -     * Initialize zero GIC instance (no multi-GIC support). Also,
>>>> set GIC
>>>> -     * as default IRQ domain to allow for GSI registration and GSI
>>>> to IRQ
>>>> -     * number translation (see acpi_register_gsi() and
>>>> acpi_gsi_to_irq()).
>>>> +     * Initialize zero GIC instance (no multi-GIC support).
>>>>        */
>>>> -    gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>>>> -    irq_set_default_host(gic_data[0].domain);
>>>> +    gic_init_bases(0, -1, dist_base, cpu_base, 0, (void
>>>> *)ACPI_IRQ_MODEL_GIC);
>>>
>>> Nit: the acpi_irq_model_id enum starts from 0, I do not think we will
>>> use the IRQ domain look-up for the ACPI_IRQ_MODEL_PIC but we have
>>> to be careful anyway.
>>
>> Yeah, I noticed that one too, but couldn't imagine the PIC being
>> migrated to that model just yet. It looks like it would be pretty
>> harmless to set ACPI_IRQ_MODEL_PIC to 1, and introduce
>> ACPI_IRQ_MODEL_ILLEGAL as zero.
>
> I think this will be a problem, because acpi_irq_model_id enum actually
> is defined by the ACPI spec, and the value is used to report to BIOS
> the current interrupt model used by OS, see section 5.8.1 _PIC Method
> in ACPI 6.0:
>
> 0 – PIC mode
> 1 – APIC mode
> 2 – SAPIC mode
> Other values –Reserved
>
> so we can't set ACPI_IRQ_MODEL_PIC to 1 as it may break the firmware,
> also _PIC method actually is not needed for ARM platform at all, we
> don't need to report to firmware the interrupt model used by OS on
> ARM, it only used by legacy IA platforms, actually I'm planning to
> remove acpi_irq_model_id on ARM64.
>
> So to me acpi_irq_model_id is suitable for the token, can we use
> another one as the token? how about the GIC ID in the MADT table?
> and this also can be used for x86 (IOAPIC IDs) too.

or just introduce a similar enum as acpi_irq_model for this purpose,
as acpi_irq_model is for _PIC method.

Thanks
Hanjun

Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Marc Zyngier]

On 22/07/15 09:35, Hanjun Guo wrote:
> On 07/22/2015 02:12 AM, Marc Zyngier wrote:
>> On 21/07/15 19:05, Lorenzo Pieralisi wrote:
>>> On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
>>>> Now that the basic ACPI GSI code is irq domain aware, make sure
>>>> that the ACPI support in the GIC doesn't pointlessly deviate from
>>>> the DT path.
>>>>
>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>> ---
>>>>   drivers/irqchip/irq-gic.c       | 17 ++++++-----------
>>>>   include/linux/irqchip/arm-gic.h |  2 +-
>>>>   2 files changed, 7 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>>> index b41ccf5..f5d365d 100644
>>>> --- a/drivers/irqchip/irq-gic.c
>>>> +++ b/drivers/irqchip/irq-gic.c
>>>> @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>>>>   {
>>>>   	unsigned long ret = 0;
>>>>
>>>> -	if (irq_domain_get_of_node(d) != controller)
>>>> -		return -EINVAL;
>>>>   	if (intsize < 3)
>>>>   		return -EINVAL;
>>>>
>>>> @@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
>>>>
>>>>   void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>>   			   void __iomem *dist_base, void __iomem *cpu_base,
>>>> -			   u32 percpu_offset, struct device_node *node)
>>>> +			   u32 percpu_offset, void *domain_token)
>>>>   {
>>>>   	irq_hw_number_t hwirq_base;
>>>>   	struct gic_chip_data *gic;
>>>> @@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>>   		gic_irqs = 1020;
>>>>   	gic->gic_irqs = gic_irqs;
>>>>
>>>> -	if (node) {		/* DT case */
>>>> -		gic->domain = irq_domain_add_linear(node, gic_irqs,
>>>> +	if (domain_token) {		/* DT/ACPI case */
>>>> +		gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
>>>>   						    &gic_irq_domain_hierarchy_ops,
>>>>   						    gic);
>>>>   	} else {		/* Non-DT case */
>>>> @@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>>   			irq_base = irq_start;
>>>>   		}
>>>>
>>>> -		gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
>>>> +		gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
>>>>   					hwirq_base, &gic_irq_domain_ops, gic);
>>>>   	}
>>>>
>>>> @@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
>>>>   	}
>>>>
>>>>   	/*
>>>> -	 * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
>>>> -	 * as default IRQ domain to allow for GSI registration and GSI to IRQ
>>>> -	 * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
>>>> +	 * Initialize zero GIC instance (no multi-GIC support).
>>>>   	 */
>>>> -	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>>>> -	irq_set_default_host(gic_data[0].domain);
>>>> +	gic_init_bases(0, -1, dist_base, cpu_base, 0, (void *)ACPI_IRQ_MODEL_GIC);
>>>
>>> Nit: the acpi_irq_model_id enum starts from 0, I do not think we will
>>> use the IRQ domain look-up for the ACPI_IRQ_MODEL_PIC but we have
>>> to be careful anyway.
>>
>> Yeah, I noticed that one too, but couldn't imagine the PIC being
>> migrated to that model just yet. It looks like it would be pretty
>> harmless to set ACPI_IRQ_MODEL_PIC to 1, and introduce
>> ACPI_IRQ_MODEL_ILLEGAL as zero.
> 
> I think this will be a problem, because acpi_irq_model_id enum actually
> is defined by the ACPI spec, and the value is used to report to BIOS
> the current interrupt model used by OS, see section 5.8.1 _PIC Method
> in ACPI 6.0:
> 
> 0 – PIC mode
> 1 – APIC mode
> 2 – SAPIC mode
> Other values –Reserved

Ah, right.

> so we can't set ACPI_IRQ_MODEL_PIC to 1 as it may break the firmware,
> also _PIC method actually is not needed for ARM platform at all, we
> don't need to report to firmware the interrupt model used by OS on
> ARM, it only used by legacy IA platforms, actually I'm planning to
> remove acpi_irq_model_id on ARM64.

Remove? I don't get it. Either ACPI_IRQ_MODEL_GIC is a leval value, and
you can't remove it, or it is not, and I wonder how it ended up here the
first place.

> So to me acpi_irq_model_id is suitable for the token, can we use
                             is *not* ?
> another one as the token? how about the GIC ID in the MADT table?
> and this also can be used for x86 (IOAPIC IDs) too.

You can use whatever you want, just not a pointer. I'll add a token
parameter to the acpi_set_irq_model function that I mentioned in my
reply to Lorenzo.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 5/5] irqchip: GIC: Switch ACPI support to stacked domains

[Hanjun Guo]

On 07/22/2015 04:53 PM, Marc Zyngier wrote:
> On 22/07/15 09:35, Hanjun Guo wrote:
>> On 07/22/2015 02:12 AM, Marc Zyngier wrote:
>>> On 21/07/15 19:05, Lorenzo Pieralisi wrote:
>>>> On Tue, Jul 21, 2015 at 11:08:00AM +0100, Marc Zyngier wrote:
>>>>> Now that the basic ACPI GSI code is irq domain aware, make sure
>>>>> that the ACPI support in the GIC doesn't pointlessly deviate from
>>>>> the DT path.
>>>>>
>>>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>>>> ---
>>>>>    drivers/irqchip/irq-gic.c       | 17 ++++++-----------
>>>>>    include/linux/irqchip/arm-gic.h |  2 +-
>>>>>    2 files changed, 7 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>>>> index b41ccf5..f5d365d 100644
>>>>> --- a/drivers/irqchip/irq-gic.c
>>>>> +++ b/drivers/irqchip/irq-gic.c
>>>>> @@ -813,8 +813,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>>>>>    {
>>>>>    	unsigned long ret = 0;
>>>>>
>>>>> -	if (irq_domain_get_of_node(d) != controller)
>>>>> -		return -EINVAL;
>>>>>    	if (intsize < 3)
>>>>>    		return -EINVAL;
>>>>>
>>>>> @@ -887,7 +885,7 @@ void gic_set_irqchip_flags(unsigned long flags)
>>>>>
>>>>>    void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>>>    			   void __iomem *dist_base, void __iomem *cpu_base,
>>>>> -			   u32 percpu_offset, struct device_node *node)
>>>>> +			   u32 percpu_offset, void *domain_token)
>>>>>    {
>>>>>    	irq_hw_number_t hwirq_base;
>>>>>    	struct gic_chip_data *gic;
>>>>> @@ -946,8 +944,8 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>>>    		gic_irqs = 1020;
>>>>>    	gic->gic_irqs = gic_irqs;
>>>>>
>>>>> -	if (node) {		/* DT case */
>>>>> -		gic->domain = irq_domain_add_linear(node, gic_irqs,
>>>>> +	if (domain_token) {		/* DT/ACPI case */
>>>>> +		gic->domain = irq_domain_add_linear(domain_token, gic_irqs,
>>>>>    						    &gic_irq_domain_hierarchy_ops,
>>>>>    						    gic);
>>>>>    	} else {		/* Non-DT case */
>>>>> @@ -973,7 +971,7 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>>>    			irq_base = irq_start;
>>>>>    		}
>>>>>
>>>>> -		gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
>>>>> +		gic->domain = irq_domain_add_legacy(NULL, gic_irqs, irq_base,
>>>>>    					hwirq_base, &gic_irq_domain_ops, gic);
>>>>>    	}
>>>>>
>>>>> @@ -1132,12 +1130,9 @@ gic_v2_acpi_init(struct acpi_table_header *table)
>>>>>    	}
>>>>>
>>>>>    	/*
>>>>> -	 * Initialize zero GIC instance (no multi-GIC support). Also, set GIC
>>>>> -	 * as default IRQ domain to allow for GSI registration and GSI to IRQ
>>>>> -	 * number translation (see acpi_register_gsi() and acpi_gsi_to_irq()).
>>>>> +	 * Initialize zero GIC instance (no multi-GIC support).
>>>>>    	 */
>>>>> -	gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
>>>>> -	irq_set_default_host(gic_data[0].domain);
>>>>> +	gic_init_bases(0, -1, dist_base, cpu_base, 0, (void *)ACPI_IRQ_MODEL_GIC);
>>>>
>>>> Nit: the acpi_irq_model_id enum starts from 0, I do not think we will
>>>> use the IRQ domain look-up for the ACPI_IRQ_MODEL_PIC but we have
>>>> to be careful anyway.
>>>
>>> Yeah, I noticed that one too, but couldn't imagine the PIC being
>>> migrated to that model just yet. It looks like it would be pretty
>>> harmless to set ACPI_IRQ_MODEL_PIC to 1, and introduce
>>> ACPI_IRQ_MODEL_ILLEGAL as zero.
>>
>> I think this will be a problem, because acpi_irq_model_id enum actually
>> is defined by the ACPI spec, and the value is used to report to BIOS
>> the current interrupt model used by OS, see section 5.8.1 _PIC Method
>> in ACPI 6.0:
>>
>> 0 – PIC mode
>> 1 – APIC mode
>> 2 – SAPIC mode
>> Other values –Reserved
>
> Ah, right.
>
>> so we can't set ACPI_IRQ_MODEL_PIC to 1 as it may break the firmware,
>> also _PIC method actually is not needed for ARM platform at all, we
>> don't need to report to firmware the interrupt model used by OS on
>> ARM, it only used by legacy IA platforms, actually I'm planning to
>> remove acpi_irq_model_id on ARM64.
>
> Remove? I don't get it. Either ACPI_IRQ_MODEL_GIC is a leval value, and
> you can't remove it, or it is not, and I wonder how it ended up here the
> first place.

The best solution is that we don't call _PIC method on ARM at all
(acpi_irq_model is used for _PIC method), but we can live with it since
there is no harm if ARM firmware don't care about the value or ARM
firmware don't define _PIC method in DSDT.

>
>> So to me acpi_irq_model_id is suitable for the token, can we use
>                               is *not* ?
>> another one as the token? how about the GIC ID in the MADT table?
>> and this also can be used for x86 (IOAPIC IDs) too.
>
> You can use whatever you want, just not a pointer. I'll add a token
> parameter to the acpi_set_irq_model function that I mentioned in my
> reply to Lorenzo.

Thank you, I will wait for the v2 to rebase my ACPI GICv3 patches.

Thanks
Hanjun

Re: [PATCH 1/5] genirq: irqdomain: Use an accessor for the of_node field

[Marc Zyngier]

On 22/07/15 08:58, Thomas Gleixner wrote:
> On Wed, 22 Jul 2015, Marc Zyngier wrote:
>>
>> Looks like my coccinelle foo is still lacking a bit... Oh well.
> 
> Please post your semantic patch and don't forget to Cc Julia Lawall :)
> 

Bah, found the problem. My semantic patch is extremely simple:

@init@
struct irq_domain *ptr;
position pos;
@@

(
ptr@pos->of_node
)


@ script:python @
p1 << init.pos;
@@

for p in p1:
    print "%s:%s" % (p.file,p.line)

as I'm just grepping through the tree (I don't feel confident enough to
do the patching, and some cases are a bit more complicated).

But I forgot to add arch/$ARCH/include/asm as an include search path for
spatch. That had the side effect of spatch being unable to identify
constructs like:

arch/$ARCH/include/asm/foo.h:

	struct foo {
		struct irq_domain *bar;
	};

arch/$ARCH/.../foo.c:

	struct foo foo;
	if (foo->bar->of_node) { ... }

Since spatch knows nothing about struct foo, it cannot identify bar as a
struct irq_domain, hence missing this occurrence.

Once I fixed the include path, I found an additional couple of victims.
I really love this thing! :-)

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 2/5] genirq: irqdomain: Remove irqdomain dependency on struct device_node

[Suravee Suthikulpanit]

On 7/22/15 15:13, Hanjun Guo wrote:
> On 07/22/2015 01:56 AM, Lorenzo Pieralisi wrote:
>> On Tue, Jul 21, 2015 at 11:07:57AM +0100, Marc Zyngier wrote:
>>> struct device_node is very much DT specific, and the original authors
>>> of the irqdomain subsystem recognized that tie, and went as far as
>>> mentionning that this could be replaced by some "void *token",
>>> should another firmware infrastructure be using it.
>>
>> Yes, that's similar to the problem Rafael solved with fwnode_handle,
>> I do not know if we can extend the fwnode_handle to manage these
>> generic tokens too, but the approach you have taken seem the right
>> one to me (and you are doing this for components that are not really
>> attached to struct device so I am not sure the fwnode_handle approach
>> can be shoehorned to solve it).
>
> If I understand correctly, fwnode_handle is not better , since
> we need to update every caller of irqdomain functions as it's a new
> type of pointer, that's will be the big change, void * is just fine.

Also, the fwnode_handle is mainly used to represent the DSDT device 
entry. In case of ACPI, GICv2m and ITS domain does not have a 
corresponded DSDT entry, but it uses other table. I am agree with just 
keeping this as void *.

Thanks,

Suravee