Re: [PATCH v4 08/10] ACPI: GIC: Add ACPI helper functions to query irq-domain tokens for for GIC MSI and ITS

From: Suravee Suthikulpanit
Date: Sun Aug 09 2015 - 04:03:07 EST

Hi Marc,

Sorry for late reply. Please see my comments below.

On 8/4/15 21:02, Marc Zyngier wrote:
On 29/07/15 11:08, Hanjun Guo wrote:
From: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx>

This patch introduces acpi_gic_get_msi_token(), which returns irq-domain
token that can be used to look up MSI doamin of a device.
In both GIC MSI and ITS cases, the base_address specified in the GIC MSI
or GIC ITS structure is used as a token for MSI domain.

In addition, this patch also provides low-level helper functions to parse
and query GIC MSI structure and GIC ITS from MADT. Once parsed, it keeps
a copy of the structure for use in subsequent queries to avoid having
to map and parse MADT multiple times.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx>
Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx>
drivers/acpi/Makefile | 1 +
drivers/acpi/acpi_gic.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++++
include/acpi/acpi_gic.h | 23 +++++
include/linux/acpi.h | 1 +
4 files changed, 259 insertions(+)
create mode 100644 drivers/acpi/acpi_gic.c
create mode 100644 include/acpi/acpi_gic.h

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 8321430..def54b9 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -54,6 +54,7 @@ acpi-$(CONFIG_ACPI_NUMA) += numa.o
acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
acpi-y += acpi_lpat.o
acpi-$(CONFIG_ACPI_GENERIC_GSI) += gsi.o
+acpi-$(CONFIG_ARM_GIC_ACPI) += acpi_gic.o

# These are (potentially) separate modules

diff --git a/drivers/acpi/acpi_gic.c b/drivers/acpi/acpi_gic.c
new file mode 100644
index 0000000..11ee4eb
--- /dev/null
+++ b/drivers/acpi/acpi_gic.c

I think this is starting badly. If this is GIC specific, it lives in
drivers/irqchip. Nothing in drivers/acpi should be interrupt-controller
specific at all. If there are things you need to expose through the ACPI
layer, add some indirections.

OK, originally, I intended for this to be an intermediate layer b/w ACPI and irqchip, but I guess that's still not what you are looking for. I'll rework this again.

+#endif /*__ACPI_GIC_H__*/
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 04dd0bb..5d58b61 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -44,6 +44,7 @@

#include <acpi/acpi_bus.h>
#include <acpi/acpi_drivers.h>
+#include <acpi/acpi_gic.h>

Ah! No.

#include <acpi/acpi_numa.h>
#include <acpi/acpi_io.h>
#include <asm/acpi.h>

Right. Very little of what is above belongs to the ACPI layer. What
would belong here is a generic acpi_get_msi_domain_token(dev) that would
call into a controller-specific function to parse the various tables and
find out which GICv2m frame (or which ITS) is serving the given device.
This would include parsing of the IORT structures if they are available.

The problem here is we would need to figure out the hook into *a controller-specific function* from a generic layer (ACPI in this case).

For GICv2m, it should be simplistic: just return the domain_token of the
v2m widget. For the ITS, it is slightly more complex, and there should
be some specific backend for that. There is no ACPI support for the ITS
yet, so that shouldn't be your concern at this point in time.

Overall, drivers/acpi should be hardware agnostic (or at least aim for
it), just like drivers/of is.

I think I understand how you don't like the current approach. Lemme try a different approach and send out another revision.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at