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;
int pci_segment;
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）and pci segment are extracted. And from MADT, we have the node id of each EIOINTC.
entry.node = node id of pci segment
entry.pci_segment = pci segment (only for msi irqdomain)
By matching node id of entry and EIOINTC to set parent.
entry.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;
3 Get parent irqdomain for PCH-MSI of pci segment:
return entrys[i].parent; (i is the index of msi irqdomain)
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.
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.
RFC V2 -> RFC V3:
1, Add support for multiple GSI domains
2, Use ACPI_GENERIC_GSI for GSI handling
3, Drop suspend-resume stuff
4, Export fwnode handles instead of irq domain handles
RFC V3 -> V12:
1, Address patch attributions of the patch series
V12 -> V13
1 Based on 5.19-rc2
2 Remove arch specified gsi code
3 Split some 'common' code into the various drivers where they belong.
4 Allow acpi_gsi_to_irq() to have an arch-specific fallback
Huacai Chen (7):
irqchip: Add Loongson PCH LPC controller support
irqchip/loongson-pch-pic: Add ACPI init support
irqchip/loongson-pch-msi: Add ACPI init support
irqchip/loongson-htvec: Add ACPI init support
irqchip/loongson-liointc: Add ACPI init support
irqchip: Add Loongson Extended I/O interrupt controller support
irqchip: Add LoongArch CPU interrupt controller support
Jianmin Lv (4):
genirq/generic_chip: export irq_unmap_generic_chip
LoongArch: Use ACPI_GENERIC_GSI for gsi handling
irqchip / ACPI: Introduce ACPI_IRQ_MODEL_LPIC for LoongArch
LoongArch: Fix irq number for timer and ipi
Marc Zyngier (2):
APCI: irq: Add support for multiple GSI domains
ACPI: irq: Allow acpi_gsi_to_irq() to have an arch-specific fallback
arch/loongarch/Kconfig | 1 +
arch/loongarch/include/asm/irq.h | 43 ++--
arch/loongarch/kernel/acpi.c | 65 ------
arch/loongarch/kernel/irq.c | 37 +++-
arch/loongarch/kernel/time.c | 2 +-
drivers/acpi/bus.c | 3 +
drivers/acpi/irq.c | 58 +++--
drivers/irqchip/Kconfig | 28 +++
drivers/irqchip/Makefile | 3 +
drivers/irqchip/irq-gic-v3.c | 18 +-
drivers/irqchip/irq-gic.c | 18 +-
drivers/irqchip/irq-loongarch-cpu.c | 168 +++++++++++++++
drivers/irqchip/irq-loongson-eiointc.c | 375 +++++++++++++++++++++++++++++++++
drivers/irqchip/irq-loongson-htvec.c | 121 ++++++++---
drivers/irqchip/irq-loongson-liointc.c | 227 +++++++++++++-------
drivers/irqchip/irq-loongson-pch-lpc.c | 203 ++++++++++++++++++
drivers/irqchip/irq-loongson-pch-msi.c | 147 +++++++++----
drivers/irqchip/irq-loongson-pch-pic.c | 200 +++++++++++++++---
include/linux/acpi.h | 4 +-
include/linux/cpuhotplug.h | 1 +
include/linux/irq.h | 1 +
kernel/irq/generic-chip.c | 2 +-
22 files changed, 1416 insertions(+), 309 deletions(-)
create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
create mode 100644 drivers/irqchip/irq-loongson-eiointc.c
create mode 100644 drivers/irqchip/irq-loongson-pch-lpc.c
--
1.8.3.1
After supporting LoongArch CPUINTC controller driver, the
irq number of interrupt source from CPUINTC is needed to transfer
from CPUINTC irqdomain, so use api of CPUINTC to transfer it.
Signed-off-by: Jianmin Lv <[email protected]>
---
arch/loongarch/kernel/irq.c | 2 +-
arch/loongarch/kernel/time.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c
index bb53ede..f342307f 100644
--- a/arch/loongarch/kernel/irq.c
+++ b/arch/loongarch/kernel/irq.c
@@ -96,7 +96,7 @@ void __init init_IRQ(void)
init_msi_parent_group();
irqchip_init();
#ifdef CONFIG_SMP
- ipi_irq = EXCCODE_IPI - EXCCODE_INT_START;
+ ipi_irq = get_ipi_irq();
irq_set_percpu_devid(ipi_irq);
r = request_percpu_irq(ipi_irq, loongson3_ipi_interrupt, "IPI", &ipi_dummy_dev);
if (r < 0)
diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c
index fe68238..4a04aa6 100644
--- a/arch/loongarch/kernel/time.c
+++ b/arch/loongarch/kernel/time.c
@@ -132,7 +132,7 @@ int constant_clockevent_init(void)
struct clock_event_device *cd;
static int timer_irq_installed = 0;
- irq = EXCCODE_TIMER - EXCCODE_INT_START;
+ irq = get_timer_irq();
cd = &per_cpu(constant_clockevent_device, cpu);
--
1.8.3.1
From: Huacai Chen <[email protected]>
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.
For systems with two chipsets, there are two related pch irqdomains,
each of which has the same node id as its parent irqdomain. 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 parent irqdomain will be set for node id in parent irqdomain driver
before pch irqdomain is created.
Co-developed-by: Jianmin Lv <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
Signed-off-by: Huacai Chen <[email protected]>
---
arch/loongarch/include/asm/irq.h | 11 +-
arch/loongarch/kernel/irq.c | 2 +-
drivers/irqchip/irq-loongson-pch-pic.c | 200 ++++++++++++++++++++++++++++-----
3 files changed, 179 insertions(+), 34 deletions(-)
diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h
index 48c0ce4..a444dc6 100644
--- a/arch/loongarch/include/asm/irq.h
+++ b/arch/loongarch/include/asm/irq.h
@@ -48,6 +48,12 @@ static inline bool on_irq_stack(int cpu, unsigned long sp)
#define MAX_IO_PICS 2
#define NR_IRQS (64 + (256 * MAX_IO_PICS))
+struct acpi_vector_group {
+ int node;
+ struct irq_domain *parent;
+};
+extern struct acpi_vector_group pch_group[MAX_IO_PICS];
+
#define CORES_PER_EIO_NODE 4
#define LOONGSON_CPU_UART0_VEC 10 /* CPU UART0 */
@@ -108,8 +114,7 @@ int pch_lpc_acpi_init(struct irq_domain *parent,
struct acpi_madt_lpc_pic *acpi_pchlpc);
struct irq_domain *pch_msi_acpi_init(struct irq_domain *parent,
struct acpi_madt_msi_pic *acpi_pchmsi);
-struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent,
- struct acpi_madt_bio_pic *acpi_pchpic);
+int find_pch_pic(u32 gsi);
extern struct acpi_madt_lio_pic *acpi_liointc;
extern struct acpi_madt_eio_pic *acpi_eiointc[MAX_IO_PICS];
@@ -123,7 +128,7 @@ struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent,
extern struct irq_domain *liointc_domain;
extern struct fwnode_handle *pch_lpc_handle;
extern struct irq_domain *pch_msi_domain[MAX_IO_PICS];
-extern struct irq_domain *pch_pic_domain[MAX_IO_PICS];
+extern struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
extern irqreturn_t loongson3_ipi_interrupt(int irq, void *dev);
diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c
index 07d6059..f2115be 100644
--- a/arch/loongarch/kernel/irq.c
+++ b/arch/loongarch/kernel/irq.c
@@ -28,7 +28,7 @@
struct irq_domain *cpu_domain;
struct irq_domain *liointc_domain;
struct irq_domain *pch_msi_domain[MAX_IO_PICS];
-struct irq_domain *pch_pic_domain[MAX_IO_PICS];
+struct acpi_vector_group pch_group[MAX_IO_PICS];
/*
* 'what should we do if we get a hw irq event on an illegal vector'.
diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index a4eb8a2..170a5b9 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -33,13 +33,40 @@
#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];
+struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
+
+int find_pch_pic(u32 gsi)
+{
+ int i;
+
+ /* Find the PCH_PIC that manages this GSI. */
+ for (i = 0; i < MAX_IO_PICS; i++) {
+ struct pch_pic *priv = pch_pic_priv[i];
+
+ if (!priv)
+ return -1;
+
+ if (gsi >= priv->gsi_base && gsi <= priv->gsi_end)
+ return i;
+ }
+
+ pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
+ return -1;
+}
+
static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
{
u32 reg;
@@ -139,6 +166,24 @@ static void pch_pic_ack_irq(struct irq_data *d)
.irq_set_type = pch_pic_set_type,
};
+static int pch_pic_domain_translate(struct irq_domain *d,
+ struct irq_fwspec *fwspec,
+ unsigned long *hwirq,
+ unsigned int *type)
+{
+ struct pch_pic *priv = d->host_data;
+ struct device_node *of_node = to_of_node(fwspec->fwnode);
+
+ if (fwspec->param_count < 1)
+ return -EINVAL;
+ if (of_node)
+ *hwirq += priv->ht_vec_base;
+ else
+ *hwirq = fwspec->param[0] - priv->gsi_base;
+ *type = IRQ_TYPE_NONE;
+
+ return 0;
+}
static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
unsigned int nr_irqs, void *arg)
{
@@ -149,13 +194,13 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
struct irq_fwspec parent_fwspec;
struct pch_pic *priv = domain->host_data;
- err = irq_domain_translate_twocell(domain, fwspec, &hwirq, &type);
+ err = pch_pic_domain_translate(domain, fwspec, &hwirq, &type);
if (err)
return err;
parent_fwspec.fwnode = domain->parent->fwnode;
parent_fwspec.param_count = 1;
- parent_fwspec.param[0] = hwirq + priv->ht_vec_base;
+ parent_fwspec.param[0] = hwirq;
err = irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
if (err)
@@ -170,7 +215,7 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
}
static const struct irq_domain_ops pch_pic_domain_ops = {
- .translate = irq_domain_translate_twocell,
+ .translate = pch_pic_domain_translate,
.alloc = pch_pic_alloc,
.free = irq_domain_free_irqs_parent,
};
@@ -180,7 +225,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 +243,41 @@ 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_handle[nr_pics] = domain_handle;
+ pch_pic_priv[nr_pics++] = priv;
return 0;
@@ -250,7 +286,111 @@ 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;
+}
+
+struct irq_domain *acpi_get_pch_parent(int node)
+{
+ int i;
+
+ for (i = 0; i < MAX_IO_PICS; i++) {
+ if (node == pch_group[i].node)
+ return pch_group[i].parent;
+ }
+ return NULL;
+}
+
+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;
+
+ 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
On Mon, 27 Jun 2022 12:39:50 +0100,
Jianmin Lv <[email protected]> wrote:
>
> From: Huacai Chen <[email protected]>
>
> 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.
Drop this paragraph.
>
> 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.
>
> For systems with two chipsets, there are two related pch irqdomains,
> each of which has the same node id as its parent irqdomain. 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 parent irqdomain will be set for node id in parent irqdomain driver
> before pch irqdomain is created.
>
> Co-developed-by: Jianmin Lv <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
> Signed-off-by: Huacai Chen <[email protected]>
> ---
> arch/loongarch/include/asm/irq.h | 11 +-
> arch/loongarch/kernel/irq.c | 2 +-
> drivers/irqchip/irq-loongson-pch-pic.c | 200 ++++++++++++++++++++++++++++-----
> 3 files changed, 179 insertions(+), 34 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h
> index 48c0ce4..a444dc6 100644
> --- a/arch/loongarch/include/asm/irq.h
> +++ b/arch/loongarch/include/asm/irq.h
> @@ -48,6 +48,12 @@ static inline bool on_irq_stack(int cpu, unsigned long sp)
> #define MAX_IO_PICS 2
> #define NR_IRQS (64 + (256 * MAX_IO_PICS))
>
> +struct acpi_vector_group {
> + int node;
> + struct irq_domain *parent;
> +};
> +extern struct acpi_vector_group pch_group[MAX_IO_PICS];
> +
> #define CORES_PER_EIO_NODE 4
>
> #define LOONGSON_CPU_UART0_VEC 10 /* CPU UART0 */
> @@ -108,8 +114,7 @@ int pch_lpc_acpi_init(struct irq_domain *parent,
> struct acpi_madt_lpc_pic *acpi_pchlpc);
> struct irq_domain *pch_msi_acpi_init(struct irq_domain *parent,
> struct acpi_madt_msi_pic *acpi_pchmsi);
> -struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent,
> - struct acpi_madt_bio_pic *acpi_pchpic);
> +int find_pch_pic(u32 gsi);
>
> extern struct acpi_madt_lio_pic *acpi_liointc;
> extern struct acpi_madt_eio_pic *acpi_eiointc[MAX_IO_PICS];
> @@ -123,7 +128,7 @@ struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent,
> extern struct irq_domain *liointc_domain;
> extern struct fwnode_handle *pch_lpc_handle;
> extern struct irq_domain *pch_msi_domain[MAX_IO_PICS];
> -extern struct irq_domain *pch_pic_domain[MAX_IO_PICS];
> +extern struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>
> extern irqreturn_t loongson3_ipi_interrupt(int irq, void *dev);
>
> diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c
> index 07d6059..f2115be 100644
> --- a/arch/loongarch/kernel/irq.c
> +++ b/arch/loongarch/kernel/irq.c
> @@ -28,7 +28,7 @@
> struct irq_domain *cpu_domain;
> struct irq_domain *liointc_domain;
> struct irq_domain *pch_msi_domain[MAX_IO_PICS];
> -struct irq_domain *pch_pic_domain[MAX_IO_PICS];
> +struct acpi_vector_group pch_group[MAX_IO_PICS];
>
> /*
> * 'what should we do if we get a hw irq event on an illegal vector'.
> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
> index a4eb8a2..170a5b9 100644
> --- a/drivers/irqchip/irq-loongson-pch-pic.c
> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
> @@ -33,13 +33,40 @@
> #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];
Why 2? Is that related to MAX_IO_PICS being 2?
> +struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
> +
> +int find_pch_pic(u32 gsi)
> +{
> + int i;
> +
> + /* Find the PCH_PIC that manages this GSI. */
> + for (i = 0; i < MAX_IO_PICS; i++) {
> + struct pch_pic *priv = pch_pic_priv[i];
> +
> + if (!priv)
> + return -1;
> +
> + if (gsi >= priv->gsi_base && gsi <= priv->gsi_end)
> + return i;
> + }
> +
> + pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
> + return -1;
> +}
> +
> static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
> {
> u32 reg;
> @@ -139,6 +166,24 @@ static void pch_pic_ack_irq(struct irq_data *d)
> .irq_set_type = pch_pic_set_type,
> };
>
> +static int pch_pic_domain_translate(struct irq_domain *d,
> + struct irq_fwspec *fwspec,
> + unsigned long *hwirq,
> + unsigned int *type)
> +{
> + struct pch_pic *priv = d->host_data;
> + struct device_node *of_node = to_of_node(fwspec->fwnode);
> +
> + if (fwspec->param_count < 1)
> + return -EINVAL;
> + if (of_node)
> + *hwirq += priv->ht_vec_base;
> + else
> + *hwirq = fwspec->param[0] - priv->gsi_base;
> + *type = IRQ_TYPE_NONE;
Have you tested this on MIPS HW? This looks like a regression on the
OF path, as it used irq_domain_translate_twocell().
> +
> + return 0;
> +}
> static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
> unsigned int nr_irqs, void *arg)
> {
> @@ -149,13 +194,13 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
> struct irq_fwspec parent_fwspec;
> struct pch_pic *priv = domain->host_data;
>
> - err = irq_domain_translate_twocell(domain, fwspec, &hwirq, &type);
> + err = pch_pic_domain_translate(domain, fwspec, &hwirq, &type);
> if (err)
> return err;
>
> parent_fwspec.fwnode = domain->parent->fwnode;
> parent_fwspec.param_count = 1;
> - parent_fwspec.param[0] = hwirq + priv->ht_vec_base;
> + parent_fwspec.param[0] = hwirq;
>
> err = irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
> if (err)
> @@ -170,7 +215,7 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
> }
>
> static const struct irq_domain_ops pch_pic_domain_ops = {
> - .translate = irq_domain_translate_twocell,
> + .translate = pch_pic_domain_translate,
> .alloc = pch_pic_alloc,
> .free = irq_domain_free_irqs_parent,
> };
> @@ -180,7 +225,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 +243,41 @@ 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;
Why isn't this pre-filled on the OF path as it isn't relevant to ACPI?
> + vec_count = ((readq(priv->base) >> 48) & 0xff) + 1;
Is that valid on the old HW?
> + priv->gsi_base = gsi_base;
Why isn't this pre-filled on the ACPI path as it isn't relevant to OF?
> + priv->gsi_end = gsi_base + vec_count - 1;
I don't think this needs to be ACPI specific. Just track the vector
count, which applies to both ACPI and DT.
>
> 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_handle[nr_pics] = domain_handle;
> + pch_pic_priv[nr_pics++] = priv;
>
> return 0;
>
> @@ -250,7 +286,111 @@ 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;
> +}
Missing blank line between functions
> +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;
> +}
> +
> +struct irq_domain *acpi_get_pch_parent(int node)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_IO_PICS; i++) {
> + if (node == pch_group[i].node)
> + return pch_group[i].parent;
> + }
> + return NULL;
> +}
> +
> +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;
> +
> + 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,
Where is this defined? It only appears in the documentation, and
nowhere else...
> + pchintc_parse_madt, 0);
> +
> + return 0;
> +}
> +early_initcall(pch_pic_acpi_init);
Why can't you use IRQCHIP_ACPI_DECLARE here? This is terribly fragile,
and will eventually break. I really don't want to rely on this.
M.
--
Without deviation from the norm, progress is not possible.
On 2022/6/29 下午7:20, Marc Zyngier wrote:
> On Mon, 27 Jun 2022 12:39:50 +0100,
> Jianmin Lv <[email protected]> wrote:
>>
>> From: Huacai Chen <[email protected]>
>>
>> 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.
>
> Drop this paragraph.
>
Ok, I'll drop it for the patch and other patches of this series.
>>
>> 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.
>>
>> For systems with two chipsets, there are two related pch irqdomains,
>> each of which has the same node id as its parent irqdomain. 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 parent irqdomain will be set for node id in parent irqdomain driver
>> before pch irqdomain is created.
>>
>> Co-developed-by: Jianmin Lv <[email protected]>
>> Signed-off-by: Jianmin Lv <[email protected]>
>> Signed-off-by: Huacai Chen <[email protected]>
>> ---
>> arch/loongarch/include/asm/irq.h | 11 +-
>> arch/loongarch/kernel/irq.c | 2 +-
>> drivers/irqchip/irq-loongson-pch-pic.c | 200 ++++++++++++++++++++++++++++-----
>> 3 files changed, 179 insertions(+), 34 deletions(-)
>>
>> diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h
>> index 48c0ce4..a444dc6 100644
>> --- a/arch/loongarch/include/asm/irq.h
>> +++ b/arch/loongarch/include/asm/irq.h
>> @@ -48,6 +48,12 @@ static inline bool on_irq_stack(int cpu, unsigned long sp)
>> #define MAX_IO_PICS 2
>> #define NR_IRQS (64 + (256 * MAX_IO_PICS))
>>
>> +struct acpi_vector_group {
>> + int node;
>> + struct irq_domain *parent;
>> +};
>> +extern struct acpi_vector_group pch_group[MAX_IO_PICS];
>> +
>> #define CORES_PER_EIO_NODE 4
>>
>> #define LOONGSON_CPU_UART0_VEC 10 /* CPU UART0 */
>> @@ -108,8 +114,7 @@ int pch_lpc_acpi_init(struct irq_domain *parent,
>> struct acpi_madt_lpc_pic *acpi_pchlpc);
>> struct irq_domain *pch_msi_acpi_init(struct irq_domain *parent,
>> struct acpi_madt_msi_pic *acpi_pchmsi);
>> -struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent,
>> - struct acpi_madt_bio_pic *acpi_pchpic);
>> +int find_pch_pic(u32 gsi);
>>
>> extern struct acpi_madt_lio_pic *acpi_liointc;
>> extern struct acpi_madt_eio_pic *acpi_eiointc[MAX_IO_PICS];
>> @@ -123,7 +128,7 @@ struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent,
>> extern struct irq_domain *liointc_domain;
>> extern struct fwnode_handle *pch_lpc_handle;
>> extern struct irq_domain *pch_msi_domain[MAX_IO_PICS];
>> -extern struct irq_domain *pch_pic_domain[MAX_IO_PICS];
>> +extern struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>>
>> extern irqreturn_t loongson3_ipi_interrupt(int irq, void *dev);
>>
>> diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c
>> index 07d6059..f2115be 100644
>> --- a/arch/loongarch/kernel/irq.c
>> +++ b/arch/loongarch/kernel/irq.c
>> @@ -28,7 +28,7 @@
>> struct irq_domain *cpu_domain;
>> struct irq_domain *liointc_domain;
>> struct irq_domain *pch_msi_domain[MAX_IO_PICS];
>> -struct irq_domain *pch_pic_domain[MAX_IO_PICS];
>> +struct acpi_vector_group pch_group[MAX_IO_PICS];
>>
>> /*
>> * 'what should we do if we get a hw irq event on an illegal vector'.
>> diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
>> index a4eb8a2..170a5b9 100644
>> --- a/drivers/irqchip/irq-loongson-pch-pic.c
>> +++ b/drivers/irqchip/irq-loongson-pch-pic.c
>> @@ -33,13 +33,40 @@
>> #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];
>
> Why 2? Is that related to MAX_IO_PICS being 2?
>
Yes, thanks, I should use MAX_IO_PICS for it.
>> +struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>> +
>> +int find_pch_pic(u32 gsi)
>> +{
>> + int i;
>> +
>> + /* Find the PCH_PIC that manages this GSI. */
>> + for (i = 0; i < MAX_IO_PICS; i++) {
>> + struct pch_pic *priv = pch_pic_priv[i];
>> +
>> + if (!priv)
>> + return -1;
>> +
>> + if (gsi >= priv->gsi_base && gsi <= priv->gsi_end)
>> + return i;
>> + }
>> +
>> + pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
>> + return -1;
>> +}
>> +
>> static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
>> {
>> u32 reg;
>> @@ -139,6 +166,24 @@ static void pch_pic_ack_irq(struct irq_data *d)
>> .irq_set_type = pch_pic_set_type,
>> };
>>
>> +static int pch_pic_domain_translate(struct irq_domain *d,
>> + struct irq_fwspec *fwspec,
>> + unsigned long *hwirq,
>> + unsigned int *type)
>> +{
>> + struct pch_pic *priv = d->host_data;
>> + struct device_node *of_node = to_of_node(fwspec->fwnode);
>> +
>> + if (fwspec->param_count < 1)
>> + return -EINVAL;
>> + if (of_node)
>> + *hwirq += priv->ht_vec_base;
>> + else
>> + *hwirq = fwspec->param[0] - priv->gsi_base;
>> + *type = IRQ_TYPE_NONE;
>
> Have you tested this on MIPS HW? This looks like a regression on the
> OF path, as it used irq_domain_translate_twocell().
>
No, I don't test on MIPS for this version patch, I want to use 'of_node'
to distinguish MIPS and LoongArch, but it seems that there is still
other problem, I'll fix it as following in next version and test it on MIPS.
if (of_node) {
*hwirq = fwspec->param[0] + priv->ht_vec_base;
*type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
} else {
*hwirq = fwspec->param[0] - priv->gsi_base;
*type = IRQ_TYPE_NONE;
}
>> +
>> + return 0;
>> +}
>> static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
>> unsigned int nr_irqs, void *arg)
>> {
>> @@ -149,13 +194,13 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
>> struct irq_fwspec parent_fwspec;
>> struct pch_pic *priv = domain->host_data;
>>
>> - err = irq_domain_translate_twocell(domain, fwspec, &hwirq, &type);
>> + err = pch_pic_domain_translate(domain, fwspec, &hwirq, &type);
>> if (err)
>> return err;
>>
>> parent_fwspec.fwnode = domain->parent->fwnode;
>> parent_fwspec.param_count = 1;
>> - parent_fwspec.param[0] = hwirq + priv->ht_vec_base;
>> + parent_fwspec.param[0] = hwirq;
>>
>> err = irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
>> if (err)
>> @@ -170,7 +215,7 @@ static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
>> }
>>
>> static const struct irq_domain_ops pch_pic_domain_ops = {
>> - .translate = irq_domain_translate_twocell,
>> + .translate = pch_pic_domain_translate,
>> .alloc = pch_pic_alloc,
>> .free = irq_domain_free_irqs_parent,
>> };
>> @@ -180,7 +225,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 +243,41 @@ 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;
>
> Why isn't this pre-filled on the OF path as it isn't relevant to ACPI?
>
No, ht_vec_base is required for both of OF and ACPI.
pch_pic_init is called from pch_pic_of_init for OF path and from
pch_pic_init_irqdomain for ACPI path, so the 'vec_base' is passed into
pch_pic_init to pre-fill 'ht_vec_base' from both path.
>> + vec_count = ((readq(priv->base) >> 48) & 0xff) + 1;
>
> Is that valid on the old HW?
>
Yes.
>> + priv->gsi_base = gsi_base;
>
> Why isn't this pre-filled on the ACPI path as it isn't relevant to OF?
>
Yes, it's only for ACPI.
>> + priv->gsi_end = gsi_base + vec_count - 1;
>
> I don't think this needs to be ACPI specific. Just track the vector
> count, which applies to both ACPI and DT.
>
Agree, thanks for your suggestion, tracking vector count is enough for both.
>>
>> 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_handle[nr_pics] = domain_handle;
>> + pch_pic_priv[nr_pics++] = priv;
>>
>> return 0;
>>
>> @@ -250,7 +286,111 @@ 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;
>> +}
>
> Missing blank line between functions
>
Ok, thanks for your correction.
>> +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;
>> +}
>> +
>> +struct irq_domain *acpi_get_pch_parent(int node)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < MAX_IO_PICS; i++) {
>> + if (node == pch_group[i].node)
>> + return pch_group[i].parent;
>> + }
>> + return NULL;
>> +}
>> +
>> +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;
>> +
>> + 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,
>
> Where is this defined? It only appears in the documentation, and
> nowhere else...
>
It's defined in the patch to ACPICA(which has been merged, please to see
https://github.com/acpica/acpica/commit/1dc530059a3e6202e941e6a9478cf30f092bfb47).
the patch will be synchronized to linux kernel by maintainer of ACPICA.
>> + pchintc_parse_madt, 0);
>> +
>> + return 0;
>> +}
>> +early_initcall(pch_pic_acpi_init);
>
> Why can't you use IRQCHIP_ACPI_DECLARE here? This is terribly fragile,
> and will eventually break. I really don't want to rely on this.
>
In early time, the change here is implemented using
IRQCHIP_ACPI_DECLARE, but we found that calling order(during
irqchip_init) of the entry declared using IRQCHIP_ACPI_DECLARE is
depended on the compiling order(driver order in Makefile) of the driver.
For removing the dependency to the compiling order, the new way here is
used(I looked into ARM, it seems that GIC driver uses
IRQCHIP_ACPI_DECLARE, and ITS driver uses early_initcall too.).
> M.
>
On Thu, 30 Jun 2022 05:36:40 +0100,
Jianmin Lv <[email protected]> wrote:
>
>
[...]
> >> +static int __init pch_pic_acpi_init(void)
> >> +{
> >> + acpi_table_parse_madt(ACPI_MADT_TYPE_BIO_PIC,
> >
> > Where is this defined? It only appears in the documentation, and
> > nowhere else...
> >
>
> It's defined in the patch to ACPICA(which has been merged, please to
> see
> https://github.com/acpica/acpica/commit/1dc530059a3e6202e941e6a9478cf30f092bfb47).
> the patch will be synchronized to linux kernel by maintainer of ACPICA.
How can I take this patch upstream if it doesn't even compile? Please
make this patch part of your series. There are tons of patches that
need Acks from the ACPI maintainers, this is only one of them.
>
>
> >> + pchintc_parse_madt, 0);
> >> +
> >> + return 0;
> >> +}
> >> +early_initcall(pch_pic_acpi_init);
> >
> > Why can't you use IRQCHIP_ACPI_DECLARE here? This is terribly fragile,
> > and will eventually break. I really don't want to rely on this.
> >
>
> In early time, the change here is implemented using
> IRQCHIP_ACPI_DECLARE, but we found that calling order(during
> irqchip_init) of the entry declared using IRQCHIP_ACPI_DECLARE is
> depended on the compiling order(driver order in Makefile) of the
> driver. For removing the dependency to the compiling order, the new
> way here is used(I looked into ARM, it seems that GIC driver uses
> IRQCHIP_ACPI_DECLARE, and ITS driver uses early_initcall too.).
It's not quite the same. The ITS part that uses early_initcall isn't
an actual driver (it only registers a domain, and nothing else).
There is also no guarantee that the initcalls at the same priority
will execute in any order, and you already have at least two such
initcalls (pch_pic_acpi_init, pch_msi_acpi_init, and I haven't quite
understood how the rest is probed yet).
One possibility would be to drive the whole probing from the root
interrupt controller, which is similar to what GICv3 does for the
actual ITS driver. You already do this sort of stuff in the CPUINTC
driver, so adding to this shouldn't be too hard.
M.
--
Without deviation from the norm, progress is not possible.
On 2022/6/30 下午3:22, Marc Zyngier wrote:
> On Thu, 30 Jun 2022 05:36:40 +0100,
> Jianmin Lv <[email protected]> wrote:
>>
>>
>
> [...]
>
>>>> +static int __init pch_pic_acpi_init(void)
>>>> +{
>>>> + acpi_table_parse_madt(ACPI_MADT_TYPE_BIO_PIC,
>>>
>>> Where is this defined? It only appears in the documentation, and
>>> nowhere else...
>>>
>>
>> It's defined in the patch to ACPICA(which has been merged, please to
>> see
>> https://github.com/acpica/acpica/commit/1dc530059a3e6202e941e6a9478cf30f092bfb47).
>> the patch will be synchronized to linux kernel by maintainer of ACPICA.
>
> How can I take this patch upstream if it doesn't even compile? Please
> make this patch part of your series. There are tons of patches that
> need Acks from the ACPI maintainers, this is only one of them.
>
Ok, I'll put the patch into my series.
>>
>>
>>>> + pchintc_parse_madt, 0);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +early_initcall(pch_pic_acpi_init);
>>>
>>> Why can't you use IRQCHIP_ACPI_DECLARE here? This is terribly fragile,
>>> and will eventually break. I really don't want to rely on this.
>>>
>>
>> In early time, the change here is implemented using
>> IRQCHIP_ACPI_DECLARE, but we found that calling order(during
>> irqchip_init) of the entry declared using IRQCHIP_ACPI_DECLARE is
>> depended on the compiling order(driver order in Makefile) of the
>> driver. For removing the dependency to the compiling order, the new
>> way here is used(I looked into ARM, it seems that GIC driver uses
>> IRQCHIP_ACPI_DECLARE, and ITS driver uses early_initcall too.).
>
> It's not quite the same. The ITS part that uses early_initcall isn't
> an actual driver (it only registers a domain, and nothing else).
>
Ok, thanks, I got it, I don't understand ARM's irqchips well.
> There is also no guarantee that the initcalls at the same priority
> will execute in any order, and you already have at least two such
> initcalls (pch_pic_acpi_init, pch_msi_acpi_init, and I haven't quite
> understood how the rest is probed yet).
>
Yes, agree, but pch_pic and pch_msi are independent, and there is no
dependencies between them, so there is no requirement on calling order
for the two entries.
> One possibility would be to drive the whole probing from the root
> interrupt controller, which is similar to what GICv3 does for the
> actual ITS driver. You already do this sort of stuff in the CPUINTC
> driver, so adding to this shouldn't be too hard.
>
Ok, thanks for your suggestion, I'll try to change it as the way in the
CPUINTC driver, it seems that way is better.
> M.
>