* [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware
@ 2015-09-14 16:43 Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 1/8] genirq: irqdomain: Use an accessor for the of_node field Marc Zyngier
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:43 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
The irqdomain code is not entierely 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. It also gives a way for non-DT systems to lookup
irqdomains using arbitrary identifiers.
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. 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, is based on 4.3-rc1, and available at:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git irq/gsi-irq-domain-v3
* From v2:
- Rebased on vanilla 4.3-rc1
- Added the IDR infrastructure to irqdomain.c so that drivers don't
have to come up with their own way of allocating identifiers
- Changed the acpi_set_irq_model() signature to directly take a
void *domain_token
* From v1:
- Improved my Coccinelle foo and hopefully caught all the
irq_domain.of_node users this time
- Decoupled acpi_irq_model from domain_token. These are now two
separate values that can be set independently
- Moved the duty of populating acpi_gsi_descriptor to the interrupt
controller, as it keeps the knowledge of the mapping with
of_phandle_args in a single location
- Generic accessor to set acpi_irq_model, domain_token and the
populate function all in one go from the interrupt controller
- General cleanup
Marc Zyngier (8):
genirq: irqdomain: Use an accessor for the of_node field
genirq: irqdomain: Remove irqdomain dependency on struct device_node
genirq: irqdomain: Allow a domain to be identified with non-DT data
genirq: irqdomain: Add irq_create_acpi_mapping
acpi: gsi: Always perform an irq domain lookup
acpi: gsi: Add acpi_set_irq_model to initialize the GSI layer
irqchip: GIC: Switch ACPI support to stacked domains
acpi: gsi: Cleanup acpi_register_gsi
arch/arm/mach-exynos/suspend.c | 4 +-
arch/arm/mach-imx/gpc.c | 4 +-
arch/arm/mach-omap2/omap-wakeupgen.c | 4 +-
arch/c6x/platforms/megamod-pic.c | 2 +-
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/pasemi/msi.c | 6 +-
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 | 60 ++++---
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 | 54 ++++--
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 | 14 ++
include/linux/irqdomain.h | 74 ++++----
kernel/irq/irqdomain.c | 235 +++++++++++++++++++++++---
34 files changed, 402 insertions(+), 126 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 1/8] genirq: irqdomain: Use an accessor for the of_node field
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
@ 2015-09-14 16:44 ` Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node Marc Zyngier
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:44 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
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/arm/mach-exynos/suspend.c | 4 ++--
arch/arm/mach-imx/gpc.c | 4 ++--
arch/arm/mach-omap2/omap-wakeupgen.c | 4 ++--
arch/c6x/platforms/megamod-pic.c | 2 +-
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/pasemi/msi.c | 6 ++++--
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 +++++
31 files changed, 57 insertions(+), 41 deletions(-)
diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c
index e00eb39..af97afc 100644
--- a/arch/arm/mach-exynos/suspend.c
+++ b/arch/arm/mach-exynos/suspend.c
@@ -184,7 +184,7 @@ static int exynos_pmu_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 */
@@ -217,7 +217,7 @@ static int exynos_pmu_domain_alloc(struct irq_domain *domain,
&exynos_pmu_chip, NULL);
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/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c
index 8c4467f..7b32255 100644
--- a/arch/arm/mach-imx/gpc.c
+++ b/arch/arm/mach-imx/gpc.c
@@ -188,7 +188,7 @@ static int imx_gpc_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 */
@@ -223,7 +223,7 @@ static int imx_gpc_domain_alloc(struct irq_domain *domain,
&imx_gpc_chip, NULL);
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, irq, nr_irqs, &parent_args);
}
diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c
index e1d2e99..f0f7ffd 100644
--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -406,7 +406,7 @@ static int wakeupgen_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 */
@@ -441,7 +441,7 @@ static int wakeupgen_domain_alloc(struct irq_domain *domain,
&wakeupgen_chip, NULL);
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/arch/c6x/platforms/megamod-pic.c b/arch/c6x/platforms/megamod-pic.c
index d487698..1d85ce1 100644
--- a/arch/c6x/platforms/megamod-pic.c
+++ b/arch/c6x/platforms/megamod-pic.c
@@ -178,7 +178,7 @@ static void __init set_megamod_mux(struct megamod_pic *pic, int src, int output)
static void __init parse_priority_map(struct megamod_pic *pic,
int *mapping, int size)
{
- struct device_node *np = pic->irqhost->of_node;
+ struct device_node *np = irq_domain_get_of_node(pic->irqhost);
const __be32 *map;
int i, maplen;
u32 val;
diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
index f26c3c6..3d9d575 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -1094,7 +1094,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)
@@ -2163,7 +2163,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/pasemi/msi.c b/arch/powerpc/platforms/pasemi/msi.c
index e66ef19..b92ca5f 100644
--- a/arch/powerpc/platforms/pasemi/msi.c
+++ b/arch/powerpc/platforms/pasemi/msi.c
@@ -143,9 +143,11 @@ int mpic_pasemi_msi_init(struct mpic *mpic)
{
int rc;
struct pci_controller *phb;
+ struct device_node *of_node;
- if (!mpic->irqhost->of_node ||
- !of_device_is_compatible(mpic->irqhost->of_node,
+ of_node = irq_domain_get_of_node(mpic->irqhost);
+ if (!of_node ||
+ !of_device_is_compatible(of_node,
"pasemi,pwrficient-openpic"))
return -ENODEV;
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 e9c6f2a..9067a95 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 a7f5626..f1d666a 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -94,7 +94,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;
@@ -172,7 +172,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 26b55c5..239d194 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1236,7 +1236,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 7deed6e..eb6c161 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -779,7 +779,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 e6b7ed5..ecf72cf 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -926,7 +926,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 a0128c7..3ad2bab 100644
--- a/drivers/irqchip/irq-hip04.c
+++ b/drivers/irqchip/irq-hip04.c
@@ -325,7 +325,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 c8753da..b072166 100644
--- a/drivers/irqchip/irq-mtk-sysirq.c
+++ b/drivers/irqchip/irq-mtk-sysirq.c
@@ -106,7 +106,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 506d9f2..9e6c16d 100644
--- a/drivers/irqchip/irq-s3c24xx.c
+++ b/drivers/irqchip/irq-s3c24xx.c
@@ -311,7 +311,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);
@@ -342,7 +342,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 2fd89eb..7bbf226 100644
--- a/drivers/irqchip/irq-tegra.c
+++ b/drivers/irqchip/irq-tegra.c
@@ -227,7 +227,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 */
@@ -267,7 +267,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 2c22558..ae82d7e 100644
--- a/drivers/irqchip/irq-vf610-mscm-ir.c
+++ b/drivers/irqchip/irq-vf610-mscm-ir.c
@@ -142,7 +142,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;
@@ -205,7 +205,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 bdfb3c8..d4a9fb3 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -657,7 +657,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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 1/8] genirq: irqdomain: Use an accessor for the of_node field Marc Zyngier
@ 2015-09-14 16:44 ` Marc Zyngier
2015-09-14 23:15 ` Rafael J. Wysocki
2015-09-15 10:58 ` Tomasz Nowicki
2015-09-14 16:44 ` [PATCH v3 3/8] genirq: irqdomain: Allow a domain to be identified with non-DT data Marc Zyngier
` (5 subsequent siblings)
7 siblings, 2 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:44 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
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 | 68 +++++++++++++++++++-----------------
kernel/irq/irqdomain.c | 89 ++++++++++++++++++++++++++++++++++-------------
2 files changed, 101 insertions(+), 56 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index f644fdb..ac7041b 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);
@@ -292,7 +293,7 @@ extern void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
extern struct irq_domain *irq_domain_add_hierarchy(struct irq_domain *parent,
unsigned int flags, unsigned int size,
- struct device_node *node,
+ void *domain_token,
const struct irq_domain_ops *ops, void *host_data);
extern int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
unsigned int nr_irqs, int node, void *arg,
@@ -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 79baaf8..a00e0ce 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);
@@ -188,10 +210,12 @@ EXPORT_SYMBOL_GPL(irq_domain_add_legacy);
/**
* irq_find_matching_host() - Locates a domain for a given device node
- * @node: device-tree node of the interrupt controller
+ * @domain_token: global identifier of the interrupt controller. If this
+ * is a valid kernel pointer, it is assumed to be a
+ * struct device_node pointer.
* @bus_token: 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 +232,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 +364,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 +389,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 +431,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 +452,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 +468,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 +495,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 +627,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);
@@ -755,7 +794,7 @@ static int irq_domain_alloc_descs(int virq, unsigned int cnt,
* @parent: Parent irq domain to associate with the new domain
* @flags: Irq domain flags associated to the domain
* @size: Size of the domain. See below
- * @node: Optional device-tree node of the interrupt controller
+ * @domain_token: Optional global identifier of the interrupt controller
* @ops: Pointer to the interrupt domain callbacks
* @host_data: Controller private data pointer
*
@@ -768,16 +807,16 @@ static int irq_domain_alloc_descs(int virq, unsigned int cnt,
struct irq_domain *irq_domain_add_hierarchy(struct irq_domain *parent,
unsigned int flags,
unsigned int size,
- struct device_node *node,
+ void *domain_token,
const struct irq_domain_ops *ops,
void *host_data)
{
struct irq_domain *domain;
if (size)
- domain = irq_domain_add_linear(node, size, ops, host_data);
+ domain = irq_domain_add_linear(domain_token, size, ops, host_data);
else
- domain = irq_domain_add_tree(node, ops, host_data);
+ domain = irq_domain_add_tree(domain_token, ops, host_data);
if (domain) {
domain->parent = parent;
domain->flags |= flags;
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 3/8] genirq: irqdomain: Allow a domain to be identified with non-DT data
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 1/8] genirq: irqdomain: Use an accessor for the of_node field Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node Marc Zyngier
@ 2015-09-14 16:44 ` Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 4/8] genirq: irqdomain: Add irq_create_acpi_mapping Marc Zyngier
` (4 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:44 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
Now that irqdomains are not directly tied to device nodes, let's
make sure that they can be identified by other types of data.
For this, we implement the method suggested in a previous patch,
where device_token is a small integer representing data stored
in an IDR.
We end-up with a bunch of functions to allocate/free a token,
retrieve the data associated to it, or even lookup a token
by data. Not all of it might prove to be necessary, but we can
probably drop some of them if they don't get enough traction.
With this, it is possible to uniquely indentify a irqdomain,
even without a device node.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
include/linux/irqdomain.h | 5 ++
kernel/irq/irqdomain.c | 134 ++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 136 insertions(+), 3 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index ac7041b..ecd0b25 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -159,6 +159,11 @@ enum {
IRQ_DOMAIN_FLAG_NONCORE = (1 << 16),
};
+extern void *irq_domain_alloc_domain_token(void *data);
+extern void irq_domain_free_domain_token(void *domain_token);
+extern void *irq_domain_token_to_data(void *domain_token);
+extern void *irq_domain_find_domain_token(void *data);
+
#ifdef CONFIG_IRQ_DOMAIN
extern struct device_node *irq_domain_token_to_of_node(void *domain_token);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index a00e0ce..619552a 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -2,6 +2,7 @@
#include <linux/debugfs.h>
#include <linux/hardirq.h>
+#include <linux/idr.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irqdesc.h>
@@ -23,20 +24,147 @@ static DEFINE_MUTEX(irq_domain_mutex);
static DEFINE_MUTEX(revmap_trees_mutex);
static struct irq_domain *irq_default_domain;
+static DEFINE_IDR(irq_domain_idr);
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);
+/*
+ * We need to differenciate between a valid pointer (to a device_node)
+ * and a "small integer". For this, we encode a "type" in the bottom
+ * two bits:
+ *
+ * - device_node, being a pointer, has encoding 00 (and is left alone)
+ * - small integer is shifted by two bits to the left, and has encoding 01
+ *
+ * Encodings 10 and 11 are reserved.
+ */
+static bool irq_domain_check_of_node(void *domain_token)
+{
+ return (virt_addr_valid(domain_token) &&
+ IS_ALIGNED((unsigned long)domain_token,
+ sizeof(unsigned long)));
+}
+
+static void *irq_domain_encode_id(int id)
+{
+ return (void *)(long)((id << 2) | 1);
+}
+
+static int irq_domain_decode_id(void *domain_token)
+{
+ int val = (long)domain_token;
+
+ WARN_ON((val & 3) != 1);
+ return val >> 2;
+}
+
+/**
+ * irq_domain_alloc_domain_token - Allocate a new domain_token
+ * @data: caller-specific data
+ *
+ * Allocate a "small integer" that can be used to identify an irqdomain.
+ * Returns this integer as a void *, or NULL on failure.
+ */
+void *irq_domain_alloc_domain_token(void *data)
+{
+ int id;
+
+ if (!data)
+ data = &irq_domain_idr;
+
+ /*
+ * Reuse the global irqdomain mutex, as this should be called
+ * in the same context. 2^24 - 1 domains should be enough for
+ * everybody.
+ */
+ mutex_lock(&irq_domain_mutex);
+ id = idr_alloc(&irq_domain_idr, data, 1, 0xffffff, GFP_KERNEL);
+ mutex_unlock(&irq_domain_mutex);
+
+ if (id < 0)
+ return NULL;
+
+ return irq_domain_encode_id(id);
+}
+
+/**
+ * irq_domain_free_domain_token - Free an existing domain_token
+ * @domain_token: A previously allocated domain token
+ *
+ * Free the "small integer" that was used to identify an irqdomain.
+ * @domain_token is not allowed to be NULL or a valid kernel address.
+ */
+void irq_domain_free_domain_token(void *domain_token)
+{
+ WARN_ON(!domain_token);
+ WARN_ON(irq_domain_check_of_node(domain_token));
+
+ mutex_lock(&irq_domain_mutex);
+ idr_remove(&irq_domain_idr, irq_domain_decode_id(domain_token));
+ mutex_unlock(&irq_domain_mutex);
+}
+
+/**
+ * irq_domain_token_to_data - Retrieve data associated with a domain_token
+ * @domain_token: An allocated domain token
+ *
+ * Returns the data previously associated with an irqdomain.
+ */
+void *irq_domain_token_to_data(void *domain_token)
+{
+ void *data;
+
+ WARN_ON(!domain_token);
+ WARN_ON(irq_domain_check_of_node(domain_token));
+
+ rcu_read_lock();
+ data = idr_find(&irq_domain_idr, irq_domain_decode_id(domain_token));
+ rcu_read_unlock();
+
+ if (data == &irq_domain_idr)
+ data = NULL;
+
+ return data;
+}
+
+static int __irq_domain_matches_id(int id, void *p, void *data)
+{
+ return (p == data) ? id : 0;
+}
+
+/**
+ * irq_domain_token_find_domain_token - Find a previously allocated domain_token
+ * @data: A unique pointer previously used to allocate a domain_token
+ *
+ * Returns the domain_token previously allocated with @data. Because
+ * this is an expensive operation, it should only be used when there
+ * is no practical way to retrieve the domain token itself. @data must
+ * have only been used to allocate a single domain_token.
+ */
+void *irq_domain_find_domain_token(void *data)
+{
+ int id;
+
+ mutex_lock(&irq_domain_mutex);
+ id = idr_for_each(&irq_domain_idr, __irq_domain_matches_id, data);
+ mutex_unlock(&irq_domain_mutex);
+
+ if (WARN_ON(id <= 0))
+ return NULL;
+
+ return irq_domain_encode_id(id);
+}
+
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.
+ * and indirected via the irqdomain IDR layer.
*/
- if (virt_addr_valid(domain_token))
+ if (irq_domain_check_of_node(domain_token))
return domain_token;
return NULL;
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 4/8] genirq: irqdomain: Add irq_create_acpi_mapping
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
` (2 preceding siblings ...)
2015-09-14 16:44 ` [PATCH v3 3/8] genirq: irqdomain: Allow a domain to be identified with non-DT data Marc Zyngier
@ 2015-09-14 16:44 ` Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 5/8] acpi: gsi: Always perform an irq domain lookup Marc Zyngier
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:44 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
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
that of 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 7235c48..4db2f01 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -201,6 +201,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 619552a..ca1301b 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/idr.h>
@@ -687,6 +688,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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 5/8] acpi: gsi: Always perform an irq domain lookup
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
` (3 preceding siblings ...)
2015-09-14 16:44 ` [PATCH v3 4/8] genirq: irqdomain: Add irq_create_acpi_mapping Marc Zyngier
@ 2015-09-14 16:44 ` Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 6/8] acpi: gsi: Add acpi_set_irq_model to initialize the GSI layer Marc Zyngier
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:44 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
Instead of directly passing NULL to the various irq_domain functions,
start by looking up the domain with a domain_token.
As domain_token is permanently set to NULL, the lookup function will
return the same value (no domain found) and the default will be used,
preserving the current behaviour.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/acpi/gsi.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index 38208f2..b2af93b 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -14,6 +14,8 @@
enum acpi_irq_model_id acpi_irq_model;
+static void *acpi_gsi_domain_token;
+
static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
{
switch (polarity) {
@@ -45,12 +47,9 @@ 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_host(acpi_gsi_domain_token);
+
+ *irq = irq_find_mapping(d, gsi);
/*
* *irq == 0 means no mapping, that should
* be reported as a failure
@@ -74,13 +73,9 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
{
unsigned int irq;
unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
+ struct irq_domain *d = irq_find_host(acpi_gsi_domain_token);
- /*
- * 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
- */
- irq = irq_create_mapping(NULL, gsi);
+ irq = irq_create_mapping(d, gsi);
if (!irq)
return -EINVAL;
@@ -98,7 +93,8 @@ 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_host(acpi_gsi_domain_token);
+ int irq = irq_find_mapping(d, gsi);
irq_dispose_mapping(irq);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 6/8] acpi: gsi: Add acpi_set_irq_model to initialize the GSI layer
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
` (4 preceding siblings ...)
2015-09-14 16:44 ` [PATCH v3 5/8] acpi: gsi: Always perform an irq domain lookup Marc Zyngier
@ 2015-09-14 16:44 ` Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 7/8] irqchip: GIC: Switch ACPI support to stacked domains Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 8/8] acpi: gsi: Cleanup acpi_register_gsi Marc Zyngier
7 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:44 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
In order to start embrassing irqdomains at the GSI level, introduce
a new initializer:
void acpi_set_irq_model(enum acpi_irq_model_id model,
unsigned long domain_token,
int (*populate)(struct acpi_gsi_descriptor *,
u32, unsigned int));
where:
- model is the value assigned to acpi_irq_model
- domain_token is the identifier for the irqdomain mapping
GSI interrupts
- populate is a function provided by the interrupt controller,
populating a struct acpi_gsi_descriptor based on a GSI and
the interrupt trigger information
As nobody calls this code yet, the current code is left in place.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/acpi/gsi.c | 32 ++++++++++++++++++++++++++++++++
include/linux/acpi.h | 5 +++++
2 files changed, 37 insertions(+)
diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index b2af93b..07f5fef5 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -16,6 +16,9 @@ enum acpi_irq_model_id acpi_irq_model;
static void *acpi_gsi_domain_token;
+static int (*acpi_gsi_descriptor_populate)(struct acpi_gsi_descriptor *data,
+ u32 gsi, unsigned int irq_type);
+
static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity)
{
switch (polarity) {
@@ -71,10 +74,20 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
int polarity)
{
+ int err;
unsigned int irq;
unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
struct irq_domain *d = irq_find_host(acpi_gsi_domain_token);
+ if (acpi_gsi_descriptor_populate) {
+ struct acpi_gsi_descriptor data;
+ err = acpi_gsi_descriptor_populate(&data, gsi, irq_type);
+ if (err)
+ return err;
+
+ return irq_create_acpi_mapping(d, &data);
+ }
+
irq = irq_create_mapping(d, gsi);
if (!irq)
return -EINVAL;
@@ -99,3 +112,22 @@ void acpi_unregister_gsi(u32 gsi)
irq_dispose_mapping(irq);
}
EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
+
+/**
+ * acpi_set_irq_model - Setup the GSI irqdomain information
+ * @model: the value assigned to acpi_irq_model
+ * @domain_token: the irq_domain identifier for mapping and looking up
+ * GSI interrupts
+ * @populate: provided by the interrupt controller, populating a
+ * struct acpi_gsi_descriptor based on a GSI and
+ * the interrupt trigger information
+ */
+void __init acpi_set_irq_model(enum acpi_irq_model_id model,
+ void *domain_token,
+ int (*populate)(struct acpi_gsi_descriptor *,
+ u32, unsigned int))
+{
+ acpi_irq_model = model;
+ acpi_gsi_domain_token = domain_token;
+ acpi_gsi_descriptor_populate = populate;
+}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4db2f01..8ad88f0 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -210,6 +210,11 @@ struct acpi_gsi_descriptor {
unsigned int irq_create_acpi_mapping(struct irq_domain *d,
struct acpi_gsi_descriptor *irq_data);
+void acpi_set_irq_model(enum acpi_irq_model_id model,
+ void *domain_token,
+ int (*populate)(struct acpi_gsi_descriptor *,
+ u32, unsigned int));
+
#ifdef CONFIG_X86_IO_APIC
extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);
#else
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 7/8] irqchip: GIC: Switch ACPI support to stacked domains
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
` (5 preceding siblings ...)
2015-09-14 16:44 ` [PATCH v3 6/8] acpi: gsi: Add acpi_set_irq_model to initialize the GSI layer Marc Zyngier
@ 2015-09-14 16:44 ` Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 8/8] acpi: gsi: Cleanup acpi_register_gsi Marc Zyngier
7 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:44 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
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 | 54 ++++++++++++++++++++++++++++++++++++-----------
1 file changed, 42 insertions(+), 12 deletions(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index ecf72cf..bc6353a 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -926,8 +926,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;
@@ -995,7 +993,7 @@ static const struct irq_domain_ops gic_irq_domain_ops = {
static 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;
@@ -1047,8 +1045,8 @@ static 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 */
@@ -1074,7 +1072,7 @@ static 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);
}
@@ -1220,10 +1218,36 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header,
return 0;
}
+static int gic_acpi_gsi_desc_populate(struct acpi_gsi_descriptor *data,
+ u32 gsi, unsigned int irq_type)
+{
+ /*
+ * Encode GSI and triggering information the way the GIC likes
+ * them.
+ */
+ if (WARN_ON(gsi < 16))
+ return -EINVAL;
+
+ if (gsi >= 32) {
+ data->param[0] = 0; /* SPI */
+ data->param[1] = gsi - 32;
+ data->param[2] = irq_type;
+ } else {
+ data->param[0] = 1; /* PPI */
+ data->param[1] = gsi - 16;
+ data->param[2] = 0xff << 4 | irq_type;
+ }
+
+ data->param_count = 3;
+
+ return 0;
+}
+
int __init
gic_v2_acpi_init(struct acpi_table_header *table)
{
void __iomem *cpu_base, *dist_base;
+ void *domain_token;
int count;
/* Collect CPU base addresses */
@@ -1274,14 +1298,20 @@ gic_v2_acpi_init(struct acpi_table_header *table)
static_key_slow_dec(&supports_deactivate);
/*
- * 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 GIC instance zero (no multi-GIC support).
*/
- __gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL);
- irq_set_default_host(gic_data[0].domain);
+ domain_token = irq_domain_alloc_domain_token(NULL);
+ if (!domain_token) {
+ pr_err("Unable to allocate domain token\n");
+ iounmap(cpu_base);
+ iounmap(dist_base);
+ return -ENOMEM;
+ }
+
+ __gic_init_bases(0, -1, dist_base, cpu_base, 0, domain_token);
- acpi_irq_model = ACPI_IRQ_MODEL_GIC;
+ acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_token,
+ gic_acpi_gsi_desc_populate);
return 0;
}
#endif
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 8/8] acpi: gsi: Cleanup acpi_register_gsi
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
` (6 preceding siblings ...)
2015-09-14 16:44 ` [PATCH v3 7/8] irqchip: GIC: Switch ACPI support to stacked domains Marc Zyngier
@ 2015-09-14 16:44 ` Marc Zyngier
7 siblings, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-14 16:44 UTC (permalink / raw)
To: Thomas Gleixner, Jiang Liu, Jason Cooper, Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
As the only user of drivers/acpi/gsi.c is now using acpi_set_irq_model
to set acpi_gsi_descriptor_populate to something meaningful, we can
always rely on that information to be present (its absence is an error),
and guarantee that new interrupt controllers will use this API.
Take this opportunity to cleanup acpi_register_gsi.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/acpi/gsi.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
index 07f5fef5..2dde6a1 100644
--- a/drivers/acpi/gsi.c
+++ b/drivers/acpi/gsi.c
@@ -75,28 +75,20 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
int polarity)
{
int err;
- 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_host(acpi_gsi_domain_token);
- if (acpi_gsi_descriptor_populate) {
- struct acpi_gsi_descriptor data;
- err = acpi_gsi_descriptor_populate(&data, gsi, irq_type);
- if (err)
- return err;
-
- return irq_create_acpi_mapping(d, &data);
+ if (WARN_ON(!acpi_gsi_descriptor_populate)) {
+ pr_warn("GSI: No registered irqchip, giving up\n");
+ return -EINVAL;
}
- irq = irq_create_mapping(d, gsi);
- if (!irq)
- return -EINVAL;
+ err = acpi_gsi_descriptor_populate(&data, gsi, irq_type);
+ if (err)
+ return err;
- /* 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);
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-14 16:44 ` [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node Marc Zyngier
@ 2015-09-14 23:15 ` Rafael J. Wysocki
2015-09-15 9:18 ` Marc Zyngier
2015-09-15 10:58 ` Tomasz Nowicki
1 sibling, 1 reply; 19+ messages in thread
From: Rafael J. Wysocki @ 2015-09-14 23:15 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, Jiang Liu, Jason Cooper, linux-acpi,
linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
On Monday, September 14, 2015 05:44:01 PM 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.
>
> 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 | 68 +++++++++++++++++++-----------------
> kernel/irq/irqdomain.c | 89 ++++++++++++++++++++++++++++++++++-------------
> 2 files changed, 101 insertions(+), 56 deletions(-)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index f644fdb..ac7041b 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;
I'm wondering if that may be something which isn't (void *), but a specific
pointer type, so the compiler warns us when something suspicious is assigned
to it?
[Somewhat along the lines struct fwnode_handle is used elsewehere.]
> enum irq_domain_bus_token bus_token;
> struct irq_domain_chip_generic *gc;
> #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> @@ -161,70 +159,73 @@ enum {
Thanks,
Rafael
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-14 23:15 ` Rafael J. Wysocki
@ 2015-09-15 9:18 ` Marc Zyngier
2015-09-16 1:53 ` Rafael J. Wysocki
0 siblings, 1 reply; 19+ messages in thread
From: Marc Zyngier @ 2015-09-15 9:18 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Thomas Gleixner, Jiang Liu, Jason Cooper, linux-acpi,
linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
On 15/09/15 00:15, Rafael J. Wysocki wrote:
> On Monday, September 14, 2015 05:44:01 PM 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.
>>
>> 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 | 68 +++++++++++++++++++-----------------
>> kernel/irq/irqdomain.c | 89 ++++++++++++++++++++++++++++++++++-------------
>> 2 files changed, 101 insertions(+), 56 deletions(-)
>>
>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>> index f644fdb..ac7041b 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;
>
> I'm wondering if that may be something which isn't (void *), but a specific
> pointer type, so the compiler warns us when something suspicious is assigned
> to it?
>
> [Somewhat along the lines struct fwnode_handle is used elsewehere.]
Yeah, I'm obviously being lazy ;-).
More seriously, I'm trying hard to avoid anything that will require an
additional memory allocation. Going from a device_node to a
fwnode_handle-like structure requires such an allocation which is going
to be a massive pain to retrofit - not impossible, but painful.
What I'm currently aiming for is tagged pointers, where the two bottom
bits indicate the type of the pointed object (see patch #3):
- 00 indicates a device node
- 01 indicates an IDR entry (shifted left by two bits)
- 10 and 11 are currently unallocated, and one of them could be used to
encode a fwnode_handle.
It would be slightly easier to replace the (void *) with a union of the
above types:
union domain_token {
struct device_node *of_node;
struct fwnode_handle *fwnode;
unsigned long id;
};
That would remove the need for an extra allocation (at the cost of the
above hack). We just need the accessors to strip the tag. Still need to
repaint all the users of irq_domain_add_*, which is going to be
amazingly invasive.
Thoughts?
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-14 16:44 ` [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node Marc Zyngier
2015-09-14 23:15 ` Rafael J. Wysocki
@ 2015-09-15 10:58 ` Tomasz Nowicki
2015-09-15 12:04 ` Thomas Gleixner
2015-09-15 12:22 ` Marc Zyngier
1 sibling, 2 replies; 19+ messages in thread
From: Tomasz Nowicki @ 2015-09-15 10:58 UTC (permalink / raw)
To: Marc Zyngier, Thomas Gleixner, Jiang Liu, Jason Cooper,
Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Hanjun Guo, Suravee Suthikulpanit, Graeme Gregory, Jake Oshins
On 14.09.2015 18:44, 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.
>
> 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 | 68 +++++++++++++++++++-----------------
> kernel/irq/irqdomain.c | 89 ++++++++++++++++++++++++++++++++++-------------
> 2 files changed, 101 insertions(+), 56 deletions(-)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index f644fdb..ac7041b 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);
> @@ -292,7 +293,7 @@ extern void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
> #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> extern struct irq_domain *irq_domain_add_hierarchy(struct irq_domain *parent,
> unsigned int flags, unsigned int size,
> - struct device_node *node,
> + void *domain_token,
> const struct irq_domain_ops *ops, void *host_data);
> extern int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
> unsigned int nr_irqs, int node, void *arg,
> @@ -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 79baaf8..a00e0ce 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));
While we are here, do you think it makes sense to abstract
of_node_to_nid as well? Then we would really remove device_node
dependency for irqdomain.
Thanks,
Tomasz
> 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);
> @@ -188,10 +210,12 @@ EXPORT_SYMBOL_GPL(irq_domain_add_legacy);
>
> /**
> * irq_find_matching_host() - Locates a domain for a given device node
> - * @node: device-tree node of the interrupt controller
> + * @domain_token: global identifier of the interrupt controller. If this
> + * is a valid kernel pointer, it is assumed to be a
> + * struct device_node pointer.
> * @bus_token: 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 +232,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 +364,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 +389,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 +431,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 +452,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 +468,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 +495,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 +627,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);
>
> @@ -755,7 +794,7 @@ static int irq_domain_alloc_descs(int virq, unsigned int cnt,
> * @parent: Parent irq domain to associate with the new domain
> * @flags: Irq domain flags associated to the domain
> * @size: Size of the domain. See below
> - * @node: Optional device-tree node of the interrupt controller
> + * @domain_token: Optional global identifier of the interrupt controller
> * @ops: Pointer to the interrupt domain callbacks
> * @host_data: Controller private data pointer
> *
> @@ -768,16 +807,16 @@ static int irq_domain_alloc_descs(int virq, unsigned int cnt,
> struct irq_domain *irq_domain_add_hierarchy(struct irq_domain *parent,
> unsigned int flags,
> unsigned int size,
> - struct device_node *node,
> + void *domain_token,
> const struct irq_domain_ops *ops,
> void *host_data)
> {
> struct irq_domain *domain;
>
> if (size)
> - domain = irq_domain_add_linear(node, size, ops, host_data);
> + domain = irq_domain_add_linear(domain_token, size, ops, host_data);
> else
> - domain = irq_domain_add_tree(node, ops, host_data);
> + domain = irq_domain_add_tree(domain_token, ops, host_data);
> if (domain) {
> domain->parent = parent;
> domain->flags |= flags;
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-15 10:58 ` Tomasz Nowicki
@ 2015-09-15 12:04 ` Thomas Gleixner
2015-09-15 12:08 ` Tomasz Nowicki
2015-09-15 12:22 ` Marc Zyngier
1 sibling, 1 reply; 19+ messages in thread
From: Thomas Gleixner @ 2015-09-15 12:04 UTC (permalink / raw)
To: Tomasz Nowicki
Cc: Marc Zyngier, Jiang Liu, Jason Cooper, Rafael J. Wysocki,
linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Hanjun Guo, Suravee Suthikulpanit, Graeme Gregory, Jake Oshins
On Tue, 15 Sep 2015, Tomasz Nowicki wrote:
Can you folks please do proper quoting on reply? It's annoying to
scroll down 250+ lines to find TWO lines of reply and then have
another 250+ lines for nothing.
Here is the real gist of your reply:
> On 14.09.2015 18:44, Marc Zyngier wrote:
> > -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));
>
> While we are here, do you think it makes sense to abstract of_node_to_nid as
> well? Then we would really remove device_node dependency for irqdomain.
15 lines of useful content versus 500+ lines of useless noise.
Thanks,
tglx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-15 12:04 ` Thomas Gleixner
@ 2015-09-15 12:08 ` Tomasz Nowicki
0 siblings, 0 replies; 19+ messages in thread
From: Tomasz Nowicki @ 2015-09-15 12:08 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Marc Zyngier, Jiang Liu, Jason Cooper, Rafael J. Wysocki,
linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Hanjun Guo, Suravee Suthikulpanit, Graeme Gregory, Jake Oshins
On 15.09.2015 14:04, Thomas Gleixner wrote:
> On Tue, 15 Sep 2015, Tomasz Nowicki wrote:
>
> Can you folks please do proper quoting on reply? It's annoying to
> scroll down 250+ lines to find TWO lines of reply and then have
> another 250+ lines for nothing.
>
Yeah, good point! Sorry.
Tomasz
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-15 10:58 ` Tomasz Nowicki
2015-09-15 12:04 ` Thomas Gleixner
@ 2015-09-15 12:22 ` Marc Zyngier
1 sibling, 0 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-15 12:22 UTC (permalink / raw)
To: Tomasz Nowicki, Thomas Gleixner, Jiang Liu, Jason Cooper,
Rafael J. Wysocki
Cc: linux-acpi, linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Hanjun Guo, Suravee Suthikulpanit, Graeme Gregory, Jake Oshins
On 15/09/15 11:58, Tomasz Nowicki wrote:
> On 14.09.2015 18:44, 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.
>>
>> 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 | 68 +++++++++++++++++++-----------------
>> kernel/irq/irqdomain.c | 89 ++++++++++++++++++++++++++++++++++-------------
>> 2 files changed, 101 insertions(+), 56 deletions(-)
>>
[...]
>> -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));
>
> While we are here, do you think it makes sense to abstract
> of_node_to_nid as well? Then we would really remove device_node
> dependency for irqdomain.
Not quite sure yet. I've decided to completely ignore the whole NUMA
thing for the time being. This is a performance issue, not a
functionality problem.
Once we have something that actually makes sense, we can have a look.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-15 9:18 ` Marc Zyngier
@ 2015-09-16 1:53 ` Rafael J. Wysocki
2015-09-16 7:49 ` Marc Zyngier
0 siblings, 1 reply; 19+ messages in thread
From: Rafael J. Wysocki @ 2015-09-16 1:53 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, Jiang Liu, Jason Cooper, linux-acpi,
linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
On Tuesday, September 15, 2015 10:18:32 AM Marc Zyngier wrote:
> On 15/09/15 00:15, Rafael J. Wysocki wrote:
> > On Monday, September 14, 2015 05:44:01 PM 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.
> >>
> >> 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 | 68 +++++++++++++++++++-----------------
> >> kernel/irq/irqdomain.c | 89 ++++++++++++++++++++++++++++++++++-------------
> >> 2 files changed, 101 insertions(+), 56 deletions(-)
> >>
> >> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> >> index f644fdb..ac7041b 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;
> >
> > I'm wondering if that may be something which isn't (void *), but a specific
> > pointer type, so the compiler warns us when something suspicious is assigned
> > to it?
> >
> > [Somewhat along the lines struct fwnode_handle is used elsewehere.]
>
> Yeah, I'm obviously being lazy ;-).
>
> More seriously, I'm trying hard to avoid anything that will require an
> additional memory allocation. Going from a device_node to a
> fwnode_handle-like structure requires such an allocation which is going
> to be a massive pain to retrofit - not impossible, but painful.
>
> What I'm currently aiming for is tagged pointers, where the two bottom
> bits indicate the type of the pointed object (see patch #3):
> - 00 indicates a device node
> - 01 indicates an IDR entry (shifted left by two bits)
> - 10 and 11 are currently unallocated, and one of them could be used to
> encode a fwnode_handle.
>
> It would be slightly easier to replace the (void *) with a union of the
> above types:
>
> union domain_token {
> struct device_node *of_node;
> struct fwnode_handle *fwnode;
> unsigned long id;
> };
>
> That would remove the need for an extra allocation (at the cost of the
> above hack). We just need the accessors to strip the tag. Still need to
> repaint all the users of irq_domain_add_*, which is going to be
> amazingly invasive.
>
> Thoughts?
Well, I'm not sure this is worth the effort to be honest.
I've just seen quite a few bugs where a pointer to something completely invalid
have been silently passed via (void *) which often results in very interesting
breakage (that is really hard to debug for that matter).
Thanks,
Rafael
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-16 1:53 ` Rafael J. Wysocki
@ 2015-09-16 7:49 ` Marc Zyngier
2015-09-16 9:00 ` Thomas Gleixner
2015-09-16 12:57 ` Rafael J. Wysocki
0 siblings, 2 replies; 19+ messages in thread
From: Marc Zyngier @ 2015-09-16 7:49 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Thomas Gleixner, Jiang Liu, Jason Cooper, linux-acpi,
linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
On 16/09/15 02:53, Rafael J. Wysocki wrote:
> On Tuesday, September 15, 2015 10:18:32 AM Marc Zyngier wrote:
>> On 15/09/15 00:15, Rafael J. Wysocki wrote:
>>> On Monday, September 14, 2015 05:44:01 PM 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.
>>>>
>>>> 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 | 68 +++++++++++++++++++-----------------
>>>> kernel/irq/irqdomain.c | 89 ++++++++++++++++++++++++++++++++++-------------
>>>> 2 files changed, 101 insertions(+), 56 deletions(-)
>>>>
>>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
>>>> index f644fdb..ac7041b 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;
>>>
>>> I'm wondering if that may be something which isn't (void *), but a specific
>>> pointer type, so the compiler warns us when something suspicious is assigned
>>> to it?
>>>
>>> [Somewhat along the lines struct fwnode_handle is used elsewehere.]
>>
>> Yeah, I'm obviously being lazy ;-).
>>
>> More seriously, I'm trying hard to avoid anything that will require an
>> additional memory allocation. Going from a device_node to a
>> fwnode_handle-like structure requires such an allocation which is going
>> to be a massive pain to retrofit - not impossible, but painful.
>>
>> What I'm currently aiming for is tagged pointers, where the two bottom
>> bits indicate the type of the pointed object (see patch #3):
>> - 00 indicates a device node
>> - 01 indicates an IDR entry (shifted left by two bits)
>> - 10 and 11 are currently unallocated, and one of them could be used to
>> encode a fwnode_handle.
>>
>> It would be slightly easier to replace the (void *) with a union of the
>> above types:
>>
>> union domain_token {
>> struct device_node *of_node;
>> struct fwnode_handle *fwnode;
>> unsigned long id;
>> };
>>
>> That would remove the need for an extra allocation (at the cost of the
>> above hack). We just need the accessors to strip the tag. Still need to
>> repaint all the users of irq_domain_add_*, which is going to be
>> amazingly invasive.
>>
>> Thoughts?
>
> Well, I'm not sure this is worth the effort to be honest.
>
> I've just seen quite a few bugs where a pointer to something completely invalid
> have been silently passed via (void *) which often results in very interesting
> breakage (that is really hard to debug for that matter).
I actually tried to prototype this yesterday, and ended up in hell. The
main issue is the point where the generic irqdomain code meets the DT
subsystem (which is basically any interrupt controller, including those
being ACPI driven). The domain_token to of_node path is dead easy, but
you cannot do the reverse conversion, so this would have to spread
around like a cancer. I gave up.
At this stage, I see two options: sticking with (void *) with the risk
of breakage and subtle bugs which we all love to track down (not!), or
do a major U-turn and make device_node a strict requirement for domain
lookup.
After the above experiment, I now see some value in actually keeping
device_node around, using it as the token, and allocating it when
required (the typical case being those interrupt controllers that are
both DT and ACPI). I'll play with it a bit more.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-16 7:49 ` Marc Zyngier
@ 2015-09-16 9:00 ` Thomas Gleixner
2015-09-16 12:57 ` Rafael J. Wysocki
1 sibling, 0 replies; 19+ messages in thread
From: Thomas Gleixner @ 2015-09-16 9:00 UTC (permalink / raw)
To: Marc Zyngier
Cc: Rafael J. Wysocki, Jiang Liu, Jason Cooper, linux-acpi,
linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
On Wed, 16 Sep 2015, Marc Zyngier wrote:
> On 16/09/15 02:53, Rafael J. Wysocki wrote:
> > I've just seen quite a few bugs where a pointer to something completely invalid
> > have been silently passed via (void *) which often results in very interesting
> > breakage (that is really hard to debug for that matter).
>
> I actually tried to prototype this yesterday, and ended up in hell. The
> main issue is the point where the generic irqdomain code meets the DT
> subsystem (which is basically any interrupt controller, including those
> being ACPI driven). The domain_token to of_node path is dead easy, but
> you cannot do the reverse conversion, so this would have to spread
> around like a cancer. I gave up.
>
> At this stage, I see two options: sticking with (void *) with the risk
> of breakage and subtle bugs which we all love to track down (not!), or
> do a major U-turn and make device_node a strict requirement for domain
> lookup.
>
> After the above experiment, I now see some value in actually keeping
> device_node around, using it as the token, and allocating it when
> required (the typical case being those interrupt controllers that are
> both DT and ACPI). I'll play with it a bit more.
Right. We already have the fwnode_type in struct device_node, so we
can reuse it. If you care about the size of the struct, then you can
make some of the struct members conditional on CONFIG_OF.
Thanks,
tglx
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node
2015-09-16 7:49 ` Marc Zyngier
2015-09-16 9:00 ` Thomas Gleixner
@ 2015-09-16 12:57 ` Rafael J. Wysocki
1 sibling, 0 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2015-09-16 12:57 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, Jiang Liu, Jason Cooper, linux-acpi,
linux-arm-kernel, linux-kernel, Lorenzo Pieralisi,
Tomasz Nowicki, Hanjun Guo, Suravee Suthikulpanit,
Graeme Gregory, Jake Oshins
On Wednesday, September 16, 2015 08:49:27 AM Marc Zyngier wrote:
> On 16/09/15 02:53, Rafael J. Wysocki wrote:
> > On Tuesday, September 15, 2015 10:18:32 AM Marc Zyngier wrote:
> >> On 15/09/15 00:15, Rafael J. Wysocki wrote:
> >>> On Monday, September 14, 2015 05:44:01 PM 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.
> >>>>
> >>>> 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 | 68 +++++++++++++++++++-----------------
> >>>> kernel/irq/irqdomain.c | 89 ++++++++++++++++++++++++++++++++++-------------
> >>>> 2 files changed, 101 insertions(+), 56 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> >>>> index f644fdb..ac7041b 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;
> >>>
> >>> I'm wondering if that may be something which isn't (void *), but a specific
> >>> pointer type, so the compiler warns us when something suspicious is assigned
> >>> to it?
> >>>
> >>> [Somewhat along the lines struct fwnode_handle is used elsewehere.]
> >>
> >> Yeah, I'm obviously being lazy ;-).
> >>
> >> More seriously, I'm trying hard to avoid anything that will require an
> >> additional memory allocation. Going from a device_node to a
> >> fwnode_handle-like structure requires such an allocation which is going
> >> to be a massive pain to retrofit - not impossible, but painful.
> >>
> >> What I'm currently aiming for is tagged pointers, where the two bottom
> >> bits indicate the type of the pointed object (see patch #3):
> >> - 00 indicates a device node
> >> - 01 indicates an IDR entry (shifted left by two bits)
> >> - 10 and 11 are currently unallocated, and one of them could be used to
> >> encode a fwnode_handle.
> >>
> >> It would be slightly easier to replace the (void *) with a union of the
> >> above types:
> >>
> >> union domain_token {
> >> struct device_node *of_node;
> >> struct fwnode_handle *fwnode;
> >> unsigned long id;
> >> };
> >>
> >> That would remove the need for an extra allocation (at the cost of the
> >> above hack). We just need the accessors to strip the tag. Still need to
> >> repaint all the users of irq_domain_add_*, which is going to be
> >> amazingly invasive.
> >>
> >> Thoughts?
> >
> > Well, I'm not sure this is worth the effort to be honest.
> >
> > I've just seen quite a few bugs where a pointer to something completely invalid
> > have been silently passed via (void *) which often results in very interesting
> > breakage (that is really hard to debug for that matter).
>
> I actually tried to prototype this yesterday, and ended up in hell. The
> main issue is the point where the generic irqdomain code meets the DT
> subsystem (which is basically any interrupt controller, including those
> being ACPI driven). The domain_token to of_node path is dead easy, but
> you cannot do the reverse conversion, so this would have to spread
> around like a cancer. I gave up.
If you replaced the struct device_node pointer with a struct fwnode_handle one,
you can get from there to the original with to_of_node() easily and backwards
too.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-09-16 12:29 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 16:43 [PATCH v3 0/8] Making the generic ACPI GSI layer irqdomain aware Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 1/8] genirq: irqdomain: Use an accessor for the of_node field Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node Marc Zyngier
2015-09-14 23:15 ` Rafael J. Wysocki
2015-09-15 9:18 ` Marc Zyngier
2015-09-16 1:53 ` Rafael J. Wysocki
2015-09-16 7:49 ` Marc Zyngier
2015-09-16 9:00 ` Thomas Gleixner
2015-09-16 12:57 ` Rafael J. Wysocki
2015-09-15 10:58 ` Tomasz Nowicki
2015-09-15 12:04 ` Thomas Gleixner
2015-09-15 12:08 ` Tomasz Nowicki
2015-09-15 12:22 ` Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 3/8] genirq: irqdomain: Allow a domain to be identified with non-DT data Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 4/8] genirq: irqdomain: Add irq_create_acpi_mapping Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 5/8] acpi: gsi: Always perform an irq domain lookup Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 6/8] acpi: gsi: Add acpi_set_irq_model to initialize the GSI layer Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 7/8] irqchip: GIC: Switch ACPI support to stacked domains Marc Zyngier
2015-09-14 16:44 ` [PATCH v3 8/8] acpi: gsi: Cleanup acpi_register_gsi Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).