[PATCH v2 0/9] Add self-probe infrastructure and stacked irqdomain support for ACPI based GICv2/3 init
This patch set introduce self-probe infrastructure to init IRQ controllers and stacked irqdomain support for ACPI based GICv2/3 init.
The self-probe infrastructure for ACPI GIC init is similar as IRQCHIP_DECLARE() and based on the GIC version support in ACPI MADT table. we match the GIC version and GIC driver and load it.
We introduce acpi_irq_domain for GICv2/3 core domain to support stacked irqdomain, and pass the gsi (global system interrupt) as the agument (void *arg) for gic_irq_domain_alloc(), then we can alloc virqs via acpi_register_gsi() with stacked irqdomain.
The stacked irq domain will be:
acpi_irq_domain == gicv2 or gicv3 core domain ^ | (parent) | ITS domain or GIv2m domain ^ | | MSI chip irq domain
Since there is one GICD supported for now, so there will be only one irqdomain created when GICv2/3 initialized, for all the wired hardware irqs for PPI, SGI and SPI, they will have unique hardware irq number (GSI), so we can match the device to the acpi_irq_domain with unique GSI and it will work. If there will be multi GICD in the future, this will still works to follow the solution for x86 of ACPI (mutil IOAPICs).
Note: patch 1~2 already merged in ACPICA core, I just send them out for helping of better understanding later 7 patches.
Any comments are warmly welcomed.
update from RFC version: - Consolidate all the GIC init code into drivers/irqchip/irq-gic-acpi.c
v1->v2: - Remove the gicv2/v3 related driver code in drivers/irqchip/irq-gic-acpi.c which I was trying to consolidate them in one file, then arm-gic.h and arm-gic-v3.h will be used separately within parent driver only
- Drop the gsi_mutex patch
- Use the GIC version to match the GIC driver, then no need to test for the version in each driver
- Move acpi_irq_init() to drivers/irqchip/irq-gic-acpi.c instead of in drivers/acpi/irq.c. maybe we need to rename acpi_irq_init() as acpi_gic_init() but I keep that name to accommodate of_irq_init(), any objections please let me know.
Bob Moore (1): ACPICA: ACPI 6.0: Add changes for MADT table.
Hanjun Guo (6): ACPICA: ACPI 6.0: Add values for MADT GIC version field. irqchip / GIC: Add GIC version support in ACPI MADT ACPI / irqchip: Add self-probe infrastructure to initialize IRQ controller irqchip / GIC / ACPI: Use IRQCHIP_ACPI_DECLARE to simplify GICv2 init code irqchip / gic: Add stacked irqdomain support for ACPI based GICv2 init irqchip / GICv3: Add stacked irqdomain support for ACPI based init
Tomasz Nowicki (2): irqchip / GICv3: Refactor gic_of_init() for GICv3 driver irqchip / GICv3: Add ACPI support for GICv3+ initialization
arch/arm64/Kconfig | 1 + arch/arm64/include/asm/acpi.h | 1 - arch/arm64/include/asm/irq.h | 13 -- arch/arm64/kernel/acpi.c | 25 ---- drivers/acpi/gsi.c | 28 ++-- drivers/irqchip/Kconfig | 3 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-gic-acpi.c | 141 +++++++++++++++++++ drivers/irqchip/irq-gic-v3.c | 264 ++++++++++++++++++++++++++++------- drivers/irqchip/irq-gic.c | 35 ++--- drivers/irqchip/irqchip.h | 12 ++ include/acpi/actbl1.h | 33 ++++- include/asm-generic/vmlinux.lds.h | 13 ++ include/linux/acpi.h | 16 +++ include/linux/acpi_irq.h | 4 +- include/linux/irqchip/arm-gic-acpi.h | 10 +- include/linux/mod_devicetable.h | 8 ++ 17 files changed, 473 insertions(+), 135 deletions(-) create mode 100644 drivers/irqchip/irq-gic-acpi.c
From: Bob Moore robert.moore@intel.com
ACPICA commit 02cbb41232bccf7a91967140cab95d5f48291f21
New subtable type. Some additions to existing subtables.
Link: https://github.com/acpica/acpica/commit/02cbb412 Signed-off-by: Bob Moore robert.moore@intel.com Signed-off-by: Lv Zheng lv.zheng@intel.com Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com ---
This patch is already merged into APCICA core, sending them out just for the help of better understanding later GIC related code.
include/acpi/actbl1.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h index b80b0e6..cadf21c 100644 --- a/include/acpi/actbl1.h +++ b/include/acpi/actbl1.h @@ -673,7 +673,8 @@ enum acpi_madt_type { ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR = 12, ACPI_MADT_TYPE_GENERIC_MSI_FRAME = 13, ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR = 14, - ACPI_MADT_TYPE_RESERVED = 15 /* 15 and greater are reserved */ + ACPI_MADT_TYPE_GENERIC_TRANSLATOR = 15, + ACPI_MADT_TYPE_RESERVED = 16 /* 16 and greater are reserved */ };
/* @@ -794,7 +795,7 @@ struct acpi_madt_local_x2apic_nmi { u8 reserved[3]; /* reserved - must be zero */ };
-/* 11: Generic Interrupt (ACPI 5.0) */ +/* 11: Generic Interrupt (ACPI 5.0 + ACPI 6.0 changes) */
struct acpi_madt_generic_interrupt { struct acpi_subtable_header header; @@ -811,6 +812,8 @@ struct acpi_madt_generic_interrupt { u32 vgic_interrupt; u64 gicr_base_address; u64 arm_mpidr; + u8 efficiency_class; + u8 reserved2[3]; };
/* Masks for Flags field above */ @@ -819,7 +822,7 @@ struct acpi_madt_generic_interrupt { #define ACPI_MADT_PERFORMANCE_IRQ_MODE (1<<1) /* 01: Performance Interrupt Mode */ #define ACPI_MADT_VGIC_IRQ_MODE (1<<2) /* 02: VGIC Maintenance Interrupt mode */
-/* 12: Generic Distributor (ACPI 5.0) */ +/* 12: Generic Distributor (ACPI 5.0 + ACPI 6.0 changes) */
struct acpi_madt_generic_distributor { struct acpi_subtable_header header; @@ -827,7 +830,8 @@ struct acpi_madt_generic_distributor { u32 gic_id; u64 base_address; u32 global_irq_base; - u32 reserved2; /* reserved - must be zero */ + u8 version; + u8 reserved2[3]; /* reserved - must be zero */ };
/* 13: Generic MSI Frame (ACPI 5.1) */ @@ -855,6 +859,16 @@ struct acpi_madt_generic_redistributor { u32 length; };
+/* 15: Generic Translator (ACPI 6.0) */ + +struct acpi_madt_generic_translator { + struct acpi_subtable_header header; + u16 reserved; /* reserved - must be zero */ + u32 translation_id; + u64 base_address; + u32 reserved2; +}; + /* * Common flags fields for MADT subtables */
ACPICA commit 4b100dc43e8baee8c8b4891b23bc7ad03eba6a28
Support for the new version field in the generic distributor subtable. Hanjun Guo hanjun.guo@linaro.org
Link: https://github.com/acpica/acpica/commit/4b100dc4 Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Bob Moore robert.moore@intel.com Signed-off-by: Lv Zheng lv.zheng@intel.com ---
This patch is also for review purpose only, already merged into ACPICA core and sent out for review for linux ACPICA by Lv Zheng. (Thank you, Bob and Lv :) )
include/acpi/actbl1.h | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h index cadf21c..a21bc50 100644 --- a/include/acpi/actbl1.h +++ b/include/acpi/actbl1.h @@ -834,6 +834,17 @@ struct acpi_madt_generic_distributor { u8 reserved2[3]; /* reserved - must be zero */ };
+/* Values for Version field above */ + +enum acpi_madt_gic_version { + ACPI_MADT_GIC_VERSION_NONE = 0, + ACPI_MADT_GIC_VERSION_V1 = 1, + ACPI_MADT_GIC_VERSION_V2 = 2, + ACPI_MADT_GIC_VERSION_V3 = 3, + ACPI_MADT_GIC_VERSION_V4 = 4, + ACPI_MADT_GIC_VERSION_RESERVED = 5 /* 5 and greater are reserved */ +}; + /* 13: Generic MSI Frame (ACPI 5.1) */
struct acpi_madt_generic_msi_frame {
There is a field added in ACPI MADT table to indicate the GIC version, so parse the table to get its value for later use.
If GIC version presented in MADT is 0, we need to fallback to hardware discovery to get the GIC version.
In ACPI MADT table there is no compatible strings to indicate various irqchips and also ACPI doesn't support irqchips which are not compatible with ARM GIC spec, so GIC version can be used to load different GIC drivers which is needed for the later patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm64/Kconfig | 1 + drivers/irqchip/Kconfig | 3 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-gic-acpi.c | 112 +++++++++++++++++++++++++++++++++++ include/linux/irqchip/arm-gic-acpi.h | 1 + 5 files changed, 118 insertions(+) create mode 100644 drivers/irqchip/irq-gic-acpi.c
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c273e4a..9d62bbb 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -16,6 +16,7 @@ config ARM64 select ARM_AMBA select ARM_ARCH_TIMER select ARM_GIC + select ARM_GIC_ACPI if ACPI select AUDIT_ARCH_COMPAT_GENERIC select ARM_GIC_V2M if PCI_MSI select ARM_GIC_V3 diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 6de62a9..0dd64c5 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -46,6 +46,9 @@ config ARM_VIC_NR The maximum number of VICs available in the system, for power management.
+config ARM_GIC_ACPI + bool + config ATMEL_AIC_IRQ bool select GENERIC_IRQ_CHIP diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index dda4927..0bd8e49 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -23,6 +23,7 @@ obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o obj-$(CONFIG_ARM_GIC_V2M) += irq-gic-v2m.o obj-$(CONFIG_ARM_GIC_V3) += irq-gic-v3.o irq-gic-common.o obj-$(CONFIG_ARM_GIC_V3_ITS) += irq-gic-v3-its.o +obj-$(CONFIG_ARM_GIC_ACPI) += irq-gic-acpi.o obj-$(CONFIG_ARM_NVIC) += irq-nvic.o obj-$(CONFIG_ARM_VIC) += irq-vic.o obj-$(CONFIG_ATMEL_AIC_IRQ) += irq-atmel-aic-common.o irq-atmel-aic.o diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c new file mode 100644 index 0000000..58f7831 --- /dev/null +++ b/drivers/irqchip/irq-gic-acpi.c @@ -0,0 +1,112 @@ +/* + * ACPI based support for ARM GIC init + * + * Copyright (C) 2015, Linaro Ltd. + * Author: Hanjun Guo hanjun.guo@linaro.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#define pr_fmt(fmt) "ACPI: GIC: " fmt + +#include <linux/acpi.h> +#include <linux/init.h> +#include <linux/irqchip/arm-gic-acpi.h> +#include <linux/irqchip/arm-gic-v3.h> + +/* GIC version presented in MADT GIC distributor structure */ +static u8 gic_version __initdata = ACPI_MADT_GIC_VERSION_NONE; + +static phys_addr_t dist_phy_base __initdata; + +static int __init +acpi_gic_parse_distributor(struct acpi_subtable_header *header, + const unsigned long end) +{ + struct acpi_madt_generic_distributor *dist; + + dist = (struct acpi_madt_generic_distributor *)header; + + if (BAD_MADT_ENTRY(dist, end)) + return -EINVAL; + + gic_version = dist->version; + dist_phy_base = dist->base_address; + return 0; +} + +static int __init +match_gic_redist(struct acpi_subtable_header *header, const unsigned long end) +{ + return 0; +} + +static bool __init acpi_gic_redist_is_present(void) +{ + int count; + + /* scan MADT table to find if we have redistributor entries */ + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR, + match_gic_redist, 0); + + /* has at least one GIC redistributor entry */ + if (count > 0) + return true; + else + return false; +} + +static int __init acpi_gic_version_init(void) +{ + int count; + void __iomem *dist_base; + u32 reg; + + count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, + acpi_gic_parse_distributor, 0); + + if (count <= 0) { + pr_err("No valid GIC distributor entry exists\n"); + return -ENODEV; + } + + if (gic_version >= ACPI_MADT_GIC_VERSION_RESERVED) { + pr_err("Invalid GIC version %d in MADT\n", gic_version); + return -EINVAL; + } + + /* + * when the GIC version is 0, we fallback to hardware discovery. + * this is also needed to keep compatiable with ACPI 5.1, + * which has no gic_version field in distributor structure and + * reserved as 0. + * + * For hardware discovery, the offset for GICv1/2 and GICv3/4 to + * get the GIC version is different (0xFE8 for GICv1/2 and 0xFFE8 + * for GICv3/4), so we need to handle it separately. + */ + if (gic_version == ACPI_MADT_GIC_VERSION_NONE) { + /* it's GICv3/v4 if redistributor is present */ + if (acpi_gic_redist_is_present()) { + dist_base = ioremap(dist_phy_base, + ACPI_GICV3_DIST_MEM_SIZE); + if (!dist_base) + return -ENOMEM; + + reg = readl_relaxed(dist_base + GICD_PIDR2) & + GIC_PIDR2_ARCH_MASK; + if (reg == GIC_PIDR2_ARCH_GICv3) + gic_version = ACPI_MADT_GIC_VERSION_V3; + else + gic_version = ACPI_MADT_GIC_VERSION_V4; + + iounmap(dist_base); + } else { + gic_version = ACPI_MADT_GIC_VERSION_V2; + } + } + + return 0; +} diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h index de3419e..13bc676 100644 --- a/include/linux/irqchip/arm-gic-acpi.h +++ b/include/linux/irqchip/arm-gic-acpi.h @@ -19,6 +19,7 @@ */ #define ACPI_GICV2_DIST_MEM_SIZE (SZ_4K) #define ACPI_GIC_CPU_IF_MEM_SIZE (SZ_8K) +#define ACPI_GICV3_DIST_MEM_SIZE (SZ_64K)
struct acpi_table_header;
On Fri, Jun 19, 2015 at 09:46:06AM +0100, Hanjun Guo wrote:
[...]
+static int __init +match_gic_redist(struct acpi_subtable_header *header, const unsigned long end) +{
- return 0;
+} +static bool __init acpi_gic_redist_is_present(void) +{
- int count;
- /* scan MADT table to find if we have redistributor entries */
- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
match_gic_redist, 0);- /* has at least one GIC redistributor entry */
- if (count > 0)
return true;- else
return false;+}
return count > 0;
What about systems where the redistributor data is in the GICC subtable ? Do you treat them as GIC V2 :) ?
On a side note, having to define an empty function like match_gic_redist is horrible.
I wonder whether it is not better to refactor map_madt_entry(), create a MADT subtable iterator out of it and make that code generic, instead of being forced to add these useless MADT handlers just to count entries, it is not the first I noticed.
+static int __init acpi_gic_version_init(void) +{
- int count;
- void __iomem *dist_base;
- u32 reg;
- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
acpi_gic_parse_distributor, 0);- if (count <= 0) {
pr_err("No valid GIC distributor entry exists\n");return -ENODEV;- }
- if (gic_version >= ACPI_MADT_GIC_VERSION_RESERVED) {
pr_err("Invalid GIC version %d in MADT\n", gic_version);return -EINVAL;- }
- /*
* when the GIC version is 0, we fallback to hardware discovery.* this is also needed to keep compatiable with ACPI 5.1,* which has no gic_version field in distributor structure and* reserved as 0.** For hardware discovery, the offset for GICv1/2 and GICv3/4 to* get the GIC version is different (0xFE8 for GICv1/2 and 0xFFE8* for GICv3/4), so we need to handle it separately.
*/- if (gic_version == ACPI_MADT_GIC_VERSION_NONE) {
/* it's GICv3/v4 if redistributor is present */if (acpi_gic_redist_is_present()) {
See above, IIUC you might have systems with GIC v3 where this call would fail, and we do not want to fall back to GIC v2.
Lorenzo
dist_base = ioremap(dist_phy_base,ACPI_GICV3_DIST_MEM_SIZE);if (!dist_base)return -ENOMEM;reg = readl_relaxed(dist_base + GICD_PIDR2) &GIC_PIDR2_ARCH_MASK;if (reg == GIC_PIDR2_ARCH_GICv3)gic_version = ACPI_MADT_GIC_VERSION_V3;elsegic_version = ACPI_MADT_GIC_VERSION_V4;iounmap(dist_base);} else {gic_version = ACPI_MADT_GIC_VERSION_V2;}
- }
- return 0;
+} diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h index de3419e..13bc676 100644 --- a/include/linux/irqchip/arm-gic-acpi.h +++ b/include/linux/irqchip/arm-gic-acpi.h @@ -19,6 +19,7 @@ */ #define ACPI_GICV2_DIST_MEM_SIZE (SZ_4K) #define ACPI_GIC_CPU_IF_MEM_SIZE (SZ_8K) +#define ACPI_GICV3_DIST_MEM_SIZE (SZ_64K) struct acpi_table_header; -- 1.9.1
On 06/23/2015 12:45 AM, Lorenzo Pieralisi wrote:
On Fri, Jun 19, 2015 at 09:46:06AM +0100, Hanjun Guo wrote:
[...]
+static int __init +match_gic_redist(struct acpi_subtable_header *header, const unsigned long end) +{
- return 0;
+} +static bool __init acpi_gic_redist_is_present(void) +{
- int count;
- /* scan MADT table to find if we have redistributor entries */
- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
match_gic_redist, 0);- /* has at least one GIC redistributor entry */
- if (count > 0)
return true;- else
return false;+}
return count > 0;
What about systems where the redistributor data is in the GICC subtable ? Do you treat them as GIC V2 :) ?
Good catch :)
It's in my TODO list and I didn't put some notes here, my bad.
If we want to support that feature, it seems we need to refactor the GICv3 code for rdist too, in gic_populate_rdist(), it use redistributor regions and jump to each cpu's redistributor using the stride, but with the redistributor data is in the GICC subtable, we only have the base address for each cpu's redistributor, have no regions.
Another way to do that we can repack all the base address for redistributors into regions then no need to refactor the code, but all the CPUs in MADT have to be listed in order which is insane.
On a side note, having to define an empty function like match_gic_redist is horrible.
I wonder whether it is not better to refactor map_madt_entry(), create a MADT subtable iterator out of it and make that code generic, instead of being forced to add these useless MADT handlers just to count entries, it is not the first I noticed.
I agree, I also think that parse GICC for each driver (SMP, GIC) is duplicate, trying to consolidate them, but I haven't figure out a good way to do that.
+static int __init acpi_gic_version_init(void) +{
- int count;
- void __iomem *dist_base;
- u32 reg;
- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
acpi_gic_parse_distributor, 0);- if (count <= 0) {
pr_err("No valid GIC distributor entry exists\n");return -ENODEV;- }
- if (gic_version >= ACPI_MADT_GIC_VERSION_RESERVED) {
pr_err("Invalid GIC version %d in MADT\n", gic_version);return -EINVAL;- }
- /*
* when the GIC version is 0, we fallback to hardware discovery.* this is also needed to keep compatiable with ACPI 5.1,* which has no gic_version field in distributor structure and* reserved as 0.** For hardware discovery, the offset for GICv1/2 and GICv3/4 to* get the GIC version is different (0xFE8 for GICv1/2 and 0xFFE8* for GICv3/4), so we need to handle it separately.
*/- if (gic_version == ACPI_MADT_GIC_VERSION_NONE) {
/* it's GICv3/v4 if redistributor is present */if (acpi_gic_redist_is_present()) {See above, IIUC you might have systems with GIC v3 where this call would fail, and we do not want to fall back to GIC v2.
Yes, but we need to find a way to refactor the GICv3 code for populating redistributors first.
Thanks Hanjun
On 06/23/2015 12:45 AM, Lorenzo Pieralisi wrote:
On Fri, Jun 19, 2015 at 09:46:06AM +0100, Hanjun Guo wrote:
[...]
+static int __init +match_gic_redist(struct acpi_subtable_header *header, const unsigned long end) +{
- return 0;
+} +static bool __init acpi_gic_redist_is_present(void) +{
- int count;
- /* scan MADT table to find if we have redistributor entries */
- count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
match_gic_redist, 0);- /* has at least one GIC redistributor entry */
- if (count > 0)
return true;- else
return false;+}
return count > 0;
What about systems where the redistributor data is in the GICC subtable ? Do you treat them as GIC V2 :) ?
On a side note, having to define an empty function like match_gic_redist is horrible.
I wonder whether it is not better to refactor map_madt_entry(), create a MADT subtable iterator out of it and make that code generic, instead of being forced to add these useless MADT handlers just to count entries, it is not the first I noticed.
After digging into the code, it seems that we need more discussion for this comment, you suggested that refactor map_madt_entry() and create a MADT subtable iterator out of it, but we still need a handler to handle each subtable entry, right? then it will become another version of acpi_parse_entries() in drivers/acpi/table.c, and no improvement to code, did I miss something?
Thanks Hanjun
This self-probe infrastructure works in the similar way as OF, but there is some different in the mechanism:
For DT, the init fn will be called once it finds compatible strings in DT, but for ACPI, we init irqchips by static tables, and in static ACPI tables, there are no compatible strings to indicate irqchips, but thanks to the GIC version presented in ACPI table, we can call the corresponding GIC drivers matching the GIC version with this framework.
This mechanism can also be used for clock declare and may also works on x86 for some table parsing too.
Part of this patch is based on Tomasz Nowicki's work.
Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/irqchip/irq-gic-acpi.c | 29 +++++++++++++++++++++++++++++ drivers/irqchip/irqchip.h | 12 ++++++++++++ include/asm-generic/vmlinux.lds.h | 13 +++++++++++++ include/linux/acpi.h | 16 ++++++++++++++++ include/linux/mod_devicetable.h | 8 ++++++++ 5 files changed, 78 insertions(+)
diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c index 58f7831..f9f1fc7 100644 --- a/drivers/irqchip/irq-gic-acpi.c +++ b/drivers/irqchip/irq-gic-acpi.c @@ -110,3 +110,32 @@ static int __init acpi_gic_version_init(void)
return 0; } + +/* + * This special acpi_table_id is the sentinel at the end of the + * acpi_table_id[] array of all irqchips. It is automatically placed at + * the end of the array by the linker, thanks to being part of a + * special section. + */ +static const struct acpi_table_id +irqchip_acpi_match_end __used __section(__irqchip_acpi_table_end); + +extern struct acpi_table_id __irqchip_acpi_table[]; + +void __init acpi_irqchip_init(void) +{ + struct acpi_table_id *id; + + if (acpi_disabled) + return; + + if (acpi_gic_version_init()) + return; + + /* scan the irqchip table to match the GIC version and its driver */ + for (id = __irqchip_acpi_table; id->id[0]; id++) { + if (gic_version == (u8)id->driver_data) + acpi_table_parse(id->id, + (acpi_tbl_table_handler)id->handler); + } +} diff --git a/drivers/irqchip/irqchip.h b/drivers/irqchip/irqchip.h index 0f6486d..241d5f8 100644 --- a/drivers/irqchip/irqchip.h +++ b/drivers/irqchip/irqchip.h @@ -11,6 +11,7 @@ #ifndef _IRQCHIP_H #define _IRQCHIP_H
+#include <linux/acpi.h> #include <linux/of.h>
/* @@ -25,4 +26,15 @@ */ #define IRQCHIP_DECLARE(name, compat, fn) OF_DECLARE_2(irqchip, name, compat, fn)
+/* + * This macro must be used by the different ARM GIC drivers to declare + * the association between their version and their initialization function. + * + * @name: name that must be unique accross all IRQCHIP_ACPI_DECLARE of the + * same file. + * @gic_version: version of GIC + * @fn: initialization function + */ +#define IRQCHIP_ACPI_DECLARE(name, gic_version, fn) \ + ACPI_DECLARE(irqchip, name, ACPI_SIG_MADT, gic_version, fn) #endif diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 8bd374d..625776c 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -181,6 +181,18 @@ #define CPUIDLE_METHOD_OF_TABLES() OF_TABLE(CONFIG_CPU_IDLE, cpuidle_method) #define EARLYCON_OF_TABLES() OF_TABLE(CONFIG_SERIAL_EARLYCON, earlycon)
+#ifdef CONFIG_ACPI +#define ACPI_TABLE(name) \ + . = ALIGN(8); \ + VMLINUX_SYMBOL(__##name##_acpi_table) = .; \ + *(__##name##_acpi_table) \ + *(__##name##_acpi_table_end) + +#define IRQCHIP_ACPI_MATCH_TABLE() ACPI_TABLE(irqchip) +#else +#define IRQCHIP_ACPI_MATCH_TABLE() +#endif + #define KERNEL_DTB() \ STRUCT_ALIGN(); \ VMLINUX_SYMBOL(__dtb_start) = .; \ @@ -516,6 +528,7 @@ CPUIDLE_METHOD_OF_TABLES() \ KERNEL_DTB() \ IRQCHIP_OF_MATCH_TABLE() \ + IRQCHIP_ACPI_MATCH_TABLE() \ EARLYCON_TABLE() \ EARLYCON_OF_TABLES()
diff --git a/include/linux/acpi.h b/include/linux/acpi.h index caead48..d383e12 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -825,4 +825,20 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev,
#endif
+#ifdef CONFIG_ACPI +#define ACPI_DECLARE(table, name, table_id, data, fn) \ + static const struct acpi_table_id __acpi_table_##name \ + __used __section(__##table##_acpi_table) \ + = { .id = table_id, \ + .handler = (void *)fn, \ + .driver_data = data } +#else +#define ACPI_DECLARE(table, name, table_id, data, fn) \ + static const struct acpi_table_id __acpi_table_##name \ + __attribute__((unused)) \ + = { .id = table_id, \ + .handler = (void *)fn, \ + .driver_data = data } +#endif + #endif /*_LINUX_ACPI_H*/ diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 3bfd567..c7fefc7 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -191,6 +191,14 @@ struct acpi_device_id { kernel_ulong_t driver_data; };
+#define ACPI_TABLE_ID_LEN 5 + +struct acpi_table_id { + __u8 id[ACPI_TABLE_ID_LEN]; + const void *handler; + kernel_ulong_t driver_data; +}; + #define PNP_ID_LEN 8 #define PNP_MAX_DEVICES 8
As the ACPI self-probe infrastructure for irqchip is ready, we use the infrastructure to simplify GICv2 init code.
acpi_irqchip_init() is renamed as acpi_irq_init() to replace the previous hardcode version of acpi_irq_init() in asm/irq.h, also cleanup the code which previously calling the GIC driver manually in arch/arm64/kernel/acpi.c.
From now on, GIC init calls reside in theirs drivers only.
This means the code becomes cleaner and it is not spread outside irqchip driver.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/arm64/include/asm/acpi.h | 1 - arch/arm64/include/asm/irq.h | 13 ------------- arch/arm64/kernel/acpi.c | 25 ------------------------- drivers/irqchip/irq-gic-acpi.c | 2 +- drivers/irqchip/irq-gic.c | 3 ++- include/linux/acpi_irq.h | 4 +++- include/linux/irqchip/arm-gic-acpi.h | 9 +-------- 7 files changed, 7 insertions(+), 50 deletions(-)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h index ed7e212..05656fc 100644 --- a/arch/arm64/include/asm/acpi.h +++ b/arch/arm64/include/asm/acpi.h @@ -12,7 +12,6 @@ #ifndef _ASM_ACPI_H #define _ASM_ACPI_H
-#include <linux/irqchip/arm-gic-acpi.h> #include <linux/mm.h> #include <linux/psci.h>
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h index bbb251b..94c5367 100644 --- a/arch/arm64/include/asm/irq.h +++ b/arch/arm64/include/asm/irq.h @@ -1,8 +1,6 @@ #ifndef __ASM_IRQ_H #define __ASM_IRQ_H
-#include <linux/irqchip/arm-gic-acpi.h> - #include <asm-generic/irq.h>
struct pt_regs; @@ -10,15 +8,4 @@ struct pt_regs; extern void migrate_irqs(void); extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
-static inline void acpi_irq_init(void) -{ - /* - * Hardcode ACPI IRQ chip initialization to GICv2 for now. - * Proper irqchip infrastructure will be implemented along with - * incoming GICv2m|GICv3|ITS bits. - */ - acpi_gic_init(); -} -#define acpi_irq_init acpi_irq_init - #endif diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 19de753..d6463bb 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -205,28 +205,3 @@ void __init acpi_boot_table_init(void) disable_acpi(); } } - -void __init acpi_gic_init(void) -{ - struct acpi_table_header *table; - acpi_status status; - acpi_size tbl_size; - int err; - - if (acpi_disabled) - return; - - status = acpi_get_table_with_size(ACPI_SIG_MADT, 0, &table, &tbl_size); - if (ACPI_FAILURE(status)) { - const char *msg = acpi_format_exception(status); - - pr_err("Failed to get MADT table, %s\n", msg); - return; - } - - err = gic_v2_acpi_init(table); - if (err) - pr_err("Failed to initialize GIC IRQ controller"); - - early_acpi_os_unmap_memory((char *)table, tbl_size); -} diff --git a/drivers/irqchip/irq-gic-acpi.c b/drivers/irqchip/irq-gic-acpi.c index f9f1fc7..236bba5 100644 --- a/drivers/irqchip/irq-gic-acpi.c +++ b/drivers/irqchip/irq-gic-acpi.c @@ -122,7 +122,7 @@ irqchip_acpi_match_end __used __section(__irqchip_acpi_table_end);
extern struct acpi_table_id __irqchip_acpi_table[];
-void __init acpi_irqchip_init(void) +void __init acpi_irq_init(void) { struct acpi_table_id *id;
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 01999d7..8fc67bc 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -1085,7 +1085,7 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, return 0; }
-int __init +static int __init gic_v2_acpi_init(struct acpi_table_header *table) { void __iomem *cpu_base, *dist_base; @@ -1141,4 +1141,5 @@ gic_v2_acpi_init(struct acpi_table_header *table) acpi_irq_model = ACPI_IRQ_MODEL_GIC; return 0; } +IRQCHIP_ACPI_DECLARE(gic_v2, ACPI_MADT_GIC_VERSION_V2, gic_v2_acpi_init); #endif diff --git a/include/linux/acpi_irq.h b/include/linux/acpi_irq.h index f10c872..4c0e108 100644 --- a/include/linux/acpi_irq.h +++ b/include/linux/acpi_irq.h @@ -3,7 +3,9 @@
#include <linux/irq.h>
-#ifndef acpi_irq_init +#ifdef CONFIG_ACPI +void acpi_irq_init(void); +#else static inline void acpi_irq_init(void) { } #endif
diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h index 13bc676..56cd82c 100644 --- a/include/linux/irqchip/arm-gic-acpi.h +++ b/include/linux/irqchip/arm-gic-acpi.h @@ -21,12 +21,5 @@ #define ACPI_GIC_CPU_IF_MEM_SIZE (SZ_8K) #define ACPI_GICV3_DIST_MEM_SIZE (SZ_64K)
-struct acpi_table_header; - -int gic_v2_acpi_init(struct acpi_table_header *table); -void acpi_gic_init(void); -#else -static inline void acpi_gic_init(void) { } -#endif - +#endif /* CONFIG_ACPI */ #endif /* ARM_GIC_ACPI_H_ */
Introduce acpi_irq_domain for GICv2 core domain instead of referring to the irq_default_domain, based on that, pass gsi as the argument and get the gsi in gic_irq_domain_alloc() to add stacked irqdomain support for ACPI based GICv2 init.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/gsi.c | 28 +++++++++++++--------------- drivers/irqchip/irq-gic.c | 32 +++++++++++++++++--------------- include/linux/irqchip/arm-gic-acpi.h | 2 ++ 3 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c index 38208f2..55b5f31 100644 --- a/drivers/acpi/gsi.c +++ b/drivers/acpi/gsi.c @@ -3,6 +3,7 @@ * * Copyright (C) 2015 ARM Ltd. * Author: Lorenzo Pieralisi lorenzo.pieralisi@arm.com + * Hanjun Guo hanjun.guo@linaro.org for stacked irqdomains support * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -13,6 +14,8 @@ #include <linux/irqdomain.h>
enum acpi_irq_model_id acpi_irq_model; +/* ACPI core domian pointing to GICv2/3 core domain */ +struct irq_domain *acpi_irq_domain __read_mostly;
static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity) { @@ -45,12 +48,7 @@ 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); + *irq = irq_find_mapping(acpi_irq_domain, gsi); /* * *irq == 0 means no mapping, that should * be reported as a failure @@ -72,16 +70,16 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) { - unsigned int irq; + int irq; unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
- /* - * 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); - if (!irq) + irq = irq_find_mapping(acpi_irq_domain, gsi); + if (irq > 0) + return irq; + + irq = irq_domain_alloc_irqs(acpi_irq_domain, 1, dev_to_node(dev), + &gsi); + if (irq <= 0) return -EINVAL;
/* Set irq type if specified and different than the current one */ @@ -98,7 +96,7 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi); */ void acpi_unregister_gsi(u32 gsi) { - int irq = irq_find_mapping(NULL, gsi); + int irq = irq_find_mapping(acpi_irq_domain, gsi);
irq_dispose_mapping(irq); } diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 8fc67bc..d1b2131 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -851,15 +851,22 @@ static struct notifier_block gic_cpu_notifier = { static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs, void *arg) { - int i, ret; + int i; irq_hw_number_t hwirq; - unsigned int type = IRQ_TYPE_NONE; - struct of_phandle_args *irq_data = arg;
- ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args, - irq_data->args_count, &hwirq, &type); - if (ret) - return ret; + if (domain->of_node) { /* DT case */ + int ret; + unsigned int type = IRQ_TYPE_NONE; + struct of_phandle_args *irq_data = arg; + + ret = gic_irq_domain_xlate(domain, irq_data->np, + irq_data->args, + irq_data->args_count, &hwirq, &type); + if (ret) + return ret; + } else { /* ACPI case */ + hwirq = (irq_hw_number_t)*(u32 *)arg; + }
for (i = 0; i < nr_irqs; i++) gic_irq_domain_map(domain, virq + i, hwirq + i); @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_irqs = 1020; gic->gic_irqs = gic_irqs;
- if (node) { /* DT case */ + if (node || !acpi_disabled) { /* DT or ACPI case */ gic->domain = irq_domain_add_linear(node, gic_irqs, &gic_irq_domain_hierarchy_ops, gic); - } else { /* Non-DT case */ + } else { /* Non-DT and ACPI case */ /* * For primary GICs, skip over SGIs. * For secondary GICs, skip over PPIs, too. @@ -1130,13 +1137,8 @@ gic_v2_acpi_init(struct acpi_table_header *table) return -ENOMEM; }
- /* - * 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()). - */ gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL); - irq_set_default_host(gic_data[0].domain); + acpi_irq_domain = gic_data[0].domain;
acpi_irq_model = ACPI_IRQ_MODEL_GIC; return 0; diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h index 56cd82c..a4a5edb 100644 --- a/include/linux/irqchip/arm-gic-acpi.h +++ b/include/linux/irqchip/arm-gic-acpi.h @@ -21,5 +21,7 @@ #define ACPI_GIC_CPU_IF_MEM_SIZE (SZ_8K) #define ACPI_GICV3_DIST_MEM_SIZE (SZ_64K)
+extern struct irq_domain *acpi_irq_domain; + #endif /* CONFIG_ACPI */ #endif /* ARM_GIC_ACPI_H_ */
On Fri, Jun 19, 2015 at 09:46:09AM +0100, Hanjun Guo wrote:
Introduce acpi_irq_domain for GICv2 core domain instead of referring to the irq_default_domain, based on that, pass gsi as the argument and get the gsi in gic_irq_domain_alloc() to add stacked irqdomain support for ACPI based GICv2 init.
This commit log does not parse and we can read the code, there is no point in trying to rewrite it here.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/gsi.c | 28 +++++++++++++--------------- drivers/irqchip/irq-gic.c | 32 +++++++++++++++++--------------- include/linux/irqchip/arm-gic-acpi.h | 2 ++ 3 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c index 38208f2..55b5f31 100644 --- a/drivers/acpi/gsi.c +++ b/drivers/acpi/gsi.c @@ -3,6 +3,7 @@
- Copyright (C) 2015 ARM Ltd.
- Author: Lorenzo Pieralisi lorenzo.pieralisi@arm.com
Hanjun Guo <hanjun.guo@linaro.org> for stacked irqdomains support- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
@@ -13,6 +14,8 @@ #include <linux/irqdomain.h> enum acpi_irq_model_id acpi_irq_model; +/* ACPI core domian pointing to GICv2/3 core domain */
This code is not GIC specific and does not have to be.
+struct irq_domain *acpi_irq_domain __read_mostly;
Make it static and create helpers to set it.
static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity) { @@ -45,12 +48,7 @@ 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);
- *irq = irq_find_mapping(acpi_irq_domain, gsi); /*
- *irq == 0 means no mapping, that should
- be reported as a failure
@@ -72,16 +70,16 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) {
- unsigned int irq;
- int irq; unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
- /*
* 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);
- if (!irq)
- irq = irq_find_mapping(acpi_irq_domain, gsi);
- if (irq > 0)
return irq;- irq = irq_domain_alloc_irqs(acpi_irq_domain, 1, dev_to_node(dev),
&gsi);- if (irq <= 0) return -EINVAL;
/* Set irq type if specified and different than the current one */ @@ -98,7 +96,7 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi); */ void acpi_unregister_gsi(u32 gsi) {
- int irq = irq_find_mapping(NULL, gsi);
- int irq = irq_find_mapping(acpi_irq_domain, gsi);
irq_dispose_mapping(irq); } diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 8fc67bc..d1b2131 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -851,15 +851,22 @@ static struct notifier_block gic_cpu_notifier = { static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs, void *arg) {
- int i, ret;
- int i; irq_hw_number_t hwirq;
- unsigned int type = IRQ_TYPE_NONE;
- struct of_phandle_args *irq_data = arg;
- ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
irq_data->args_count, &hwirq, &type);- if (ret)
return ret;
- if (domain->of_node) { /* DT case */
int ret;unsigned int type = IRQ_TYPE_NONE;struct of_phandle_args *irq_data = arg;ret = gic_irq_domain_xlate(domain, irq_data->np,irq_data->args,irq_data->args_count, &hwirq, &type);if (ret)return ret;- } else { /* ACPI case */
hwirq = (irq_hw_number_t)*(u32 *)arg;- }
If domain->of_node is NULL and system booted with DT the code above does not fail (and if it fails almost certainly that won't be graceful) but it should.
for (i = 0; i < nr_irqs; i++) gic_irq_domain_map(domain, virq + i, hwirq + i); @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_irqs = 1020; gic->gic_irqs = gic_irqs;
- if (node) { /* DT case */
- if (node || !acpi_disabled) { /* DT or ACPI case */ gic->domain = irq_domain_add_linear(node, gic_irqs, &gic_irq_domain_hierarchy_ops, gic);
- } else { /* Non-DT case */
- } else { /* Non-DT and ACPI case */ /*
- For primary GICs, skip over SGIs.
- For secondary GICs, skip over PPIs, too.
@@ -1130,13 +1137,8 @@ gic_v2_acpi_init(struct acpi_table_header *table) return -ENOMEM; }
- /*
* 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()).*/- irq_set_default_host(gic_data[0].domain);
- acpi_irq_domain = gic_data[0].domain;
acpi_irq_model = ACPI_IRQ_MODEL_GIC; return 0; diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h index 56cd82c..a4a5edb 100644 --- a/include/linux/irqchip/arm-gic-acpi.h +++ b/include/linux/irqchip/arm-gic-acpi.h @@ -21,5 +21,7 @@ #define ACPI_GIC_CPU_IF_MEM_SIZE (SZ_8K) #define ACPI_GICV3_DIST_MEM_SIZE (SZ_64K) +extern struct irq_domain *acpi_irq_domain;
This declaration does not belong here, see above.
Lorenzo
On 06/23/2015 01:20 AM, Lorenzo Pieralisi wrote:
On Fri, Jun 19, 2015 at 09:46:09AM +0100, Hanjun Guo wrote:
Introduce acpi_irq_domain for GICv2 core domain instead of referring to the irq_default_domain, based on that, pass gsi as the argument and get the gsi in gic_irq_domain_alloc() to add stacked irqdomain support for ACPI based GICv2 init.
This commit log does not parse and we can read the code, there is no point in trying to rewrite it here.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/gsi.c | 28 +++++++++++++--------------- drivers/irqchip/irq-gic.c | 32 +++++++++++++++++--------------- include/linux/irqchip/arm-gic-acpi.h | 2 ++ 3 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c index 38208f2..55b5f31 100644 --- a/drivers/acpi/gsi.c +++ b/drivers/acpi/gsi.c @@ -3,6 +3,7 @@
- Copyright (C) 2015 ARM Ltd.
- Author: Lorenzo Pieralisi lorenzo.pieralisi@arm.com
Hanjun Guo <hanjun.guo@linaro.org> for stacked irqdomains support- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
@@ -13,6 +14,8 @@ #include <linux/irqdomain.h>
enum acpi_irq_model_id acpi_irq_model; +/* ACPI core domian pointing to GICv2/3 core domain */
This code is not GIC specific and does not have to be.
+struct irq_domain *acpi_irq_domain __read_mostly;
Make it static and create helpers to set it.
static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity) { @@ -45,12 +48,7 @@ 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);
- *irq = irq_find_mapping(acpi_irq_domain, gsi); /*
- *irq == 0 means no mapping, that should
- be reported as a failure
@@ -72,16 +70,16 @@ EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity) {
- unsigned int irq;
- int irq; unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity);
- /*
* 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);
- if (!irq)
irq = irq_find_mapping(acpi_irq_domain, gsi);
if (irq > 0)
return irq;irq = irq_domain_alloc_irqs(acpi_irq_domain, 1, dev_to_node(dev),
&gsi);if (irq <= 0) return -EINVAL;
/* Set irq type if specified and different than the current one */
@@ -98,7 +96,7 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi); */ void acpi_unregister_gsi(u32 gsi) {
- int irq = irq_find_mapping(NULL, gsi);
int irq = irq_find_mapping(acpi_irq_domain, gsi);
irq_dispose_mapping(irq); }
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 8fc67bc..d1b2131 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -851,15 +851,22 @@ static struct notifier_block gic_cpu_notifier = { static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs, void *arg) {
- int i, ret;
- int i; irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
struct of_phandle_args *irq_data = arg;
ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
irq_data->args_count, &hwirq, &type);if (ret)
return ret;
- if (domain->of_node) { /* DT case */
int ret;unsigned int type = IRQ_TYPE_NONE;struct of_phandle_args *irq_data = arg;ret = gic_irq_domain_xlate(domain, irq_data->np,irq_data->args,irq_data->args_count, &hwirq, &type);if (ret)return ret;- } else { /* ACPI case */
hwirq = (irq_hw_number_t)*(u32 *)arg;- }
If domain->of_node is NULL and system booted with DT the code above does not fail (and if it fails almost certainly that won't be graceful) but it should.
how about the following logic?
if (!domain->of_node && acpi_disabled) return -ENODEV; else if (domain->of_node) dt related code; else ACPI related code;
for (i = 0; i < nr_irqs; i++) gic_irq_domain_map(domain, virq + i, hwirq + i); @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_irqs = 1020; gic->gic_irqs = gic_irqs;
- if (node) { /* DT case */
- if (node || !acpi_disabled) { /* DT or ACPI case */ gic->domain = irq_domain_add_linear(node, gic_irqs, &gic_irq_domain_hierarchy_ops, gic);
- } else { /* Non-DT case */
- } else { /* Non-DT and ACPI case */ /*
- For primary GICs, skip over SGIs.
- For secondary GICs, skip over PPIs, too.
@@ -1130,13 +1137,8 @@ gic_v2_acpi_init(struct acpi_table_header *table) return -ENOMEM; }
- /*
* 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()).*/- irq_set_default_host(gic_data[0].domain);
acpi_irq_domain = gic_data[0].domain;
acpi_irq_model = ACPI_IRQ_MODEL_GIC; return 0;
diff --git a/include/linux/irqchip/arm-gic-acpi.h b/include/linux/irqchip/arm-gic-acpi.h index 56cd82c..a4a5edb 100644 --- a/include/linux/irqchip/arm-gic-acpi.h +++ b/include/linux/irqchip/arm-gic-acpi.h @@ -21,5 +21,7 @@ #define ACPI_GIC_CPU_IF_MEM_SIZE (SZ_8K) #define ACPI_GICV3_DIST_MEM_SIZE (SZ_64K)
+extern struct irq_domain *acpi_irq_domain;
This declaration does not belong here, see above.
will update in next version.
Thanks Hanjun
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 8fc67bc..d1b2131 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -851,15 +851,22 @@ static struct notifier_block gic_cpu_notifier = { static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs, void *arg) {
- int i, ret;
- int i; irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
struct of_phandle_args *irq_data = arg;
ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
irq_data->args_count, &hwirq, &type);if (ret)
return ret;
- if (domain->of_node) { /* DT case */
int ret;unsigned int type = IRQ_TYPE_NONE;struct of_phandle_args *irq_data = arg;ret = gic_irq_domain_xlate(domain, irq_data->np,irq_data->args,irq_data->args_count, &hwirq, &type);if (ret)return ret;- } else { /* ACPI case */
hwirq = (irq_hw_number_t)*(u32 *)arg;- }
If domain->of_node is NULL and system booted with DT the code above does not fail (and if it fails almost certainly that won't be graceful) but it should.
how about the following logic?
if (!domain->of_node && acpi_disabled) return -ENODEV; else if (domain->of_node) dt related code; else ACPI related code;
Code is not checking the node at present so:
if (acpi_disabled) dt code; else ACPI code;
would do, but that's a nit.
for (i = 0; i < nr_irqs; i++) gic_irq_domain_map(domain, virq + i, hwirq + i); @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_irqs = 1020; gic->gic_irqs = gic_irqs;
- if (node) { /* DT case */
- if (node || !acpi_disabled) { /* DT or ACPI case */ gic->domain = irq_domain_add_linear(node, gic_irqs, &gic_irq_domain_hierarchy_ops, gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI (for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Lorenzo
On 06/24/2015 01:38 AM, Lorenzo Pieralisi wrote:
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 8fc67bc..d1b2131 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -851,15 +851,22 @@ static struct notifier_block gic_cpu_notifier = { static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs, void *arg) {
- int i, ret;
- int i; irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
struct of_phandle_args *irq_data = arg;
ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
irq_data->args_count, &hwirq, &type);if (ret)
return ret;
- if (domain->of_node) { /* DT case */
int ret;unsigned int type = IRQ_TYPE_NONE;struct of_phandle_args *irq_data = arg;ret = gic_irq_domain_xlate(domain, irq_data->np,irq_data->args,irq_data->args_count, &hwirq, &type);if (ret)return ret;- } else { /* ACPI case */
hwirq = (irq_hw_number_t)*(u32 *)arg;- }
If domain->of_node is NULL and system booted with DT the code above does not fail (and if it fails almost certainly that won't be graceful) but it should.
how about the following logic?
if (!domain->of_node && acpi_disabled) return -ENODEV; else if (domain->of_node) dt related code; else ACPI related code;
Code is not checking the node at present so:
if (acpi_disabled) dt code; else ACPI code;
would do, but that's a nit.
for (i = 0; i < nr_irqs; i++) gic_irq_domain_map(domain, virq + i, hwirq + i);@@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_irqs = 1020; gic->gic_irqs = gic_irqs;
- if (node) { /* DT case */
- if (node || !acpi_disabled) { /* DT or ACPI case */ gic->domain = irq_domain_add_linear(node, gic_irqs, &gic_irq_domain_hierarchy_ops, gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI
I'm little confused here, would you mind explaining more for your worrying? To me, node pointer is optional and it's ok for ACPI case.
(for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Sure.
Thanks Hanjun
On 27/06/15 04:52, Hanjun Guo wrote:
On 06/24/2015 01:38 AM, Lorenzo Pieralisi wrote:
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 8fc67bc..d1b2131 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -851,15 +851,22 @@ static struct notifier_block gic_cpu_notifier = { static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs, void *arg) {
- int i, ret;
- int i; irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
struct of_phandle_args *irq_data = arg;
ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
irq_data->args_count, &hwirq, &type);if (ret)
return ret;
- if (domain->of_node) { /* DT case */
int ret;unsigned int type = IRQ_TYPE_NONE;struct of_phandle_args *irq_data = arg;ret = gic_irq_domain_xlate(domain, irq_data->np,irq_data->args,irq_data->args_count, &hwirq, &type);if (ret)return ret;- } else { /* ACPI case */
hwirq = (irq_hw_number_t)*(u32 *)arg;- }
If domain->of_node is NULL and system booted with DT the code above does not fail (and if it fails almost certainly that won't be graceful) but it should.
how about the following logic?
if (!domain->of_node && acpi_disabled) return -ENODEV; else if (domain->of_node) dt related code; else ACPI related code;
Code is not checking the node at present so:
if (acpi_disabled) dt code; else ACPI code;
would do, but that's a nit.
for (i = 0; i < nr_irqs; i++) gic_irq_domain_map(domain, virq + i, hwirq + i);@@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_irqs = 1020; gic->gic_irqs = gic_irqs;
- if (node) { /* DT case */
- if (node || !acpi_disabled) { /* DT or ACPI case */ gic->domain = irq_domain_add_linear(node, gic_irqs, &gic_irq_domain_hierarchy_ops, gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI
I'm little confused here, would you mind explaining more for your worrying? To me, node pointer is optional and it's ok for ACPI case.
(for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Sure.
Frankly, I'd really like to see ACPI using the "node" parameter for something useful. This would save having to cache pointers all over the place, will make find_irq_host() work as expected... etc.
See the comment at the top of linux/irqdomain.h :
"... 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."
Maybe it is time to bite the bullet.
Thanks,
M.
Hi Marc,
On 06/29/2015 04:39 PM, Marc Zyngier wrote:
On 27/06/15 04:52, Hanjun Guo wrote:
On 06/24/2015 01:38 AM, Lorenzo Pieralisi wrote:
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
for (i = 0; i < nr_irqs; i++) gic_irq_domain_map(domain, virq + i, hwirq + i);@@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, gic_irqs = 1020; gic->gic_irqs = gic_irqs;
- if (node) { /* DT case */
- if (node || !acpi_disabled) { /* DT or ACPI case */ gic->domain = irq_domain_add_linear(node, gic_irqs, &gic_irq_domain_hierarchy_ops, gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI
I'm little confused here, would you mind explaining more for your worrying? To me, node pointer is optional and it's ok for ACPI case.
(for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Sure.
Frankly, I'd really like to see ACPI using the "node" parameter for something useful. This would save having to cache pointers all over the place, will make find_irq_host() work as expected... etc.
See the comment at the top of linux/irqdomain.h :
"... 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."
To init GIC in ACPI, we can only use the table entry pointer as the token, but the ACPI static tables are early mem/io remapped memory at boot stage, and it will be not available after boot, also we need muti types of MADT enties to init GIC (GICC and GICD for GICv2, GICC or GICR and GICD for GICv3), not as DT, just one single node to include all the information needed to init the GIC.
We use ACPI handle for devices as node for DT when the namespace is available, but that's pretty late in the boot stage which GIC, SMP and timers were already initialized, so ACPI handle can not use as the token too.
I see multi places just pass NULL as the pointer directly for irq_domain_add_linear() which works fine, and for ACPI, we tested this patch and also it works.
Maybe it is time to bite the bullet.
Hope comment above explains :)
Thanks Hanjun
On 30/06/15 12:50, Hanjun Guo wrote:
Hi Marc,
On 06/29/2015 04:39 PM, Marc Zyngier wrote:
On 27/06/15 04:52, Hanjun Guo wrote:
On 06/24/2015 01:38 AM, Lorenzo Pieralisi wrote:
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
> for (i = 0; i < nr_irqs; i++) > gic_irq_domain_map(domain, virq + i, hwirq + i); > @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, > gic_irqs = 1020; > gic->gic_irqs = gic_irqs; > > - if (node) { /* DT case */ > + if (node || !acpi_disabled) { /* DT or ACPI case */ > gic->domain = irq_domain_add_linear(node, gic_irqs, > &gic_irq_domain_hierarchy_ops, > gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI
I'm little confused here, would you mind explaining more for your worrying? To me, node pointer is optional and it's ok for ACPI case.
(for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Sure.
Frankly, I'd really like to see ACPI using the "node" parameter for something useful. This would save having to cache pointers all over the place, will make find_irq_host() work as expected... etc.
See the comment at the top of linux/irqdomain.h :
"... 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."
To init GIC in ACPI, we can only use the table entry pointer as the token, but the ACPI static tables are early mem/io remapped memory at boot stage, and it will be not available after boot, also we need muti types of MADT enties to init GIC (GICC and GICD for GICv2, GICC or GICR and GICD for GICv3), not as DT, just one single node to include all the information needed to init the GIC.
A single pointer would be enough, you don't need all of them.
We use ACPI handle for devices as node for DT when the namespace is available, but that's pretty late in the boot stage which GIC, SMP and timers were already initialized, so ACPI handle can not use as the token too.
I see multi places just pass NULL as the pointer directly for irq_domain_add_linear() which works fine, and for ACPI, we tested this patch and also it works.
Yes it works. But you're reinventing the wheel by keeping references outside of the normal framework, which is simply going to make the code more difficult to maintain in the long run.
Putting NULL as the device_node parameter really means "this is a domain I don't need to look up later". In your case, you will have to lookup that domain, all the time. You're just doing it in your own little corner, which is what bothers me.
M.
On Tue, Jun 30, 2015 at 01:17:14PM +0100, Marc Zyngier wrote:
On 30/06/15 12:50, Hanjun Guo wrote:
Hi Marc,
On 06/29/2015 04:39 PM, Marc Zyngier wrote:
On 27/06/15 04:52, Hanjun Guo wrote:
On 06/24/2015 01:38 AM, Lorenzo Pieralisi wrote:
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
>> for (i = 0; i < nr_irqs; i++) >> gic_irq_domain_map(domain, virq + i, hwirq + i); >> @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, >> gic_irqs = 1020; >> gic->gic_irqs = gic_irqs; >> >> - if (node) { /* DT case */ >> + if (node || !acpi_disabled) { /* DT or ACPI case */ >> gic->domain = irq_domain_add_linear(node, gic_irqs, >> &gic_irq_domain_hierarchy_ops, >> gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI
I'm little confused here, would you mind explaining more for your worrying? To me, node pointer is optional and it's ok for ACPI case.
(for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Sure.
Frankly, I'd really like to see ACPI using the "node" parameter for something useful. This would save having to cache pointers all over the place, will make find_irq_host() work as expected... etc.
See the comment at the top of linux/irqdomain.h :
"... 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."
To init GIC in ACPI, we can only use the table entry pointer as the token, but the ACPI static tables are early mem/io remapped memory at boot stage, and it will be not available after boot, also we need muti types of MADT enties to init GIC (GICC and GICD for GICv2, GICC or GICR and GICD for GICv3), not as DT, just one single node to include all the information needed to init the GIC.
A single pointer would be enough, you don't need all of them.
We use ACPI handle for devices as node for DT when the namespace is available, but that's pretty late in the boot stage which GIC, SMP and timers were already initialized, so ACPI handle can not use as the token too.
I see multi places just pass NULL as the pointer directly for irq_domain_add_linear() which works fine, and for ACPI, we tested this patch and also it works.
Yes it works. But you're reinventing the wheel by keeping references outside of the normal framework, which is simply going to make the code more difficult to maintain in the long run.
Putting NULL as the device_node parameter really means "this is a domain I don't need to look up later". In your case, you will have to lookup that domain, all the time. You're just doing it in your own little corner, which is what bothers me.
Hanjun, I think it should be possible that instead of looking up the domains in our own bit of code. We can instead use a ptr to the appropriate information as the token instead.
I don't think we have to replicate the behaviour of node, in the DT case, but just do what is sensible for ACPI in this case.
Graeme
On 06/30/2015 11:07 PM, Graeme Gregory wrote:
On Tue, Jun 30, 2015 at 01:17:14PM +0100, Marc Zyngier wrote:
On 30/06/15 12:50, Hanjun Guo wrote:
Hi Marc,
On 06/29/2015 04:39 PM, Marc Zyngier wrote:
On 27/06/15 04:52, Hanjun Guo wrote:
On 06/24/2015 01:38 AM, Lorenzo Pieralisi wrote:
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
>>> for (i = 0; i < nr_irqs; i++) >>> gic_irq_domain_map(domain, virq + i, hwirq + i); >>> @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, >>> gic_irqs = 1020; >>> gic->gic_irqs = gic_irqs; >>> >>> - if (node) { /* DT case */ >>> + if (node || !acpi_disabled) { /* DT or ACPI case */ >>> gic->domain = irq_domain_add_linear(node, gic_irqs, >>> &gic_irq_domain_hierarchy_ops, >>> gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI
I'm little confused here, would you mind explaining more for your worrying? To me, node pointer is optional and it's ok for ACPI case.
(for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Sure.
Frankly, I'd really like to see ACPI using the "node" parameter for something useful. This would save having to cache pointers all over the place, will make find_irq_host() work as expected... etc.
See the comment at the top of linux/irqdomain.h :
"... 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."
To init GIC in ACPI, we can only use the table entry pointer as the token, but the ACPI static tables are early mem/io remapped memory at boot stage, and it will be not available after boot, also we need muti types of MADT enties to init GIC (GICC and GICD for GICv2, GICC or GICR and GICD for GICv3), not as DT, just one single node to include all the information needed to init the GIC.
A single pointer would be enough, you don't need all of them.
We use ACPI handle for devices as node for DT when the namespace is available, but that's pretty late in the boot stage which GIC, SMP and timers were already initialized, so ACPI handle can not use as the token too.
I see multi places just pass NULL as the pointer directly for irq_domain_add_linear() which works fine, and for ACPI, we tested this patch and also it works.
Yes it works. But you're reinventing the wheel by keeping references outside of the normal framework, which is simply going to make the code more difficult to maintain in the long run.
Putting NULL as the device_node parameter really means "this is a domain I don't need to look up later". In your case, you will have to lookup that domain, all the time. You're just doing it in your own little corner, which is what bothers me.
Hanjun, I think it should be possible that instead of looking up the domains in our own bit of code. We can instead use a ptr to the appropriate information as the token instead.
I don't think we have to replicate the behaviour of node, in the DT case, but just do what is sensible for ACPI in this case.
I'm trying to introduce a pointer which point to the GICD when GIC is initialized, this pointer will be used to match the irqdomain. In driver code, I will find this pointer with the gsi, then find the irqdomain match the pointer, then I hope I can remove acpi_irq_domain in the end.
Marc, beyond this issue, do you have any more comments about this patch set? the framework of init GIC? and the code to init GICV3? if yes, I can address it along with this one together.
Thanks Hanjun
Hi Marc,
On 06/30/2015 08:17 PM, Marc Zyngier wrote:
On 30/06/15 12:50, Hanjun Guo wrote:
Hi Marc,
On 06/29/2015 04:39 PM, Marc Zyngier wrote:
On 27/06/15 04:52, Hanjun Guo wrote:
On 06/24/2015 01:38 AM, Lorenzo Pieralisi wrote:
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
>> for (i = 0; i < nr_irqs; i++) >> gic_irq_domain_map(domain, virq + i, hwirq + i); >> @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, >> gic_irqs = 1020; >> gic->gic_irqs = gic_irqs; >> >> - if (node) { /* DT case */ >> + if (node || !acpi_disabled) { /* DT or ACPI case */ >> gic->domain = irq_domain_add_linear(node, gic_irqs, >> &gic_irq_domain_hierarchy_ops, >> gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI
I'm little confused here, would you mind explaining more for your worrying? To me, node pointer is optional and it's ok for ACPI case.
(for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Sure.
Frankly, I'd really like to see ACPI using the "node" parameter for something useful. This would save having to cache pointers all over the place, will make find_irq_host() work as expected... etc.
See the comment at the top of linux/irqdomain.h :
"... 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."
I'm stuck here, I think the token for now is tied to device tree as the device node pointer for now in the code if the pointer is not NULL, just see the of_node_get(of_node) in __irq_domain_add(), if dynamic dt is enabled:
struct device_node *of_node_get(struct device_node *node) { if (node) kobject_get(&node->kobj); return node; } EXPORT_SYMBOL(of_node_get);
so if the node is not NULL, it will be used to get the node's kobject, that will be a problem for acpi case if we pass a token as a parameter.
I rethink for quite a while and have a discussion with Graeme, I come out another idea and we don't need the token at all for ACPI case.
To init GIC in ACPI, we can only use the table entry pointer as the token, but the ACPI static tables are early mem/io remapped memory at boot stage, and it will be not available after boot, also we need muti types of MADT enties to init GIC (GICC and GICD for GICv2, GICC or GICR and GICD for GICv3), not as DT, just one single node to include all the information needed to init the GIC.
A single pointer would be enough, you don't need all of them.
We use ACPI handle for devices as node for DT when the namespace is available, but that's pretty late in the boot stage which GIC, SMP and timers were already initialized, so ACPI handle can not use as the token too.
I see multi places just pass NULL as the pointer directly for irq_domain_add_linear() which works fine, and for ACPI, we tested this patch and also it works.
Yes it works. But you're reinventing the wheel by keeping references outside of the normal framework, which is simply going to make the code
That's the framework for DT not suitable for ACPI, just see the comments below.
more difficult to maintain in the long run.
Putting NULL as the device_node parameter really means "this is a domain I don't need to look up later". In your case, you will have to lookup
I agree that we need to look up the domain not just using the global acpi_irq_domain pointer everywhere, which make the code not scalable, but I don't think it's not OK to put NULL as the device_node parameter in ACPI case.
The reason is that we use different model of mapping interrupt with device in ACPI, for ACPI device in DSDT, it's using another mapping of device and hwirq number than DT, in DT, we using the interrupt parent property to get the device node of irqchip, that's why we need the matching function that match the device node with the one associated with the irqdomain. But for ACPI, we only can get the gsi which the device is using, no interrupt parent will be used, that because gsi is a flat hwirq number which is unique in the system, then we can get its associated irq domain by matching the gsi supported by this irqchip (see drawings below), then we can be without the token pointer matching the interrupt controller.
------------ ---> gsi_base0 | | | | irqdomain <----| irqchip 0 | | | | | |____________| ---> gsi_end0
------------ ---> gsi_base1 (probably gsi_end0+1) | | | | irqdomain <----| irqchip 1 | | | | | |____________| ---> gsi_end1
.....
so if a device is using gsi number n, we can match it with the compare of if (n <= gsi_base && gsi_end), then find its associated irq domain.
that domain, all the time. You're just doing it in your own little corner, which is what bothers me.
Not really the corner, that's because how ACPI works, I prepared a patch on top of this patch set following your suggestion to lookup the irqdomain to make the code scalable, could you please review it? I can squash it to this patch set if it's ok to you:
[RFC PATCH] ACPI / gsi: use gsi to find its irqdomain
As we use the acpi_irq_domain as the global pointer to the acpi core irq domain, which lead to multi places referring it and the way is not scalable, so introduce a struct gsi_cfg_data to abstract the gsi data for the irqchips then match the irq domain with the gsi.
For a ACPI based irqchip for example a GICD, it defines System Vector Base in the GICD entry, which means the global system interrupt number where this GIC Distributor’s interrupt inputs start, then we can get the irq number supported by reading the register for this GICD, so we can explictly get the gsi's associated irqdomain if the gsi is within the range of gsi supported by this GICD.
For ACPI device in DSDT, it using another mapping of device and hwirq number than DT, in DT, we using the interrupt parent property to get the device node of irqchip, that's why we need the matching function that match the device node with the one associated with the irqdomain. For ACPI, we only can get the gsi which the device is using, no interrupt parent will be used, that because gsi is a flat hwirq number which is unique in the system, then we can get its associated irq domain as I said above, which can be without the token pointer matching the interrupt controller.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/gsi.c | 58 ++++++++++++++++++++++++++++++++++++++------ drivers/irqchip/irq-gic-v3.c | 4 ++- drivers/irqchip/irq-gic.c | 7 +++++- include/linux/acpi.h | 2 +- 4 files changed, 61 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c index 02c8408..85dee53 100644 --- a/drivers/acpi/gsi.c +++ b/drivers/acpi/gsi.c @@ -15,11 +15,51 @@
enum acpi_irq_model_id acpi_irq_model;
-static struct irq_domain *acpi_irq_domain __read_mostly; +struct gsi_cfg_data { + struct list_head list; + u32 gsi_base; + u32 gsi_end; + struct irq_domain *domain; +};
-void set_acpi_irq_domain(struct irq_domain *domain) +static LIST_HEAD(gsi_cfg_data_list); +static DEFINE_MUTEX(gsi_mutex); + +int gsi_cfg_data_add(struct irq_domain *domain, u32 gsi_base, u32 gsi_end) +{ + struct gsi_cfg_data *data; + + data = kzalloc(sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + data->domain = domain; + data->gsi_base = gsi_base; + data->gsi_end = gsi_end; + + mutex_lock(&gsi_mutex); + list_add(&data->list, &gsi_cfg_data_list); + mutex_unlock(&gsi_mutex); + + return 0; +} + +/* Find irqdomain with GSI (hwirq number) */ +static struct irq_domain *acpi_find_irqdomain(u32 gsi) { - acpi_irq_domain = domain; + struct gsi_cfg_data *data; + struct irq_domain *domain = NULL; + + mutex_lock(&gsi_mutex); + list_for_each_entry(data, &gsi_cfg_data_list, list) { + if (gsi >= data->gsi_base && gsi <= data->gsi_end) { + domain = data->domain; + break; + } + } + mutex_unlock(&gsi_mutex); + + return domain; }
static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity) @@ -53,7 +93,9 @@ static unsigned int acpi_gsi_get_irq_type(int trigger, int polarity) */ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) { - *irq = irq_find_mapping(acpi_irq_domain, gsi); + struct irq_domain *domain = acpi_find_irqdomain(gsi); + + *irq = irq_find_mapping(domain, gsi); /* * *irq == 0 means no mapping, that should * be reported as a failure @@ -77,12 +119,13 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, { int irq; unsigned int irq_type = acpi_gsi_get_irq_type(trigger, polarity); + struct irq_domain *domain = acpi_find_irqdomain(gsi);
- irq = irq_find_mapping(acpi_irq_domain, gsi); + irq = irq_find_mapping(domain, gsi); if (irq > 0) return irq;
- irq = irq_domain_alloc_irqs(acpi_irq_domain, 1, dev_to_node(dev), + irq = irq_domain_alloc_irqs(domain, 1, dev_to_node(dev), &gsi); if (irq <= 0) return -EINVAL; @@ -101,7 +144,8 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi); */ void acpi_unregister_gsi(u32 gsi) { - int irq = irq_find_mapping(acpi_irq_domain, gsi); + struct irq_domain *domain = acpi_find_irqdomain(gsi); + int irq = irq_find_mapping(domain, gsi);
irq_dispose_mapping(irq); } diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index a11c020..21a7404 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -947,6 +947,7 @@ static LIST_HEAD(redist_list); static struct redist_region *redist_regs __initdata; static u32 nr_redist_regions __initdata; static phys_addr_t dist_phy_base __initdata; +static u32 gsi_base __initdata;
static int __init gic_acpi_register_redist(u64 phys_base, u64 size) @@ -1014,6 +1015,7 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, if (BAD_MADT_ENTRY(dist, end)) return -EINVAL;
+ gsi_base = dist->global_irq_base; dist_phy_base = dist->base_address; return 0; } @@ -1168,7 +1170,7 @@ init_base: if (err) goto out_release_redist;
- set_acpi_irq_domain(gic_data.domain); + gsi_cfg_data_add(gic_data.domain, gsi_base, gsi_base + gic_data.irq_nr); return 0;
out_release_redist: diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 7f943ef..15934fb 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -1051,6 +1051,7 @@ IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
#ifdef CONFIG_ACPI static phys_addr_t dist_phy_base, cpu_phy_base __initdata; +static u32 gsi_base __initdata;
static int __init gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, @@ -1089,6 +1090,7 @@ gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, if (BAD_MADT_ENTRY(dist, end)) return -EINVAL;
+ gsi_base = dist->global_irq_base; dist_phy_base = dist->base_address; return 0; } @@ -1139,7 +1141,10 @@ gic_v2_acpi_init(struct acpi_table_header *table) }
gic_init_bases(0, -1, dist_base, cpu_base, 0, NULL); - set_acpi_irq_domain(gic_data[0].domain); + + /* since we have only one GICD, we can safely use gic_data[0] here */ + gsi_cfg_data_add(gic_data[0].domain, gsi_base, + gsi_base + gic_data[0].gic_irqs);
acpi_irq_model = ACPI_IRQ_MODEL_GIC; return 0; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 6de4b0e..8154359 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -223,7 +223,7 @@ void acpi_unregister_gsi (u32 gsi);
#ifdef CONFIG_ACPI_GENERIC_GSI struct irq_domain; -void set_acpi_irq_domain(struct irq_domain *domain); +int gsi_cfg_data_add(struct irq_domain *domain, u32 gsi_base, u32 gsi_end); #endif
struct pci_dev;
On 08/07/15 04:40, Hanjun Guo wrote:
Hi Marc,
On 06/30/2015 08:17 PM, Marc Zyngier wrote:
On 30/06/15 12:50, Hanjun Guo wrote:
Hi Marc,
On 06/29/2015 04:39 PM, Marc Zyngier wrote:
On 27/06/15 04:52, Hanjun Guo wrote:
On 06/24/2015 01:38 AM, Lorenzo Pieralisi wrote:
On Tue, Jun 23, 2015 at 04:11:38PM +0100, Hanjun Guo wrote:
[...]
>>> for (i = 0; i < nr_irqs; i++) >>> gic_irq_domain_map(domain, virq + i, hwirq + i); >>> @@ -945,11 +952,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start, >>> gic_irqs = 1020; >>> gic->gic_irqs = gic_irqs; >>> >>> - if (node) { /* DT case */ >>> + if (node || !acpi_disabled) { /* DT or ACPI case */ >>> gic->domain = irq_domain_add_linear(node, gic_irqs, >>> &gic_irq_domain_hierarchy_ops, >>> gic);
I think this is a bit more worrying, I mean passing a NULL node pointer to the irqdomain layer which basically means you are booting out of ACPI
I'm little confused here, would you mind explaining more for your worrying? To me, node pointer is optional and it's ok for ACPI case.
(for you, if that's true for the irq_domain_add_linear implementation that's another story), the node pointer should be optional but you need feedback from IRQ layer maintainers here.
Sure.
Frankly, I'd really like to see ACPI using the "node" parameter for something useful. This would save having to cache pointers all over the place, will make find_irq_host() work as expected... etc.
See the comment at the top of linux/irqdomain.h :
"... 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."
I'm stuck here, I think the token for now is tied to device tree as the device node pointer for now in the code if the pointer is not NULL, just see the of_node_get(of_node) in __irq_domain_add(), if dynamic dt is enabled:
struct device_node *of_node_get(struct device_node *node) { if (node) kobject_get(&node->kobj); return node; } EXPORT_SYMBOL(of_node_get);
so if the node is not NULL, it will be used to get the node's kobject, that will be a problem for acpi case if we pass a token as a parameter.
Well, I'm sure this could be worked around, and we could have different handling for different type of objects.
I rethink for quite a while and have a discussion with Graeme, I come out another idea and we don't need the token at all for ACPI case.
To init GIC in ACPI, we can only use the table entry pointer as the token, but the ACPI static tables are early mem/io remapped memory at boot stage, and it will be not available after boot, also we need muti types of MADT enties to init GIC (GICC and GICD for GICv2, GICC or GICR and GICD for GICv3), not as DT, just one single node to include all the information needed to init the GIC.
A single pointer would be enough, you don't need all of them.
We use ACPI handle for devices as node for DT when the namespace is available, but that's pretty late in the boot stage which GIC, SMP and timers were already initialized, so ACPI handle can not use as the token too.
I see multi places just pass NULL as the pointer directly for irq_domain_add_linear() which works fine, and for ACPI, we tested this patch and also it works.
Yes it works. But you're reinventing the wheel by keeping references outside of the normal framework, which is simply going to make the code
That's the framework for DT not suitable for ACPI, just see the comments below.
more difficult to maintain in the long run.
Putting NULL as the device_node parameter really means "this is a domain I don't need to look up later". In your case, you will have to lookup
I agree that we need to look up the domain not just using the global acpi_irq_domain pointer everywhere, which make the code not scalable, but I don't think it's not OK to put NULL as the device_node parameter in ACPI case.
The reason is that we use different model of mapping interrupt with device in ACPI, for ACPI device in DSDT, it's using another mapping of device and hwirq number than DT, in DT, we using the interrupt parent property to get the device node of irqchip, that's why we need the matching function that match the device node with the one associated with the irqdomain. But for ACPI, we only can get the gsi which the device is using, no interrupt parent will be used, that because gsi is a flat hwirq number which is unique in the system, then we can get its associated irq domain by matching the gsi supported by this irqchip (see drawings below), then we can be without the token pointer matching the interrupt controller.
But that's for only GSI. GSI are not the *real* problem. The real issue is when you have a system that composes domains. We're moving away from a model where you can store the various domains yourself. For example, have a look at this:
https://lwn.net/Articles/650418/
and specially patch 13. See how the PCI/MSI domain looks up the core ITS domain? This is where we're going. Various *independent* entities building a hierarchy based on a common repository.
So please stop saying that ACPI is different, because it really isn't. You create domains based on some global identifier, you look them up using this identifier. There is nothing more to it. The core code has some issues with that? Let's fix it.
Look at Suravee's patches introducing ACPI support for GICv2m. He seems to be doing a rather good job reusing the existing framework, and adapting it to fit the ACPI model. It doesn't mean I agree with everything in his series (I need to review it in more details), but this is the direction I'd like to see things taken.
Thanks,
M.
From: Tomasz Nowicki tomasz.nowicki@linaro.org
Isolate hardware abstraction (FDT) code to gic_of_init(). Rest of the logic goes to gic_init_bases() and expects well defined data to initialize GIC properly. The same solution is used for GICv2 driver.
This is needed for ACPI initialization later.
Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/irqchip/irq-gic-v3.c | 105 +++++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 44 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 49875ad..87cf81b 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -765,17 +765,69 @@ static const struct irq_domain_ops gic_irq_domain_ops = { .free = gic_irq_domain_free, };
+static int __init gic_init_bases(void __iomem *dist_base, + struct redist_region *rdist_regs, + u32 nr_redist_regions, + u64 redist_stride, + struct device_node *node) +{ + u32 typer; + int gic_irqs; + int err; + + gic_data.dist_base = dist_base; + gic_data.redist_regions = rdist_regs; + gic_data.nr_redist_regions = nr_redist_regions; + gic_data.redist_stride = redist_stride; + + /* + * Find out how many interrupts are supported. + * The GIC only supports up to 1020 interrupt sources (SGI+PPI+SPI) + */ + typer = readl_relaxed(gic_data.dist_base + GICD_TYPER); + gic_data.rdists.id_bits = GICD_TYPER_ID_BITS(typer); + gic_irqs = GICD_TYPER_IRQS(typer); + if (gic_irqs > 1020) + gic_irqs = 1020; + gic_data.irq_nr = gic_irqs; + + gic_data.domain = irq_domain_add_tree(node, &gic_irq_domain_ops, + &gic_data); + gic_data.rdists.rdist = alloc_percpu(typeof(*gic_data.rdists.rdist)); + + if (WARN_ON(!gic_data.domain) || WARN_ON(!gic_data.rdists.rdist)) { + err = -ENOMEM; + goto out_free; + } + + set_handle_irq(gic_handle_irq); + + if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis()) + its_init(node, &gic_data.rdists, gic_data.domain); + + gic_smp_init(); + gic_dist_init(); + gic_cpu_init(); + gic_cpu_pm_init(); + + return 0; + +out_free: + if (gic_data.domain) + irq_domain_remove(gic_data.domain); + free_percpu(gic_data.rdists.rdist); + return err; +} + +#ifdef CONFIG_OF static int __init gic_of_init(struct device_node *node, struct device_node *parent) { void __iomem *dist_base; struct redist_region *rdist_regs; u64 redist_stride; u32 nr_redist_regions; - u32 typer; u32 reg; - int gic_irqs; - int err; - int i; + int err, i;
dist_base = of_iomap(node, 0); if (!dist_base) { @@ -819,47 +871,11 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare if (of_property_read_u64(node, "redistributor-stride", &redist_stride)) redist_stride = 0;
- gic_data.dist_base = dist_base; - gic_data.redist_regions = rdist_regs; - gic_data.nr_redist_regions = nr_redist_regions; - gic_data.redist_stride = redist_stride; - - /* - * Find out how many interrupts are supported. - * The GIC only supports up to 1020 interrupt sources (SGI+PPI+SPI) - */ - typer = readl_relaxed(gic_data.dist_base + GICD_TYPER); - gic_data.rdists.id_bits = GICD_TYPER_ID_BITS(typer); - gic_irqs = GICD_TYPER_IRQS(typer); - if (gic_irqs > 1020) - gic_irqs = 1020; - gic_data.irq_nr = gic_irqs; + err = gic_init_bases(dist_base, rdist_regs, nr_redist_regions, + redist_stride, node); + if (!err) + return 0;
- gic_data.domain = irq_domain_add_tree(node, &gic_irq_domain_ops, - &gic_data); - gic_data.rdists.rdist = alloc_percpu(typeof(*gic_data.rdists.rdist)); - - if (WARN_ON(!gic_data.domain) || WARN_ON(!gic_data.rdists.rdist)) { - err = -ENOMEM; - goto out_free; - } - - set_handle_irq(gic_handle_irq); - - if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis()) - its_init(node, &gic_data.rdists, gic_data.domain); - - gic_smp_init(); - gic_dist_init(); - gic_cpu_init(); - gic_cpu_pm_init(); - - return 0; - -out_free: - if (gic_data.domain) - irq_domain_remove(gic_data.domain); - free_percpu(gic_data.rdists.rdist); out_unmap_rdist: for (i = 0; i < nr_redist_regions; i++) if (rdist_regs[i].redist_base) @@ -871,3 +887,4 @@ out_unmap_dist: }
IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init); +#endif
From: Tomasz Nowicki tomasz.nowicki@linaro.org
With the refator of gic_of_init(), GICv3/4 can be initialized by gic_init_bases() with gic distributor base address and gic redistributor region(s).
So get the redistributor region base addresses from MADT GIC redistributor subtable, and the distributor base address from GICD subtable to init GICv3 irqchip in ACPI way.
Note: GIC redistributor base address may also be provided in GICC structures on systems supporting GICv3 and above if the GIC Redistributors are not in the always-on power domain, this patch didn't implement such feature yet.
Signed-off-by: Tomasz Nowicki tomasz.nowicki@linaro.org [hj: Rework this patch and fix multi issues] Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/irqchip/irq-gic-v3.c | 143 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 139 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 87cf81b..d2eef3c 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -15,6 +15,7 @@ * along with this program. If not, see http://www.gnu.org/licenses/. */
+#include <linux/acpi.h> #include <linux/cpu.h> #include <linux/cpu_pm.h> #include <linux/delay.h> @@ -25,6 +26,7 @@ #include <linux/percpu.h> #include <linux/slab.h>
+#include <linux/irqchip/arm-gic-acpi.h> #include <linux/irqchip/arm-gic-v3.h>
#include <asm/cputype.h> @@ -819,6 +821,16 @@ out_free: return err; }
+static int __init detect_distributor(void __iomem *dist_base) +{ + u32 reg = readl_relaxed(dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK; + + if (reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4) + return -ENODEV; + + return 0; +} + #ifdef CONFIG_OF static int __init gic_of_init(struct device_node *node, struct device_node *parent) { @@ -826,7 +838,6 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare struct redist_region *rdist_regs; u64 redist_stride; u32 nr_redist_regions; - u32 reg; int err, i;
dist_base = of_iomap(node, 0); @@ -836,11 +847,10 @@ static int __init gic_of_init(struct device_node *node, struct device_node *pare return -ENXIO; }
- reg = readl_relaxed(dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK; - if (reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4) { + err = detect_distributor(dist_base); + if (err) { pr_err("%s: no distributor detected, giving up\n", node->full_name); - err = -ENODEV; goto out_unmap_dist; }
@@ -888,3 +898,128 @@ out_unmap_dist:
IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init); #endif + +#ifdef CONFIG_ACPI +static struct redist_region *redist_regs __initdata; +static u32 nr_redist_regions __initdata; +static phys_addr_t dist_phy_base __initdata; + +static int __init +gic_acpi_register_redist(u64 phys_base, u64 size) +{ + struct redist_region *redist_regs_new; + void __iomem *redist_base; + + redist_regs_new = krealloc(redist_regs, + sizeof(*redist_regs) * (nr_redist_regions + 1), + GFP_KERNEL); + if (!redist_regs_new) { + pr_err("Couldn't allocate resource for GICR region\n"); + return -ENOMEM; + } + + redist_regs = redist_regs_new; + + redist_base = ioremap(phys_base, size); + if (!redist_base) { + pr_err("Couldn't map GICR region @%llx\n", phys_base); + return -ENOMEM; + } + + redist_regs[nr_redist_regions].phys_base = phys_base; + redist_regs[nr_redist_regions].redist_base = redist_base; + nr_redist_regions++; + return 0; +} + +static int __init +gic_acpi_parse_madt_redist(struct acpi_subtable_header *header, + const unsigned long end) +{ + struct acpi_madt_generic_redistributor *redist; + + if (BAD_MADT_ENTRY(header, end)) + return -EINVAL; + + redist = (struct acpi_madt_generic_redistributor *)header; + if (!redist->base_address) + return -EINVAL; + + return gic_acpi_register_redist(redist->base_address, redist->length); +} + +static int __init +gic_acpi_parse_madt_distributor(struct acpi_subtable_header *header, + const unsigned long end) +{ + struct acpi_madt_generic_distributor *dist; + + dist = (struct acpi_madt_generic_distributor *)header; + + if (BAD_MADT_ENTRY(dist, end)) + return -EINVAL; + + dist_phy_base = dist->base_address; + return 0; +} + +static int __init +gic_v3_acpi_init(struct acpi_table_header *table) +{ + int count, i, err = 0; + void __iomem *dist_base; + + /* Get distributor base address */ + count = acpi_parse_entries(ACPI_SIG_MADT, + sizeof(struct acpi_table_madt), + gic_acpi_parse_madt_distributor, table, + ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0); + if (count <= 0) { + pr_err("No valid GICD entry exist\n"); + return -EINVAL; + } else if (count > 1) { + pr_err("More than one GICD entry detected\n"); + return -EINVAL; + } + + dist_base = ioremap(dist_phy_base, ACPI_GICV3_DIST_MEM_SIZE); + if (!dist_base) { + pr_err("Unable to map GICD registers\n"); + return -ENOMEM; + } + + err = detect_distributor(dist_base); + if (err) { + pr_err("No distributor detected at @%p, giving up", dist_base); + goto out_dist_unmap; + } + + /* Collect redistributor base addresses */ + count = acpi_parse_entries(ACPI_SIG_MADT, + sizeof(struct acpi_table_madt), + gic_acpi_parse_madt_redist, table, + ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR, 0); + if (count <= 0) { + pr_info("No valid GICR entries exist\n"); + err = -EINVAL; + goto out_redist_unmap; + } + + err = gic_init_bases(dist_base, redist_regs, nr_redist_regions, 0, NULL); + if (err) + goto out_redist_unmap; + + irq_set_default_host(gic_data.domain); + return 0; + +out_redist_unmap: + for (i = 0; i < nr_redist_regions; i++) + if (redist_regs[i].redist_base) + iounmap(redist_regs[i].redist_base); + kfree(redist_regs); +out_dist_unmap: + iounmap(dist_base); + return err; +} +IRQCHIP_ACPI_DECLARE(gic_v3, ACPI_MADT_GIC_VERSION_V3, gic_v3_acpi_init); +#endif
Similar as stacked domain support for ACPI based GICv2 init, let acpi_irq_domain point to the core domain of GICv3 and pass the gsi as the arg to support stacked irqdomain.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/irqchip/irq-gic-v3.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index d2eef3c..a1c4c74 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -733,15 +733,21 @@ static int gic_irq_domain_xlate(struct irq_domain *d, static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs, void *arg) { - int i, ret; + int i; irq_hw_number_t hwirq; - unsigned int type = IRQ_TYPE_NONE; - struct of_phandle_args *irq_data = arg;
- ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args, - irq_data->args_count, &hwirq, &type); - if (ret) - return ret; + if (domain->of_node) { /* DT case */ + int ret; + unsigned int type = IRQ_TYPE_NONE; + struct of_phandle_args *irq_data = arg; + + ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args, + irq_data->args_count, &hwirq, &type); + if (ret) + return ret; + } else { /* ACPI case */ + hwirq = (irq_hw_number_t)*(u32 *)arg; + }
for (i = 0; i < nr_irqs; i++) gic_irq_domain_map(domain, virq + i, hwirq + i); @@ -1009,7 +1015,7 @@ gic_v3_acpi_init(struct acpi_table_header *table) if (err) goto out_redist_unmap;
- irq_set_default_host(gic_data.domain); + acpi_irq_domain = gic_data.domain; return 0;
out_redist_unmap:
-
Graeme Gregory -
Hanjun Guo -
Lorenzo Pieralisi -
Marc Zyngier