* [PATCH RFC V2 00/10] irqchip: Add LoongArch-related irqchip drivers
@ 2022-05-27 11:02 Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 01/10] irqchip: Adjust Kconfig for Loongson Jianmin Lv
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Jianmin Lv @ 2022-05-27 11:02 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen,
Jianmin Lv
LoongArch is a new RISC ISA, which is a bit like MIPS or RISC-V.
LoongArch includes a reduced 32-bit version (LA32R), a standard 32-bit
version (LA32S) and a 64-bit version (LA64). LoongArch use ACPI as its
boot protocol LoongArch-specific interrupt controllers (similar to APIC)
are already added in the ACPI Specification 6.5(which may be published in
early June this year and the board is reviewing the draft).
Currently, LoongArch based processors (e.g. Loongson-3A5000) can only
work together with LS7A chipsets. The irq chips in LoongArch computers
include CPUINTC (CPU Core Interrupt Controller), LIOINTC (Legacy I/O
Interrupt Controller), EIOINTC (Extended I/O Interrupt Controller),
HTVECINTC (Hyper-Transport Vector Interrupt Controller), PCH-PIC (Main
Interrupt Controller in LS7A chipset), PCH-LPC (LPC Interrupt Controller
in LS7A chipset) and PCH-MSI (MSI Interrupt Controller).
CPUINTC is a per-core controller (in CPU), LIOINTC/EIOINTC/HTVECINTC are
per-package controllers (in CPU), while PCH-PIC/PCH-LPC/PCH-MSI are all
controllers out of CPU (i.e., in chipsets). These controllers (in other
words, irqchips) are linked in a hierarchy, and there are two models of
hierarchy (legacy model and extended model).
Legacy IRQ model:
In this model, the IPI (Inter-Processor Interrupt) and CPU Local Timer
interrupt go to CPUINTC directly, CPU UARTS interrupts go to LIOINTC,
while all other devices interrupts go to PCH-PIC/PCH-LPC/PCH-MSI and
gathered by HTVECINTC, and then go to LIOINTC, and then CPUINTC.
+---------------------------------------------+
| |
| +-----+ +---------+ +-------+ |
| | IPI | --> | CPUINTC | <-- | Timer | |
| +-----+ +---------+ +-------+ |
| ^ |
| | |
| +---------+ +-------+ |
| | LIOINTC | <-- | UARTs | |
| +---------+ +-------+ |
| ^ |
| | |
| +-----------+ |
| | HTVECINTC | |
| +-----------+ |
| ^ ^ |
| | | |
| +---------+ +---------+ |
| | PCH-PIC | | PCH-MSI | |
| +---------+ +---------+ |
| ^ ^ ^ |
| | | | |
| +---------+ +---------+ +---------+ |
| | PCH-LPC | | Devices | | Devices | |
| +---------+ +---------+ +---------+ |
| ^ |
| | |
| +---------+ |
| | Devices | |
| +---------+ |
| |
| |
+---------------------------------------------+
Extended IRQ model:
In this model, the IPI (Inter-Processor Interrupt) and CPU Local Timer
interrupt go to CPUINTC directly, CPU UARTS interrupts go to LIOINTC,
while all other devices interrupts go to PCH-PIC/PCH-LPC/PCH-MSI and
gathered by EIOINTC, and then go to to CPUINTC directly.
+--------------------------------------------------------+
| |
| +-----+ +---------+ +-------+ |
| | IPI | --> | CPUINTC | <-- | Timer | |
| +-----+ +---------+ +-------+ |
| ^ ^ |
| | | |
| +---------+ +---------+ +-------+ |
| | EIOINTC | | LIOINTC | <-- | UARTs | |
| +---------+ +---------+ +-------+ |
| ^ ^ |
| | | |
| +---------+ +---------+ |
| | PCH-PIC | | PCH-MSI | |
| +---------+ +---------+ |
| ^ ^ ^ |
| | | | |
| +---------+ +---------+ +---------+ |
| | PCH-LPC | | Devices | | Devices | |
| +---------+ +---------+ +---------+ |
| ^ |
| | |
| +---------+ |
| | Devices | |
| +---------+ |
| |
| |
+--------------------------------------------------------+
The hierarchy model is constructed by parsing irq contronler structures
in MADT. Some controllers((e.g. LIOINTC, HTVECINTC, EIOINTC and PCH-LPC)
are hardcodingly connected to their parents, so their irqdomins are
separately routed to their parents in a fixed way. Some controllers
(e.g. PCH-PIC and PCH-MSI) could be routed to different parents for different
CPU. The firmware will config EIOINTC for the newer CPU and config HTVECINTC
for old CPU in MADT. By this way, PCH-PIC and PCH-MSI irqdomain can only be
routed one parent irqdomin: HTVECINTC or EIOINTC.
Example of irqchip topology in a system with two chipsets:
+------------------------------------------------------------+
| |
| +------------------+ |
| | CPUINTC | |
| +------------------+ |
| ^ ^ |
| | | |
| +----------+ +----------+ |
| | EIOINTC 0| | EIOINTC 1| |
| +----------+ +----------+ |
| ^ ^ ^ ^ |
| | | | | |
| +----------+ +----------+ +----------+ +----------+ |
| | PCH-PIC 0| | PCH-MSI 0| | PCH-PIC 1| | PCH-MSI 1| |
| +----------+ +----------+ +----------+ +----------+ |
| |
| |
+------------------------------------------------------------+
For systems with two chipsets, there are tow group(consists of EIOINTC, PCH-PIC and PCH-MSI) irqdomains,
and each group has same node id. So we defined a structure to mantain the relation of node and it's parent irqdomain.
struct acpi_vector_group {
int node;
struct irq_domain *parent;
};
The initialization and use of acpi_vector_group array are following:
1 Entry of struct acpi_vector_group array initialization:
By parsing MCFG, the node id（from bit44-47 of Base Address）of each pci segment is extracted. And from MADT, we have the node id of each EIOINTC.
entrys[pci segment].node = node id of pci segment
entrys[pci segment].parent = EIOINTC irqdomain(node id of EIOINTC == node id of pci segment)
2 Get parent irqdomain for PCH-PIC:
From MADT, we have the node id of each PCH-PIC(from bit44-47 of Base Address).
if (node of entry i == node of PCH-PIC)
return entrys[i].parent;
entrys[pci segment].node = node id of pci segment
entrys[pci segment].parent = EIOINTC irqdomain(node id of EIOINTC == node id of pci segment)
3 Get parent irqdomain for PCH-MSI of pci segment:
return entrys[pci segment].parent;
4 How to select a correct irqdomain to map irq for a device?
For devices using legacy irq behind PCH-PIC, GSI is used to select correct PCH-PIC irqdomain.
For devices using msi irq behind PCH-MSI, the pci segmen of the device is used to select correct PCH-MSI irqdomain.
Cross-compile tool chain to build kernel:
https://github.com/loongson/build-tools/releases/latest/download/loongarch64-clfs-20211202-cross-tools.tar.xz
A CLFS-based Linux distro:
https://github.com/loongson/build-tools/releases/latest/download/loongarch64-clfs-system-2021-12-02.tar.bz2
Loongson and LoongArch documentations:
https://github.com/loongson/LoongArch-Documentation
V1 -> V2:
1, Remove queued patches;
2, Move common logic of DT/ACPI probing to common functions;
3, Split .suspend()/.resume() functions to separate patches.
V2 -> V3:
1, Fix a bug for loongson-pch-pic probe;
2, Some minor improvements for LPC controller.
V3 -> V4:
1, Rework the CPU interrupt controller driver;
2, Some minor improvements for other controllers.
V4 -> V5:
1, Add a description of LoonArch's IRQ model;
2, Support multiple EIOINTCs in one system;
3, Some minor improvements for other controllers.
V5 -> V6:
1, Attach a fwnode to CPUINTC irq domain;
2, Use raw spinlock instead of generic spinlock;
3, Improve the method of restoring EIOINTC state;
4, Update documentation, comments and commit messages.
V6 -> V7:
1, Fix build warnings reported by kernel test robot.
V7 -> V8:
1, Add arguments sanity checking for irqchip init functions;
2, Support Loongson-3C5000 (One NUMA Node includes 4 EIOINTC Node).
V8 -> V9:
1, Rebase on 5.17-rc5;
2, Update cover letter;
3, Some small improvements.
V9 -> V10:
1, Rebase on 5.17-rc6;
2, Fix build warnings reported by kernel test robot.
V10 -> V11:
1, Rebase on 5.18-rc4;
2, Fix irq affinity setting for EIOINTC;
3, Fix hwirq allocation failure for EIOINTC.
V11 -> RFC:
1, Refactored the way to build irqchip hierarchy topology.
RFC -> RFC V2:
1, Move all IO-interrupt related code to driver/irqchip from arch directory.
2. Add description for an example of two chipsets system.
Huacai Chen (1):
irqchip: Adjust Kconfig for Loongson
Jianmin Lv (9):
irqchip: Add LoongArch CPU interrupt controller support
irqchip/loongson-pch-pic: Add ACPI init support
irqchip/loongson-pch-pic: Add suspend/resume support
irqchip/loongson-pch-msi: Add ACPI init support
irqchip/loongson-htvec: Add ACPI init support
irqchip/loongson-htvec: Add suspend/resume support
irqchip/loongson-liointc: Add ACPI init support
irqchip: Add Loongson Extended I/O interrupt controller support
irqchip: Add Loongson PCH LPC controller support
drivers/irqchip/Kconfig | 38 ++-
drivers/irqchip/Makefile | 3 +
drivers/irqchip/irq-loongarch-cpu.c | 115 +++++++++
drivers/irqchip/irq-loongarch-pic-common.c | 201 +++++++++++++++
drivers/irqchip/irq-loongarch-pic-common.h | 44 ++++
drivers/irqchip/irq-loongson-eiointc.c | 385 +++++++++++++++++++++++++++++
drivers/irqchip/irq-loongson-htvec.c | 148 ++++++++---
drivers/irqchip/irq-loongson-liointc.c | 216 ++++++++++------
drivers/irqchip/irq-loongson-pch-lpc.c | 220 +++++++++++++++++
drivers/irqchip/irq-loongson-pch-msi.c | 138 +++++++----
drivers/irqchip/irq-loongson-pch-pic.c | 197 +++++++++++++--
include/linux/cpuhotplug.h | 1 +
12 files changed, 1519 insertions(+), 187 deletions(-)
create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c
create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h
create mode 100644 drivers/irqchip/irq-loongson-eiointc.c
create mode 100644 drivers/irqchip/irq-loongson-pch-lpc.c
--
1.8.3.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC V2 01/10] irqchip: Adjust Kconfig for Loongson
2022-05-27 11:02 [PATCH RFC V2 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
@ 2022-05-27 11:02 ` Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support Jianmin Lv
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Jianmin Lv @ 2022-05-27 11:02 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen,
Jianmin Lv
From: Huacai Chen <chenhuacai@loongson.cn>
We are preparing to add new Loongson (based on LoongArch, not compatible
with old MIPS-based Loongson) support. HTVEC will be shared by both old
and new Loongson processors, so we adjust its description. HTPIC is only
used by MIPS-based Loongson, so we add a MIPS dependency. PCH_PIC and
PCH_MSI will have some arch-specific code, so we remove the COMPILE_TEST
dependency to avoid build warnings.
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/irqchip/Kconfig | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 15edb9a..39d6be2 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -556,7 +556,7 @@ config LOONGSON_LIOINTC
config LOONGSON_HTPIC
bool "Loongson3 HyperTransport PIC Controller"
- depends on MACH_LOONGSON64
+ depends on (MACH_LOONGSON64 && MIPS)
default y
select IRQ_DOMAIN
select GENERIC_IRQ_CHIP
@@ -564,16 +564,16 @@ config LOONGSON_HTPIC
Support for the Loongson-3 HyperTransport PIC Controller.
config LOONGSON_HTVEC
- bool "Loongson3 HyperTransport Interrupt Vector Controller"
+ bool "Loongson HyperTransport Interrupt Vector Controller"
depends on MACH_LOONGSON64
default MACH_LOONGSON64
select IRQ_DOMAIN_HIERARCHY
help
- Support for the Loongson3 HyperTransport Interrupt Vector Controller.
+ Support for the Loongson HyperTransport Interrupt Vector Controller.
config LOONGSON_PCH_PIC
bool "Loongson PCH PIC Controller"
- depends on MACH_LOONGSON64 || COMPILE_TEST
+ depends on MACH_LOONGSON64
default MACH_LOONGSON64
select IRQ_DOMAIN_HIERARCHY
select IRQ_FASTEOI_HIERARCHY_HANDLERS
@@ -582,7 +582,7 @@ config LOONGSON_PCH_PIC
config LOONGSON_PCH_MSI
bool "Loongson PCH MSI Controller"
- depends on MACH_LOONGSON64 || COMPILE_TEST
+ depends on MACH_LOONGSON64
depends on PCI
default MACH_LOONGSON64
select IRQ_DOMAIN_HIERARCHY
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-05-27 11:02 [PATCH RFC V2 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 01/10] irqchip: Adjust Kconfig for Loongson Jianmin Lv
@ 2022-05-27 11:02 ` Jianmin Lv
2022-05-30 16:20 ` Marc Zyngier
2022-05-27 11:02 ` [PATCH RFC V2 03/10] irqchip/loongson-pch-pic: Add ACPI init support Jianmin Lv
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Jianmin Lv @ 2022-05-27 11:02 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen,
Jianmin Lv
We are preparing to add new Loongson (based on LoongArch, not compatible
with old MIPS-based Loongson) support. This patch add the LoongArch CPU
interrupt controller support.
LoongArch CPUINTC stands for CSR.ECFG/CSR.ESTAT and related interrupt
controller that described in Section 7.4 of "LoongArch Reference Manual,
Vol 1". For more information please refer Documentation/loongarch/irq-
chip-model.rst.
LoongArch CPUINTC has 13 interrupt sources: SWI0~1, HWI0~7, IPI, TI
(Timer) and PCOV (PMC). IRQ mappings of HWI0~7 are configurable (can be
created from DT/ACPI), but IPI, TI (Timer) and PCOV (PMC) are hardcoded
bits, so we define get_xxx_irq() for them.
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/irqchip/Kconfig | 10 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-loongarch-cpu.c | 115 +++++++++++++++++
drivers/irqchip/irq-loongarch-pic-common.c | 201 +++++++++++++++++++++++++++++
drivers/irqchip/irq-loongarch-pic-common.h | 44 +++++++
5 files changed, 371 insertions(+)
create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
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/Kconfig b/drivers/irqchip/Kconfig
index 39d6be2..a596ee7 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -545,6 +545,16 @@ config EXYNOS_IRQ_COMBINER
Say yes here to add support for the IRQ combiner devices embedded
in Samsung Exynos chips.
+config IRQ_LOONGARCH_CPU
+ bool
+ select GENERIC_IRQ_CHIP
+ select IRQ_DOMAIN
+ select GENERIC_IRQ_EFFECTIVE_AFF_MASK
+ help
+ Support for the LoongArch CPU Interrupt Controller. For details of
+ irq chip hierarchy on LoongArch platforms please read the document
+ Documentation/loongarch/irq-chip-model.rst.
+
config LOONGSON_LIOINTC
bool "Loongson Local I/O Interrupt Controller"
depends on MACH_LOONGSON64
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 160a1d8..736f030 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -105,6 +105,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 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-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
new file mode 100644
index 0000000..26f948f
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-cpu.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+
+#include <asm/loongarch.h>
+#include <asm/setup.h>
+#include "irq-loongarch-pic-common.h"
+
+static struct irq_domain *irq_domain;
+
+static void mask_loongarch_irq(struct irq_data *d)
+{
+ clear_csr_ecfg(ECFGF(d->hwirq));
+}
+
+static void unmask_loongarch_irq(struct irq_data *d)
+{
+ set_csr_ecfg(ECFGF(d->hwirq));
+}
+
+static struct irq_chip cpu_irq_controller = {
+ .name = "LoongArch",
+ .irq_mask = mask_loongarch_irq,
+ .irq_unmask = unmask_loongarch_irq,
+};
+
+static void handle_cpu_irq(struct pt_regs *regs)
+{
+ int hwirq;
+ unsigned int estat = read_csr_estat() & CSR_ESTAT_IS;
+
+ while ((hwirq = ffs(estat))) {
+ estat &= ~BIT(hwirq - 1);
+ generic_handle_domain_irq(irq_domain, hwirq - 1);
+ }
+}
+
+static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_noprobe(irq);
+ irq_set_chip_and_handler(irq, &cpu_irq_controller, handle_percpu_irq);
+
+ return 0;
+}
+
+static const struct irq_domain_ops loongarch_cpu_intc_irq_domain_ops = {
+ .map = loongarch_cpu_intc_map,
+ .xlate = irq_domain_xlate_onecell,
+};
+
+struct irq_domain * __init loongarch_cpu_irq_init(void)
+{
+ /* Mask interrupts. */
+ clear_csr_ecfg(ECFG0_IM);
+ clear_csr_estat(ESTATF_IP);
+
+ irq_domain = irq_domain_add_legacy(NULL, EXCCODE_INT_NUM, 0, 0,
+ &loongarch_cpu_intc_irq_domain_ops, NULL);
+ if (!irq_domain)
+ panic("Failed to add irqdomain for LoongArch CPU");
+
+ set_handle_irq(&handle_cpu_irq);
+
+ return irq_domain;
+}
+#ifdef CONFIG_ACPI
+static int __init
+liointc_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_lio_pic *liointc_entry = (struct acpi_madt_lio_pic *)header;
+
+ return liointc_acpi_init(irq_domain, liointc_entry);
+}
+
+static int __init
+eiointc_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_eio_pic *eiointc_entry = (struct acpi_madt_eio_pic *)header;
+
+ return eiointc_acpi_init(irq_domain, eiointc_entry);
+}
+static int __init acpi_cascade_irqdomain_init(void)
+{
+ acpi_table_parse_madt(ACPI_MADT_TYPE_LIO_PIC,
+ liointc_parse_madt, 0);
+ acpi_table_parse_madt(ACPI_MADT_TYPE_EIO_PIC,
+ eiointc_parse_madt, 0);
+ return 0;
+}
+static int __init coreintc_acpi_init_v1(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ if (irq_domain)
+ return 0;
+
+ init_vector_parent_group();
+ loongarch_cpu_irq_init();
+ acpi_cascade_irqdomain_init();
+ return 0;
+}
+IRQCHIP_ACPI_DECLARE(coreintc_v1, ACPI_MADT_TYPE_CORE_PIC,
+ NULL, ACPI_MADT_CORE_PIC_VERSION_V1,
+ coreintc_acpi_init_v1);
+#endif
diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
new file mode 100644
index 0000000..94437e4
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-pic-common.c
@@ -0,0 +1,201 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
+ */
+
+#include <linux/irq.h>
+#include <linux/acpi.h>
+#include <linux/pci.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 irq_domain *liointc_domain;
+struct irq_domain *pch_lpc_domain;
+struct irq_domain *pch_msi_domain[MAX_IO_PICS];
+struct irq_domain *pch_pic_domain[MAX_IO_PICS];
+
+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;
+}
+
+int pcibios_device_add(struct pci_dev *dev)
+{
+ int id = pci_domain_nr(dev->bus);
+
+ dev_set_msi_domain(&dev->dev, pch_msi_domain[id]);
+
+ return 0;
+}
+
+int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
+{
+ if (irqp != NULL)
+ *irqp = acpi_register_gsi(NULL, gsi, -1, -1);
+ return (*irqp >= 0) ? 0 : -EINVAL;
+}
+EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
+
+int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
+{
+ if (gsi)
+ *gsi = isa_irq;
+ return 0;
+}
+
+/*
+ * success: return IRQ number (>=0)
+ * failure: return < 0
+ */
+int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
+{
+ int id;
+ struct irq_fwspec fwspec;
+
+ switch (gsi) {
+ case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
+ fwspec.fwnode = liointc_domain->fwnode;
+ fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
+ fwspec.param_count = 1;
+
+ return irq_create_fwspec_mapping(&fwspec);
+
+ case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
+ if (!pch_lpc_domain)
+ return -EINVAL;
+
+ fwspec.fwnode = pch_lpc_domain->fwnode;
+ fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
+ fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
+ fwspec.param_count = 2;
+
+ return irq_create_fwspec_mapping(&fwspec);
+
+ case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
+ id = find_pch_pic(gsi);
+ if (id < 0)
+ return -EINVAL;
+
+ fwspec.fwnode = pch_pic_domain[id]->fwnode;
+ fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
+ fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
+ fwspec.param_count = 2;
+
+ return irq_create_fwspec_mapping(&fwspec);
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(acpi_register_gsi);
+
+void acpi_unregister_gsi(u32 gsi)
+{
+ int id, irq, hw_irq;
+ struct irq_domain *d;
+
+ switch (gsi) {
+ case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
+ if (!liointc_domain)
+ return;
+ d = liointc_domain;
+ hw_irq = gsi - GSI_MIN_CPU_IRQ;
+ break;
+
+ case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
+ if (!pch_lpc_domain)
+ return;
+ d = pch_lpc_domain;
+ hw_irq = gsi - GSI_MIN_LPC_IRQ;
+ break;
+
+ case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
+ id = find_pch_pic(gsi);
+ if (id < 0)
+ return;
+ if (!pch_pic_domain[id])
+ return;
+ d = pch_pic_domain[id];
+
+ hw_irq = gsi - acpi_pchpic[id]->gsi_base;
+ break;
+ }
+ irq = irq_find_mapping(d, hw_irq);
+ irq_dispose_mapping(irq);
+
+ return;
+}
+EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
+
+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);
+}
+
+void acpi_set_vector_parent(int node, struct irq_domain *parent)
+{
+ int i;
+
+ if (cpu_has_flatmode)
+ node = cpu_to_node(node * CORES_PER_EIO_NODE);
+
+ for (i = 0; i < MAX_IO_PICS; i++) {
+ if (node == vector_group[i].node) {
+ vector_group[i].parent = parent;
+ return;
+ }
+ }
+}
+
+struct irq_domain *acpi_get_msi_parent(int index)
+{
+ return vector_group[index].parent;
+}
+
+struct irq_domain *acpi_get_pch_parent(int node)
+{
+ int i;
+
+ for (i = 0; i < MAX_IO_PICS; i++) {
+ if (node == vector_group[i].node)
+ return vector_group[i].parent;
+ }
+ return NULL;
+}
diff --git a/drivers/irqchip/irq-loongarch-pic-common.h b/drivers/irqchip/irq-loongarch-pic-common.h
new file mode 100644
index 0000000..3815fc9
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-pic-common.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
+ */
+
+#ifndef _IRQ_LOONGARCH_PIC_COMMON_H
+#define _IRQ_LOONGARCH_PIC_COMMON_H
+
+#include <linux/of.h>
+#include <linux/irqdomain.h>
+
+struct acpi_vector_group {
+ int node;
+ struct irq_domain *parent;
+};
+
+/* IRQ number definitions */
+#define LOONGSON_LPC_IRQ_BASE 0
+#define LOONGSON_CPU_IRQ_BASE 16
+#define LOONGSON_PCH_IRQ_BASE 64
+
+#define GSI_MIN_LPC_IRQ LOONGSON_LPC_IRQ_BASE
+#define GSI_MAX_LPC_IRQ (LOONGSON_LPC_IRQ_BASE + 16 - 1)
+#define GSI_MIN_CPU_IRQ LOONGSON_CPU_IRQ_BASE
+#define GSI_MAX_CPU_IRQ (LOONGSON_CPU_IRQ_BASE + 48 - 1)
+#define GSI_MIN_PCH_IRQ LOONGSON_PCH_IRQ_BASE
+#define GSI_MAX_PCH_IRQ (LOONGSON_PCH_IRQ_BASE + 256 - 1)
+
+extern struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
+extern struct irq_domain *liointc_domain;
+extern struct irq_domain *pch_lpc_domain;
+extern struct irq_domain *pch_msi_domain[MAX_IO_PICS];
+extern struct irq_domain *pch_pic_domain[MAX_IO_PICS];
+
+int liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic *acpi_liointc);
+int eiointc_acpi_init(struct irq_domain *parent, struct acpi_madt_eio_pic *acpi_eiointc);
+int htvec_acpi_init(struct irq_domain *parent, struct acpi_madt_ht_pic *acpi_htvec);
+int pch_lpc_acpi_init(struct irq_domain *parent, struct acpi_madt_lpc_pic *acpi_pchlpc);
+void init_vector_parent_group(void);
+void acpi_set_vector_parent(int node, struct irq_domain *parent);
+struct irq_domain *acpi_get_msi_parent(int index);
+struct irq_domain *acpi_get_pch_parent(int node);
+
+#endif /* _IRQ_LOONGARCH_PIC_COMMON_H */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH RFC V2 03/10] irqchip/loongson-pch-pic: Add ACPI init support
2022-05-27 11:02 [PATCH RFC V2 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 01/10] irqchip: Adjust Kconfig for Loongson Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support Jianmin Lv
@ 2022-05-27 11:02 ` Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 04/10] irqchip/loongson-pch-pic: Add suspend/resume support Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 05/10] irqchip/loongson-pch-msi: Add ACPI init support Jianmin Lv
4 siblings, 0 replies; 15+ messages in thread
From: Jianmin Lv @ 2022-05-27 11:02 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen,
Jianmin Lv
We are preparing to add new Loongson (based on LoongArch, not compatible
with old MIPS-based Loongson) support. LoongArch use ACPI other than DT
as its boot protocol, so add ACPI init support.
PCH-PIC/PCH-MSI stands for "Interrupt Controller" that described in
Section 5 of "Loongson 7A1000 Bridge User Manual". For more information
please refer Documentation/loongarch/irq-chip-model.rst.
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/irqchip/irq-loongson-pch-pic.c | 150 +++++++++++++++++++++++++++------
1 file changed, 123 insertions(+), 27 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index a4eb8a2..e3eb5c6 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -15,6 +15,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include "irq-loongarch-pic-common.h"
/* Registers */
#define PCH_PIC_MASK 0x20
@@ -33,13 +34,20 @@
#define PIC_REG_IDX(irq_id) ((irq_id) / PIC_COUNT_PER_REG)
#define PIC_REG_BIT(irq_id) ((irq_id) % PIC_COUNT_PER_REG)
+static int nr_pics;
+
struct pch_pic {
void __iomem *base;
struct irq_domain *pic_domain;
+ struct fwnode_handle *domain_handle;
u32 ht_vec_base;
raw_spinlock_t pic_lock;
+ u32 gsi_end;
+ u32 gsi_base;
};
+static struct pch_pic *pch_pic_priv[2];
+
static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
{
u32 reg;
@@ -180,7 +188,7 @@ static void pch_pic_reset(struct pch_pic *priv)
int i;
for (i = 0; i < PIC_COUNT; i++) {
- /* Write vectored ID */
+ /* Write vector ID */
writeb(priv->ht_vec_base + i, priv->base + PCH_INT_HTVEC(i));
/* Hardcode route to HT0 Lo */
writeb(1, priv->base + PCH_INT_ROUTE(i));
@@ -198,50 +206,44 @@ static void pch_pic_reset(struct pch_pic *priv)
}
}
-static int pch_pic_of_init(struct device_node *node,
- struct device_node *parent)
+static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
+ struct irq_domain *parent_domain, struct fwnode_handle *domain_handle,
+ u32 gsi_base)
{
+ int vec_count;
struct pch_pic *priv;
- struct irq_domain *parent_domain;
- int err;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
raw_spin_lock_init(&priv->pic_lock);
- priv->base = of_iomap(node, 0);
- if (!priv->base) {
- err = -ENOMEM;
+ priv->base = ioremap(addr, size);
+ if (!priv->base)
goto free_priv;
- }
- parent_domain = irq_find_host(parent);
- if (!parent_domain) {
- pr_err("Failed to find the parent domain\n");
- err = -ENXIO;
- goto iounmap_base;
- }
+ priv->domain_handle = domain_handle;
- if (of_property_read_u32(node, "loongson,pic-base-vec",
- &priv->ht_vec_base)) {
- pr_err("Failed to determine pic-base-vec\n");
- err = -EINVAL;
- goto iounmap_base;
- }
+ priv->ht_vec_base = vec_base;
+ vec_count = ((readq(priv->base) >> 48) & 0xff) + 1;
+ priv->gsi_base = gsi_base;
+ priv->gsi_end = gsi_base + vec_count - 1;
priv->pic_domain = irq_domain_create_hierarchy(parent_domain, 0,
- PIC_COUNT,
- of_node_to_fwnode(node),
- &pch_pic_domain_ops,
- priv);
+ vec_count, priv->domain_handle,
+ &pch_pic_domain_ops, priv);
+
if (!priv->pic_domain) {
pr_err("Failed to create IRQ domain\n");
- err = -ENOMEM;
goto iounmap_base;
}
pch_pic_reset(priv);
+ pch_pic_domain[nr_pics] = priv->pic_domain;
+ pch_pic_priv[nr_pics++] = priv;
+ register_syscore_ops(&pch_pic_syscore_ops);
+
+ register_syscore_ops(&pch_pic_syscore_ops);
return 0;
@@ -250,7 +252,101 @@ static int pch_pic_of_init(struct device_node *node,
free_priv:
kfree(priv);
- return err;
+ return -EINVAL;
+}
+
+#ifdef CONFIG_OF
+
+static int pch_pic_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ int err, vec_base;
+ struct resource res;
+ struct irq_domain *parent_domain;
+
+ if (of_address_to_resource(node, 0, &res))
+ return -EINVAL;
+
+ parent_domain = irq_find_host(parent);
+ if (!parent_domain) {
+ pr_err("Failed to find the parent domain\n");
+ return -ENXIO;
+ }
+
+ if (of_property_read_u32(node, "loongson,pic-base-vec", &vec_base)) {
+ pr_err("Failed to determine pic-base-vec\n");
+ return -EINVAL;
+ }
+
+ err = pch_pic_init(res.start, resource_size(&res), vec_base,
+ parent_domain, of_node_to_fwnode(node), 0);
+ if (err < 0)
+ return err;
+
+ return 0;
}
IRQCHIP_DECLARE(pch_pic, "loongson,pch-pic-1.0", pch_pic_of_init);
+
+#endif
+
+#ifdef CONFIG_ACPI
+static int __init
+lpcintc_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_lpc_pic *lpcintc_entry = (struct acpi_madt_lpc_pic *)header;
+
+ return pch_lpc_acpi_init(pch_pic_priv[0]->pic_domain, lpcintc_entry);
+}
+
+static int __init acpi_cascade_irqdomain_init(void)
+{
+ acpi_table_parse_madt(ACPI_MADT_TYPE_LPC_PIC,
+ lpcintc_parse_madt, 0);
+ return 0;
+}
+int __init pch_pic_init_irqdomain(struct irq_domain *parent,
+ struct acpi_madt_bio_pic *acpi_pchpic)
+{
+ int ret, vec_base;
+ struct fwnode_handle *domain_handle;
+
+ if (!acpi_pchpic)
+ return -EINVAL;
+
+ vec_base = acpi_pchpic->gsi_base - GSI_MIN_PCH_IRQ;
+
+ domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_pchpic);
+ if (!domain_handle) {
+ pr_err("Unable to allocate domain handle\n");
+ return -ENOMEM;
+ }
+
+ ret = pch_pic_init(acpi_pchpic->address, acpi_pchpic->size,
+ vec_base, parent, domain_handle, acpi_pchpic->gsi_base);
+ if (ret == 0 && acpi_pchpic->id == 0)
+ acpi_cascade_irqdomain_init();
+
+ return ret;
+}
+
+static int __init pchintc_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_bio_pic *pch_pic_entry = (struct acpi_madt_bio_pic *)header;
+
+ acpi_pchpic[nr_pics] = pch_pic_entry;
+ return pch_pic_init_irqdomain(acpi_get_pch_parent((pch_pic_entry->address >> 44) & 0xf),
+ pch_pic_entry);
+}
+
+static int __init pch_pic_acpi_init(void)
+{
+ acpi_table_parse_madt(ACPI_MADT_TYPE_BIO_PIC,
+ pchintc_parse_madt, 0);
+
+ return 0;
+}
+early_initcall(pch_pic_acpi_init);
+#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH RFC V2 04/10] irqchip/loongson-pch-pic: Add suspend/resume support
2022-05-27 11:02 [PATCH RFC V2 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
` (2 preceding siblings ...)
2022-05-27 11:02 ` [PATCH RFC V2 03/10] irqchip/loongson-pch-pic: Add ACPI init support Jianmin Lv
@ 2022-05-27 11:02 ` Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 05/10] irqchip/loongson-pch-msi: Add ACPI init support Jianmin Lv
4 siblings, 0 replies; 15+ messages in thread
From: Jianmin Lv @ 2022-05-27 11:02 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen,
Jianmin Lv
Add suspend/resume support for PCH-PIC irqchip, which is needed for
suspend/hibernation.
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/irqchip/irq-loongson-pch-pic.c | 47 ++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index e3eb5c6..a88a40e 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -15,6 +15,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include <linux/syscore_ops.h>
#include "irq-loongarch-pic-common.h"
/* Registers */
@@ -42,6 +43,9 @@ struct pch_pic {
struct fwnode_handle *domain_handle;
u32 ht_vec_base;
raw_spinlock_t pic_lock;
+ u32 saved_vec_en[PIC_REG_COUNT];
+ u32 saved_vec_pol[PIC_REG_COUNT];
+ u32 saved_vec_edge[PIC_REG_COUNT];
u32 gsi_end;
u32 gsi_base;
};
@@ -145,6 +149,7 @@ static void pch_pic_ack_irq(struct irq_data *d)
.irq_ack = pch_pic_ack_irq,
.irq_set_affinity = irq_chip_set_affinity_parent,
.irq_set_type = pch_pic_set_type,
+ .flags = IRQCHIP_SKIP_SET_WAKE,
};
static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
@@ -206,6 +211,46 @@ static void pch_pic_reset(struct pch_pic *priv)
}
}
+static int pch_pic_suspend(void)
+{
+ int i, j;
+
+ for (i = 0; i < nr_pics; i++) {
+ for (j = 0; j < PIC_REG_COUNT; j++) {
+ pch_pic_priv[i]->saved_vec_pol[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_POL + 4 * j);
+ pch_pic_priv[i]->saved_vec_edge[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_EDGE + 4 * j);
+ pch_pic_priv[i]->saved_vec_en[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_MASK + 4 * j);
+ }
+ }
+
+ return 0;
+}
+
+static void pch_pic_resume(void)
+{
+ int i, j;
+
+ for (i = 0; i < nr_pics; i++) {
+ pch_pic_reset(pch_pic_priv[i]);
+ for (j = 0; j < PIC_REG_COUNT; j++) {
+ writel(pch_pic_priv[i]->saved_vec_pol[j],
+ pch_pic_priv[i]->base + PCH_PIC_POL + 4 * j);
+ writel(pch_pic_priv[i]->saved_vec_edge[j],
+ pch_pic_priv[i]->base + PCH_PIC_EDGE + 4 * j);
+ writel(pch_pic_priv[i]->saved_vec_en[j],
+ pch_pic_priv[i]->base + PCH_PIC_MASK + 4 * j);
+ }
+ }
+}
+
+static struct syscore_ops pch_pic_syscore_ops = {
+ .suspend = pch_pic_suspend,
+ .resume = pch_pic_resume,
+};
+
static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
struct irq_domain *parent_domain, struct fwnode_handle *domain_handle,
u32 gsi_base)
@@ -245,6 +290,8 @@ static int pch_pic_init(phys_addr_t addr, unsigned long size, int vec_base,
register_syscore_ops(&pch_pic_syscore_ops);
+ register_syscore_ops(&pch_pic_syscore_ops);
+
return 0;
iounmap_base:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH RFC V2 05/10] irqchip/loongson-pch-msi: Add ACPI init support
2022-05-27 11:02 [PATCH RFC V2 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
` (3 preceding siblings ...)
2022-05-27 11:02 ` [PATCH RFC V2 04/10] irqchip/loongson-pch-pic: Add suspend/resume support Jianmin Lv
@ 2022-05-27 11:02 ` Jianmin Lv
4 siblings, 0 replies; 15+ messages in thread
From: Jianmin Lv @ 2022-05-27 11:02 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen,
Jianmin Lv
We are preparing to add new Loongson (based on LoongArch, not compatible
with old MIPS-based Loongson) support. LoongArch use ACPI other than DT
as its boot protocol, so add ACPI init support.
PCH-PIC/PCH-MSI stands for "Interrupt Controller" that described in
Section 5 of "Loongson 7A1000 Bridge User Manual". For more information
please refer Documentation/loongarch/irq-chip-model.rst.
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/irqchip/irq-loongson-pch-msi.c | 138 +++++++++++++++++++++++----------
1 file changed, 96 insertions(+), 42 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-pch-msi.c b/drivers/irqchip/irq-loongson-pch-msi.c
index e3801c4..eed3c2e 100644
--- a/drivers/irqchip/irq-loongson-pch-msi.c
+++ b/drivers/irqchip/irq-loongson-pch-msi.c
@@ -14,6 +14,9 @@
#include <linux/of_pci.h>
#include <linux/pci.h>
#include <linux/slab.h>
+#include "irq-loongarch-pic-common.h"
+
+static int nr_pics;
struct pch_msi_data {
struct mutex msi_map_lock;
@@ -21,8 +24,11 @@ struct pch_msi_data {
u32 irq_first; /* The vector number that MSIs starts */
u32 num_irqs; /* The number of vectors for MSIs */
unsigned long *msi_map;
+ struct fwnode_handle *domain_handle;
};
+static struct pch_msi_data *pch_msi_priv[2];
+
static void pch_msi_mask_msi_irq(struct irq_data *d)
{
pci_msi_mask_irq(d);
@@ -154,12 +160,14 @@ static void pch_msi_middle_domain_free(struct irq_domain *domain,
};
static int pch_msi_init_domains(struct pch_msi_data *priv,
- struct device_node *node,
- struct irq_domain *parent)
+ struct irq_domain *parent,
+ struct fwnode_handle *domain_handle)
{
struct irq_domain *middle_domain, *msi_domain;
- middle_domain = irq_domain_create_linear(of_node_to_fwnode(node),
+ priv->domain_handle = domain_handle;
+
+ middle_domain = irq_domain_create_linear(priv->domain_handle,
priv->num_irqs,
&pch_msi_middle_domain_ops,
priv);
@@ -171,7 +179,7 @@ static int pch_msi_init_domains(struct pch_msi_data *priv,
middle_domain->parent = parent;
irq_domain_update_bus_token(middle_domain, DOMAIN_BUS_NEXUS);
- msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(node),
+ msi_domain = pci_msi_create_irq_domain(priv->domain_handle,
&pch_msi_domain_info,
middle_domain);
if (!msi_domain) {
@@ -183,19 +191,11 @@ static int pch_msi_init_domains(struct pch_msi_data *priv,
return 0;
}
-static int pch_msi_init(struct device_node *node,
- struct device_node *parent)
+static int pch_msi_init(phys_addr_t msg_address, int irq_base, int irq_count,
+ struct irq_domain *parent_domain, struct fwnode_handle *domain_handle)
{
- struct pch_msi_data *priv;
- struct irq_domain *parent_domain;
- struct resource res;
int ret;
-
- parent_domain = irq_find_host(parent);
- if (!parent_domain) {
- pr_err("Failed to find the parent domain\n");
- return -ENXIO;
- }
+ struct pch_msi_data *priv;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
@@ -203,48 +203,102 @@ static int pch_msi_init(struct device_node *node,
mutex_init(&priv->msi_map_lock);
- ret = of_address_to_resource(node, 0, &res);
- if (ret) {
- pr_err("Failed to allocate resource\n");
- goto err_priv;
- }
-
- priv->doorbell = res.start;
-
- if (of_property_read_u32(node, "loongson,msi-base-vec",
- &priv->irq_first)) {
- pr_err("Unable to parse MSI vec base\n");
- ret = -EINVAL;
- goto err_priv;
- }
-
- if (of_property_read_u32(node, "loongson,msi-num-vecs",
- &priv->num_irqs)) {
- pr_err("Unable to parse MSI vec number\n");
- ret = -EINVAL;
- goto err_priv;
- }
+ priv->doorbell = msg_address;
+ priv->irq_first = irq_base;
+ priv->num_irqs = irq_count;
priv->msi_map = bitmap_zalloc(priv->num_irqs, GFP_KERNEL);
- if (!priv->msi_map) {
- ret = -ENOMEM;
+ if (!priv->msi_map)
goto err_priv;
- }
pr_debug("Registering %d MSIs, starting at %d\n",
priv->num_irqs, priv->irq_first);
- ret = pch_msi_init_domains(priv, node, parent_domain);
+ ret = pch_msi_init_domains(priv, parent_domain, domain_handle);
if (ret)
goto err_map;
+ pch_msi_domain[nr_pics] = irq_find_matching_fwnode(domain_handle, DOMAIN_BUS_PCI_MSI);
+ pch_msi_priv[nr_pics++] = priv;
return 0;
err_map:
bitmap_free(priv->msi_map);
err_priv:
kfree(priv);
- return ret;
+
+ return -EINVAL;
+}
+
+#ifdef CONFIG_OF
+
+static int pch_msi_of_init(struct device_node *node, struct device_node *parent)
+{
+ int err;
+ int irq_base, irq_count;
+ struct resource res;
+ struct irq_domain *parent_domain;
+
+ parent_domain = irq_find_host(parent);
+ if (!parent_domain) {
+ pr_err("Failed to find the parent domain\n");
+ return -ENXIO;
+ }
+
+ if (of_address_to_resource(node, 0, &res)) {
+ pr_err("Failed to allocate resource\n");
+ return -EINVAL;
+ }
+
+ if (of_property_read_u32(node, "loongson,msi-base-vec", &irq_base)) {
+ pr_err("Unable to parse MSI vec base\n");
+ return -EINVAL;
+ }
+
+ if (of_property_read_u32(node, "loongson,msi-num-vecs", &irq_count)) {
+ pr_err("Unable to parse MSI vec number\n");
+ return -EINVAL;
+ }
+
+ err = pch_msi_init(res.start, irq_base, irq_count, parent_domain, of_node_to_fwnode(node));
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+IRQCHIP_DECLARE(pch_msi, "loongson,pch-msi-1.0", pch_msi_of_init);
+
+#endif
+
+#ifdef CONFIG_ACPI
+int __init pch_msi_init_irqdomain(struct irq_domain *parent,
+ struct acpi_madt_msi_pic *acpi_pchmsi)
+{
+ struct fwnode_handle *domain_handle;
+
+ if (!acpi_pchmsi)
+ return -EINVAL;
+
+ domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_pchmsi);
+
+ return pch_msi_init(acpi_pchmsi->msg_address, acpi_pchmsi->start,
+ acpi_pchmsi->count, parent, domain_handle);
+}
+static int __init msiintc_parse_madt(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_msi_pic *pch_msi_entry = (struct acpi_madt_msi_pic *)header;
+
+ return pch_msi_init_irqdomain(acpi_get_msi_parent(nr_pics), pch_msi_entry);
}
-IRQCHIP_DECLARE(pch_msi, "loongson,pch-msi-1.0", pch_msi_init);
+static int __init pch_msi_acpi_init(void)
+{
+ acpi_table_parse_madt(ACPI_MADT_TYPE_MSI_PIC,
+ msiintc_parse_madt, 0);
+
+ return 0;
+}
+early_initcall(pch_msi_acpi_init);
+#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-05-27 11:02 ` [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support Jianmin Lv
@ 2022-05-30 16:20 ` Marc Zyngier
2022-05-31 11:01 ` 吕建民
0 siblings, 1 reply; 15+ messages in thread
From: Marc Zyngier @ 2022-05-30 16:20 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen
On Fri, 27 May 2022 12:02:12 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
> We are preparing to add new Loongson (based on LoongArch, not compatible
> with old MIPS-based Loongson) support. This patch add the LoongArch CPU
> interrupt controller support.
Please drop this paragraph, as it doesn't help at all.
>
> LoongArch CPUINTC stands for CSR.ECFG/CSR.ESTAT and related interrupt
> controller that described in Section 7.4 of "LoongArch Reference Manual,
> Vol 1". For more information please refer Documentation/loongarch/irq-
> chip-model.rst.
Where is this the patch adding this document?
>
> LoongArch CPUINTC has 13 interrupt sources: SWI0~1, HWI0~7, IPI, TI
> (Timer) and PCOV (PMC). IRQ mappings of HWI0~7 are configurable (can be
> created from DT/ACPI), but IPI, TI (Timer) and PCOV (PMC) are hardcoded
> bits, so we define get_xxx_irq() for them.
Where are these functions? How are they used?
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> ---
> drivers/irqchip/Kconfig | 10 ++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-loongarch-cpu.c | 115 +++++++++++++++++
> drivers/irqchip/irq-loongarch-pic-common.c | 201 +++++++++++++++++++++++++++++
One patch per driver, please.
> drivers/irqchip/irq-loongarch-pic-common.h | 44 +++++++
> 5 files changed, 371 insertions(+)
> create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
> 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/Kconfig b/drivers/irqchip/Kconfig
> index 39d6be2..a596ee7 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -545,6 +545,16 @@ config EXYNOS_IRQ_COMBINER
> Say yes here to add support for the IRQ combiner devices embedded
> in Samsung Exynos chips.
>
> +config IRQ_LOONGARCH_CPU
> + bool
> + select GENERIC_IRQ_CHIP
> + select IRQ_DOMAIN
> + select GENERIC_IRQ_EFFECTIVE_AFF_MASK
> + help
> + Support for the LoongArch CPU Interrupt Controller. For details of
> + irq chip hierarchy on LoongArch platforms please read the document
> + Documentation/loongarch/irq-chip-model.rst.
> +
> config LOONGSON_LIOINTC
> bool "Loongson Local I/O Interrupt Controller"
> depends on MACH_LOONGSON64
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 160a1d8..736f030 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -105,6 +105,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 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-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
> new file mode 100644
> index 0000000..26f948f
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongarch-cpu.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +
> +#include <asm/loongarch.h>
> +#include <asm/setup.h>
> +#include "irq-loongarch-pic-common.h"
> +
> +static struct irq_domain *irq_domain;
> +
> +static void mask_loongarch_irq(struct irq_data *d)
> +{
> + clear_csr_ecfg(ECFGF(d->hwirq));
> +}
> +
> +static void unmask_loongarch_irq(struct irq_data *d)
> +{
> + set_csr_ecfg(ECFGF(d->hwirq));
> +}
> +
> +static struct irq_chip cpu_irq_controller = {
> + .name = "LoongArch",
> + .irq_mask = mask_loongarch_irq,
> + .irq_unmask = unmask_loongarch_irq,
> +};
> +
> +static void handle_cpu_irq(struct pt_regs *regs)
> +{
> + int hwirq;
> + unsigned int estat = read_csr_estat() & CSR_ESTAT_IS;
> +
> + while ((hwirq = ffs(estat))) {
> + estat &= ~BIT(hwirq - 1);
> + generic_handle_domain_irq(irq_domain, hwirq - 1);
> + }
> +}
> +
> +static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + irq_set_noprobe(irq);
> + irq_set_chip_and_handler(irq, &cpu_irq_controller, handle_percpu_irq);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops loongarch_cpu_intc_irq_domain_ops = {
> + .map = loongarch_cpu_intc_map,
> + .xlate = irq_domain_xlate_onecell,
> +};
> +
> +struct irq_domain * __init loongarch_cpu_irq_init(void)
> +{
> + /* Mask interrupts. */
> + clear_csr_ecfg(ECFG0_IM);
> + clear_csr_estat(ESTATF_IP);
> +
> + irq_domain = irq_domain_add_legacy(NULL, EXCCODE_INT_NUM, 0, 0,
> + &loongarch_cpu_intc_irq_domain_ops, NULL);
I already commented on this in the past, and my position is still the
same: this isn't a legacy architecture, you are not converting
anything from a board file, so there is no reason why you get to use a
legacy domain.
Since you are using ACPI, irq_domain_add_*() really is the wrong API,
as they take an of_node. Use irq_domain_create_linear(), and pass an
actual fwnode there (there are plenty of examples in the tree).
> + if (!irq_domain)
> + panic("Failed to add irqdomain for LoongArch CPU");
> +
> + set_handle_irq(&handle_cpu_irq);
> +
> + return irq_domain;
What uses this irq_domain in the arch code?
> +}
> +#ifdef CONFIG_ACPI
Why the #ifdef? Isn't this system supposed to be ACPI only? There is
no DT support anyway, so you should make the driver depend on ACPI and
that's about it.
> +static int __init
> +liointc_parse_madt(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_lio_pic *liointc_entry = (struct acpi_madt_lio_pic *)header;
> +
> + return liointc_acpi_init(irq_domain, liointc_entry);
> +}
> +
> +static int __init
> +eiointc_parse_madt(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_eio_pic *eiointc_entry = (struct acpi_madt_eio_pic *)header;
> +
> + return eiointc_acpi_init(irq_domain, eiointc_entry);
> +}
> +static int __init acpi_cascade_irqdomain_init(void)
> +{
> + acpi_table_parse_madt(ACPI_MADT_TYPE_LIO_PIC,
> + liointc_parse_madt, 0);
> + acpi_table_parse_madt(ACPI_MADT_TYPE_EIO_PIC,
> + eiointc_parse_madt, 0);
> + return 0;
> +}
> +static int __init coreintc_acpi_init_v1(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + if (irq_domain)
> + return 0;
> +
> + init_vector_parent_group();
> + loongarch_cpu_irq_init();
> + acpi_cascade_irqdomain_init();
> + return 0;
> +}
> +IRQCHIP_ACPI_DECLARE(coreintc_v1, ACPI_MADT_TYPE_CORE_PIC,
> + NULL, ACPI_MADT_CORE_PIC_VERSION_V1,
> + coreintc_acpi_init_v1);
> +#endif
> diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
> new file mode 100644
> index 0000000..94437e4
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongarch-pic-common.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/acpi.h>
> +#include <linux/pci.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 irq_domain *liointc_domain;
> +struct irq_domain *pch_lpc_domain;
> +struct irq_domain *pch_msi_domain[MAX_IO_PICS];
> +struct irq_domain *pch_pic_domain[MAX_IO_PICS];
Why isn't this static? If someone needs to know, why isn't there an
accessor?
> +
> +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;
> +}
> +
> +int pcibios_device_add(struct pci_dev *dev)
> +{
> + int id = pci_domain_nr(dev->bus);
> +
> + dev_set_msi_domain(&dev->dev, pch_msi_domain[id]);
> +
> + return 0;
> +}
This doesn't belong here at all. Please move it to the PCI code.
> +
> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
> +{
> + if (irqp != NULL)
> + *irqp = acpi_register_gsi(NULL, gsi, -1, -1);
> + return (*irqp >= 0) ? 0 : -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> +
> +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
> +{
> + if (gsi)
> + *gsi = isa_irq;
> + return 0;
> +}
> +
> +/*
> + * success: return IRQ number (>=0)
> + * failure: return < 0
> + */
> +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
> +{
> + int id;
> + struct irq_fwspec fwspec;
> +
> + switch (gsi) {
> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> + fwspec.fwnode = liointc_domain->fwnode;
> + fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
> + fwspec.param_count = 1;
> +
> + return irq_create_fwspec_mapping(&fwspec);
> +
> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> + if (!pch_lpc_domain)
> + return -EINVAL;
> +
> + fwspec.fwnode = pch_lpc_domain->fwnode;
> + fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
> + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
> + fwspec.param_count = 2;
> +
> + return irq_create_fwspec_mapping(&fwspec);
> +
> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> + id = find_pch_pic(gsi);
> + if (id < 0)
> + return -EINVAL;
> +
> + fwspec.fwnode = pch_pic_domain[id]->fwnode;
> + fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
> + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
> + fwspec.param_count = 2;
> +
> + return irq_create_fwspec_mapping(&fwspec);
> + }
So all the complexity here seems to stem from the fact that you deal
with three ranges of interrupts, managed by three different pieces of
code?
Other architectures have similar requirements, and don't require to
re-implement a private version of the ACPI API. Instead, they expose a
single irqdomain, and deal with the various ranges internally.
Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue.
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(acpi_register_gsi);
> +
> +void acpi_unregister_gsi(u32 gsi)
> +{
> + int id, irq, hw_irq;
> + struct irq_domain *d;
> +
> + switch (gsi) {
> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> + if (!liointc_domain)
> + return;
> + d = liointc_domain;
> + hw_irq = gsi - GSI_MIN_CPU_IRQ;
> + break;
> +
> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> + if (!pch_lpc_domain)
> + return;
> + d = pch_lpc_domain;
> + hw_irq = gsi - GSI_MIN_LPC_IRQ;
> + break;
> +
> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> + id = find_pch_pic(gsi);
> + if (id < 0)
> + return;
> + if (!pch_pic_domain[id])
> + return;
> + d = pch_pic_domain[id];
> +
> + hw_irq = gsi - acpi_pchpic[id]->gsi_base;
> + break;
> + }
> + irq = irq_find_mapping(d, hw_irq);
> + irq_dispose_mapping(irq);
> +
> + return;
> +}
> +EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
> +
> +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;
> +}
Again, why can't you reuse drivers/acpi/pci_mcfg.c?
> +
> +void __init init_vector_parent_group(void)
> +{
> + acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
> +}
> +
> +void acpi_set_vector_parent(int node, struct irq_domain *parent)
> +{
> + int i;
> +
> + if (cpu_has_flatmode)
> + node = cpu_to_node(node * CORES_PER_EIO_NODE);
> +
> + for (i = 0; i < MAX_IO_PICS; i++) {
> + if (node == vector_group[i].node) {
> + vector_group[i].parent = parent;
> + return;
> + }
> + }
> +}
> +
> +struct irq_domain *acpi_get_msi_parent(int index)
> +{
> + return vector_group[index].parent;
> +}
> +
> +struct irq_domain *acpi_get_pch_parent(int node)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_IO_PICS; i++) {
> + if (node == vector_group[i].node)
> + return vector_group[i].parent;
> + }
> + return NULL;
> +}
> diff --git a/drivers/irqchip/irq-loongarch-pic-common.h b/drivers/irqchip/irq-loongarch-pic-common.h
> new file mode 100644
> index 0000000..3815fc9
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongarch-pic-common.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
> + */
> +
> +#ifndef _IRQ_LOONGARCH_PIC_COMMON_H
> +#define _IRQ_LOONGARCH_PIC_COMMON_H
> +
> +#include <linux/of.h>
> +#include <linux/irqdomain.h>
> +
> +struct acpi_vector_group {
> + int node;
> + struct irq_domain *parent;
> +};
> +
> +/* IRQ number definitions */
> +#define LOONGSON_LPC_IRQ_BASE 0
> +#define LOONGSON_CPU_IRQ_BASE 16
> +#define LOONGSON_PCH_IRQ_BASE 64
> +
> +#define GSI_MIN_LPC_IRQ LOONGSON_LPC_IRQ_BASE
> +#define GSI_MAX_LPC_IRQ (LOONGSON_LPC_IRQ_BASE + 16 - 1)
> +#define GSI_MIN_CPU_IRQ LOONGSON_CPU_IRQ_BASE
> +#define GSI_MAX_CPU_IRQ (LOONGSON_CPU_IRQ_BASE + 48 - 1)
> +#define GSI_MIN_PCH_IRQ LOONGSON_PCH_IRQ_BASE
> +#define GSI_MAX_PCH_IRQ (LOONGSON_PCH_IRQ_BASE + 256 - 1)
> +
> +extern struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
> +extern struct irq_domain *liointc_domain;
> +extern struct irq_domain *pch_lpc_domain;
> +extern struct irq_domain *pch_msi_domain[MAX_IO_PICS];
> +extern struct irq_domain *pch_pic_domain[MAX_IO_PICS];
> +
> +int liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic *acpi_liointc);
> +int eiointc_acpi_init(struct irq_domain *parent, struct acpi_madt_eio_pic *acpi_eiointc);
> +int htvec_acpi_init(struct irq_domain *parent, struct acpi_madt_ht_pic *acpi_htvec);
> +int pch_lpc_acpi_init(struct irq_domain *parent, struct acpi_madt_lpc_pic *acpi_pchlpc);
> +void init_vector_parent_group(void);
> +void acpi_set_vector_parent(int node, struct irq_domain *parent);
> +struct irq_domain *acpi_get_msi_parent(int index);
> +struct irq_domain *acpi_get_pch_parent(int node);
> +
> +#endif /* _IRQ_LOONGARCH_PIC_COMMON_H */
> --
> 1.8.3.1
>
>
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-05-30 16:20 ` Marc Zyngier
@ 2022-05-31 11:01 ` 吕建民
2022-05-31 13:56 ` Marc Zyngier
0 siblings, 1 reply; 15+ messages in thread
From: 吕建民 @ 2022-05-31 11:01 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen
> -----Original Messages-----
> From: "Marc Zyngier" <maz@kernel.org>
> Sent Time: 2022-05-31 00:20:31 (Tuesday)
> To: "Jianmin Lv" <lvjianmin@loongson.cn>
> Cc: "Thomas Gleixner" <tglx@linutronix.de>, linux-kernel@vger.kernel.org, "Xuefeng Li" <lixuefeng@loongson.cn>, "Huacai Chen" <chenhuacai@gmail.com>, "Jiaxun Yang" <jiaxun.yang@flygoat.com>, "Huacai Chen" <chenhuacai@loongson.cn>
> Subject: Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
>
> On Fri, 27 May 2022 12:02:12 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
> >
> > We are preparing to add new Loongson (based on LoongArch, not compatible
> > with old MIPS-based Loongson) support. This patch add the LoongArch CPU
> > interrupt controller support.
>
> Please drop this paragraph, as it doesn't help at all.
>
Ok,thanks, I'll drop it.
> >
> > LoongArch CPUINTC stands for CSR.ECFG/CSR.ESTAT and related interrupt
> > controller that described in Section 7.4 of "LoongArch Reference Manual,
> > Vol 1". For more information please refer Documentation/loongarch/irq-
> > chip-model.rst.
>
> Where is this the patch adding this document?
>
See https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html
> >
> > LoongArch CPUINTC has 13 interrupt sources: SWI0~1, HWI0~7, IPI, TI
> > (Timer) and PCOV (PMC). IRQ mappings of HWI0~7 are configurable (can be
> > created from DT/ACPI), but IPI, TI (Timer) and PCOV (PMC) are hardcoded
> > bits, so we define get_xxx_irq() for them.
>
> Where are these functions? How are they used?
>
Sorry, these functions are implemented in previours version, and they are deleted in current version because I changed to use legacy irqdomain in this version so that we don't have to use these functions to create irq mapping for IPI, PMC and TIMER. Of cause, if you sugguest us to use linear irqdomain, I'll restore them to be what like as last version.
> >
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> > ---
> > drivers/irqchip/Kconfig | 10 ++
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-loongarch-cpu.c | 115 +++++++++++++++++
> > drivers/irqchip/irq-loongarch-pic-common.c | 201 +++++++++++++++++++++++++++++
>
> One patch per driver, please.
>
Ok, I'll split them to be seperate patch.
> > drivers/irqchip/irq-loongarch-pic-common.h | 44 +++++++
> > 5 files changed, 371 insertions(+)
> > create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
> > 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/Kconfig b/drivers/irqchip/Kconfig
> > index 39d6be2..a596ee7 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -545,6 +545,16 @@ config EXYNOS_IRQ_COMBINER
> > Say yes here to add support for the IRQ combiner devices embedded
> > in Samsung Exynos chips.
> >
> > +config IRQ_LOONGARCH_CPU
> > + bool
> > + select GENERIC_IRQ_CHIP
> > + select IRQ_DOMAIN
> > + select GENERIC_IRQ_EFFECTIVE_AFF_MASK
> > + help
> > + Support for the LoongArch CPU Interrupt Controller. For details of
> > + irq chip hierarchy on LoongArch platforms please read the document
> > + Documentation/loongarch/irq-chip-model.rst.
> > +
> > config LOONGSON_LIOINTC
> > bool "Loongson Local I/O Interrupt Controller"
> > depends on MACH_LOONGSON64
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index 160a1d8..736f030 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -105,6 +105,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 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-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
> > new file mode 100644
> > index 0000000..26f948f
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-loongarch-cpu.c
> > @@ -0,0 +1,115 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
> > + */
> > +
> > +#include <linux init.h="">
> > +#include <linux kernel.h="">
> > +#include <linux interrupt.h="">
> > +#include <linux irq.h="">
> > +#include <linux irqchip.h="">
> > +#include <linux irqdomain.h="">
> > +
> > +#include <asm loongarch.h="">
> > +#include <asm setup.h="">
> > +#include "irq-loongarch-pic-common.h"
> > +
> > +static struct irq_domain *irq_domain;
> > +
> > +static void mask_loongarch_irq(struct irq_data *d)
> > +{
> > + clear_csr_ecfg(ECFGF(d->hwirq));
> > +}
> > +
> > +static void unmask_loongarch_irq(struct irq_data *d)
> > +{
> > + set_csr_ecfg(ECFGF(d->hwirq));
> > +}
> > +
> > +static struct irq_chip cpu_irq_controller = {
> > + .name = "LoongArch",
> > + .irq_mask = mask_loongarch_irq,
> > + .irq_unmask = unmask_loongarch_irq,
> > +};
> > +
> > +static void handle_cpu_irq(struct pt_regs *regs)
> > +{
> > + int hwirq;
> > + unsigned int estat = read_csr_estat() & CSR_ESTAT_IS;
> > +
> > + while ((hwirq = ffs(estat))) {
> > + estat &= ~BIT(hwirq - 1);
> > + generic_handle_domain_irq(irq_domain, hwirq - 1);
> > + }
> > +}
> > +
> > +static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
> > + irq_hw_number_t hwirq)
> > +{
> > + irq_set_noprobe(irq);
> > + irq_set_chip_and_handler(irq, &cpu_irq_controller, handle_percpu_irq);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct irq_domain_ops loongarch_cpu_intc_irq_domain_ops = {
> > + .map = loongarch_cpu_intc_map,
> > + .xlate = irq_domain_xlate_onecell,
> > +};
> > +
> > +struct irq_domain * __init loongarch_cpu_irq_init(void)
> > +{
> > + /* Mask interrupts. */
> > + clear_csr_ecfg(ECFG0_IM);
> > + clear_csr_estat(ESTATF_IP);
> > +
> > + irq_domain = irq_domain_add_legacy(NULL, EXCCODE_INT_NUM, 0, 0,
> > + &loongarch_cpu_intc_irq_domain_ops, NULL);
>
> I already commented on this in the past, and my position is still the
> same: this isn't a legacy architecture, you are not converting
> anything from a board file, so there is no reason why you get to use a
> legacy domain.
>
> Since you are using ACPI, irq_domain_add_*() really is the wrong API,
> as they take an of_node. Use irq_domain_create_linear(), and pass an
> actual fwnode there (there are plenty of examples in the tree).
>
Sorry, as I mentioned above, the only reason that I use legacy irqdomain here is to avoid to export get_xxx_irq functions for others(like arch files). As you recommend here, I'll recover them in next version.
> > + if (!irq_domain)
> > + panic("Failed to add irqdomain for LoongArch CPU");
> > +
> > + set_handle_irq(&handle_cpu_irq);
> > +
> > + return irq_domain;
>
> What uses this irq_domain in the arch code?
>
Thanks, sure, there is no need to return irq_domain， and I'll change it in next version.
> > +}
> > +#ifdef CONFIG_ACPI
>
> Why the #ifdef? Isn't this system supposed to be ACPI only? There is
> no DT support anyway, so you should make the driver depend on ACPI and
> that's about it.
>
Yes, we'll support DT in future(in fatct, DT for the driver has been supported in our internel repo for SOC products) for the driver as other irqchip drivers. Should we delete it now and take it into count later when adding DT supporting?
> > +static int __init
> > +liointc_parse_madt(union acpi_subtable_headers *header,
> > + const unsigned long end)
> > +{
> > + struct acpi_madt_lio_pic *liointc_entry = (struct acpi_madt_lio_pic *)header;
> > +
> > + return liointc_acpi_init(irq_domain, liointc_entry);
> > +}
> > +
> > +static int __init
> > +eiointc_parse_madt(union acpi_subtable_headers *header,
> > + const unsigned long end)
> > +{
> > + struct acpi_madt_eio_pic *eiointc_entry = (struct acpi_madt_eio_pic *)header;
> > +
> > + return eiointc_acpi_init(irq_domain, eiointc_entry);
> > +}
> > +static int __init acpi_cascade_irqdomain_init(void)
> > +{
> > + acpi_table_parse_madt(ACPI_MADT_TYPE_LIO_PIC,
> > + liointc_parse_madt, 0);
> > + acpi_table_parse_madt(ACPI_MADT_TYPE_EIO_PIC,
> > + eiointc_parse_madt, 0);
> > + return 0;
> > +}
> > +static int __init coreintc_acpi_init_v1(union acpi_subtable_headers *header,
> > + const unsigned long end)
> > +{
> > + if (irq_domain)
> > + return 0;
> > +
> > + init_vector_parent_group();
> > + loongarch_cpu_irq_init();
> > + acpi_cascade_irqdomain_init();
> > + return 0;
> > +}
> > +IRQCHIP_ACPI_DECLARE(coreintc_v1, ACPI_MADT_TYPE_CORE_PIC,
> > + NULL, ACPI_MADT_CORE_PIC_VERSION_V1,
> > + coreintc_acpi_init_v1);
> > +#endif
> > diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
> > new file mode 100644
> > index 0000000..94437e4
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-loongarch-pic-common.c
> > @@ -0,0 +1,201 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
> > + */
> > +
> > +#include <linux irq.h="">
> > +#include <linux acpi.h="">
> > +#include <linux pci.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 irq_domain *liointc_domain;
> > +struct irq_domain *pch_lpc_domain;
> > +struct irq_domain *pch_msi_domain[MAX_IO_PICS];
> > +struct irq_domain *pch_pic_domain[MAX_IO_PICS];
>
> Why isn't this static? If someone needs to know, why isn't there an
> accessor?
>
These irq_domains will be initialized in other irqchip drivers(e.g. liointc_domain is set in liointc driver).
>
> > +
> > +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;
> > +}
> > +
> > +int pcibios_device_add(struct pci_dev *dev)
> > +{
> > + int id = pci_domain_nr(dev->bus);
> > +
> > + dev_set_msi_domain(&dev->dev, pch_msi_domain[id]);
> > +
> > + return 0;
> > +}
>
> This doesn't belong here at all. Please move it to the PCI code.
>
Ok, I'll put them into PCI code of arch directory.
> > +
> > +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
> > +{
> > + if (irqp != NULL)
> > + *irqp = acpi_register_gsi(NULL, gsi, -1, -1);
> > + return (*irqp >= 0) ? 0 : -EINVAL;
> > +}
> > +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> > +
> > +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
> > +{
> > + if (gsi)
> > + *gsi = isa_irq;
> > + return 0;
> > +}
> > +
> > +/*
> > + * success: return IRQ number (>=0)
> > + * failure: return < 0
> > + */
> > +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
> > +{
> > + int id;
> > + struct irq_fwspec fwspec;
> > +
> > + switch (gsi) {
> > + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> > + fwspec.fwnode = liointc_domain->fwnode;
> > + fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
> > + fwspec.param_count = 1;
> > +
> > + return irq_create_fwspec_mapping(&fwspec);
> > +
> > + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> > + if (!pch_lpc_domain)
> > + return -EINVAL;
> > +
> > + fwspec.fwnode = pch_lpc_domain->fwnode;
> > + fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
> > + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
> > + fwspec.param_count = 2;
> > +
> > + return irq_create_fwspec_mapping(&fwspec);
> > +
> > + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> > + id = find_pch_pic(gsi);
> > + if (id < 0)
> > + return -EINVAL;
> > +
> > + fwspec.fwnode = pch_pic_domain[id]->fwnode;
> > + fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
> > + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
> > + fwspec.param_count = 2;
> > +
> > + return irq_create_fwspec_mapping(&fwspec);
> > + }
>
> So all the complexity here seems to stem from the fact that you deal
> with three ranges of interrupts, managed by three different pieces of
> code?
>
Yes.
> Other architectures have similar requirements, and don't require to
> re-implement a private version of the ACPI API. Instead, they expose a
> single irqdomain, and deal with the various ranges internally.
>
> Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue.
>
Thanks, I agree, that sounds a good and reasonable suggestion, and I'll reserach it further and reuse code from drivers/acpi/irq.c as can as possible.
> > +
> > + return -EINVAL;
> > +}
> > +EXPORT_SYMBOL_GPL(acpi_register_gsi);
> > +
> > +void acpi_unregister_gsi(u32 gsi)
> > +{
> > + int id, irq, hw_irq;
> > + struct irq_domain *d;
> > +
> > + switch (gsi) {
> > + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> > + if (!liointc_domain)
> > + return;
> > + d = liointc_domain;
> > + hw_irq = gsi - GSI_MIN_CPU_IRQ;
> > + break;
> > +
> > + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> > + if (!pch_lpc_domain)
> > + return;
> > + d = pch_lpc_domain;
> > + hw_irq = gsi - GSI_MIN_LPC_IRQ;
> > + break;
> > +
> > + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> > + id = find_pch_pic(gsi);
> > + if (id < 0)
> > + return;
> > + if (!pch_pic_domain[id])
> > + return;
> > + d = pch_pic_domain[id];
> > +
> > + hw_irq = gsi - acpi_pchpic[id]->gsi_base;
> > + break;
> > + }
> > + irq = irq_find_mapping(d, hw_irq);
> > + irq_dispose_mapping(irq);
> > +
> > + return;
> > +}
> > +EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
> > +
> > +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;
> > +}
>
> Again, why can't you reuse drivers/acpi/pci_mcfg.c?
>
Yes, I really want to reuse code from pci_mcfg.c, but I found that pci_mmcfg_late_init() is called from acpi_init during subsys_initcall. vector_group entries here are
needed initialzed during irqchip_init flow before EIOINTC, PCH PIC and PCH MSI initialization as I descripted info 'Example of irqchip topology in a system with two chipsets' in [PATCH RFC V2 00/10].
> > +
> > +void __init init_vector_parent_group(void)
> > +{
> > + acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
> > +}
> > +
> > +void acpi_set_vector_parent(int node, struct irq_domain *parent)
> > +{
> > + int i;
> > +
> > + if (cpu_has_flatmode)
> > + node = cpu_to_node(node * CORES_PER_EIO_NODE);
> > +
> > + for (i = 0; i < MAX_IO_PICS; i++) {
> > + if (node == vector_group[i].node) {
> > + vector_group[i].parent = parent;
> > + return;
> > + }
> > + }
> > +}
> > +
> > +struct irq_domain *acpi_get_msi_parent(int index)
> > +{
> > + return vector_group[index].parent;
> > +}
> > +
> > +struct irq_domain *acpi_get_pch_parent(int node)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < MAX_IO_PICS; i++) {
> > + if (node == vector_group[i].node)
> > + return vector_group[i].parent;
> > + }
> > + return NULL;
> > +}
> > diff --git a/drivers/irqchip/irq-loongarch-pic-common.h b/drivers/irqchip/irq-loongarch-pic-common.h
> > new file mode 100644
> > index 0000000..3815fc9
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-loongarch-pic-common.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
> > + */
> > +
> > +#ifndef _IRQ_LOONGARCH_PIC_COMMON_H
> > +#define _IRQ_LOONGARCH_PIC_COMMON_H
> > +
> > +#include <linux of.h="">
> > +#include <linux irqdomain.h="">
> > +
> > +struct acpi_vector_group {
> > + int node;
> > + struct irq_domain *parent;
> > +};
> > +
> > +/* IRQ number definitions */
> > +#define LOONGSON_LPC_IRQ_BASE 0
> > +#define LOONGSON_CPU_IRQ_BASE 16
> > +#define LOONGSON_PCH_IRQ_BASE 64
> > +
> > +#define GSI_MIN_LPC_IRQ LOONGSON_LPC_IRQ_BASE
> > +#define GSI_MAX_LPC_IRQ (LOONGSON_LPC_IRQ_BASE + 16 - 1)
> > +#define GSI_MIN_CPU_IRQ LOONGSON_CPU_IRQ_BASE
> > +#define GSI_MAX_CPU_IRQ (LOONGSON_CPU_IRQ_BASE + 48 - 1)
> > +#define GSI_MIN_PCH_IRQ LOONGSON_PCH_IRQ_BASE
> > +#define GSI_MAX_PCH_IRQ (LOONGSON_PCH_IRQ_BASE + 256 - 1)
> > +
> > +extern struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
> > +extern struct irq_domain *liointc_domain;
> > +extern struct irq_domain *pch_lpc_domain;
> > +extern struct irq_domain *pch_msi_domain[MAX_IO_PICS];
> > +extern struct irq_domain *pch_pic_domain[MAX_IO_PICS];
> > +
> > +int liointc_acpi_init(struct irq_domain *parent, struct acpi_madt_lio_pic *acpi_liointc);
> > +int eiointc_acpi_init(struct irq_domain *parent, struct acpi_madt_eio_pic *acpi_eiointc);
> > +int htvec_acpi_init(struct irq_domain *parent, struct acpi_madt_ht_pic *acpi_htvec);
> > +int pch_lpc_acpi_init(struct irq_domain *parent, struct acpi_madt_lpc_pic *acpi_pchlpc);
> > +void init_vector_parent_group(void);
> > +void acpi_set_vector_parent(int node, struct irq_domain *parent);
> > +struct irq_domain *acpi_get_msi_parent(int index);
> > +struct irq_domain *acpi_get_pch_parent(int node);
> > +
> > +#endif /* _IRQ_LOONGARCH_PIC_COMMON_H */
> > --
> > 1.8.3.1
> >
> >
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
</linux></linux></linux></linux></linux></asm></asm></linux></linux></linux></linux></linux></linux></lvjianmin@loongson.cn></chenhuacai@loongson.cn></lvjianmin@loongson.cn></chenhuacai@loongson.cn></jiaxun.yang@flygoat.com></chenhuacai@gmail.com></lixuefeng@loongson.cn></tglx@linutronix.de></lvjianmin@loongson.cn></maz@kernel.org>
本邮件及其附件含有龙芯中科的商业秘密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制或散发）本邮件及其附件中的信息。如果您错收本邮件，请您立即电话或邮件通知发件人并删除本邮件。
This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-05-31 11:01 ` 吕建民
@ 2022-05-31 13:56 ` Marc Zyngier
2022-06-01 3:35 ` Jianmin Lv
2022-06-02 3:16 ` Jianmin Lv
0 siblings, 2 replies; 15+ messages in thread
From: Marc Zyngier @ 2022-05-31 13:56 UTC (permalink / raw)
To: 吕建民
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen
On Tue, 31 May 2022 12:01:09 +0100,
吕建民 <lvjianmin@loongson.cn> wrote:
Please fix your email setup. I've done some cleanup to be able read
your email and reply to it, but it'd be better if it was clean the
first place.
>
> > -----Original Messages-----
> > From: "Marc Zyngier" <maz@kernel.org>
> > Sent Time: 2022-05-31 00:20:31 (Tuesday)
> > To: "Jianmin Lv" <lvjianmin@loongson.cn>
> > Cc: "Thomas Gleixner" <tglx@linutronix.de>, linux-kernel@vger.kernel.org, "Xuefeng Li" <lixuefeng@loongson.cn>, "Huacai Chen" <chenhuacai@gmail.com>, "Jiaxun Yang" <jiaxun.yang@flygoat.com>, "Huacai Chen" <chenhuacai@loongson.cn>
> > Subject: Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
> >
> > On Fri, 27 May 2022 12:02:12 +0100,
> > Jianmin Lv <lvjianmin@loongson.cn> wrote:
> > >
> > > We are preparing to add new Loongson (based on LoongArch, not compatible
> > > with old MIPS-based Loongson) support. This patch add the LoongArch CPU
> > > interrupt controller support.
> >
> > Please drop this paragraph, as it doesn't help at all.
> >
> Ok,thanks, I'll drop it.
>
> > >
> > > LoongArch CPUINTC stands for CSR.ECFG/CSR.ESTAT and related interrupt
> > > controller that described in Section 7.4 of "LoongArch Reference Manual,
> > > Vol 1". For more information please refer Documentation/loongarch/irq-
> > > chip-model.rst.
> >
> > Where is this the patch adding this document?
> >
> See https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html
>
I was referring to the irq-chip-mode.rst file. I don't see it as part
of the series, while it would probably be useful.
> > >
> > > LoongArch CPUINTC has 13 interrupt sources: SWI0~1, HWI0~7, IPI, TI
> > > (Timer) and PCOV (PMC). IRQ mappings of HWI0~7 are configurable (can be
> > > created from DT/ACPI), but IPI, TI (Timer) and PCOV (PMC) are hardcoded
> > > bits, so we define get_xxx_irq() for them.
> >
> > Where are these functions? How are they used?
> >
> Sorry, these functions are implemented in previours version, and
> they are deleted in current version because I changed to use legacy
> irqdomain in this version so that we don't have to use these
> functions to create irq mapping for IPI, PMC and TIMER. Of cause, if
> you sugguest us to use linear irqdomain, I'll restore them to be
> what like as last version.
I'm not sure there is any need for them. Why does the irqchip care
about the interrupt number of a function or the other? The hwirq->irq
mapping is always a SW construct, and if the driver cannot extract the
information from DT/ACPI, it can still pull the number from wherever
it wants.
>
> > >
> > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> > > ---
> > > drivers/irqchip/Kconfig | 10 ++
> > > drivers/irqchip/Makefile | 1 +
> > > drivers/irqchip/irq-loongarch-cpu.c | 115 +++++++++++++++++
> > > drivers/irqchip/irq-loongarch-pic-common.c | 201 +++++++++++++++++++++++++++++
> >
> > One patch per driver, please.
> >
> Ok, I'll split them to be seperate patch.
>
> > > drivers/irqchip/irq-loongarch-pic-common.h | 44 +++++++
> > > 5 files changed, 371 insertions(+)
> > > create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
> > > 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/Kconfig b/drivers/irqchip/Kconfig
> > > index 39d6be2..a596ee7 100644
> > > --- a/drivers/irqchip/Kconfig
> > > +++ b/drivers/irqchip/Kconfig
> > > @@ -545,6 +545,16 @@ config EXYNOS_IRQ_COMBINER
> > > Say yes here to add support for the IRQ combiner devices embedded
> > > in Samsung Exynos chips.
> > >
> > > +config IRQ_LOONGARCH_CPU
> > > + bool
> > > + select GENERIC_IRQ_CHIP
> > > + select IRQ_DOMAIN
> > > + select GENERIC_IRQ_EFFECTIVE_AFF_MASK
> > > + help
> > > + Support for the LoongArch CPU Interrupt Controller. For details of
> > > + irq chip hierarchy on LoongArch platforms please read the document
> > > + Documentation/loongarch/irq-chip-model.rst.
> > > +
> > > config LOONGSON_LIOINTC
> > > bool "Loongson Local I/O Interrupt Controller"
> > > depends on MACH_LOONGSON64
> > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > > index 160a1d8..736f030 100644
> > > --- a/drivers/irqchip/Makefile
> > > +++ b/drivers/irqchip/Makefile
> > > @@ -105,6 +105,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 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-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
> > > new file mode 100644
> > > index 0000000..26f948f
> > > --- /dev/null
> > > +++ b/drivers/irqchip/irq-loongarch-cpu.c
> > > @@ -0,0 +1,115 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
> > > + */
> > > +
> > > +#include <linux init.h="">
> > > +#include <linux kernel.h="">
> > > +#include <linux interrupt.h="">
> > > +#include <linux irq.h="">
> > > +#include <linux irqchip.h="">
> > > +#include <linux irqdomain.h="">
> > > +
> > > +#include <asm loongarch.h="">
> > > +#include <asm setup.h="">
> > > +#include "irq-loongarch-pic-common.h"
> > > +
> > > +static struct irq_domain *irq_domain;
> > > +
> > > +static void mask_loongarch_irq(struct irq_data *d)
> > > +{
> > > + clear_csr_ecfg(ECFGF(d->hwirq));
> > > +}
> > > +
> > > +static void unmask_loongarch_irq(struct irq_data *d)
> > > +{
> > > + set_csr_ecfg(ECFGF(d->hwirq));
> > > +}
> > > +
> > > +static struct irq_chip cpu_irq_controller = {
> > > + .name = "LoongArch",
> > > + .irq_mask = mask_loongarch_irq,
> > > + .irq_unmask = unmask_loongarch_irq,
> > > +};
> > > +
> > > +static void handle_cpu_irq(struct pt_regs *regs)
> > > +{
> > > + int hwirq;
> > > + unsigned int estat = read_csr_estat() & CSR_ESTAT_IS;
> > > +
> > > + while ((hwirq = ffs(estat))) {
> > > + estat &= ~BIT(hwirq - 1);
> > > + generic_handle_domain_irq(irq_domain, hwirq - 1);
> > > + }
> > > +}
> > > +
> > > +static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
> > > + irq_hw_number_t hwirq)
> > > +{
> > > + irq_set_noprobe(irq);
> > > + irq_set_chip_and_handler(irq, &cpu_irq_controller, handle_percpu_irq);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct irq_domain_ops loongarch_cpu_intc_irq_domain_ops = {
> > > + .map = loongarch_cpu_intc_map,
> > > + .xlate = irq_domain_xlate_onecell,
> > > +};
> > > +
> > > +struct irq_domain * __init loongarch_cpu_irq_init(void)
> > > +{
> > > + /* Mask interrupts. */
> > > + clear_csr_ecfg(ECFG0_IM);
> > > + clear_csr_estat(ESTATF_IP);
> > > +
> > > + irq_domain = irq_domain_add_legacy(NULL, EXCCODE_INT_NUM, 0, 0,
> > > + &loongarch_cpu_intc_irq_domain_ops, NULL);
> >
> > I already commented on this in the past, and my position is still the
> > same: this isn't a legacy architecture, you are not converting
> > anything from a board file, so there is no reason why you get to use a
> > legacy domain.
> >
> > Since you are using ACPI, irq_domain_add_*() really is the wrong API,
> > as they take an of_node. Use irq_domain_create_linear(), and pass an
> > actual fwnode there (there are plenty of examples in the tree).
> >
> Sorry, as I mentioned above, the only reason that I use legacy
> irqdomain here is to avoid to export get_xxx_irq functions for
> others(like arch files). As you recommend here, I'll recover them in
> next version.
>
> > > + if (!irq_domain)
> > > + panic("Failed to add irqdomain for LoongArch CPU");
> > > +
> > > + set_handle_irq(&handle_cpu_irq);
> > > +
> > > + return irq_domain;
> >
> > What uses this irq_domain in the arch code?
> >
> Thanks, sure, there is no need to return irq_domain， and I'll
> change it in next version.
>
> > > +}
> > > +#ifdef CONFIG_ACPI
> >
> > Why the #ifdef? Isn't this system supposed to be ACPI only? There is
> > no DT support anyway, so you should make the driver depend on ACPI and
> > that's about it.
> >
> Yes, we'll support DT in future(in fatct, DT for the driver has been
> supported in our internel repo for SOC products) for the driver as
> other irqchip drivers. Should we delete it now and take it into
> count later when adding DT supporting?
Drop everything that isn't immediately useful. For example, who cares
about suspend-resume, which is half of your series? Please focus on
the bare minimal to get your system up and running.
>
> > > +static int __init
> > > +liointc_parse_madt(union acpi_subtable_headers *header,
> > > + const unsigned long end)
> > > +{
> > > + struct acpi_madt_lio_pic *liointc_entry = (struct acpi_madt_lio_pic *)header;
> > > +
> > > + return liointc_acpi_init(irq_domain, liointc_entry);
> > > +}
> > > +
> > > +static int __init
> > > +eiointc_parse_madt(union acpi_subtable_headers *header,
> > > + const unsigned long end)
> > > +{
> > > + struct acpi_madt_eio_pic *eiointc_entry = (struct acpi_madt_eio_pic *)header;
> > > +
> > > + return eiointc_acpi_init(irq_domain, eiointc_entry);
> > > +}
> > > +static int __init acpi_cascade_irqdomain_init(void)
> > > +{
> > > + acpi_table_parse_madt(ACPI_MADT_TYPE_LIO_PIC,
> > > + liointc_parse_madt, 0);
> > > + acpi_table_parse_madt(ACPI_MADT_TYPE_EIO_PIC,
> > > + eiointc_parse_madt, 0);
> > > + return 0;
> > > +}
> > > +static int __init coreintc_acpi_init_v1(union acpi_subtable_headers *header,
> > > + const unsigned long end)
> > > +{
> > > + if (irq_domain)
> > > + return 0;
> > > +
> > > + init_vector_parent_group();
> > > + loongarch_cpu_irq_init();
> > > + acpi_cascade_irqdomain_init();
> > > + return 0;
> > > +}
> > > +IRQCHIP_ACPI_DECLARE(coreintc_v1, ACPI_MADT_TYPE_CORE_PIC,
> > > + NULL, ACPI_MADT_CORE_PIC_VERSION_V1,
> > > + coreintc_acpi_init_v1);
> > > +#endif
> > > diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
> > > new file mode 100644
> > > index 0000000..94437e4
> > > --- /dev/null
> > > +++ b/drivers/irqchip/irq-loongarch-pic-common.c
> > > @@ -0,0 +1,201 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
> > > + */
> > > +
> > > +#include <linux irq.h="">
> > > +#include <linux acpi.h="">
> > > +#include <linux pci.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 irq_domain *liointc_domain;
> > > +struct irq_domain *pch_lpc_domain;
> > > +struct irq_domain *pch_msi_domain[MAX_IO_PICS];
> > > +struct irq_domain *pch_pic_domain[MAX_IO_PICS];
> >
> > Why isn't this static? If someone needs to know, why isn't there an
> > accessor?
> >
> These irq_domains will be initialized in other irqchip
> drivers(e.g. liointc_domain is set in liointc driver).
Really, there shouldn't any need to keep domain references around at
all. That's why we have fwnodes, to be able to retrive them from the
list of existing domains. If you have to keep all these domain
references around, you're doing something wrong.
>
> >
> > > +
> > > +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;
> > > +}
> > > +
> > > +int pcibios_device_add(struct pci_dev *dev)
> > > +{
> > > + int id = pci_domain_nr(dev->bus);
> > > +
> > > + dev_set_msi_domain(&dev->dev, pch_msi_domain[id]);
> > > +
> > > + return 0;
> > > +}
> >
> > This doesn't belong here at all. Please move it to the PCI code.
> >
> Ok, I'll put them into PCI code of arch directory.
>
> > > +
> > > +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
> > > +{
> > > + if (irqp != NULL)
> > > + *irqp = acpi_register_gsi(NULL, gsi, -1, -1);
> > > + return (*irqp >= 0) ? 0 : -EINVAL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> > > +
> > > +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
> > > +{
> > > + if (gsi)
> > > + *gsi = isa_irq;
> > > + return 0;
> > > +}
> > > +
> > > +/*
> > > + * success: return IRQ number (>=0)
> > > + * failure: return < 0
> > > + */
> > > +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
> > > +{
> > > + int id;
> > > + struct irq_fwspec fwspec;
> > > +
> > > + switch (gsi) {
> > > + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> > > + fwspec.fwnode = liointc_domain->fwnode;
> > > + fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
> > > + fwspec.param_count = 1;
> > > +
> > > + return irq_create_fwspec_mapping(&fwspec);
> > > +
> > > + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> > > + if (!pch_lpc_domain)
> > > + return -EINVAL;
> > > +
> > > + fwspec.fwnode = pch_lpc_domain->fwnode;
> > > + fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
> > > + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
> > > + fwspec.param_count = 2;
> > > +
> > > + return irq_create_fwspec_mapping(&fwspec);
> > > +
> > > + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> > > + id = find_pch_pic(gsi);
> > > + if (id < 0)
> > > + return -EINVAL;
> > > +
> > > + fwspec.fwnode = pch_pic_domain[id]->fwnode;
> > > + fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
> > > + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
> > > + fwspec.param_count = 2;
> > > +
> > > + return irq_create_fwspec_mapping(&fwspec);
> > > + }
> >
> > So all the complexity here seems to stem from the fact that you deal
> > with three ranges of interrupts, managed by three different pieces of
> > code?
> >
> Yes.
>
> > Other architectures have similar requirements, and don't require to
> > re-implement a private version of the ACPI API. Instead, they expose a
> > single irqdomain, and deal with the various ranges internally.
> >
> > Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue.
> >
> Thanks, I agree, that sounds a good and reasonable suggestion, and
> I'll reserach it further and reuse code from drivers/acpi/irq.c as
> can as possible.
>
> > > +
> > > + return -EINVAL;
> > > +}
> > > +EXPORT_SYMBOL_GPL(acpi_register_gsi);
> > > +
> > > +void acpi_unregister_gsi(u32 gsi)
> > > +{
> > > + int id, irq, hw_irq;
> > > + struct irq_domain *d;
> > > +
> > > + switch (gsi) {
> > > + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> > > + if (!liointc_domain)
> > > + return;
> > > + d = liointc_domain;
> > > + hw_irq = gsi - GSI_MIN_CPU_IRQ;
> > > + break;
> > > +
> > > + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> > > + if (!pch_lpc_domain)
> > > + return;
> > > + d = pch_lpc_domain;
> > > + hw_irq = gsi - GSI_MIN_LPC_IRQ;
> > > + break;
> > > +
> > > + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> > > + id = find_pch_pic(gsi);
> > > + if (id < 0)
> > > + return;
> > > + if (!pch_pic_domain[id])
> > > + return;
> > > + d = pch_pic_domain[id];
> > > +
> > > + hw_irq = gsi - acpi_pchpic[id]->gsi_base;
> > > + break;
> > > + }
> > > + irq = irq_find_mapping(d, hw_irq);
> > > + irq_dispose_mapping(irq);
> > > +
> > > + return;
> > > +}
> > > +EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
> > > +
> > > +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;
> > > +}
> >
> > Again, why can't you reuse drivers/acpi/pci_mcfg.c?
> >
> Yes, I really want to reuse code from pci_mcfg.c, but I found that
> pci_mmcfg_late_init() is called from acpi_init during
> subsys_initcall. vector_group entries here are needed initialzed
> during irqchip_init flow before EIOINTC, PCH PIC and PCH MSI
> initialization as I descripted info 'Example of irqchip topology in
> a system with two chipsets' in [PATCH RFC V2 00/10].
I'm not sure why this is needed. Can't this be done at a later time?
Surely, no PCI device can come up without the ACPI resources having
been populated. And if the PCI bus comes up before, you should be able
to defer it.
In any case, this doesn't seem to belong the an irqchip driver, and is
much more a PCI thing.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-05-31 13:56 ` Marc Zyngier
@ 2022-06-01 3:35 ` Jianmin Lv
2022-06-02 3:16 ` Jianmin Lv
1 sibling, 0 replies; 15+ messages in thread
From: Jianmin Lv @ 2022-06-01 3:35 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen
On 2022/5/31 下午9:56, Marc Zyngier wrote:
> On Tue, 31 May 2022 12:01:09 +0100,
> 吕建民 <lvjianmin@loongson.cn> wrote:
>
> Please fix your email setup. I've done some cleanup to be able read
> your email and reply to it, but it'd be better if it was clean the
> first place.
Thanks, Marc. I'm so sorry to reply with a wrong setup computer last time.
>>> -----Original Messages-----
>>> From: "Marc Zyngier" <maz@kernel.org>
>>> Sent Time: 2022-05-31 00:20:31 (Tuesday)
>>> To: "Jianmin Lv" <lvjianmin@loongson.cn>
>>> Cc: "Thomas Gleixner" <tglx@linutronix.de>, linux-kernel@vger.kernel.org, "Xuefeng Li" <lixuefeng@loongson.cn>, "Huacai Chen" <chenhuacai@gmail.com>, "Jiaxun Yang" <jiaxun.yang@flygoat.com>, "Huacai Chen" <chenhuacai@loongson.cn>
>>> Subject: Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
>>>
>>> On Fri, 27 May 2022 12:02:12 +0100,
>>> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>>> We are preparing to add new Loongson (based on LoongArch, not compatible
>>>> with old MIPS-based Loongson) support. This patch add the LoongArch CPU
>>>> interrupt controller support.
>>> Please drop this paragraph, as it doesn't help at all.
>>>
>> Ok,thanks, I'll drop it.
>>
>>>> LoongArch CPUINTC stands for CSR.ECFG/CSR.ESTAT and related interrupt
>>>> controller that described in Section 7.4 of "LoongArch Reference Manual,
>>>> Vol 1". For more information please refer Documentation/loongarch/irq-
>>>> chip-model.rst.
>>> Where is this the patch adding this document?
>>>
>> See https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html
>>
> I was referring to the irq-chip-mode.rst file. I don't see it as part
> of the series, while it would probably be useful.
The irq-chip-mode.rst file is in the link:
https://lore.kernel.org/linux-arch/20220518092619.1269111-1-chenhuacai@loongson.cn/T/#mb9699cd9aec2708b39e1c7a2a59d888dc520bb1f
>>>> LoongArch CPUINTC has 13 interrupt sources: SWI0~1, HWI0~7, IPI, TI
>>>> (Timer) and PCOV (PMC). IRQ mappings of HWI0~7 are configurable (can be
>>>> created from DT/ACPI), but IPI, TI (Timer) and PCOV (PMC) are hardcoded
>>>> bits, so we define get_xxx_irq() for them.
>>> Where are these functions? How are they used?
>>>
>> Sorry, these functions are implemented in previours version, and
>> they are deleted in current version because I changed to use legacy
>> irqdomain in this version so that we don't have to use these
>> functions to create irq mapping for IPI, PMC and TIMER. Of cause, if
>> you sugguest us to use linear irqdomain, I'll restore them to be
>> what like as last version.
> I'm not sure there is any need for them. Why does the irqchip care
> about the interrupt number of a function or the other? The hwirq->irq
> mapping is always a SW construct, and if the driver cannot extract the
> information from DT/ACPI, it can still pull the number from wherever
> it wants.
In previous version, these functions are defined as following:
int get_ipi_irq(void)
{
return irq_create_mapping(irq_domain, EXCCODE_IPI -
EXCCODE_INT_START);
}
int get_pmc_irq(void)
{
return irq_create_mapping(irq_domain, EXCCODE_PMC -
EXCCODE_INT_START);
}
int get_timer_irq(void)
{
return irq_create_mapping(irq_domain, EXCCODE_TIMER -
EXCCODE_INT_START);
}
After exporting these functions, in arch directory files, ipi and timer
irq will be setuped with irq number from these functions. For example,
in /loongarch/kernel/time.c, timer irq is setuped as following:
irq = get_timer_irq();
if (request_irq(irq, constant_timer_interrupt, IRQF_PERCPU | IRQF_TIMER,
"timer", NULL))
pr_err("Failed to request irq %d (timer)\n", irq);
When handing irq, the irq is fatched for the hwirq from the mapping in
the irq_domain.
>>>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>>>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>>>> ---
>>>> drivers/irqchip/Kconfig | 10 ++
>>>> drivers/irqchip/Makefile | 1 +
>>>> drivers/irqchip/irq-loongarch-cpu.c | 115 +++++++++++++++++
>>>> drivers/irqchip/irq-loongarch-pic-common.c | 201 +++++++++++++++++++++++++++++
>>> One patch per driver, please.
>>>
>> Ok, I'll split them to be seperate patch.
>>
>>>> drivers/irqchip/irq-loongarch-pic-common.h | 44 +++++++
>>>> 5 files changed, 371 insertions(+)
>>>> create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
>>>> 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/Kconfig b/drivers/irqchip/Kconfig
>>>> index 39d6be2..a596ee7 100644
>>>> --- a/drivers/irqchip/Kconfig
>>>> +++ b/drivers/irqchip/Kconfig
>>>> @@ -545,6 +545,16 @@ config EXYNOS_IRQ_COMBINER
>>>> Say yes here to add support for the IRQ combiner devices embedded
>>>> in Samsung Exynos chips.
>>>>
>>>> +config IRQ_LOONGARCH_CPU
>>>> + bool
>>>> + select GENERIC_IRQ_CHIP
>>>> + select IRQ_DOMAIN
>>>> + select GENERIC_IRQ_EFFECTIVE_AFF_MASK
>>>> + help
>>>> + Support for the LoongArch CPU Interrupt Controller. For details of
>>>> + irq chip hierarchy on LoongArch platforms please read the document
>>>> + Documentation/loongarch/irq-chip-model.rst.
>>>> +
>>>> config LOONGSON_LIOINTC
>>>> bool "Loongson Local I/O Interrupt Controller"
>>>> depends on MACH_LOONGSON64
>>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>>> index 160a1d8..736f030 100644
>>>> --- a/drivers/irqchip/Makefile
>>>> +++ b/drivers/irqchip/Makefile
>>>> @@ -105,6 +105,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 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-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
>>>> new file mode 100644
>>>> index 0000000..26f948f
>>>> --- /dev/null
>>>> +++ b/drivers/irqchip/irq-loongarch-cpu.c
>>>> @@ -0,0 +1,115 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
>>>> + */
>>>> +
>>>> +#include <linux init.h="">
>>>> +#include <linux kernel.h="">
>>>> +#include <linux interrupt.h="">
>>>> +#include <linux irq.h="">
>>>> +#include <linux irqchip.h="">
>>>> +#include <linux irqdomain.h="">
>>>> +
>>>> +#include <asm loongarch.h="">
>>>> +#include <asm setup.h="">
>>>> +#include "irq-loongarch-pic-common.h"
>>>> +
>>>> +static struct irq_domain *irq_domain;
>>>> +
>>>> +static void mask_loongarch_irq(struct irq_data *d)
>>>> +{
>>>> + clear_csr_ecfg(ECFGF(d->hwirq));
>>>> +}
>>>> +
>>>> +static void unmask_loongarch_irq(struct irq_data *d)
>>>> +{
>>>> + set_csr_ecfg(ECFGF(d->hwirq));
>>>> +}
>>>> +
>>>> +static struct irq_chip cpu_irq_controller = {
>>>> + .name = "LoongArch",
>>>> + .irq_mask = mask_loongarch_irq,
>>>> + .irq_unmask = unmask_loongarch_irq,
>>>> +};
>>>> +
>>>> +static void handle_cpu_irq(struct pt_regs *regs)
>>>> +{
>>>> + int hwirq;
>>>> + unsigned int estat = read_csr_estat() & CSR_ESTAT_IS;
>>>> +
>>>> + while ((hwirq = ffs(estat))) {
>>>> + estat &= ~BIT(hwirq - 1);
>>>> + generic_handle_domain_irq(irq_domain, hwirq - 1);
>>>> + }
>>>> +}
>>>> +
>>>> +static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
>>>> + irq_hw_number_t hwirq)
>>>> +{
>>>> + irq_set_noprobe(irq);
>>>> + irq_set_chip_and_handler(irq, &cpu_irq_controller, handle_percpu_irq);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct irq_domain_ops loongarch_cpu_intc_irq_domain_ops = {
>>>> + .map = loongarch_cpu_intc_map,
>>>> + .xlate = irq_domain_xlate_onecell,
>>>> +};
>>>> +
>>>> +struct irq_domain * __init loongarch_cpu_irq_init(void)
>>>> +{
>>>> + /* Mask interrupts. */
>>>> + clear_csr_ecfg(ECFG0_IM);
>>>> + clear_csr_estat(ESTATF_IP);
>>>> +
>>>> + irq_domain = irq_domain_add_legacy(NULL, EXCCODE_INT_NUM, 0, 0,
>>>> + &loongarch_cpu_intc_irq_domain_ops, NULL);
>>> I already commented on this in the past, and my position is still the
>>> same: this isn't a legacy architecture, you are not converting
>>> anything from a board file, so there is no reason why you get to use a
>>> legacy domain.
>>>
>>> Since you are using ACPI, irq_domain_add_*() really is the wrong API,
>>> as they take an of_node. Use irq_domain_create_linear(), and pass an
>>> actual fwnode there (there are plenty of examples in the tree).
>>>
>> Sorry, as I mentioned above, the only reason that I use legacy
>> irqdomain here is to avoid to export get_xxx_irq functions for
>> others(like arch files). As you recommend here, I'll recover them in
>> next version.
>>
>>>> + if (!irq_domain)
>>>> + panic("Failed to add irqdomain for LoongArch CPU");
>>>> +
>>>> + set_handle_irq(&handle_cpu_irq);
>>>> +
>>>> + return irq_domain;
>>> What uses this irq_domain in the arch code?
>>>
>> Thanks, sure, there is no need to return irq_domain， and I'll
>> change it in next version.
>>
>>>> +}
>>>> +#ifdef CONFIG_ACPI
>>> Why the #ifdef? Isn't this system supposed to be ACPI only? There is
>>> no DT support anyway, so you should make the driver depend on ACPI and
>>> that's about it.
>>>
>> Yes, we'll support DT in future(in fatct, DT for the driver has been
>> supported in our internel repo for SOC products) for the driver as
>> other irqchip drivers. Should we delete it now and take it into
>> count later when adding DT supporting?
> Drop everything that isn't immediately useful. For example, who cares
> about suspend-resume, which is half of your series? Please focus on
> the bare minimal to get your system up and running.
Ok, thanks for your suggestion, I'll drop them all in next version and
submit them as separate series.
>>>> +static int __init
>>>> +liointc_parse_madt(union acpi_subtable_headers *header,
>>>> + const unsigned long end)
>>>> +{
>>>> + struct acpi_madt_lio_pic *liointc_entry = (struct acpi_madt_lio_pic *)header;
>>>> +
>>>> + return liointc_acpi_init(irq_domain, liointc_entry);
>>>> +}
>>>> +
>>>> +static int __init
>>>> +eiointc_parse_madt(union acpi_subtable_headers *header,
>>>> + const unsigned long end)
>>>> +{
>>>> + struct acpi_madt_eio_pic *eiointc_entry = (struct acpi_madt_eio_pic *)header;
>>>> +
>>>> + return eiointc_acpi_init(irq_domain, eiointc_entry);
>>>> +}
>>>> +static int __init acpi_cascade_irqdomain_init(void)
>>>> +{
>>>> + acpi_table_parse_madt(ACPI_MADT_TYPE_LIO_PIC,
>>>> + liointc_parse_madt, 0);
>>>> + acpi_table_parse_madt(ACPI_MADT_TYPE_EIO_PIC,
>>>> + eiointc_parse_madt, 0);
>>>> + return 0;
>>>> +}
>>>> +static int __init coreintc_acpi_init_v1(union acpi_subtable_headers *header,
>>>> + const unsigned long end)
>>>> +{
>>>> + if (irq_domain)
>>>> + return 0;
>>>> +
>>>> + init_vector_parent_group();
>>>> + loongarch_cpu_irq_init();
>>>> + acpi_cascade_irqdomain_init();
>>>> + return 0;
>>>> +}
>>>> +IRQCHIP_ACPI_DECLARE(coreintc_v1, ACPI_MADT_TYPE_CORE_PIC,
>>>> + NULL, ACPI_MADT_CORE_PIC_VERSION_V1,
>>>> + coreintc_acpi_init_v1);
>>>> +#endif
>>>> diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
>>>> new file mode 100644
>>>> index 0000000..94437e4
>>>> --- /dev/null
>>>> +++ b/drivers/irqchip/irq-loongarch-pic-common.c
>>>> @@ -0,0 +1,201 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
>>>> + */
>>>> +
>>>> +#include <linux irq.h="">
>>>> +#include <linux acpi.h="">
>>>> +#include <linux pci.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 irq_domain *liointc_domain;
>>>> +struct irq_domain *pch_lpc_domain;
>>>> +struct irq_domain *pch_msi_domain[MAX_IO_PICS];
>>>> +struct irq_domain *pch_pic_domain[MAX_IO_PICS];
>>> Why isn't this static? If someone needs to know, why isn't there an
>>> accessor?
>>>
>> These irq_domains will be initialized in other irqchip
>> drivers(e.g. liointc_domain is set in liointc driver).
> Really, there shouldn't any need to keep domain references around at
> all. That's why we have fwnodes, to be able to retrive them from the
> list of existing domains. If you have to keep all these domain
> references around, you're doing something wrong.
Thanks, I got what you mean, maybe I should export fwnode_handle as
following:
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];
and find domain by these handle in the created domins list, yes?
>>>> +
>>>> +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;
>>>> +}
>>>> +
>>>> +int pcibios_device_add(struct pci_dev *dev)
>>>> +{
>>>> + int id = pci_domain_nr(dev->bus);
>>>> +
>>>> + dev_set_msi_domain(&dev->dev, pch_msi_domain[id]);
>>>> +
>>>> + return 0;
>>>> +}
>>> This doesn't belong here at all. Please move it to the PCI code.
>>>
>> Ok, I'll put them into PCI code of arch directory.
>>
>>>> +
>>>> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
>>>> +{
>>>> + if (irqp != NULL)
>>>> + *irqp = acpi_register_gsi(NULL, gsi, -1, -1);
>>>> + return (*irqp >= 0) ? 0 : -EINVAL;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>>>> +
>>>> +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
>>>> +{
>>>> + if (gsi)
>>>> + *gsi = isa_irq;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * success: return IRQ number (>=0)
>>>> + * failure: return < 0
>>>> + */
>>>> +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
>>>> +{
>>>> + int id;
>>>> + struct irq_fwspec fwspec;
>>>> +
>>>> + switch (gsi) {
>>>> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
>>>> + fwspec.fwnode = liointc_domain->fwnode;
>>>> + fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
>>>> + fwspec.param_count = 1;
>>>> +
>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>> +
>>>> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
>>>> + if (!pch_lpc_domain)
>>>> + return -EINVAL;
>>>> +
>>>> + fwspec.fwnode = pch_lpc_domain->fwnode;
>>>> + fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
>>>> + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
>>>> + fwspec.param_count = 2;
>>>> +
>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>> +
>>>> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
>>>> + id = find_pch_pic(gsi);
>>>> + if (id < 0)
>>>> + return -EINVAL;
>>>> +
>>>> + fwspec.fwnode = pch_pic_domain[id]->fwnode;
>>>> + fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
>>>> + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
>>>> + fwspec.param_count = 2;
>>>> +
>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>> + }
>>> So all the complexity here seems to stem from the fact that you deal
>>> with three ranges of interrupts, managed by three different pieces of
>>> code?
>>>
>> Yes.
>>
>>> Other architectures have similar requirements, and don't require to
>>> re-implement a private version of the ACPI API. Instead, they expose a
>>> single irqdomain, and deal with the various ranges internally.
>>>
>>> Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue.
>>>
>> Thanks, I agree, that sounds a good and reasonable suggestion, and
>> I'll reserach it further and reuse code from drivers/acpi/irq.c as
>> can as possible.
>>
>>>> +
>>>> + return -EINVAL;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(acpi_register_gsi);
>>>> +
>>>> +void acpi_unregister_gsi(u32 gsi)
>>>> +{
>>>> + int id, irq, hw_irq;
>>>> + struct irq_domain *d;
>>>> +
>>>> + switch (gsi) {
>>>> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
>>>> + if (!liointc_domain)
>>>> + return;
>>>> + d = liointc_domain;
>>>> + hw_irq = gsi - GSI_MIN_CPU_IRQ;
>>>> + break;
>>>> +
>>>> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
>>>> + if (!pch_lpc_domain)
>>>> + return;
>>>> + d = pch_lpc_domain;
>>>> + hw_irq = gsi - GSI_MIN_LPC_IRQ;
>>>> + break;
>>>> +
>>>> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
>>>> + id = find_pch_pic(gsi);
>>>> + if (id < 0)
>>>> + return;
>>>> + if (!pch_pic_domain[id])
>>>> + return;
>>>> + d = pch_pic_domain[id];
>>>> +
>>>> + hw_irq = gsi - acpi_pchpic[id]->gsi_base;
>>>> + break;
>>>> + }
>>>> + irq = irq_find_mapping(d, hw_irq);
>>>> + irq_dispose_mapping(irq);
>>>> +
>>>> + return;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
>>>> +
>>>> +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;
>>>> +}
>>> Again, why can't you reuse drivers/acpi/pci_mcfg.c?
>>>
>> Yes, I really want to reuse code from pci_mcfg.c, but I found that
>> pci_mmcfg_late_init() is called from acpi_init during
>> subsys_initcall. vector_group entries here are needed initialzed
>> during irqchip_init flow before EIOINTC, PCH PIC and PCH MSI
>> initialization as I descripted info 'Example of irqchip topology in
>> a system with two chipsets' in [PATCH RFC V2 00/10].
> I'm not sure why this is needed. Can't this be done at a later time?
> Surely, no PCI device can come up without the ACPI resources having
> been populated. And if the PCI bus comes up before, you should be able
> to defer it.
>
> In any case, this doesn't seem to belong the an irqchip driver, and is
> much more a PCI thing.
For ARM, the parent domain of pci msi domain is matched to ITS domain by
pci segment
in IORT. For LoongArch, we didn't haveIORT-like table, so we used pci
segment in
MCFG to match the parent domain of pci msi domain to EIOINTC domain. An
example of
irqchip topology in a system with two chipsets as following(same as
[PATCH RFC V2 00/10]):
+------------------------------------------------------------+
| |
| +------------------+ |
| | CPUINTC | |
| +------------------+ |
| ^ ^ |
| | | |
| +----------+ +----------+ |
| | EIOINTC 0| | EIOINTC 1| |
| +----------+ +----------+ |
| ^ ^ ^ ^ |
| | | | | |
| +----------+ +----------+ +----------+ +----------+ |
| | PCH-PIC 0| | PCH-MSI 0| | PCH-PIC 1| | PCH-MSI 1| |
| +----------+ +----------+ +----------+ +----------+ |
| |
| |
+------------------------------------------------------------+
For systems with two chipsets, there are tow group(consists of EIOINTC,
PCH-PIC and PCH-MSI) irqdomains,
and each group has same node id. So we defined a structure to mantain
the relation of node and it's parent irqdomain.
struct acpi_vector_group {
int node;
struct irq_domain *parent;
};
The initialization and use of acpi_vector_group array are following:
- Entry of struct acpi_vector_group array initialization:
By parsing MCFG, the node id£šfrom bit44-47 of Base Address£©of each pci
segment is extracted. And from MADT, we have the node id of each EIOINTC.
entrys[pci segment].node = node id of pci segment
entrys[pci segment].parent = EIOINTC irqdomain(node id of EIOINTC ==
node id of pci segment)
- Get parent irqdomain for PCH-PIC:
From MADT, we have the node id of each PCH-PIC(from bit44-47 of Base
Address).
if (node of entry i == node of PCH-PIC)
return entrys[i].parent;
entrys[pci segment].node = node id of pci segment
entrys[pci segment].parent = EIOINTC irqdomain(node id of EIOINTC ==
node id of pci segment)
- Get parent irqdomain for PCH-MSI of pci segment:
return entrys[pci segment].parent;
Then, how to select a correct irqdomain to map irq for a device?
For devices using legacy irq behind PCH-PIC, GSI is used to select
correct PCH-PIC irqdomain.
For devices using msi irq behind PCH-MSI, the pci segmen of the device
is used to select correct PCH-MSI irqdomain.
I don't know whether we should create IORT-like table and add related
code. If so, I'll create new table for LoongArch and submit it to UEFI
and ASWG after it's verified.
> Thanks,
>
> M.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-05-31 13:56 ` Marc Zyngier
2022-06-01 3:35 ` Jianmin Lv
@ 2022-06-02 3:16 ` Jianmin Lv
2022-06-06 10:02 ` Marc Zyngier
1 sibling, 1 reply; 15+ messages in thread
From: Jianmin Lv @ 2022-06-02 3:16 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen
>>>> +
>>>> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
>>>> +{
>>>> + if (irqp != NULL)
>>>> + *irqp = acpi_register_gsi(NULL, gsi, -1, -1);
>>>> + return (*irqp >= 0) ? 0 : -EINVAL;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>>>> +
>>>> +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
>>>> +{
>>>> + if (gsi)
>>>> + *gsi = isa_irq;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/*
>>>> + * success: return IRQ number (>=0)
>>>> + * failure: return < 0
>>>> + */
>>>> +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
>>>> +{
>>>> + int id;
>>>> + struct irq_fwspec fwspec;
>>>> +
>>>> + switch (gsi) {
>>>> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
>>>> + fwspec.fwnode = liointc_domain->fwnode;
>>>> + fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
>>>> + fwspec.param_count = 1;
>>>> +
>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>> +
>>>> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
>>>> + if (!pch_lpc_domain)
>>>> + return -EINVAL;
>>>> +
>>>> + fwspec.fwnode = pch_lpc_domain->fwnode;
>>>> + fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
>>>> + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
>>>> + fwspec.param_count = 2;
>>>> +
>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>> +
>>>> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
>>>> + id = find_pch_pic(gsi);
>>>> + if (id < 0)
>>>> + return -EINVAL;
>>>> +
>>>> + fwspec.fwnode = pch_pic_domain[id]->fwnode;
>>>> + fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
>>>> + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
>>>> + fwspec.param_count = 2;
>>>> +
>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>> + }
>>> So all the complexity here seems to stem from the fact that you deal
>>> with three ranges of interrupts, managed by three different pieces of
>>> code?
>>>
>> Yes.
>>
>>> Other architectures have similar requirements, and don't require to
>>> re-implement a private version of the ACPI API. Instead, they expose a
>>> single irqdomain, and deal with the various ranges internally.
>>>
>>> Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue.
>>>
>> Thanks, I agree, that sounds a good and reasonable suggestion, and
>> I'll reserach it further and reuse code from drivers/acpi/irq.c as
>> can as possible.
>>
Hi, Marc, according to your suggestion, I carefully looked into gic
driver of ARM, and I found one possible gsi mapping path as following:
acpi_register_gsi /* whatever the gsi is, gic domain for ARM is only
single domain to use.*/
->irq_create_fwspec_mapping
->irq_find_mapping /* return irq in the mapping of irqdomain with
fwnode_handle of acpi_gsi_domain_id if configured. */
->irq_domain_alloc_irqs /* if not configured and hierarchy domain */
->irq_domain_alloc_irqs_hierarchy
->domain->ops->alloc /* call gic_irq_domain_alloc */
->gic_irq_domain_map /* handle different GSI range as
following code: */
switch (__get_intid_range(hw)) {
case SGI_RANGE:
case PPI_RANGE:
case EPPI_RANGE:
irq_set_percpu_devid(irq);
irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_percpu_devid_irq, NULL, NULL);
break;
case SPI_RANGE:
case ESPI_RANGE:
irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_fasteoi_irq, NULL, NULL);
irq_set_probe(irq);
irqd_set_single_target(irqd);
break;
case LPI_RANGE:
if (!gic_dist_supports_lpis())
return -EPERM;
irq_domain_set_info(d, irq, hw, chip, d->host_data,
handle_fasteoi_irq, NULL, NULL);
break;
Yes, it's well for ARM by this way, and I found that only one
domain(specified by acpi_gsi_domain_id)is used.
But for LoongArch, different GSI range have to be handled in different
domain(e.g. GSI for LIOINTC irqchip can be only mapped in LIOINTC
domain.). The hwirq->irq mapping of different GSI range is stored in
related separate domain. The reason leading to this is that an interrupt
source is hardcodingly to connected to an interrupt vector for these
irqchip(LIOINTC,LPC-PIC and PCH-PIC), and the interrupt source of them
need to be configured with GSI in DSDT or FADT(e.g. SCI).
If only exposing one domain for LoongArch, when calling irq_find_mapping
in acpi_register_gsi flow, the irq is returned only from the domain
specfied by acpi_gsi_domain_id, so I'm afraid it's unsuitable to expose
a single domain for acpi_register_gsi.
I'm so sorry, I really don't find a way to reuse driver/acpi/irq.c after
my humble work.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-06-02 3:16 ` Jianmin Lv
@ 2022-06-06 10:02 ` Marc Zyngier
2022-06-07 3:41 ` Jianmin Lv
0 siblings, 1 reply; 15+ messages in thread
From: Marc Zyngier @ 2022-06-06 10:02 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen, Hanjun Guo, Lorenzo Pieralisi
+ Lorenzo and Hanjun who maintain the ACPI irq code
On Thu, 02 Jun 2022 04:16:30 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
> >>>> +
> >>>> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
> >>>> +{
> >>>> + if (irqp != NULL)
> >>>> + *irqp = acpi_register_gsi(NULL, gsi, -1, -1);
> >>>> + return (*irqp >= 0) ? 0 : -EINVAL;
> >>>> +}
> >>>> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> >>>> +
> >>>> +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
> >>>> +{
> >>>> + if (gsi)
> >>>> + *gsi = isa_irq;
> >>>> + return 0;
> >>>> +}
> >>>> +
> >>>> +/*
> >>>> + * success: return IRQ number (>=0)
> >>>> + * failure: return < 0
> >>>> + */
> >>>> +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
> >>>> +{
> >>>> + int id;
> >>>> + struct irq_fwspec fwspec;
> >>>> +
> >>>> + switch (gsi) {
> >>>> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> >>>> + fwspec.fwnode = liointc_domain->fwnode;
> >>>> + fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
> >>>> + fwspec.param_count = 1;
> >>>> +
> >>>> + return irq_create_fwspec_mapping(&fwspec);
> >>>> +
> >>>> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> >>>> + if (!pch_lpc_domain)
> >>>> + return -EINVAL;
> >>>> +
> >>>> + fwspec.fwnode = pch_lpc_domain->fwnode;
> >>>> + fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
> >>>> + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
> >>>> + fwspec.param_count = 2;
> >>>> +
> >>>> + return irq_create_fwspec_mapping(&fwspec);
> >>>> +
> >>>> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> >>>> + id = find_pch_pic(gsi);
> >>>> + if (id < 0)
> >>>> + return -EINVAL;
> >>>> +
> >>>> + fwspec.fwnode = pch_pic_domain[id]->fwnode;
> >>>> + fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
> >>>> + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
> >>>> + fwspec.param_count = 2;
> >>>> +
> >>>> + return irq_create_fwspec_mapping(&fwspec);
> >>>> + }
> >>> So all the complexity here seems to stem from the fact that you deal
> >>> with three ranges of interrupts, managed by three different pieces of
> >>> code?
> >>>
> >> Yes.
> >>
> >>> Other architectures have similar requirements, and don't require to
> >>> re-implement a private version of the ACPI API. Instead, they expose a
> >>> single irqdomain, and deal with the various ranges internally.
> >>>
> >>> Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue.
> >>>
> >> Thanks, I agree, that sounds a good and reasonable suggestion, and
> >> I'll reserach it further and reuse code from drivers/acpi/irq.c as
> >> can as possible.
> >>
> Hi, Marc, according to your suggestion, I carefully looked into gic
> driver of ARM, and I found one possible gsi mapping path as following:
>
> acpi_register_gsi /* whatever the gsi is, gic domain for ARM is only
> single domain to use.*/
> ->irq_create_fwspec_mapping
> ->irq_find_mapping /* return irq in the mapping of irqdomain with
> fwnode_handle of acpi_gsi_domain_id if configured. */
> ->irq_domain_alloc_irqs /* if not configured and hierarchy domain */
> ->irq_domain_alloc_irqs_hierarchy
> ->domain->ops->alloc /* call gic_irq_domain_alloc */
> ->gic_irq_domain_map /* handle different GSI range as
> following code: */
>
>
> switch (__get_intid_range(hw)) {
> case SGI_RANGE:
> case PPI_RANGE:
> case EPPI_RANGE:
> irq_set_percpu_devid(irq);
> irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_percpu_devid_irq, NULL, NULL);
> break;
>
> case SPI_RANGE:
> case ESPI_RANGE:
> irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_fasteoi_irq, NULL, NULL);
> irq_set_probe(irq);
> irqd_set_single_target(irqd);
> break;
>
> case LPI_RANGE:
> if (!gic_dist_supports_lpis())
> return -EPERM;
> irq_domain_set_info(d, irq, hw, chip, d->host_data,
> handle_fasteoi_irq, NULL, NULL);
> break;
>
> Yes, it's well for ARM by this way, and I found that only one
> domain(specified by acpi_gsi_domain_id)is used.
>
> But for LoongArch, different GSI range have to be handled in different
> domain(e.g. GSI for LIOINTC irqchip can be only mapped in LIOINTC
> domain.). The hwirq->irq mapping of different GSI range is stored in
> related separate domain. The reason leading to this is that an
> interrupt source is hardcodingly to connected to an interrupt vector
> for these irqchip(LIOINTC,LPC-PIC and PCH-PIC), and the interrupt
> source of them need to be configured with GSI in DSDT or
> FADT(e.g. SCI).
>
> If only exposing one domain for LoongArch, when calling
> irq_find_mapping in acpi_register_gsi flow, the irq is returned only
> from the domain specfied by acpi_gsi_domain_id, so I'm afraid it's
> unsuitable to expose a single domain for acpi_register_gsi.
>
> I'm so sorry, I really don't find a way to reuse driver/acpi/irq.c
> after my humble work.
I don't think reimplementing ACPI is the solution. What could be a
reasonable approach is a way to overload the retrieval of the
acpi_gsi_domain_id fwnode with a GSI parameter.
I hacked the following patch, which will give you an idea of what I
have in mind (only compile-tested).
Thanks,
M.
From a3fdc06a53cbcc0e6b77863aae9a7b01a0848fd0 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Mon, 6 Jun 2022 10:49:14 +0100
Subject: [PATCH] APCI: irq: Add support for multiple GSI domains
In an unfortunate departure from the ACPI spec, the LoongArch
architecture split its GSI space across multiple interrupt
controllers.
In order to be able to reuse sthe core code and prevent
rachitectures from reinventing an already square wheel, offer
the arch code the ability to register a dispatcher function
that will return the domain fwnode for a given GSI.
The ARM GIC drivers are updated to support this (with a single
domain, as intended).
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: Hanjun Guo <guohanjun@huawei.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
drivers/acpi/irq.c | 41 +++++++++++++++++++++++-------------
drivers/irqchip/irq-gic-v3.c | 18 ++++++++++------
drivers/irqchip/irq-gic.c | 18 ++++++++++------
include/linux/acpi.h | 2 +-
4 files changed, 51 insertions(+), 28 deletions(-)
diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
index c68e694fca26..6e1633ac1756 100644
--- a/drivers/acpi/irq.c
+++ b/drivers/acpi/irq.c
@@ -12,7 +12,7 @@
enum acpi_irq_model_id acpi_irq_model;
-static struct fwnode_handle *acpi_gsi_domain_id;
+static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
/**
* acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
@@ -26,8 +26,10 @@ static struct fwnode_handle *acpi_gsi_domain_id;
*/
int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
{
- struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
- DOMAIN_BUS_ANY);
+ struct irq_domain *d;
+
+ d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(gsi),
+ DOMAIN_BUS_ANY);
*irq = irq_find_mapping(d, gsi);
/*
@@ -53,12 +55,12 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
{
struct irq_fwspec fwspec;
- if (WARN_ON(!acpi_gsi_domain_id)) {
+ fwspec.fwnode = acpi_get_gsi_domain_id(gsi);
+ if (WARN_ON(!fwspec.fwnode)) {
pr_warn("GSI: No registered irqchip, giving up\n");
return -EINVAL;
}
- fwspec.fwnode = acpi_gsi_domain_id;
fwspec.param[0] = gsi;
fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
fwspec.param_count = 2;
@@ -73,13 +75,14 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi);
*/
void acpi_unregister_gsi(u32 gsi)
{
- struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
- DOMAIN_BUS_ANY);
+ struct irq_domain *d;
int irq;
if (WARN_ON(acpi_irq_model == ACPI_IRQ_MODEL_GIC && gsi < 16))
return;
+ d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(gsi),
+ DOMAIN_BUS_ANY);
irq = irq_find_mapping(d, gsi);
irq_dispose_mapping(irq);
}
@@ -97,7 +100,8 @@ EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
* The referenced device fwhandle or NULL on failure
*/
static struct fwnode_handle *
-acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source)
+acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source,
+ u32 gsi)
{
struct fwnode_handle *result;
struct acpi_device *device;
@@ -105,7 +109,7 @@ acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source)
acpi_status status;
if (!source->string_length)
- return acpi_gsi_domain_id;
+ return acpi_get_gsi_domain_id(gsi);
status = acpi_get_handle(NULL, source->string_ptr, &handle);
if (WARN_ON(ACPI_FAILURE(status)))
@@ -194,7 +198,7 @@ static acpi_status acpi_irq_parse_one_cb(struct acpi_resource *ares,
ctx->index -= irq->interrupt_count;
return AE_OK;
}
- fwnode = acpi_gsi_domain_id;
+ fwnode = acpi_get_gsi_domain_id(ctx->index);
acpi_irq_parse_one_match(fwnode, irq->interrupts[ctx->index],
irq->triggering, irq->polarity,
irq->shareable, ctx);
@@ -207,7 +211,8 @@ static acpi_status acpi_irq_parse_one_cb(struct acpi_resource *ares,
ctx->index -= eirq->interrupt_count;
return AE_OK;
}
- fwnode = acpi_get_irq_source_fwhandle(&eirq->resource_source);
+ fwnode = acpi_get_irq_source_fwhandle(&eirq->resource_source,
+ eirq->interrupts[ctx->index]);
acpi_irq_parse_one_match(fwnode, eirq->interrupts[ctx->index],
eirq->triggering, eirq->polarity,
eirq->shareable, ctx);
@@ -291,10 +296,10 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
* GSI interrupts
*/
void __init acpi_set_irq_model(enum acpi_irq_model_id model,
- struct fwnode_handle *fwnode)
+ struct fwnode_handle *(*fn)(u32))
{
acpi_irq_model = model;
- acpi_gsi_domain_id = fwnode;
+ acpi_get_gsi_domain_id = fn;
}
/**
@@ -312,8 +317,14 @@ struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
const struct irq_domain_ops *ops,
void *host_data)
{
- struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
- DOMAIN_BUS_ANY);
+ struct irq_domain *d;
+
+ /* This only works for the GIC model... */
+ if (acpi_irq_model != ACPI_IRQ_MODEL_GIC)
+ return NULL;
+
+ d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(0),
+ DOMAIN_BUS_ANY);
if (!d)
return NULL;
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 2be8dea6b6b0..87b1f53a65ec 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -2357,11 +2357,17 @@ static void __init gic_acpi_setup_kvm_info(void)
vgic_set_kvm_info(&gic_v3_kvm_info);
}
+static struct fwnode_handle *gsi_domain_handle;
+
+static struct fwnode_handle *gic_v3_get_gsi_domain_id(u32 gsi)
+{
+ return gsi_domain_handle;
+}
+
static int __init
gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
{
struct acpi_madt_generic_distributor *dist;
- struct fwnode_handle *domain_handle;
size_t size;
int i, err;
@@ -2393,18 +2399,18 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
if (err)
goto out_redist_unmap;
- domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
- if (!domain_handle) {
+ gsi_domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
+ if (!gsi_domain_handle) {
err = -ENOMEM;
goto out_redist_unmap;
}
err = gic_init_bases(acpi_data.dist_base, acpi_data.redist_regs,
- acpi_data.nr_redist_regions, 0, domain_handle);
+ acpi_data.nr_redist_regions, 0, gsi_domain_handle);
if (err)
goto out_fwhandle_free;
- acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
+ acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, gic_v3_get_gsi_domain_id);
if (static_branch_likely(&supports_deactivate_key))
gic_acpi_setup_kvm_info();
@@ -2412,7 +2418,7 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
return 0;
out_fwhandle_free:
- irq_domain_free_fwnode(domain_handle);
+ irq_domain_free_fwnode(gsi_domain_handle);
out_redist_unmap:
for (i = 0; i < acpi_data.nr_redist_regions; i++)
if (acpi_data.redist_regs[i].redist_base)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 820404cb56bc..4c7bae0ec8f9 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1682,11 +1682,17 @@ static void __init gic_acpi_setup_kvm_info(void)
vgic_set_kvm_info(&gic_v2_kvm_info);
}
+static struct fwnode_handle *gsi_domain_handle;
+
+static struct fwnode_handle *gic_v2_get_gsi_domain_id(u32 gsi)
+{
+ return gsi_domain_handle;
+}
+
static int __init gic_v2_acpi_init(union acpi_subtable_headers *header,
const unsigned long end)
{
struct acpi_madt_generic_distributor *dist;
- struct fwnode_handle *domain_handle;
struct gic_chip_data *gic = &gic_data[0];
int count, ret;
@@ -1724,22 +1730,22 @@ static int __init gic_v2_acpi_init(union acpi_subtable_headers *header,
/*
* Initialize GIC instance zero (no multi-GIC support).
*/
- domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
- if (!domain_handle) {
+ gsi_domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
+ if (!gsi_domain_handle) {
pr_err("Unable to allocate domain handle\n");
gic_teardown(gic);
return -ENOMEM;
}
- ret = __gic_init_bases(gic, domain_handle);
+ ret = __gic_init_bases(gic, gsi_domain_handle);
if (ret) {
pr_err("Failed to initialise GIC\n");
- irq_domain_free_fwnode(domain_handle);
+ irq_domain_free_fwnode(gsi_domain_handle);
gic_teardown(gic);
return ret;
}
- acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
+ acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, gic_v2_get_gsi_domain_id);
if (IS_ENABLED(CONFIG_ARM_GIC_V2M))
gicv2m_init(NULL, gic_data[0].domain);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4f82a5bc6d98..957e23f727ea 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -356,7 +356,7 @@ int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
void acpi_set_irq_model(enum acpi_irq_model_id model,
- struct fwnode_handle *fwnode);
+ struct fwnode_handle *(*)(u32));
struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
unsigned int size,
--
2.34.1
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-06-06 10:02 ` Marc Zyngier
@ 2022-06-07 3:41 ` Jianmin Lv
2022-06-07 6:56 ` Marc Zyngier
0 siblings, 1 reply; 15+ messages in thread
From: Jianmin Lv @ 2022-06-07 3:41 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen, Hanjun Guo, Lorenzo Pieralisi
On 2022/6/6 下午6:02, Marc Zyngier wrote:
> + Lorenzo and Hanjun who maintain the ACPI irq code
>
> On Thu, 02 Jun 2022 04:16:30 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>>>>> +
>>>>>> +int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
>>>>>> +{
>>>>>> + if (irqp != NULL)
>>>>>> + *irqp = acpi_register_gsi(NULL, gsi, -1, -1);
>>>>>> + return (*irqp >= 0) ? 0 : -EINVAL;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>>>>>> +
>>>>>> +int acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
>>>>>> +{
>>>>>> + if (gsi)
>>>>>> + *gsi = isa_irq;
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * success: return IRQ number (>=0)
>>>>>> + * failure: return < 0
>>>>>> + */
>>>>>> +int acpi_register_gsi(struct device *dev, u32 gsi, int trigger, int polarity)
>>>>>> +{
>>>>>> + int id;
>>>>>> + struct irq_fwspec fwspec;
>>>>>> +
>>>>>> + switch (gsi) {
>>>>>> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
>>>>>> + fwspec.fwnode = liointc_domain->fwnode;
>>>>>> + fwspec.param[0] = gsi - GSI_MIN_CPU_IRQ;
>>>>>> + fwspec.param_count = 1;
>>>>>> +
>>>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>>>> +
>>>>>> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
>>>>>> + if (!pch_lpc_domain)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + fwspec.fwnode = pch_lpc_domain->fwnode;
>>>>>> + fwspec.param[0] = gsi - GSI_MIN_LPC_IRQ;
>>>>>> + fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
>>>>>> + fwspec.param_count = 2;
>>>>>> +
>>>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>>>> +
>>>>>> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
>>>>>> + id = find_pch_pic(gsi);
>>>>>> + if (id < 0)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + fwspec.fwnode = pch_pic_domain[id]->fwnode;
>>>>>> + fwspec.param[0] = gsi - acpi_pchpic[id]->gsi_base;
>>>>>> + fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
>>>>>> + fwspec.param_count = 2;
>>>>>> +
>>>>>> + return irq_create_fwspec_mapping(&fwspec);
>>>>>> + }
>>>>> So all the complexity here seems to stem from the fact that you deal
>>>>> with three ranges of interrupts, managed by three different pieces of
>>>>> code?
>>>>>
>>>> Yes.
>>>>
>>>>> Other architectures have similar requirements, and don't require to
>>>>> re-implement a private version of the ACPI API. Instead, they expose a
>>>>> single irqdomain, and deal with the various ranges internally.
>>>>>
>>>>> Clearly, not being able to reuse drivers/acpi/irq.c *is* an issue.
>>>>>
>>>> Thanks, I agree, that sounds a good and reasonable suggestion, and
>>>> I'll reserach it further and reuse code from drivers/acpi/irq.c as
>>>> can as possible.
>>>>
>> Hi, Marc, according to your suggestion, I carefully looked into gic
>> driver of ARM, and I found one possible gsi mapping path as following:
>>
>> acpi_register_gsi /* whatever the gsi is, gic domain for ARM is only
>> single domain to use.*/
>> ->irq_create_fwspec_mapping
>> ->irq_find_mapping /* return irq in the mapping of irqdomain with
>> fwnode_handle of acpi_gsi_domain_id if configured. */
>> ->irq_domain_alloc_irqs /* if not configured and hierarchy domain */
>> ->irq_domain_alloc_irqs_hierarchy
>> ->domain->ops->alloc /* call gic_irq_domain_alloc */
>> ->gic_irq_domain_map /* handle different GSI range as
>> following code: */
>>
>>
>> switch (__get_intid_range(hw)) {
>> case SGI_RANGE:
>> case PPI_RANGE:
>> case EPPI_RANGE:
>> irq_set_percpu_devid(irq);
>> irq_domain_set_info(d, irq, hw, chip, d->host_data,
>> handle_percpu_devid_irq, NULL, NULL);
>> break;
>>
>> case SPI_RANGE:
>> case ESPI_RANGE:
>> irq_domain_set_info(d, irq, hw, chip, d->host_data,
>> handle_fasteoi_irq, NULL, NULL);
>> irq_set_probe(irq);
>> irqd_set_single_target(irqd);
>> break;
>>
>> case LPI_RANGE:
>> if (!gic_dist_supports_lpis())
>> return -EPERM;
>> irq_domain_set_info(d, irq, hw, chip, d->host_data,
>> handle_fasteoi_irq, NULL, NULL);
>> break;
>>
>> Yes, it's well for ARM by this way, and I found that only one
>> domain(specified by acpi_gsi_domain_id)is used.
>>
>> But for LoongArch, different GSI range have to be handled in different
>> domain(e.g. GSI for LIOINTC irqchip can be only mapped in LIOINTC
>> domain.). The hwirq->irq mapping of different GSI range is stored in
>> related separate domain. The reason leading to this is that an
>> interrupt source is hardcodingly to connected to an interrupt vector
>> for these irqchip(LIOINTC,LPC-PIC and PCH-PIC), and the interrupt
>> source of them need to be configured with GSI in DSDT or
>> FADT(e.g. SCI).
>>
>> If only exposing one domain for LoongArch, when calling
>> irq_find_mapping in acpi_register_gsi flow, the irq is returned only
>> from the domain specfied by acpi_gsi_domain_id, so I'm afraid it's
>> unsuitable to expose a single domain for acpi_register_gsi.
>>
>> I'm so sorry, I really don't find a way to reuse driver/acpi/irq.c
>> after my humble work.
> I don't think reimplementing ACPI is the solution. What could be a
> reasonable approach is a way to overload the retrieval of the
> acpi_gsi_domain_id fwnode with a GSI parameter.
>
> I hacked the following patch, which will give you an idea of what I
> have in mind (only compile-tested).
Hi, Marc, thanks so much for your patch. I have verified it on my
LoongArch machine and it works well.
BTW, in acpi_get_irq_source_fwhandle(), maybe
acpi_get_gsi_domain_id(ctx->index) is needed to changed to
acpi_get_gsi_domain_id(irq->interrupts[ctx->index])?
I have another question, for LoongArch, acpi_isa_irq_to_gsi is required
to implemente, but no common version, do we need to implemente an weak
version in driver/acpi/irq.c as following?
int __weak acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
{
if (gsi)
*gsi = isa_irq;
return 0;
}
I'll use the way you provided here to reuse driver/acpi/irq.c in next
version. How should I do next? Should I integrate your patch into my
next version or wait for you to merge it first?
> Thanks,
>
> M.
>
> From a3fdc06a53cbcc0e6b77863aae9a7b01a0848fd0 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Mon, 6 Jun 2022 10:49:14 +0100
> Subject: [PATCH] APCI: irq: Add support for multiple GSI domains
>
> In an unfortunate departure from the ACPI spec, the LoongArch
> architecture split its GSI space across multiple interrupt
> controllers.
>
> In order to be able to reuse sthe core code and prevent
> rachitectures from reinventing an already square wheel, offer
> the arch code the ability to register a dispatcher function
> that will return the domain fwnode for a given GSI.
>
> The ARM GIC drivers are updated to support this (with a single
> domain, as intended).
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: Hanjun Guo <guohanjun@huawei.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
> drivers/acpi/irq.c | 41 +++++++++++++++++++++++-------------
> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++------
> drivers/irqchip/irq-gic.c | 18 ++++++++++------
> include/linux/acpi.h | 2 +-
> 4 files changed, 51 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index c68e694fca26..6e1633ac1756 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -12,7 +12,7 @@
>
> enum acpi_irq_model_id acpi_irq_model;
>
> -static struct fwnode_handle *acpi_gsi_domain_id;
> +static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
>
> /**
> * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
> @@ -26,8 +26,10 @@ static struct fwnode_handle *acpi_gsi_domain_id;
> */
> int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
> {
> - struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
> - DOMAIN_BUS_ANY);
> + struct irq_domain *d;
> +
> + d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(gsi),
> + DOMAIN_BUS_ANY);
>
> *irq = irq_find_mapping(d, gsi);
> /*
> @@ -53,12 +55,12 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
> {
> struct irq_fwspec fwspec;
>
> - if (WARN_ON(!acpi_gsi_domain_id)) {
> + fwspec.fwnode = acpi_get_gsi_domain_id(gsi);
> + if (WARN_ON(!fwspec.fwnode)) {
> pr_warn("GSI: No registered irqchip, giving up\n");
> return -EINVAL;
> }
>
> - fwspec.fwnode = acpi_gsi_domain_id;
> fwspec.param[0] = gsi;
> fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
> fwspec.param_count = 2;
> @@ -73,13 +75,14 @@ EXPORT_SYMBOL_GPL(acpi_register_gsi);
> */
> void acpi_unregister_gsi(u32 gsi)
> {
> - struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
> - DOMAIN_BUS_ANY);
> + struct irq_domain *d;
> int irq;
>
> if (WARN_ON(acpi_irq_model == ACPI_IRQ_MODEL_GIC && gsi < 16))
> return;
>
> + d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(gsi),
> + DOMAIN_BUS_ANY);
> irq = irq_find_mapping(d, gsi);
> irq_dispose_mapping(irq);
> }
> @@ -97,7 +100,8 @@ EXPORT_SYMBOL_GPL(acpi_unregister_gsi);
> * The referenced device fwhandle or NULL on failure
> */
> static struct fwnode_handle *
> -acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source)
> +acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source,
> + u32 gsi)
> {
> struct fwnode_handle *result;
> struct acpi_device *device;
> @@ -105,7 +109,7 @@ acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source)
> acpi_status status;
>
> if (!source->string_length)
> - return acpi_gsi_domain_id;
> + return acpi_get_gsi_domain_id(gsi);
>
> status = acpi_get_handle(NULL, source->string_ptr, &handle);
> if (WARN_ON(ACPI_FAILURE(status)))
> @@ -194,7 +198,7 @@ static acpi_status acpi_irq_parse_one_cb(struct acpi_resource *ares,
> ctx->index -= irq->interrupt_count;
> return AE_OK;
> }
> - fwnode = acpi_gsi_domain_id;
> + fwnode = acpi_get_gsi_domain_id(ctx->index);
> acpi_irq_parse_one_match(fwnode, irq->interrupts[ctx->index],
> irq->triggering, irq->polarity,
> irq->shareable, ctx);
> @@ -207,7 +211,8 @@ static acpi_status acpi_irq_parse_one_cb(struct acpi_resource *ares,
> ctx->index -= eirq->interrupt_count;
> return AE_OK;
> }
> - fwnode = acpi_get_irq_source_fwhandle(&eirq->resource_source);
> + fwnode = acpi_get_irq_source_fwhandle(&eirq->resource_source,
> + eirq->interrupts[ctx->index]);
> acpi_irq_parse_one_match(fwnode, eirq->interrupts[ctx->index],
> eirq->triggering, eirq->polarity,
> eirq->shareable, ctx);
> @@ -291,10 +296,10 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
> * GSI interrupts
> */
> void __init acpi_set_irq_model(enum acpi_irq_model_id model,
> - struct fwnode_handle *fwnode)
> + struct fwnode_handle *(*fn)(u32))
> {
> acpi_irq_model = model;
> - acpi_gsi_domain_id = fwnode;
> + acpi_get_gsi_domain_id = fn;
> }
>
> /**
> @@ -312,8 +317,14 @@ struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> const struct irq_domain_ops *ops,
> void *host_data)
> {
> - struct irq_domain *d = irq_find_matching_fwnode(acpi_gsi_domain_id,
> - DOMAIN_BUS_ANY);
> + struct irq_domain *d;
> +
> + /* This only works for the GIC model... */
> + if (acpi_irq_model != ACPI_IRQ_MODEL_GIC)
> + return NULL;
> +
> + d = irq_find_matching_fwnode(acpi_get_gsi_domain_id(0),
> + DOMAIN_BUS_ANY);
>
> if (!d)
> return NULL;
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 2be8dea6b6b0..87b1f53a65ec 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -2357,11 +2357,17 @@ static void __init gic_acpi_setup_kvm_info(void)
> vgic_set_kvm_info(&gic_v3_kvm_info);
> }
>
> +static struct fwnode_handle *gsi_domain_handle;
> +
> +static struct fwnode_handle *gic_v3_get_gsi_domain_id(u32 gsi)
> +{
> + return gsi_domain_handle;
> +}
> +
> static int __init
> gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
> {
> struct acpi_madt_generic_distributor *dist;
> - struct fwnode_handle *domain_handle;
> size_t size;
> int i, err;
>
> @@ -2393,18 +2399,18 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
> if (err)
> goto out_redist_unmap;
>
> - domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
> - if (!domain_handle) {
> + gsi_domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
> + if (!gsi_domain_handle) {
> err = -ENOMEM;
> goto out_redist_unmap;
> }
>
> err = gic_init_bases(acpi_data.dist_base, acpi_data.redist_regs,
> - acpi_data.nr_redist_regions, 0, domain_handle);
> + acpi_data.nr_redist_regions, 0, gsi_domain_handle);
> if (err)
> goto out_fwhandle_free;
>
> - acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
> + acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, gic_v3_get_gsi_domain_id);
>
> if (static_branch_likely(&supports_deactivate_key))
> gic_acpi_setup_kvm_info();
> @@ -2412,7 +2418,7 @@ gic_acpi_init(union acpi_subtable_headers *header, const unsigned long end)
> return 0;
>
> out_fwhandle_free:
> - irq_domain_free_fwnode(domain_handle);
> + irq_domain_free_fwnode(gsi_domain_handle);
> out_redist_unmap:
> for (i = 0; i < acpi_data.nr_redist_regions; i++)
> if (acpi_data.redist_regs[i].redist_base)
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 820404cb56bc..4c7bae0ec8f9 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1682,11 +1682,17 @@ static void __init gic_acpi_setup_kvm_info(void)
> vgic_set_kvm_info(&gic_v2_kvm_info);
> }
>
> +static struct fwnode_handle *gsi_domain_handle;
> +
> +static struct fwnode_handle *gic_v2_get_gsi_domain_id(u32 gsi)
> +{
> + return gsi_domain_handle;
> +}
> +
> static int __init gic_v2_acpi_init(union acpi_subtable_headers *header,
> const unsigned long end)
> {
> struct acpi_madt_generic_distributor *dist;
> - struct fwnode_handle *domain_handle;
> struct gic_chip_data *gic = &gic_data[0];
> int count, ret;
>
> @@ -1724,22 +1730,22 @@ static int __init gic_v2_acpi_init(union acpi_subtable_headers *header,
> /*
> * Initialize GIC instance zero (no multi-GIC support).
> */
> - domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
> - if (!domain_handle) {
> + gsi_domain_handle = irq_domain_alloc_fwnode(&dist->base_address);
> + if (!gsi_domain_handle) {
> pr_err("Unable to allocate domain handle\n");
> gic_teardown(gic);
> return -ENOMEM;
> }
>
> - ret = __gic_init_bases(gic, domain_handle);
> + ret = __gic_init_bases(gic, gsi_domain_handle);
> if (ret) {
> pr_err("Failed to initialise GIC\n");
> - irq_domain_free_fwnode(domain_handle);
> + irq_domain_free_fwnode(gsi_domain_handle);
> gic_teardown(gic);
> return ret;
> }
>
> - acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, domain_handle);
> + acpi_set_irq_model(ACPI_IRQ_MODEL_GIC, gic_v2_get_gsi_domain_id);
>
> if (IS_ENABLED(CONFIG_ARM_GIC_V2M))
> gicv2m_init(NULL, gic_data[0].domain);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4f82a5bc6d98..957e23f727ea 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -356,7 +356,7 @@ int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
> int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
>
> void acpi_set_irq_model(enum acpi_irq_model_id model,
> - struct fwnode_handle *fwnode);
> + struct fwnode_handle *(*)(u32));
>
> struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> unsigned int size,
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-06-07 3:41 ` Jianmin Lv
@ 2022-06-07 6:56 ` Marc Zyngier
2022-06-08 8:55 ` Jianmin Lv
0 siblings, 1 reply; 15+ messages in thread
From: Marc Zyngier @ 2022-06-07 6:56 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen, Hanjun Guo, Lorenzo Pieralisi
On Tue, 07 Jun 2022 04:41:22 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
>
> On 2022/6/6 下午6:02, Marc Zyngier wrote:
> > + Lorenzo and Hanjun who maintain the ACPI irq code
> >
> > On Thu, 02 Jun 2022 04:16:30 +0100,
> > Jianmin Lv <lvjianmin@loongson.cn> wrote:
[...]
> >> I'm so sorry, I really don't find a way to reuse driver/acpi/irq.c
> >> after my humble work.
> > I don't think reimplementing ACPI is the solution. What could be a
> > reasonable approach is a way to overload the retrieval of the
> > acpi_gsi_domain_id fwnode with a GSI parameter.
> >
> > I hacked the following patch, which will give you an idea of what I
> > have in mind (only compile-tested).
>
>
> Hi, Marc, thanks so much for your patch. I have verified it on my
> LoongArch machine and it works well.
>
>
> BTW, in acpi_get_irq_source_fwhandle(), maybe
> acpi_get_gsi_domain_id(ctx->index) is needed to changed to
> acpi_get_gsi_domain_id(irq->interrupts[ctx->index])?
Yes, absolutely. Thanks for spotting it.
> I have another question, for LoongArch, acpi_isa_irq_to_gsi is
> required to implemente, but no common version, do we need to
> implemente an weak version in driver/acpi/irq.c as following?
>
>
> int __weak acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
> {
> if (gsi)
> *gsi = isa_irq;
> return 0;
> }
Do you actually have CONFIG_ISA? In 2022? For a brand new architecture?
If you really have to, then this needs to be a bit more involved:
#ifdef CONFIG_ISA
int __weak acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
{
if (irq < nr_legacy_irqs()) {
*gsi = isa_irq;
return 0;
}
return -1;
}
#endif
But I'd rather you get rid of any such legacy if this can be avoided.
> I'll use the way you provided here to reuse driver/acpi/irq.c in next
> version. How should I do next? Should I integrate your patch into my
> next version or wait for you to merge it first?
Please pick up the patch (with the above fix), and use it as a prefix
to your series. It needs to be reviewed by the relevant maintainers
anyway.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support
2022-06-07 6:56 ` Marc Zyngier
@ 2022-06-08 8:55 ` Jianmin Lv
0 siblings, 0 replies; 15+ messages in thread
From: Jianmin Lv @ 2022-06-08 8:55 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Xuefeng Li, Huacai Chen,
Jiaxun Yang, Huacai Chen, Hanjun Guo, Lorenzo Pieralisi
On 2022/6/7 下午2:56, Marc Zyngier wrote:
> On Tue, 07 Jun 2022 04:41:22 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>>
>> On 2022/6/6 下午6:02, Marc Zyngier wrote:
>>> + Lorenzo and Hanjun who maintain the ACPI irq code
>>>
>>> On Thu, 02 Jun 2022 04:16:30 +0100,
>>> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
> [...]
>
>>>> I'm so sorry, I really don't find a way to reuse driver/acpi/irq.c
>>>> after my humble work.
>>> I don't think reimplementing ACPI is the solution. What could be a
>>> reasonable approach is a way to overload the retrieval of the
>>> acpi_gsi_domain_id fwnode with a GSI parameter.
>>>
>>> I hacked the following patch, which will give you an idea of what I
>>> have in mind (only compile-tested).
>>
>>
>> Hi, Marc, thanks so much for your patch. I have verified it on my
>> LoongArch machine and it works well.
>>
>>
>> BTW, in acpi_get_irq_source_fwhandle(), maybe
>> acpi_get_gsi_domain_id(ctx->index) is needed to changed to
>> acpi_get_gsi_domain_id(irq->interrupts[ctx->index])?
>
> Yes, absolutely. Thanks for spotting it.
>
>> I have another question, for LoongArch, acpi_isa_irq_to_gsi is
>> required to implemente, but no common version, do we need to
>> implemente an weak version in driver/acpi/irq.c as following?
>>
>>
>> int __weak acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
>> {
>> if (gsi)
>> *gsi = isa_irq;
>> return 0;
>> }
>
> Do you actually have CONFIG_ISA? In 2022? For a brand new architecture?
>
> If you really have to, then this needs to be a bit more involved:
>
> #ifdef CONFIG_ISA
> int __weak acpi_isa_irq_to_gsi(unsigned int isa_irq, u32 *gsi)
> {
> if (irq < nr_legacy_irqs()) {
> *gsi = isa_irq;
> return 0;
> }
>
> return -1;
> }
> #endif
>
> But I'd rather you get rid of any such legacy if this can be avoided.
>
Thanks for your suggestion, I have confirmed that we don't need
CONFIT_ISA for LoongArch, so acpi_isa_irq_to_gsi is not needed either.
>> I'll use the way you provided here to reuse driver/acpi/irq.c in next
>> version. How should I do next? Should I integrate your patch into my
>> next version or wait for you to merge it first?
>
> Please pick up the patch (with the above fix), and use it as a prefix
> to your series. It needs to be reviewed by the relevant maintainers
> anyway.
>
Ok, thanks, I'll add relevant maintainers to review for next version.
> Thanks,
>
> M.
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-06-08 9:33 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27 11:02 [PATCH RFC V2 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 01/10] irqchip: Adjust Kconfig for Loongson Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 02/10] irqchip: Add LoongArch CPU interrupt controller support Jianmin Lv
2022-05-30 16:20 ` Marc Zyngier
2022-05-31 11:01 ` 吕建民
2022-05-31 13:56 ` Marc Zyngier
2022-06-01 3:35 ` Jianmin Lv
2022-06-02 3:16 ` Jianmin Lv
2022-06-06 10:02 ` Marc Zyngier
2022-06-07 3:41 ` Jianmin Lv
2022-06-07 6:56 ` Marc Zyngier
2022-06-08 8:55 ` Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 03/10] irqchip/loongson-pch-pic: Add ACPI init support Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 04/10] irqchip/loongson-pch-pic: Add suspend/resume support Jianmin Lv
2022-05-27 11:02 ` [PATCH RFC V2 05/10] irqchip/loongson-pch-msi: Add ACPI init support Jianmin Lv
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).