Messages in this thread | | | Date | Mon, 27 Jun 2022 08:34:14 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver |
| |
On Mon, 20 Jun 2022 04:12:10 +0100, Jianmin Lv <lvjianmin@loongson.cn> wrote: > > > > On 2022/6/19 上午1:22, Marc Zyngier wrote: > > On Wed, 15 Jun 2022 07:07:24 +0100, > > Jianmin Lv <lvjianmin@loongson.cn> 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@loongson.cn> > >> --- > >> 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.
M.
-- Without deviation from the norm, progress is not possible.
| |