Re: [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver

From: Jianmin Lv
Date: Mon Jun 27 2022 - 04:05:10 EST




On 2022/6/27 下午3:34, Marc Zyngier wrote:
On Mon, 20 Jun 2022 04:12:10 +0100,
Jianmin Lv <lvjianmin@xxxxxxxxxxx> wrote:



On 2022/6/19 上午1:22, Marc Zyngier wrote:
On Wed, 15 Jun 2022 07:07:24 +0100,
Jianmin Lv <lvjianmin@xxxxxxxxxxx> wrote:

The library file contains following content:
- Implement acpi_get_gsi_domain_id callback.
- Implement initialization of vector group entries and APIs
for building hierachy irqdomains.

Signed-off-by: Jianmin Lv <lvjianmin@xxxxxxxxxxx>
---
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-loongarch-pic-common.c | 122 +++++++++++++++++++++++++++++
drivers/irqchip/irq-loongarch-pic-common.h | 39 +++++++++
3 files changed, 162 insertions(+), 1 deletion(-)
create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c
create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 6894a13..2d0d871 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -103,7 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
-obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
+obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-pic-common.o
obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o
obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
new file mode 100644
index 0000000..2f75362
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-pic-common.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
+ */
+
+#include <linux/irq.h>
+#include <linux/pci.h>
+#include <linux/acpi.h>
+#include "irq-loongarch-pic-common.h"
+
+static struct acpi_vector_group vector_group[MAX_IO_PICS];
+
+struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
+
+struct fwnode_handle *liointc_handle;
+struct fwnode_handle *pch_lpc_handle;
+struct fwnode_handle *pch_msi_handle[MAX_IO_PICS];
+struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];

Why aren't these in individual drivers, and then have accessors to
retrieve them?


Ok, I'll try to put them in individual drivers.


+
+static int find_pch_pic(u32 gsi)
+{
+ int i, start, end;
+
+ /* Find the PCH_PIC that manages this GSI. */
+ for (i = 0; i < MAX_IO_PICS; i++) {
+ struct acpi_madt_bio_pic *irq_cfg = acpi_pchpic[i];
+
+ if (!irq_cfg)
+ return -1;
+
+ start = irq_cfg->gsi_base;
+ end = irq_cfg->gsi_base + irq_cfg->size;
+ if (gsi >= start && gsi < end)
+ return i;
+ }
+
+ pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
+ return -1;
+}

Same thing. This really should be in the PCH driver, and be called by
lpic_get_gsi_domain().


Ok, I'll try to put them in PCH driver.


+
+struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi)
+{
+ int id;
+ struct fwnode_handle *domain_handle = NULL;
+
+ switch (gsi) {
+ case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
+ if (liointc_handle)
+ domain_handle = liointc_handle;
+ break;
+
+ case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
+ if (pch_lpc_handle)
+ domain_handle = pch_lpc_handle;
+ break;
+
+ case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
+ id = find_pch_pic(gsi);
+ if (id >= 0 && pch_pic_handle[id])
+ domain_handle = pch_pic_handle[id];
+
+ break;
+ }
+
+ return domain_handle;
+}
+
+static int pci_mcfg_parse(struct acpi_table_header *header)
+{
+ struct acpi_table_mcfg *mcfg;
+ struct acpi_mcfg_allocation *mptr;
+ int i, n;
+
+ if (header->length < sizeof(struct acpi_table_mcfg))
+ return -EINVAL;
+
+ n = (header->length - sizeof(struct acpi_table_mcfg)) /
+ sizeof(struct acpi_mcfg_allocation);
+ mcfg = (struct acpi_table_mcfg *)header;
+ mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
+
+ for (i = 0; i < n; i++, mptr++)
+ vector_group[mptr->pci_segment].node = (mptr->address >> 44) & 0xf;
+
+ return 0;
+}
+
+void __init init_vector_parent_group(void)
+{
+ acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
+}

I really don't think the PCI code should be anywhere near
this. Frankly, this file looks like a dumping ground for totally
unrelated stuff.



For ARM, the msi domain of a pci device is matched by comparing PCI
segment of it and PCI segment of msi domain's parent domain(its
domain).
The PCI segment number here is as in MCFG and as returned by _SEG in
the namespace.

For LoongArch, a similar way is used, but we don't use
IORT-like table, rather, we directly used PCI segment and Base
address(node information is in bit44-47 of the address) in MCFG for
it, so we need to get them by early parsing MCFG before creating MSI
and PCH irqdomain(MSI and PCH irqdomain have the same node as their
parent
irqdomain).

If the related code is not suitable to be here, Maybe I should put
them in arch/loongarch/kernel/irq.c and call init_vector_parent_group
before irqchip_init().

That'd probably be better. This really is arch-specific stuff, and not
interrupt-controller related.


Ok, I'll do that in next version.

M.