* [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers
@ 2022-06-15 6:07 Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains Jianmin Lv
` (7 more replies)
0 siblings, 8 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 6:07 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Hanjun Guo, Lorenzo Pieralisi, Jiaxun Yang, Huacai Chen
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 12083 bytes --]
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.
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
Huacai Chen (7):
irqchip: Add LoongArch CPU interrupt 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 Loongson PCH LPC controller support
Jianmin Lv (2):
genirq/generic_chip: export irq_unmap_generic_chip
irqchip: create library file for LoongArch irqchip driver
Marc Zyngier (1):
APCI: irq: Add support for multiple GSI domains
drivers/acpi/bus.c | 3 +
drivers/acpi/irq.c | 40 ++--
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 | 134 +++++++++++
drivers/irqchip/irq-loongarch-pic-common.c | 122 ++++++++++
drivers/irqchip/irq-loongarch-pic-common.h | 39 ++++
drivers/irqchip/irq-loongson-eiointc.c | 347 +++++++++++++++++++++++++++++
drivers/irqchip/irq-loongson-htvec.c | 119 +++++++---
drivers/irqchip/irq-loongson-liointc.c | 225 ++++++++++++-------
drivers/irqchip/irq-loongson-pch-lpc.c | 202 +++++++++++++++++
drivers/irqchip/irq-loongson-pch-msi.c | 138 ++++++++----
drivers/irqchip/irq-loongson-pch-pic.c | 171 +++++++++++---
include/linux/acpi.h | 3 +-
include/linux/cpuhotplug.h | 1 +
include/linux/irq.h | 1 +
kernel/irq/generic-chip.c | 2 +-
19 files changed, 1402 insertions(+), 212 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] 27+ messages in thread
* [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
@ 2022-06-15 6:07 ` Jianmin Lv
2022-06-15 7:14 ` Marc Zyngier
2022-06-15 6:07 ` [PATCH V12 02/10] genirq/generic_chip: export irq_unmap_generic_chip Jianmin Lv
` (6 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 6:07 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Hanjun Guo, Lorenzo Pieralisi, Jiaxun Yang, Huacai Chen
From: Marc Zyngier <maz@kernel.org>
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
architectures 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).
Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: Hanjun Guo <guohanjun@huawei.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
drivers/irqchip/irq-gic.c | 18 ++++++++++++------
include/linux/acpi.h | 2 +-
4 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
index c68e694..b7460ab 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,10 +26,7 @@
*/
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);
-
- *irq = irq_find_mapping(d, gsi);
+ *irq = acpi_register_gsi(NULL, gsi, -1, -1);
/*
* *irq == 0 means no mapping, that should
* be reported as a failure
@@ -53,12 +50,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 +70,14 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
*/
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 +95,8 @@ void acpi_unregister_gsi(u32 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 +104,7 @@ void acpi_unregister_gsi(u32 gsi)
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 +193,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(irq->interrupts[ctx->index]);
acpi_irq_parse_one_match(fwnode, irq->interrupts[ctx->index],
irq->triggering, irq->polarity,
irq->shareable, ctx);
@@ -207,7 +206,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 +291,10 @@ int acpi_irq_get(acpi_handle handle, unsigned int index, struct resource *res)
* 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 +312,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 2be8dea..87b1f53 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 @@ static void __init gic_acpi_setup_kvm_info(void)
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 @@ static void __init gic_acpi_setup_kvm_info(void)
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 820404c..4c7bae0 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 4f82a5b..957e23f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -356,7 +356,7 @@ static inline bool acpi_sci_irq_valid(void)
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,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH V12 02/10] genirq/generic_chip: export irq_unmap_generic_chip
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains Jianmin Lv
@ 2022-06-15 6:07 ` Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 03/10] irqchip: Add LoongArch CPU interrupt controller support Jianmin Lv
` (5 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 6:07 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Hanjun Guo, Lorenzo Pieralisi, Jiaxun Yang, Huacai Chen
Some irq controllers have to re-implement a private version for
irq_generic_chip_ops, because they have a different xlate to translate
hwirq. Export irq_unmap_generic_chip for reusing in user defined
ops.
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
include/linux/irq.h | 1 +
kernel/irq/generic-chip.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 5053082..83a4574 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1121,6 +1121,7 @@ struct irq_domain_chip_generic {
/* Setup functions for irq_chip_generic */
int irq_map_generic_chip(struct irq_domain *d, unsigned int virq,
irq_hw_number_t hw_irq);
+void irq_unmap_generic_chip(struct irq_domain *d, unsigned int virq);
struct irq_chip_generic *
irq_alloc_generic_chip(const char *name, int nr_ct, unsigned int irq_base,
void __iomem *reg_base, irq_flow_handler_t handler);
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index f0862eb..c653cd3 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -431,7 +431,7 @@ int irq_map_generic_chip(struct irq_domain *d, unsigned int virq,
return 0;
}
-static void irq_unmap_generic_chip(struct irq_domain *d, unsigned int virq)
+void irq_unmap_generic_chip(struct irq_domain *d, unsigned int virq)
{
struct irq_data *data = irq_domain_get_irq_data(d, virq);
struct irq_domain_chip_generic *dgc = d->gc;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH V12 03/10] irqchip: Add LoongArch CPU interrupt controller support
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 02/10] genirq/generic_chip: export irq_unmap_generic_chip Jianmin Lv
@ 2022-06-15 6:07 ` Jianmin Lv
2022-06-18 10:59 ` Marc Zyngier
2022-06-15 6:07 ` [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver Jianmin Lv
` (4 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 6:07 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Hanjun Guo, Lorenzo Pieralisi, Jiaxun Yang, Huacai Chen
From: Huacai Chen <chenhuacai@loongson.cn>
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.
Change-Id: I53fb0be768daeeecc90d0ccc0bb0becd3d4e6984
Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
drivers/acpi/bus.c | 3 +
drivers/irqchip/Kconfig | 10 +++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-loongarch-cpu.c | 134 ++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 1 +
5 files changed, 149 insertions(+)
create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 86fa61a..63fbf00 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1145,6 +1145,9 @@ static int __init acpi_bus_init_irq(void)
case ACPI_IRQ_MODEL_PLATFORM:
message = "platform specific model";
break;
+ case ACPI_IRQ_MODEL_LPIC:
+ message = "LPIC";
+ break;
default:
pr_info("Unknown interrupt routing model\n");
return -ENODEV;
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 4ab1038..4126b1c 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -546,6 +546,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 5b67450..6894a13 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -103,6 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
+obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
obj-$(CONFIG_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..c382bd9
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-cpu.c
@@ -0,0 +1,134 @@
+// 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);
+ }
+}
+
+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);
+}
+
+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)
+{
+ struct fwnode_handle *domain_handle;
+
+ /* Mask interrupts. */
+ clear_csr_ecfg(ECFG0_IM);
+ clear_csr_estat(ESTATF_IP);
+
+ domain_handle = irq_domain_alloc_fwnode(NULL);
+ irq_domain = irq_domain_create_linear(domain_handle, EXCCODE_INT_NUM,
+ &loongarch_cpu_intc_irq_domain_ops, NULL);
+
+ if (!irq_domain)
+ panic("Failed to add irqdomain for LoongArch CPU");
+
+ set_handle_irq(&handle_cpu_irq);
+ acpi_set_irq_model(ACPI_IRQ_MODEL_LPIC, lpic_get_gsi_domain_id);
+
+ return irq_domain;
+}
+
+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);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 957e23f..d2f5108 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -105,6 +105,7 @@ enum acpi_irq_model_id {
ACPI_IRQ_MODEL_IOSAPIC,
ACPI_IRQ_MODEL_PLATFORM,
ACPI_IRQ_MODEL_GIC,
+ ACPI_IRQ_MODEL_LPIC,
ACPI_IRQ_MODEL_COUNT
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
` (2 preceding siblings ...)
2022-06-15 6:07 ` [PATCH V12 03/10] irqchip: Add LoongArch CPU interrupt controller support Jianmin Lv
@ 2022-06-15 6:07 ` Jianmin Lv
2022-06-18 17:22 ` Marc Zyngier
2022-06-15 6:07 ` [PATCH V12 05/10] irqchip/loongson-pch-pic: Add ACPI init support Jianmin Lv
` (3 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 6:07 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Hanjun Guo, Lorenzo Pieralisi, Jiaxun Yang, Huacai Chen
The library file contains following content:
- Implement acpi_get_gsi_domain_id callback.
- Implement initialization of vector group entries and APIs
for building hierachy irqdomains.
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-loongarch-pic-common.c | 122 +++++++++++++++++++++++++++++
drivers/irqchip/irq-loongarch-pic-common.h | 39 +++++++++
3 files changed, 162 insertions(+), 1 deletion(-)
create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c
create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 6894a13..2d0d871 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -103,7 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
-obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
+obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-pic-common.o
obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o
obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
new file mode 100644
index 0000000..2f75362
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-pic-common.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
+ */
+
+#include <linux/irq.h>
+#include <linux/pci.h>
+#include <linux/acpi.h>
+#include "irq-loongarch-pic-common.h"
+
+static struct acpi_vector_group vector_group[MAX_IO_PICS];
+
+struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
+
+struct fwnode_handle *liointc_handle;
+struct fwnode_handle *pch_lpc_handle;
+struct fwnode_handle *pch_msi_handle[MAX_IO_PICS];
+struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
+
+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;
+}
+
+struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi)
+{
+ int id;
+ struct fwnode_handle *domain_handle = NULL;
+
+ switch (gsi) {
+ case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
+ if (liointc_handle)
+ domain_handle = liointc_handle;
+ break;
+
+ case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
+ if (pch_lpc_handle)
+ domain_handle = pch_lpc_handle;
+ break;
+
+ case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
+ id = find_pch_pic(gsi);
+ if (id >= 0 && pch_pic_handle[id])
+ domain_handle = pch_pic_handle[id];
+
+ break;
+ }
+
+ return domain_handle;
+}
+
+static int pci_mcfg_parse(struct acpi_table_header *header)
+{
+ struct acpi_table_mcfg *mcfg;
+ struct acpi_mcfg_allocation *mptr;
+ int i, n;
+
+ if (header->length < sizeof(struct acpi_table_mcfg))
+ return -EINVAL;
+
+ n = (header->length - sizeof(struct acpi_table_mcfg)) /
+ sizeof(struct acpi_mcfg_allocation);
+ mcfg = (struct acpi_table_mcfg *)header;
+ mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
+
+ for (i = 0; i < n; i++, mptr++)
+ vector_group[mptr->pci_segment].node = (mptr->address >> 44) & 0xf;
+
+ return 0;
+}
+
+void __init init_vector_parent_group(void)
+{
+ acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
+}
+
+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..3e068c3
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-pic-common.h
@@ -0,0 +1,39 @@
+/* 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)
+
+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);
+struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi);
+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] 27+ messages in thread
* [PATCH V12 05/10] irqchip/loongson-pch-pic: Add ACPI init support
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
` (3 preceding siblings ...)
2022-06-15 6:07 ` [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver Jianmin Lv
@ 2022-06-15 6:07 ` Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 06/10] irqchip/loongson-pch-msi: " Jianmin Lv
` (2 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 6:07 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Hanjun Guo, Lorenzo Pieralisi, Jiaxun Yang, Huacai Chen
From: Huacai Chen <chenhuacai@loongson.cn>
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.
Change-Id: I40e9b9e5298d7b3f6ae2ab2ba69f5bcfa50c1418
Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
drivers/irqchip/irq-loongson-pch-pic.c | 171 +++++++++++++++++++++++++++------
1 file changed, 141 insertions(+), 30 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index a4eb8a2..1426b93 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;
@@ -139,6 +147,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 +175,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 +196,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 +206,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 +224,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 +267,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] 27+ messages in thread
* [PATCH V12 06/10] irqchip/loongson-pch-msi: Add ACPI init support
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
` (4 preceding siblings ...)
2022-06-15 6:07 ` [PATCH V12 05/10] irqchip/loongson-pch-pic: Add ACPI init support Jianmin Lv
@ 2022-06-15 6:07 ` Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 07/10] irqchip/loongson-htvec: " Jianmin Lv
2022-06-18 10:39 ` [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Marc Zyngier
7 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 6:07 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Hanjun Guo, Lorenzo Pieralisi, Jiaxun Yang, Huacai Chen
From: Huacai Chen <chenhuacai@loongson.cn>
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.
Change-Id: Ifaa229d36212d645ca423f1e2ff7c0cb5fd0cc01
Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@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..a2e94f3 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_handle[nr_pics] = domain_handle;
+ 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] 27+ messages in thread
* [PATCH V12 07/10] irqchip/loongson-htvec: Add ACPI init support
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
` (5 preceding siblings ...)
2022-06-15 6:07 ` [PATCH V12 06/10] irqchip/loongson-pch-msi: " Jianmin Lv
@ 2022-06-15 6:07 ` Jianmin Lv
2022-06-18 10:39 ` [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Marc Zyngier
7 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 6:07 UTC (permalink / raw)
To: Thomas Gleixner, Marc Zyngier
Cc: linux-kernel, Hanjun Guo, Lorenzo Pieralisi, Jiaxun Yang, Huacai Chen
From: Huacai Chen <chenhuacai@loongson.cn>
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.
HTVECINTC stands for "HyperTransport Interrupts" that described in
Section 14.3 of "Loongson 3A5000 Processor Reference Manual". For more
information please refer Documentation/loongarch/irq-chip-model.rst.
Change-Id: Id79050b46d31a9118a669972f3419585de8d8b11
Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
drivers/irqchip/irq-loongson-htvec.c | 119 +++++++++++++++++++++++++----------
1 file changed, 85 insertions(+), 34 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-htvec.c b/drivers/irqchip/irq-loongson-htvec.c
index 60a335d..72b3874 100644
--- a/drivers/irqchip/irq-loongson-htvec.c
+++ b/drivers/irqchip/irq-loongson-htvec.c
@@ -16,11 +16,11 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include "irq-loongarch-pic-common.h"
/* Registers */
#define HTVEC_EN_OFF 0x20
#define HTVEC_MAX_PARENT_IRQ 8
-
#define VEC_COUNT_PER_REG 32
#define VEC_REG_IDX(irq_id) ((irq_id) / VEC_COUNT_PER_REG)
#define VEC_REG_BIT(irq_id) ((irq_id) % VEC_COUNT_PER_REG)
@@ -30,8 +30,11 @@ struct htvec {
void __iomem *base;
struct irq_domain *htvec_domain;
raw_spinlock_t htvec_lock;
+ struct fwnode_handle *domain_handle;
};
+static struct htvec *htvec_priv;
+
static void htvec_irq_dispatch(struct irq_desc *desc)
{
int i;
@@ -155,64 +158,112 @@ static void htvec_reset(struct htvec *priv)
}
}
-static int htvec_of_init(struct device_node *node,
- struct device_node *parent)
+static int htvec_init(phys_addr_t addr, unsigned long size,
+ int num_parents, int parent_irq[], struct fwnode_handle *domain_handle)
{
+ int i;
struct htvec *priv;
- int err, parent_irq[8], i;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
+ priv->num_parents = num_parents;
+ priv->base = ioremap(addr, size);
+ priv->domain_handle = domain_handle;
raw_spin_lock_init(&priv->htvec_lock);
- priv->base = of_iomap(node, 0);
- if (!priv->base) {
- err = -ENOMEM;
- goto free_priv;
- }
-
- /* Interrupt may come from any of the 8 interrupt lines */
- for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
- parent_irq[i] = irq_of_parse_and_map(node, i);
- if (parent_irq[i] <= 0)
- break;
-
- priv->num_parents++;
- }
- if (!priv->num_parents) {
- pr_err("Failed to get parent irqs\n");
- err = -ENODEV;
- goto iounmap_base;
- }
-
- priv->htvec_domain = irq_domain_create_linear(of_node_to_fwnode(node),
+ /* Setup IRQ domain */
+ priv->htvec_domain = irq_domain_create_linear(priv->domain_handle,
(VEC_COUNT_PER_REG * priv->num_parents),
&htvec_domain_ops, priv);
if (!priv->htvec_domain) {
- pr_err("Failed to create IRQ domain\n");
- err = -ENOMEM;
- goto irq_dispose;
+ pr_err("loongson-htvec: cannot add IRQ domain\n");
+ goto iounmap_base;
}
htvec_reset(priv);
- for (i = 0; i < priv->num_parents; i++)
+ for (i = 0; i < priv->num_parents; i++) {
irq_set_chained_handler_and_data(parent_irq[i],
htvec_irq_dispatch, priv);
+ }
+
+ htvec_priv = priv;
return 0;
-irq_dispose:
- for (; i > 0; i--)
- irq_dispose_mapping(parent_irq[i - 1]);
iounmap_base:
iounmap(priv->base);
-free_priv:
+ priv->domain_handle = NULL;
kfree(priv);
- return err;
+ return -EINVAL;
+}
+
+#ifdef CONFIG_OF
+
+static int htvec_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ int i, err;
+ int parent_irq[8];
+ int num_parents = 0;
+ struct resource res;
+
+ if (of_address_to_resource(node, 0, &res))
+ return -EINVAL;
+
+ /* Interrupt may come from any of the 8 interrupt lines */
+ for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
+ parent_irq[i] = irq_of_parse_and_map(node, i);
+ if (parent_irq[i] <= 0)
+ break;
+
+ num_parents++;
+ }
+
+ err = htvec_init(res.start, resource_size(&res),
+ num_parents, parent_irq, of_node_to_fwnode(node));
+ if (err < 0)
+ return err;
+
+ return 0;
}
IRQCHIP_DECLARE(htvec, "loongson,htvec-1.0", htvec_of_init);
+
+#endif
+
+#ifdef CONFIG_ACPI
+int __init htvec_acpi_init(struct irq_domain *parent,
+ struct acpi_madt_ht_pic *acpi_htvec)
+{
+ int i, ret;
+ int num_parents, parent_irq[8];
+ struct fwnode_handle *domain_handle;
+
+ if (!acpi_htvec)
+ return -EINVAL;
+
+ num_parents = HTVEC_MAX_PARENT_IRQ;
+
+ domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_htvec);
+ if (!domain_handle) {
+ pr_err("Unable to allocate domain handle\n");
+ return -ENOMEM;
+ }
+
+ /* Interrupt may come from any of the 8 interrupt lines */
+ for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++)
+ parent_irq[i] = irq_create_mapping(parent, acpi_htvec->cascade[i]);
+
+ ret = htvec_init(acpi_htvec->address, acpi_htvec->size,
+ num_parents, parent_irq, domain_handle);
+ if (ret == 0)
+ acpi_set_vector_parent(0, htvec_priv->htvec_domain);
+
+ return ret;
+}
+
+#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-15 6:07 ` [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains Jianmin Lv
@ 2022-06-15 7:14 ` Marc Zyngier
2022-06-15 9:28 ` Jianmin Lv
2022-06-15 9:43 ` Jianmin Lv
0 siblings, 2 replies; 27+ messages in thread
From: Marc Zyngier @ 2022-06-15 7:14 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On Wed, 15 Jun 2022 07:07:21 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
> From: Marc Zyngier <maz@kernel.org>
>
> 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
> architectures 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).
>
> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
I don't think this tag is appropriate here.
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: Hanjun Guo <guohanjun@huawei.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> ---
> drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
> drivers/irqchip/irq-gic.c | 18 ++++++++++++------
> include/linux/acpi.h | 2 +-
> 4 files changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index c68e694..b7460ab 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,10 +26,7 @@
> */
> 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);
> -
> - *irq = irq_find_mapping(d, gsi);
> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
What is this?
- This wasn't part of my initial patch, and randomly changing patches
without mentioning it isn't acceptable
- you *cannot* trigger a registration here, as this isn't what the API
advertises
- what makes you think that passing random values (NULL, -1... )to
acpi_register_gsi() is an acceptable thing to do?
The original patch had:
@@ -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);
/*
and I don't think it needs anything else. If something breaks, let's
discuss it, but don't abuse the API nor the fact that I usually don't
review my own patches to sneak things in...
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-15 7:14 ` Marc Zyngier
@ 2022-06-15 9:28 ` Jianmin Lv
2022-06-18 10:36 ` Marc Zyngier
2022-06-15 9:43 ` Jianmin Lv
1 sibling, 1 reply; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 9:28 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/15 下午3:14, Marc Zyngier wrote:
> On Wed, 15 Jun 2022 07:07:21 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>> From: Marc Zyngier <maz@kernel.org>
>>
>> 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
>> architectures 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).
>>
>> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
>
> I don't think this tag is appropriate here.
>
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> Cc: Hanjun Guo <guohanjun@huawei.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>> ---
>> drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
>> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
>> drivers/irqchip/irq-gic.c | 18 ++++++++++++------
>> include/linux/acpi.h | 2 +-
>> 4 files changed, 48 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>> index c68e694..b7460ab 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,10 +26,7 @@
>> */
>> 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);
>> -
>> - *irq = irq_find_mapping(d, gsi);
>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
>
> What is this?
>
> - This wasn't part of my initial patch, and randomly changing patches
> without mentioning it isn't acceptable
>
> - you *cannot* trigger a registration here, as this isn't what the API
> advertises
>
> - what makes you think that passing random values (NULL, -1... )to
> acpi_register_gsi() is an acceptable thing to do?
>
> The original patch had:
>
> @@ -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);
> /*
>
> and I don't think it needs anything else. If something breaks, let's
> discuss it, but don't abuse the API nor the fact that I usually don't
> review my own patches to sneak things in...
>
Sorry, Marc, I don't know how to communicate with you for my change
here before submitting the patch, maybe I should mention it in the
patch commit or code.
When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
only handle existed mapping and will return -EINVAL if mapping not
found. When I test on my machine, a calling stack is as following:
acpi_bus_init
->acpi_enable_subsystem
->acpi_ev_install_xrupt_handlers
->acpi_ev_install_sci_handler
->acpi_os_install_interrupt_handler
->acpi_gsi_to_irq
the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it so
that new mapping for gsi is created if no mapping is found.
I looked into generic acpi_register_gsi, the existed mapping will be
checked first by calling irq_find_mapping, so I think calling
acpi_register_gsi in acpi_gsi_to_irq can address the problem.
But you're right, I'm wrong that I passed random value of -1 to
acpi_register_gsi. I don't find a right way to address the problem
without changing acpi_gsi_to_irq. I'll continue to work for the problem.
> M.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-15 7:14 ` Marc Zyngier
2022-06-15 9:28 ` Jianmin Lv
@ 2022-06-15 9:43 ` Jianmin Lv
1 sibling, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-15 9:43 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/15 下午3:14, Marc Zyngier wrote:
> On Wed, 15 Jun 2022 07:07:21 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>> From: Marc Zyngier <maz@kernel.org>
>>
>> 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
>> architectures 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).
>>
>> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
>
> I don't think this tag is appropriate here.
>
Ok, I'll drop it.
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> Cc: Hanjun Guo <guohanjun@huawei.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>> ---
>> drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
>> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
>> drivers/irqchip/irq-gic.c | 18 ++++++++++++------
>> include/linux/acpi.h | 2 +-
>> 4 files changed, 48 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>> index c68e694..b7460ab 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,10 +26,7 @@
>> */
>> 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);
>> -
>> - *irq = irq_find_mapping(d, gsi);
>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
>
> What is this?
>
> - This wasn't part of my initial patch, and randomly changing patches
> without mentioning it isn't acceptable
>
> - you *cannot* trigger a registration here, as this isn't what the API
> advertises
>
> - what makes you think that passing random values (NULL, -1... )to
> acpi_register_gsi() is an acceptable thing to do?
>
> The original patch had:
>
> @@ -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);
> /*
>
> and I don't think it needs anything else. If something breaks, let's
> discuss it, but don't abuse the API nor the fact that I usually don't
> review my own patches to sneak things in...
>
> M.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-15 9:28 ` Jianmin Lv
@ 2022-06-18 10:36 ` Marc Zyngier
2022-06-20 2:46 ` Jianmin Lv
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Marc Zyngier @ 2022-06-18 10:36 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On Wed, 15 Jun 2022 10:28:47 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
>
>
> On 2022/6/15 下午3:14, Marc Zyngier wrote:
> > On Wed, 15 Jun 2022 07:07:21 +0100,
> > Jianmin Lv <lvjianmin@loongson.cn> wrote:
> >>
> >> From: Marc Zyngier <maz@kernel.org>
> >>
> >> 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
> >> architectures 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).
> >>
> >> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
> >
> > I don't think this tag is appropriate here.
> >
> >> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >> Cc: Hanjun Guo <guohanjun@huawei.com>
> >> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> >> ---
> >> drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
> >> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
> >> drivers/irqchip/irq-gic.c | 18 ++++++++++++------
> >> include/linux/acpi.h | 2 +-
> >> 4 files changed, 48 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> >> index c68e694..b7460ab 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,10 +26,7 @@
> >> */
> >> 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);
> >> -
> >> - *irq = irq_find_mapping(d, gsi);
> >> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
> >
> > What is this?
> >
> > - This wasn't part of my initial patch, and randomly changing patches
> > without mentioning it isn't acceptable
> >
> > - you *cannot* trigger a registration here, as this isn't what the API
> > advertises
> >
> > - what makes you think that passing random values (NULL, -1... )to
> > acpi_register_gsi() is an acceptable thing to do?
> >
> > The original patch had:
> >
> > @@ -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);
> > /*
> >
> > and I don't think it needs anything else. If something breaks, let's
> > discuss it, but don't abuse the API nor the fact that I usually don't
> > review my own patches to sneak things in...
> >
>
> Sorry, Marc, I don't know how to communicate with you for my change
> here before submitting the patch, maybe I should mention it in the
> patch commit or code.
It should at least be discussed first, like you are doing it here.
> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
> only handle existed mapping and will return -EINVAL if mapping not
> found. When I test on my machine, a calling stack is as following:
>
>
> acpi_bus_init
> ->acpi_enable_subsystem
> ->acpi_ev_install_xrupt_handlers
> ->acpi_ev_install_sci_handler
> ->acpi_os_install_interrupt_handler
> ->acpi_gsi_to_irq
>
>
> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it
> so that new mapping for gsi is created if no mapping is found.
So it looks like we have a discrepancy between the x86 and ARM on that
front.
Lorenzo, Hanjun, can you please have a look at this and shed some
light on what the expected behaviour is? It looks like we never
encountered an issue with this on arm64, which tends to indicate that
we don't usually use the above path.
> I looked into generic acpi_register_gsi, the existed mapping will be
> checked first by calling irq_find_mapping, so I think calling
> acpi_register_gsi in acpi_gsi_to_irq can address the problem.
>
> But you're right, I'm wrong that I passed random value of -1 to
> acpi_register_gsi. I don't find a right way to address the problem
> without changing acpi_gsi_to_irq. I'll continue to work for the
> problem.
At the very least, this should be indirected so that the existing
behaviour isn't affected, no matter how badly broken arm64 may or may
not be here. Please have a look at the patch below that should help
you with this.
Thanks,
M.
From 3e6b87ea49473d0eb384f42e76d584a1495a538c Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Sat, 18 Jun 2022 11:29:33 +0100
Subject: [PATCH] ACPI: irq: Allow acpi_gsi_to_irq() to have an arch-specific
fallback
It appears that the generic version of acpi_gsi_to_irq() doesn't
fallback to establishing a mapping if there is no pre-existing
one while the x86 version does.
While arm64 seems unaffected by it, LoongArch is relying on the x86
behaviour. In an effort to prevent new architectures from reinventing
the proverbial wheel, provide an optional callback that the arch code
can set to restore the x86 behaviour.
Hopefully we can eventually get rid of this in the future once
the expected behaviour has been clarified.
Reported-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/acpi/irq.c | 8 ++++++--
include/linux/acpi.h | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
index 6e1633ac1756..66c5f01995d0 100644
--- a/drivers/acpi/irq.c
+++ b/drivers/acpi/irq.c
@@ -13,6 +13,7 @@
enum acpi_irq_model_id acpi_irq_model;
static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
+static int (*acpi_gsi_to_irq_fallback)(u32 gsi);
/**
* acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
@@ -33,9 +34,12 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
*irq = irq_find_mapping(d, gsi);
/*
- * *irq == 0 means no mapping, that should
- * be reported as a failure
+ * *irq == 0 means no mapping, that should be reported as a
+ * failure, unless there is an arch-specific fallback handler.
*/
+ if (!*irq && acpi_gsi_to_irq_fallback)
+ *irq = acpi_gsi_to_irq_fallback(gsi);
+
return (*irq > 0) ? 0 : -EINVAL;
}
EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 957e23f727ea..71d3719e3ec4 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -357,6 +357,7 @@ 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 *(*)(u32));
+void acpi_set_gsi_to_irq_fallback(int (*)(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] 27+ messages in thread
* Re: [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
` (6 preceding siblings ...)
2022-06-15 6:07 ` [PATCH V12 07/10] irqchip/loongson-htvec: " Jianmin Lv
@ 2022-06-18 10:39 ` Marc Zyngier
2022-06-20 2:28 ` Jianmin Lv
7 siblings, 1 reply; 27+ messages in thread
From: Marc Zyngier @ 2022-06-18 10:39 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On Wed, 15 Jun 2022 07:07:20 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
> 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.
>
> 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
>
> Huacai Chen (7):
> irqchip: Add LoongArch CPU interrupt 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 Loongson PCH LPC controller support
>
> Jianmin Lv (2):
> genirq/generic_chip: export irq_unmap_generic_chip
> irqchip: create library file for LoongArch irqchip driver
>
> Marc Zyngier (1):
> APCI: irq: Add support for multiple GSI domains
>
> drivers/acpi/bus.c | 3 +
> drivers/acpi/irq.c | 40 ++--
> 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 | 134 +++++++++++
> drivers/irqchip/irq-loongarch-pic-common.c | 122 ++++++++++
> drivers/irqchip/irq-loongarch-pic-common.h | 39 ++++
> drivers/irqchip/irq-loongson-eiointc.c | 347 +++++++++++++++++++++++++++++
> drivers/irqchip/irq-loongson-htvec.c | 119 +++++++---
> drivers/irqchip/irq-loongson-liointc.c | 225 ++++++++++++-------
> drivers/irqchip/irq-loongson-pch-lpc.c | 202 +++++++++++++++++
> drivers/irqchip/irq-loongson-pch-msi.c | 138 ++++++++----
> drivers/irqchip/irq-loongson-pch-pic.c | 171 +++++++++++---
> include/linux/acpi.h | 3 +-
> include/linux/cpuhotplug.h | 1 +
> include/linux/irq.h | 1 +
> kernel/irq/generic-chip.c | 2 +-
> 19 files changed, 1402 insertions(+), 212 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
One thing I don't see here is the removal of the irq code that
currently lives in arch/loongarch/kernel/acpi.c. It really should be
removed as part of this series with the patch that enables the common
ACPI irq code for this architecture.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 03/10] irqchip: Add LoongArch CPU interrupt controller support
2022-06-15 6:07 ` [PATCH V12 03/10] irqchip: Add LoongArch CPU interrupt controller support Jianmin Lv
@ 2022-06-18 10:59 ` Marc Zyngier
2022-06-20 3:06 ` Jianmin Lv
0 siblings, 1 reply; 27+ messages in thread
From: Marc Zyngier @ 2022-06-18 10:59 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On Wed, 15 Jun 2022 07:07:23 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
> From: Huacai Chen <chenhuacai@loongson.cn>
>
> 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.
>
> Change-Id: I53fb0be768daeeecc90d0ccc0bb0becd3d4e6984
Please drop this Change-Id. The upstream kernel doesn't use Gerrit.
> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> drivers/acpi/bus.c | 3 +
> drivers/irqchip/Kconfig | 10 +++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-loongarch-cpu.c | 134 ++++++++++++++++++++++++++++++++++++
> include/linux/acpi.h | 1 +
> 5 files changed, 149 insertions(+)
> create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 86fa61a..63fbf00 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1145,6 +1145,9 @@ static int __init acpi_bus_init_irq(void)
> case ACPI_IRQ_MODEL_PLATFORM:
> message = "platform specific model";
> break;
> + case ACPI_IRQ_MODEL_LPIC:
> + message = "LPIC";
> + break;
This should be part of the patch that deals with the ACPI-specific
part of the architecture, which is the following patch.
> default:
> pr_info("Unknown interrupt routing model\n");
> return -ENODEV;
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 4ab1038..4126b1c 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -546,6 +546,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 5b67450..6894a13 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -103,6 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> obj-$(CONFIG_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..c382bd9
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongarch-cpu.c
> @@ -0,0 +1,134 @@
> +// 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",
Why is it "LoongArch" and not "CPUINTC", which would make a lot more
sense?
> + .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);
> + }
> +}
> +
> +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);
> +}
> +
> +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)
> +{
> + struct fwnode_handle *domain_handle;
> +
> + /* Mask interrupts. */
> + clear_csr_ecfg(ECFG0_IM);
> + clear_csr_estat(ESTATF_IP);
> +
> + domain_handle = irq_domain_alloc_fwnode(NULL);
Please don't use NULL here, as this is supposed to be a physical
address. If you don't have any physical address at hand (because this
driver isn't using MMIO), use irq_domain_alloc_named_fwnode() instead.
> + irq_domain = irq_domain_create_linear(domain_handle, EXCCODE_INT_NUM,
> + &loongarch_cpu_intc_irq_domain_ops, NULL);
> +
> + if (!irq_domain)
> + panic("Failed to add irqdomain for LoongArch CPU");
> +
> + set_handle_irq(&handle_cpu_irq);
> + acpi_set_irq_model(ACPI_IRQ_MODEL_LPIC, lpic_get_gsi_domain_id);
lpic_get_gsi_domain_id only gets defined in the following patch, so
the series cannot be bisected. Please fix this (the series should
compile every step of the way).
> +
> + return irq_domain;
> +}
> +
> +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);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 957e23f..d2f5108 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -105,6 +105,7 @@ enum acpi_irq_model_id {
> ACPI_IRQ_MODEL_IOSAPIC,
> ACPI_IRQ_MODEL_PLATFORM,
> ACPI_IRQ_MODEL_GIC,
> + ACPI_IRQ_MODEL_LPIC,
This hunk should be moved to the patch that introduces
lpic_get_gsi_domain_id.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver
2022-06-15 6:07 ` [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver Jianmin Lv
@ 2022-06-18 17:22 ` Marc Zyngier
2022-06-20 3:12 ` Jianmin Lv
0 siblings, 1 reply; 27+ messages in thread
From: Marc Zyngier @ 2022-06-18 17:22 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On Wed, 15 Jun 2022 07:07:24 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
> The library file contains following content:
> - Implement acpi_get_gsi_domain_id callback.
> - Implement initialization of vector group entries and APIs
> for building hierachy irqdomains.
>
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> ---
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/irq-loongarch-pic-common.c | 122 +++++++++++++++++++++++++++++
> drivers/irqchip/irq-loongarch-pic-common.h | 39 +++++++++
> 3 files changed, 162 insertions(+), 1 deletion(-)
> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c
> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 6894a13..2d0d871 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -103,7 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> -obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-pic-common.o
> obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o
> obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
> obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
> diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
> new file mode 100644
> index 0000000..2f75362
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongarch-pic-common.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/pci.h>
> +#include <linux/acpi.h>
> +#include "irq-loongarch-pic-common.h"
> +
> +static struct acpi_vector_group vector_group[MAX_IO_PICS];
> +
> +struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
> +
> +struct fwnode_handle *liointc_handle;
> +struct fwnode_handle *pch_lpc_handle;
> +struct fwnode_handle *pch_msi_handle[MAX_IO_PICS];
> +struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
Why aren't these in individual drivers, and then have accessors to
retrieve them?
> +
> +static int find_pch_pic(u32 gsi)
> +{
> + int i, start, end;
> +
> + /* Find the PCH_PIC that manages this GSI. */
> + for (i = 0; i < MAX_IO_PICS; i++) {
> + struct acpi_madt_bio_pic *irq_cfg = acpi_pchpic[i];
> +
> + if (!irq_cfg)
> + return -1;
> +
> + start = irq_cfg->gsi_base;
> + end = irq_cfg->gsi_base + irq_cfg->size;
> + if (gsi >= start && gsi < end)
> + return i;
> + }
> +
> + pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
> + return -1;
> +}
Same thing. This really should be in the PCH driver, and be called by
lpic_get_gsi_domain().
> +
> +struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi)
> +{
> + int id;
> + struct fwnode_handle *domain_handle = NULL;
> +
> + switch (gsi) {
> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> + if (liointc_handle)
> + domain_handle = liointc_handle;
> + break;
> +
> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> + if (pch_lpc_handle)
> + domain_handle = pch_lpc_handle;
> + break;
> +
> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> + id = find_pch_pic(gsi);
> + if (id >= 0 && pch_pic_handle[id])
> + domain_handle = pch_pic_handle[id];
> +
> + break;
> + }
> +
> + return domain_handle;
> +}
> +
> +static int pci_mcfg_parse(struct acpi_table_header *header)
> +{
> + struct acpi_table_mcfg *mcfg;
> + struct acpi_mcfg_allocation *mptr;
> + int i, n;
> +
> + if (header->length < sizeof(struct acpi_table_mcfg))
> + return -EINVAL;
> +
> + n = (header->length - sizeof(struct acpi_table_mcfg)) /
> + sizeof(struct acpi_mcfg_allocation);
> + mcfg = (struct acpi_table_mcfg *)header;
> + mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
> +
> + for (i = 0; i < n; i++, mptr++)
> + vector_group[mptr->pci_segment].node = (mptr->address >> 44) & 0xf;
> +
> + return 0;
> +}
> +
> +void __init init_vector_parent_group(void)
> +{
> + acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
> +}
I really don't think the PCI code should be anywhere near
this. Frankly, this file looks like a dumping ground for totally
unrelated stuff.
> +
> +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;
> +}
Same thing. There is nothing "common" here. All this should be split
in the various drivers, where they belong.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers
2022-06-18 10:39 ` [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Marc Zyngier
@ 2022-06-20 2:28 ` Jianmin Lv
0 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-20 2:28 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/18 下午6:39, Marc Zyngier wrote:
> On Wed, 15 Jun 2022 07:07:20 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>> 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.
>>
>> 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
>>
>> Huacai Chen (7):
>> irqchip: Add LoongArch CPU interrupt 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 Loongson PCH LPC controller support
>>
>> Jianmin Lv (2):
>> genirq/generic_chip: export irq_unmap_generic_chip
>> irqchip: create library file for LoongArch irqchip driver
>>
>> Marc Zyngier (1):
>> APCI: irq: Add support for multiple GSI domains
>>
>> drivers/acpi/bus.c | 3 +
>> drivers/acpi/irq.c | 40 ++--
>> 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 | 134 +++++++++++
>> drivers/irqchip/irq-loongarch-pic-common.c | 122 ++++++++++
>> drivers/irqchip/irq-loongarch-pic-common.h | 39 ++++
>> drivers/irqchip/irq-loongson-eiointc.c | 347 +++++++++++++++++++++++++++++
>> drivers/irqchip/irq-loongson-htvec.c | 119 +++++++---
>> drivers/irqchip/irq-loongson-liointc.c | 225 ++++++++++++-------
>> drivers/irqchip/irq-loongson-pch-lpc.c | 202 +++++++++++++++++
>> drivers/irqchip/irq-loongson-pch-msi.c | 138 ++++++++----
>> drivers/irqchip/irq-loongson-pch-pic.c | 171 +++++++++++---
>> include/linux/acpi.h | 3 +-
>> include/linux/cpuhotplug.h | 1 +
>> include/linux/irq.h | 1 +
>> kernel/irq/generic-chip.c | 2 +-
>> 19 files changed, 1402 insertions(+), 212 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
>
> One thing I don't see here is the removal of the irq code that
> currently lives in arch/loongarch/kernel/acpi.c. It really should be
> removed as part of this series with the patch that enables the common
> ACPI irq code for this architecture.
>
Ok, I'll add the change of removing arch irq code and enabling common
irq code to the patch series.
> Thanks,
>
> M.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-18 10:36 ` Marc Zyngier
@ 2022-06-20 2:46 ` Jianmin Lv
2022-06-25 9:34 ` Jianmin Lv
2022-06-28 7:42 ` Hanjun Guo
2 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-20 2:46 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/18 下午6:36, Marc Zyngier wrote:
> On Wed, 15 Jun 2022 10:28:47 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>>
>>
>> On 2022/6/15 下午3:14, Marc Zyngier wrote:
>>> On Wed, 15 Jun 2022 07:07:21 +0100,
>>> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>>>
>>>> From: Marc Zyngier <maz@kernel.org>
>>>>
>>>> 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
>>>> architectures 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).
>>>>
>>>> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
>>>
>>> I don't think this tag is appropriate here.
>>>
>>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>>> Cc: Hanjun Guo <guohanjun@huawei.com>
>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>>>> ---
>>>> drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
>>>> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
>>>> drivers/irqchip/irq-gic.c | 18 ++++++++++++------
>>>> include/linux/acpi.h | 2 +-
>>>> 4 files changed, 48 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>>>> index c68e694..b7460ab 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,10 +26,7 @@
>>>> */
>>>> 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);
>>>> -
>>>> - *irq = irq_find_mapping(d, gsi);
>>>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
>>>
>>> What is this?
>>>
>>> - This wasn't part of my initial patch, and randomly changing patches
>>> without mentioning it isn't acceptable
>>>
>>> - you *cannot* trigger a registration here, as this isn't what the API
>>> advertises
>>>
>>> - what makes you think that passing random values (NULL, -1... )to
>>> acpi_register_gsi() is an acceptable thing to do?
>>>
>>> The original patch had:
>>>
>>> @@ -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);
>>> /*
>>>
>>> and I don't think it needs anything else. If something breaks, let's
>>> discuss it, but don't abuse the API nor the fact that I usually don't
>>> review my own patches to sneak things in...
>>>
>>
>> Sorry, Marc, I don't know how to communicate with you for my change
>> here before submitting the patch, maybe I should mention it in the
>> patch commit or code.
>
> It should at least be discussed first, like you are doing it here.
>
Ok, thanks for your correcting.
>> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
>> only handle existed mapping and will return -EINVAL if mapping not
>> found. When I test on my machine, a calling stack is as following:
>>
>>
>> acpi_bus_init
>> ->acpi_enable_subsystem
>> ->acpi_ev_install_xrupt_handlers
>> ->acpi_ev_install_sci_handler
>> ->acpi_os_install_interrupt_handler
>> ->acpi_gsi_to_irq
>>
>>
>> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
>> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it
>> so that new mapping for gsi is created if no mapping is found.
>
> So it looks like we have a discrepancy between the x86 and ARM on that
> front.
>
Yes, I think so.
> Lorenzo, Hanjun, can you please have a look at this and shed some
> light on what the expected behaviour is? It looks like we never
> encountered an issue with this on arm64, which tends to indicate that
> we don't usually use the above path.
>
>> I looked into generic acpi_register_gsi, the existed mapping will be
>> checked first by calling irq_find_mapping, so I think calling
>> acpi_register_gsi in acpi_gsi_to_irq can address the problem.
>>
>> But you're right, I'm wrong that I passed random value of -1 to
>> acpi_register_gsi. I don't find a right way to address the problem
>> without changing acpi_gsi_to_irq. I'll continue to work for the
>> problem.
>
> At the very least, this should be indirected so that the existing
> behaviour isn't affected, no matter how badly broken arm64 may or may
> not be here. Please have a look at the patch below that should help
> you with this.
>
Ok, Marc, thanks very much for your way to address the problem, I'll try
it on my machine. And I'll put the patch in my patch series without
change( :) ) if that works well.
> Thanks,
>
> M.
>
> From 3e6b87ea49473d0eb384f42e76d584a1495a538c Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Sat, 18 Jun 2022 11:29:33 +0100
> Subject: [PATCH] ACPI: irq: Allow acpi_gsi_to_irq() to have an arch-specific
> fallback
>
> It appears that the generic version of acpi_gsi_to_irq() doesn't
> fallback to establishing a mapping if there is no pre-existing
> one while the x86 version does.
>
> While arm64 seems unaffected by it, LoongArch is relying on the x86
> behaviour. In an effort to prevent new architectures from reinventing
> the proverbial wheel, provide an optional callback that the arch code
> can set to restore the x86 behaviour.
>
> Hopefully we can eventually get rid of this in the future once
> the expected behaviour has been clarified.
>
> Reported-by: Jianmin Lv <lvjianmin@loongson.cn>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> drivers/acpi/irq.c | 8 ++++++--
> include/linux/acpi.h | 1 +
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index 6e1633ac1756..66c5f01995d0 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -13,6 +13,7 @@
> enum acpi_irq_model_id acpi_irq_model;
>
> static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
> +static int (*acpi_gsi_to_irq_fallback)(u32 gsi);
>
> /**
> * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
> @@ -33,9 +34,12 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
>
> *irq = irq_find_mapping(d, gsi);
> /*
> - * *irq == 0 means no mapping, that should
> - * be reported as a failure
> + * *irq == 0 means no mapping, that should be reported as a
> + * failure, unless there is an arch-specific fallback handler.
> */
> + if (!*irq && acpi_gsi_to_irq_fallback)
> + *irq = acpi_gsi_to_irq_fallback(gsi);
> +
> return (*irq > 0) ? 0 : -EINVAL;
> }
> EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 957e23f727ea..71d3719e3ec4 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -357,6 +357,7 @@ 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 *(*)(u32));
> +void acpi_set_gsi_to_irq_fallback(int (*)(u32));
>
> struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> unsigned int size,
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 03/10] irqchip: Add LoongArch CPU interrupt controller support
2022-06-18 10:59 ` Marc Zyngier
@ 2022-06-20 3:06 ` Jianmin Lv
0 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-20 3:06 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/18 下午6:59, Marc Zyngier wrote:
> On Wed, 15 Jun 2022 07:07:23 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>> From: Huacai Chen <chenhuacai@loongson.cn>
>>
>> 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.
>>
>> Change-Id: I53fb0be768daeeecc90d0ccc0bb0becd3d4e6984
>
> Please drop this Change-Id. The upstream kernel doesn't use Gerrit.
>
Ok, I'll check it before submitting patch series to avoid this.
>> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>> ---
>> drivers/acpi/bus.c | 3 +
>> drivers/irqchip/Kconfig | 10 +++
>> drivers/irqchip/Makefile | 1 +
>> drivers/irqchip/irq-loongarch-cpu.c | 134 ++++++++++++++++++++++++++++++++++++
>> include/linux/acpi.h | 1 +
>> 5 files changed, 149 insertions(+)
>> create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
>>
>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> index 86fa61a..63fbf00 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -1145,6 +1145,9 @@ static int __init acpi_bus_init_irq(void)
>> case ACPI_IRQ_MODEL_PLATFORM:
>> message = "platform specific model";
>> break;
>> + case ACPI_IRQ_MODEL_LPIC:
>> + message = "LPIC";
>> + break;
>
> This should be part of the patch that deals with the ACPI-specific
> part of the architecture, which is the following patch.
>
Ok, I'll put the change in the following another patch.
>> default:
>> pr_info("Unknown interrupt routing model\n");
>> return -ENODEV;
>> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
>> index 4ab1038..4126b1c 100644
>> --- a/drivers/irqchip/Kconfig
>> +++ b/drivers/irqchip/Kconfig
>> @@ -546,6 +546,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 5b67450..6894a13 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -103,6 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
>> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
>> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
>> obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
>> +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
>> obj-$(CONFIG_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..c382bd9
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-loongarch-cpu.c
>> @@ -0,0 +1,134 @@
>> +// 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",
>
> Why is it "LoongArch" and not "CPUINTC", which would make a lot more
> sense?
>
I think "CPUINTC" should make more sense, maybe Huacai think "LoongArch"
is better, like "MIPS" in mips cpu irqchip driver, but I prefer
"CPUINTC", like "LAPIC" of x86 and "GIC" of arm, a name of irqchip
instead of a name of an architecture.
Huacai, what do you think?
>> + .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);
>> + }
>> +}
>> +
>> +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);
>> +}
>> +
>> +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)
>> +{
>> + struct fwnode_handle *domain_handle;
>> +
>> + /* Mask interrupts. */
>> + clear_csr_ecfg(ECFG0_IM);
>> + clear_csr_estat(ESTATF_IP);
>> +
>> + domain_handle = irq_domain_alloc_fwnode(NULL);
>
> Please don't use NULL here, as this is supposed to be a physical
> address. If you don't have any physical address at hand (because this
> driver isn't using MMIO), use irq_domain_alloc_named_fwnode() instead.
>
Ok, I'll fix it in next version.
>> + irq_domain = irq_domain_create_linear(domain_handle, EXCCODE_INT_NUM,
>> + &loongarch_cpu_intc_irq_domain_ops, NULL);
>> +
>> + if (!irq_domain)
>> + panic("Failed to add irqdomain for LoongArch CPU");
>> +
>> + set_handle_irq(&handle_cpu_irq);
>> + acpi_set_irq_model(ACPI_IRQ_MODEL_LPIC, lpic_get_gsi_domain_id);
>
> lpic_get_gsi_domain_id only gets defined in the following patch, so
> the series cannot be bisected. Please fix this (the series should
> compile every step of the way).
>
Ok, I'll fix it in next version.
>> +
>> + return irq_domain;
>> +}
>> +
>> +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);
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 957e23f..d2f5108 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -105,6 +105,7 @@ enum acpi_irq_model_id {
>> ACPI_IRQ_MODEL_IOSAPIC,
>> ACPI_IRQ_MODEL_PLATFORM,
>> ACPI_IRQ_MODEL_GIC,
>> + ACPI_IRQ_MODEL_LPIC,
>
> This hunk should be moved to the patch that introduces
> lpic_get_gsi_domain_id.
>
Ok, I'll move it to the patch that introduces lpic_get_gsi_domain_id in
next version.
> Thanks,
>
> M.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver
2022-06-18 17:22 ` Marc Zyngier
@ 2022-06-20 3:12 ` Jianmin Lv
2022-06-27 7:34 ` Marc Zyngier
0 siblings, 1 reply; 27+ messages in thread
From: Jianmin Lv @ 2022-06-20 3:12 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/19 上午1:22, Marc Zyngier wrote:
> On Wed, 15 Jun 2022 07:07:24 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>> The library file contains following content:
>> - Implement acpi_get_gsi_domain_id callback.
>> - Implement initialization of vector group entries and APIs
>> for building hierachy irqdomains.
>>
>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>> ---
>> drivers/irqchip/Makefile | 2 +-
>> drivers/irqchip/irq-loongarch-pic-common.c | 122 +++++++++++++++++++++++++++++
>> drivers/irqchip/irq-loongarch-pic-common.h | 39 +++++++++
>> 3 files changed, 162 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c
>> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h
>>
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index 6894a13..2d0d871 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -103,7 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
>> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
>> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
>> obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
>> -obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
>> +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-pic-common.o
>> obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o
>> obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
>> obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
>> diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
>> new file mode 100644
>> index 0000000..2f75362
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-loongarch-pic-common.c
>> @@ -0,0 +1,122 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
>> + */
>> +
>> +#include <linux/irq.h>
>> +#include <linux/pci.h>
>> +#include <linux/acpi.h>
>> +#include "irq-loongarch-pic-common.h"
>> +
>> +static struct acpi_vector_group vector_group[MAX_IO_PICS];
>> +
>> +struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
>> +
>> +struct fwnode_handle *liointc_handle;
>> +struct fwnode_handle *pch_lpc_handle;
>> +struct fwnode_handle *pch_msi_handle[MAX_IO_PICS];
>> +struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>
> Why aren't these in individual drivers, and then have accessors to
> retrieve them?
>
Ok, I'll try to put them in individual drivers.
>> +
>> +static int find_pch_pic(u32 gsi)
>> +{
>> + int i, start, end;
>> +
>> + /* Find the PCH_PIC that manages this GSI. */
>> + for (i = 0; i < MAX_IO_PICS; i++) {
>> + struct acpi_madt_bio_pic *irq_cfg = acpi_pchpic[i];
>> +
>> + if (!irq_cfg)
>> + return -1;
>> +
>> + start = irq_cfg->gsi_base;
>> + end = irq_cfg->gsi_base + irq_cfg->size;
>> + if (gsi >= start && gsi < end)
>> + return i;
>> + }
>> +
>> + pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
>> + return -1;
>> +}
>
> Same thing. This really should be in the PCH driver, and be called by
> lpic_get_gsi_domain().
>
Ok, I'll try to put them in PCH driver.
>> +
>> +struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi)
>> +{
>> + int id;
>> + struct fwnode_handle *domain_handle = NULL;
>> +
>> + switch (gsi) {
>> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
>> + if (liointc_handle)
>> + domain_handle = liointc_handle;
>> + break;
>> +
>> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
>> + if (pch_lpc_handle)
>> + domain_handle = pch_lpc_handle;
>> + break;
>> +
>> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
>> + id = find_pch_pic(gsi);
>> + if (id >= 0 && pch_pic_handle[id])
>> + domain_handle = pch_pic_handle[id];
>> +
>> + break;
>> + }
>> +
>> + return domain_handle;
>> +}
>> +
>> +static int pci_mcfg_parse(struct acpi_table_header *header)
>> +{
>> + struct acpi_table_mcfg *mcfg;
>> + struct acpi_mcfg_allocation *mptr;
>> + int i, n;
>> +
>> + if (header->length < sizeof(struct acpi_table_mcfg))
>> + return -EINVAL;
>> +
>> + n = (header->length - sizeof(struct acpi_table_mcfg)) /
>> + sizeof(struct acpi_mcfg_allocation);
>> + mcfg = (struct acpi_table_mcfg *)header;
>> + mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
>> +
>> + for (i = 0; i < n; i++, mptr++)
>> + vector_group[mptr->pci_segment].node = (mptr->address >> 44) & 0xf;
>> +
>> + return 0;
>> +}
>> +
>> +void __init init_vector_parent_group(void)
>> +{
>> + acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
>> +}
>
> I really don't think the PCI code should be anywhere near
> this. Frankly, this file looks like a dumping ground for totally
> unrelated stuff.
>
For ARM, the msi domain of a pci device is matched by comparing PCI
segment of it and PCI segment of msi domain's parent domain(its domain).
The PCI segment number here is as in MCFG and as returned by _SEG in the
namespace.
For LoongArch, a similar way is used, but we don't use
IORT-like table, rather, we directly used PCI segment and Base
address(node information is in bit44-47 of the address) in MCFG for it,
so we need to get them by early parsing MCFG before creating MSI and PCH
irqdomain(MSI and PCH irqdomain have the same node as their parent
irqdomain).
If the related code is not suitable to be here, Maybe I should put them
in arch/loongarch/kernel/irq.c and call init_vector_parent_group before
irqchip_init().
>> +
>> +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;
>> +}
>
> Same thing. There is nothing "common" here. All this should be split
> in the various drivers, where they belong.
>
Ok, I'll try to put them in individual drivers. So the "common" file
here will be not needed any more.
> M.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-18 10:36 ` Marc Zyngier
2022-06-20 2:46 ` Jianmin Lv
@ 2022-06-25 9:34 ` Jianmin Lv
2022-06-27 7:32 ` Marc Zyngier
2022-06-28 7:42 ` Hanjun Guo
2 siblings, 1 reply; 27+ messages in thread
From: Jianmin Lv @ 2022-06-25 9:34 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/18 下午6:36, Marc Zyngier wrote:
> On Wed, 15 Jun 2022 10:28:47 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>>
>>
>> On 2022/6/15 下午3:14, Marc Zyngier wrote:
>>> On Wed, 15 Jun 2022 07:07:21 +0100,
>>> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>>>
>>>> From: Marc Zyngier <maz@kernel.org>
>>>>
>>>> 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
>>>> architectures 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).
>>>>
>>>> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
>>>
>>> I don't think this tag is appropriate here.
>>>
>>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>>> Cc: Hanjun Guo <guohanjun@huawei.com>
>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>>>> ---
>>>> drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
>>>> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
>>>> drivers/irqchip/irq-gic.c | 18 ++++++++++++------
>>>> include/linux/acpi.h | 2 +-
>>>> 4 files changed, 48 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>>>> index c68e694..b7460ab 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,10 +26,7 @@
>>>> */
>>>> 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);
>>>> -
>>>> - *irq = irq_find_mapping(d, gsi);
>>>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
>>>
>>> What is this?
>>>
>>> - This wasn't part of my initial patch, and randomly changing patches
>>> without mentioning it isn't acceptable
>>>
>>> - you *cannot* trigger a registration here, as this isn't what the API
>>> advertises
>>>
>>> - what makes you think that passing random values (NULL, -1... )to
>>> acpi_register_gsi() is an acceptable thing to do?
>>>
>>> The original patch had:
>>>
>>> @@ -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);
>>> /*
>>>
>>> and I don't think it needs anything else. If something breaks, let's
>>> discuss it, but don't abuse the API nor the fact that I usually don't
>>> review my own patches to sneak things in...
>>>
>>
>> Sorry, Marc, I don't know how to communicate with you for my change
>> here before submitting the patch, maybe I should mention it in the
>> patch commit or code.
>
> It should at least be discussed first, like you are doing it here.
>
>> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
>> only handle existed mapping and will return -EINVAL if mapping not
>> found. When I test on my machine, a calling stack is as following:
>>
>>
>> acpi_bus_init
>> ->acpi_enable_subsystem
>> ->acpi_ev_install_xrupt_handlers
>> ->acpi_ev_install_sci_handler
>> ->acpi_os_install_interrupt_handler
>> ->acpi_gsi_to_irq
>>
>>
>> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
>> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it
>> so that new mapping for gsi is created if no mapping is found.
>
> So it looks like we have a discrepancy between the x86 and ARM on that
> front.
>
> Lorenzo, Hanjun, can you please have a look at this and shed some
> light on what the expected behaviour is? It looks like we never
> encountered an issue with this on arm64, which tends to indicate that
> we don't usually use the above path.
>
>> I looked into generic acpi_register_gsi, the existed mapping will be
>> checked first by calling irq_find_mapping, so I think calling
>> acpi_register_gsi in acpi_gsi_to_irq can address the problem.
>>
>> But you're right, I'm wrong that I passed random value of -1 to
>> acpi_register_gsi. I don't find a right way to address the problem
>> without changing acpi_gsi_to_irq. I'll continue to work for the
>> problem.
>
> At the very least, this should be indirected so that the existing
> behaviour isn't affected, no matter how badly broken arm64 may or may
> not be here. Please have a look at the patch below that should help
> you with this.
>
> Thanks,
>
> M.
>
> From 3e6b87ea49473d0eb384f42e76d584a1495a538c Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Sat, 18 Jun 2022 11:29:33 +0100
> Subject: [PATCH] ACPI: irq: Allow acpi_gsi_to_irq() to have an arch-specific
> fallback
>
> It appears that the generic version of acpi_gsi_to_irq() doesn't
> fallback to establishing a mapping if there is no pre-existing
> one while the x86 version does.
>
> While arm64 seems unaffected by it, LoongArch is relying on the x86
> behaviour. In an effort to prevent new architectures from reinventing
> the proverbial wheel, provide an optional callback that the arch code
> can set to restore the x86 behaviour.
>
> Hopefully we can eventually get rid of this in the future once
> the expected behaviour has been clarified.
>
> Reported-by: Jianmin Lv <lvjianmin@loongson.cn>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> drivers/acpi/irq.c | 8 ++++++--
> include/linux/acpi.h | 1 +
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index 6e1633ac1756..66c5f01995d0 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -13,6 +13,7 @@
> enum acpi_irq_model_id acpi_irq_model;
>
> static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
> +static int (*acpi_gsi_to_irq_fallback)(u32 gsi);
>
> /**
> * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
> @@ -33,9 +34,12 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
>
> *irq = irq_find_mapping(d, gsi);
> /*
> - * *irq == 0 means no mapping, that should
> - * be reported as a failure
> + * *irq == 0 means no mapping, that should be reported as a
> + * failure, unless there is an arch-specific fallback handler.
> */
> + if (!*irq && acpi_gsi_to_irq_fallback)
> + *irq = acpi_gsi_to_irq_fallback(gsi);
> +
> return (*irq > 0) ? 0 : -EINVAL;
> }
> EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 957e23f727ea..71d3719e3ec4 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -357,6 +357,7 @@ 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 *(*)(u32));
> +void acpi_set_gsi_to_irq_fallback(int (*)(u32));
>
Hi, Marc
I want to make sure that if acpi_set_gsi_to_irq_fallback should be
implemented in driver/acpi/irq.c as acpi_set_irq_model, e.g.:
void __init acpi_set_gsi_to_irq_fallback(int (*fn)(u32))
{
acpi_gsi_to_irq_fallback = fn;
}
And then, arch related code can call acpi_set_gsi_to_irq_fallback
to register a callback.
> struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> unsigned int size,
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-25 9:34 ` Jianmin Lv
@ 2022-06-27 7:32 ` Marc Zyngier
2022-06-27 8:00 ` Jianmin Lv
0 siblings, 1 reply; 27+ messages in thread
From: Marc Zyngier @ 2022-06-27 7:32 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On Sat, 25 Jun 2022 10:34:34 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
>
>
> On 2022/6/18 下午6:36, Marc Zyngier wrote:
> > On Wed, 15 Jun 2022 10:28:47 +0100,
> > Jianmin Lv <lvjianmin@loongson.cn> wrote:
> >>
> >>
> >>
> >> On 2022/6/15 下午3:14, Marc Zyngier wrote:
> >>> On Wed, 15 Jun 2022 07:07:21 +0100,
> >>> Jianmin Lv <lvjianmin@loongson.cn> wrote:
> >>>>
> >>>> From: Marc Zyngier <maz@kernel.org>
> >>>>
> >>>> 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
> >>>> architectures 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).
> >>>>
> >>>> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
> >>>
> >>> I don't think this tag is appropriate here.
> >>>
> >>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >>>> Cc: Hanjun Guo <guohanjun@huawei.com>
> >>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >>>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> >>>> ---
> >>>> drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
> >>>> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
> >>>> drivers/irqchip/irq-gic.c | 18 ++++++++++++------
> >>>> include/linux/acpi.h | 2 +-
> >>>> 4 files changed, 48 insertions(+), 30 deletions(-)
> >>>>
> >>>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> >>>> index c68e694..b7460ab 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,10 +26,7 @@
> >>>> */
> >>>> 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);
> >>>> -
> >>>> - *irq = irq_find_mapping(d, gsi);
> >>>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
> >>>
> >>> What is this?
> >>>
> >>> - This wasn't part of my initial patch, and randomly changing patches
> >>> without mentioning it isn't acceptable
> >>>
> >>> - you *cannot* trigger a registration here, as this isn't what the API
> >>> advertises
> >>>
> >>> - what makes you think that passing random values (NULL, -1... )to
> >>> acpi_register_gsi() is an acceptable thing to do?
> >>>
> >>> The original patch had:
> >>>
> >>> @@ -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);
> >>> /*
> >>>
> >>> and I don't think it needs anything else. If something breaks, let's
> >>> discuss it, but don't abuse the API nor the fact that I usually don't
> >>> review my own patches to sneak things in...
> >>>
> >>
> >> Sorry, Marc, I don't know how to communicate with you for my change
> >> here before submitting the patch, maybe I should mention it in the
> >> patch commit or code.
> >
> > It should at least be discussed first, like you are doing it here.
> >
> >> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
> >> only handle existed mapping and will return -EINVAL if mapping not
> >> found. When I test on my machine, a calling stack is as following:
> >>
> >>
> >> acpi_bus_init
> >> ->acpi_enable_subsystem
> >> ->acpi_ev_install_xrupt_handlers
> >> ->acpi_ev_install_sci_handler
> >> ->acpi_os_install_interrupt_handler
> >> ->acpi_gsi_to_irq
> >>
> >>
> >> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
> >> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it
> >> so that new mapping for gsi is created if no mapping is found.
> >
> > So it looks like we have a discrepancy between the x86 and ARM on that
> > front.
> >
> > Lorenzo, Hanjun, can you please have a look at this and shed some
> > light on what the expected behaviour is? It looks like we never
> > encountered an issue with this on arm64, which tends to indicate that
> > we don't usually use the above path.
> >
> >> I looked into generic acpi_register_gsi, the existed mapping will be
> >> checked first by calling irq_find_mapping, so I think calling
> >> acpi_register_gsi in acpi_gsi_to_irq can address the problem.
> >>
> >> But you're right, I'm wrong that I passed random value of -1 to
> >> acpi_register_gsi. I don't find a right way to address the problem
> >> without changing acpi_gsi_to_irq. I'll continue to work for the
> >> problem.
> >
> > At the very least, this should be indirected so that the existing
> > behaviour isn't affected, no matter how badly broken arm64 may or may
> > not be here. Please have a look at the patch below that should help
> > you with this.
> >
> > Thanks,
> >
> > M.
> >
> > From 3e6b87ea49473d0eb384f42e76d584a1495a538c Mon Sep 17 00:00:00 2001
> > From: Marc Zyngier <maz@kernel.org>
> > Date: Sat, 18 Jun 2022 11:29:33 +0100
> > Subject: [PATCH] ACPI: irq: Allow acpi_gsi_to_irq() to have an arch-specific
> > fallback
> >
> > It appears that the generic version of acpi_gsi_to_irq() doesn't
> > fallback to establishing a mapping if there is no pre-existing
> > one while the x86 version does.
> >
> > While arm64 seems unaffected by it, LoongArch is relying on the x86
> > behaviour. In an effort to prevent new architectures from reinventing
> > the proverbial wheel, provide an optional callback that the arch code
> > can set to restore the x86 behaviour.
> >
> > Hopefully we can eventually get rid of this in the future once
> > the expected behaviour has been clarified.
> >
> > Reported-by: Jianmin Lv <lvjianmin@loongson.cn>
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> > drivers/acpi/irq.c | 8 ++++++--
> > include/linux/acpi.h | 1 +
> > 2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> > index 6e1633ac1756..66c5f01995d0 100644
> > --- a/drivers/acpi/irq.c
> > +++ b/drivers/acpi/irq.c
> > @@ -13,6 +13,7 @@
> > enum acpi_irq_model_id acpi_irq_model;
> > static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
> > +static int (*acpi_gsi_to_irq_fallback)(u32 gsi);
> > /**
> > * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
> > @@ -33,9 +34,12 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
> > *irq = irq_find_mapping(d, gsi);
> > /*
> > - * *irq == 0 means no mapping, that should
> > - * be reported as a failure
> > + * *irq == 0 means no mapping, that should be reported as a
> > + * failure, unless there is an arch-specific fallback handler.
> > */
> > + if (!*irq && acpi_gsi_to_irq_fallback)
> > + *irq = acpi_gsi_to_irq_fallback(gsi);
> > +
> > return (*irq > 0) ? 0 : -EINVAL;
> > }
> > EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 957e23f727ea..71d3719e3ec4 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -357,6 +357,7 @@ 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 *(*)(u32));
> > +void acpi_set_gsi_to_irq_fallback(int (*)(u32));
> >
>
> Hi, Marc
>
> I want to make sure that if acpi_set_gsi_to_irq_fallback should be
> implemented in driver/acpi/irq.c as acpi_set_irq_model, e.g.:
>
> void __init acpi_set_gsi_to_irq_fallback(int (*fn)(u32))
> {
> acpi_gsi_to_irq_fallback = fn;
> }
>
> And then, arch related code can call acpi_set_gsi_to_irq_fallback
> to register a callback.
Yes. I had something like that, but forgot to add it to the patch,
apparently.
\x17 M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver
2022-06-20 3:12 ` Jianmin Lv
@ 2022-06-27 7:34 ` Marc Zyngier
2022-06-27 8:04 ` Jianmin Lv
0 siblings, 1 reply; 27+ messages in thread
From: Marc Zyngier @ 2022-06-27 7:34 UTC (permalink / raw)
To: Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On Mon, 20 Jun 2022 04:12:10 +0100,
Jianmin Lv <lvjianmin@loongson.cn> wrote:
>
>
>
> On 2022/6/19 上午1:22, Marc Zyngier wrote:
> > On Wed, 15 Jun 2022 07:07:24 +0100,
> > Jianmin Lv <lvjianmin@loongson.cn> wrote:
> >>
> >> The library file contains following content:
> >> - Implement acpi_get_gsi_domain_id callback.
> >> - Implement initialization of vector group entries and APIs
> >> for building hierachy irqdomains.
> >>
> >> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> >> ---
> >> drivers/irqchip/Makefile | 2 +-
> >> drivers/irqchip/irq-loongarch-pic-common.c | 122 +++++++++++++++++++++++++++++
> >> drivers/irqchip/irq-loongarch-pic-common.h | 39 +++++++++
> >> 3 files changed, 162 insertions(+), 1 deletion(-)
> >> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c
> >> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h
> >>
> >> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> >> index 6894a13..2d0d871 100644
> >> --- a/drivers/irqchip/Makefile
> >> +++ b/drivers/irqchip/Makefile
> >> @@ -103,7 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> >> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> >> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> >> obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> >> -obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> >> +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-pic-common.o
> >> obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o
> >> obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
> >> obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
> >> diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
> >> new file mode 100644
> >> index 0000000..2f75362
> >> --- /dev/null
> >> +++ b/drivers/irqchip/irq-loongarch-pic-common.c
> >> @@ -0,0 +1,122 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
> >> + */
> >> +
> >> +#include <linux/irq.h>
> >> +#include <linux/pci.h>
> >> +#include <linux/acpi.h>
> >> +#include "irq-loongarch-pic-common.h"
> >> +
> >> +static struct acpi_vector_group vector_group[MAX_IO_PICS];
> >> +
> >> +struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
> >> +
> >> +struct fwnode_handle *liointc_handle;
> >> +struct fwnode_handle *pch_lpc_handle;
> >> +struct fwnode_handle *pch_msi_handle[MAX_IO_PICS];
> >> +struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
> >
> > Why aren't these in individual drivers, and then have accessors to
> > retrieve them?
> >
>
> Ok, I'll try to put them in individual drivers.
>
>
> >> +
> >> +static int find_pch_pic(u32 gsi)
> >> +{
> >> + int i, start, end;
> >> +
> >> + /* Find the PCH_PIC that manages this GSI. */
> >> + for (i = 0; i < MAX_IO_PICS; i++) {
> >> + struct acpi_madt_bio_pic *irq_cfg = acpi_pchpic[i];
> >> +
> >> + if (!irq_cfg)
> >> + return -1;
> >> +
> >> + start = irq_cfg->gsi_base;
> >> + end = irq_cfg->gsi_base + irq_cfg->size;
> >> + if (gsi >= start && gsi < end)
> >> + return i;
> >> + }
> >> +
> >> + pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
> >> + return -1;
> >> +}
> >
> > Same thing. This really should be in the PCH driver, and be called by
> > lpic_get_gsi_domain().
> >
>
> Ok, I'll try to put them in PCH driver.
>
>
> >> +
> >> +struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi)
> >> +{
> >> + int id;
> >> + struct fwnode_handle *domain_handle = NULL;
> >> +
> >> + switch (gsi) {
> >> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> >> + if (liointc_handle)
> >> + domain_handle = liointc_handle;
> >> + break;
> >> +
> >> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> >> + if (pch_lpc_handle)
> >> + domain_handle = pch_lpc_handle;
> >> + break;
> >> +
> >> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> >> + id = find_pch_pic(gsi);
> >> + if (id >= 0 && pch_pic_handle[id])
> >> + domain_handle = pch_pic_handle[id];
> >> +
> >> + break;
> >> + }
> >> +
> >> + return domain_handle;
> >> +}
> >> +
> >> +static int pci_mcfg_parse(struct acpi_table_header *header)
> >> +{
> >> + struct acpi_table_mcfg *mcfg;
> >> + struct acpi_mcfg_allocation *mptr;
> >> + int i, n;
> >> +
> >> + if (header->length < sizeof(struct acpi_table_mcfg))
> >> + return -EINVAL;
> >> +
> >> + n = (header->length - sizeof(struct acpi_table_mcfg)) /
> >> + sizeof(struct acpi_mcfg_allocation);
> >> + mcfg = (struct acpi_table_mcfg *)header;
> >> + mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
> >> +
> >> + for (i = 0; i < n; i++, mptr++)
> >> + vector_group[mptr->pci_segment].node = (mptr->address >> 44) & 0xf;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +void __init init_vector_parent_group(void)
> >> +{
> >> + acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
> >> +}
> >
> > I really don't think the PCI code should be anywhere near
> > this. Frankly, this file looks like a dumping ground for totally
> > unrelated stuff.
> >
>
>
> For ARM, the msi domain of a pci device is matched by comparing PCI
> segment of it and PCI segment of msi domain's parent domain(its
> domain).
> The PCI segment number here is as in MCFG and as returned by _SEG in
> the namespace.
>
> For LoongArch, a similar way is used, but we don't use
> IORT-like table, rather, we directly used PCI segment and Base
> address(node information is in bit44-47 of the address) in MCFG for
> it, so we need to get them by early parsing MCFG before creating MSI
> and PCH irqdomain(MSI and PCH irqdomain have the same node as their
> parent
> irqdomain).
>
> If the related code is not suitable to be here, Maybe I should put
> them in arch/loongarch/kernel/irq.c and call init_vector_parent_group
> before irqchip_init().
That'd probably be better. This really is arch-specific stuff, and not
interrupt-controller related.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-27 7:32 ` Marc Zyngier
@ 2022-06-27 8:00 ` Jianmin Lv
0 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-27 8:00 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/27 下午3:32, Marc Zyngier wrote:
> On Sat, 25 Jun 2022 10:34:34 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>>
>>
>> On 2022/6/18 下午6:36, Marc Zyngier wrote:
>>> On Wed, 15 Jun 2022 10:28:47 +0100,
>>> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>>>
>>>>
>>>>
>>>> On 2022/6/15 下午3:14, Marc Zyngier wrote:
>>>>> On Wed, 15 Jun 2022 07:07:21 +0100,
>>>>> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>>>>>
>>>>>> From: Marc Zyngier <maz@kernel.org>
>>>>>>
>>>>>> 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
>>>>>> architectures 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).
>>>>>>
>>>>>> Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn>
>>>>>
>>>>> I don't think this tag is appropriate here.
>>>>>
>>>>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>>>>> Cc: Hanjun Guo <guohanjun@huawei.com>
>>>>>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>>>>>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>>>>>> ---
>>>>>> drivers/acpi/irq.c | 40 +++++++++++++++++++++++-----------------
>>>>>> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
>>>>>> drivers/irqchip/irq-gic.c | 18 ++++++++++++------
>>>>>> include/linux/acpi.h | 2 +-
>>>>>> 4 files changed, 48 insertions(+), 30 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>>>>>> index c68e694..b7460ab 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,10 +26,7 @@
>>>>>> */
>>>>>> 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);
>>>>>> -
>>>>>> - *irq = irq_find_mapping(d, gsi);
>>>>>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
>>>>>
>>>>> What is this?
>>>>>
>>>>> - This wasn't part of my initial patch, and randomly changing patches
>>>>> without mentioning it isn't acceptable
>>>>>
>>>>> - you *cannot* trigger a registration here, as this isn't what the API
>>>>> advertises
>>>>>
>>>>> - what makes you think that passing random values (NULL, -1... )to
>>>>> acpi_register_gsi() is an acceptable thing to do?
>>>>>
>>>>> The original patch had:
>>>>>
>>>>> @@ -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);
>>>>> /*
>>>>>
>>>>> and I don't think it needs anything else. If something breaks, let's
>>>>> discuss it, but don't abuse the API nor the fact that I usually don't
>>>>> review my own patches to sneak things in...
>>>>>
>>>>
>>>> Sorry, Marc, I don't know how to communicate with you for my change
>>>> here before submitting the patch, maybe I should mention it in the
>>>> patch commit or code.
>>>
>>> It should at least be discussed first, like you are doing it here.
>>>
>>>> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
>>>> only handle existed mapping and will return -EINVAL if mapping not
>>>> found. When I test on my machine, a calling stack is as following:
>>>>
>>>>
>>>> acpi_bus_init
>>>> ->acpi_enable_subsystem
>>>> ->acpi_ev_install_xrupt_handlers
>>>> ->acpi_ev_install_sci_handler
>>>> ->acpi_os_install_interrupt_handler
>>>> ->acpi_gsi_to_irq
>>>>
>>>>
>>>> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
>>>> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it
>>>> so that new mapping for gsi is created if no mapping is found.
>>>
>>> So it looks like we have a discrepancy between the x86 and ARM on that
>>> front.
>>>
>>> Lorenzo, Hanjun, can you please have a look at this and shed some
>>> light on what the expected behaviour is? It looks like we never
>>> encountered an issue with this on arm64, which tends to indicate that
>>> we don't usually use the above path.
>>>
>>>> I looked into generic acpi_register_gsi, the existed mapping will be
>>>> checked first by calling irq_find_mapping, so I think calling
>>>> acpi_register_gsi in acpi_gsi_to_irq can address the problem.
>>>>
>>>> But you're right, I'm wrong that I passed random value of -1 to
>>>> acpi_register_gsi. I don't find a right way to address the problem
>>>> without changing acpi_gsi_to_irq. I'll continue to work for the
>>>> problem.
>>>
>>> At the very least, this should be indirected so that the existing
>>> behaviour isn't affected, no matter how badly broken arm64 may or may
>>> not be here. Please have a look at the patch below that should help
>>> you with this.
>>>
>>> Thanks,
>>>
>>> M.
>>>
>>> From 3e6b87ea49473d0eb384f42e76d584a1495a538c Mon Sep 17 00:00:00 2001
>>> From: Marc Zyngier <maz@kernel.org>
>>> Date: Sat, 18 Jun 2022 11:29:33 +0100
>>> Subject: [PATCH] ACPI: irq: Allow acpi_gsi_to_irq() to have an arch-specific
>>> fallback
>>>
>>> It appears that the generic version of acpi_gsi_to_irq() doesn't
>>> fallback to establishing a mapping if there is no pre-existing
>>> one while the x86 version does.
>>>
>>> While arm64 seems unaffected by it, LoongArch is relying on the x86
>>> behaviour. In an effort to prevent new architectures from reinventing
>>> the proverbial wheel, provide an optional callback that the arch code
>>> can set to restore the x86 behaviour.
>>>
>>> Hopefully we can eventually get rid of this in the future once
>>> the expected behaviour has been clarified.
>>>
>>> Reported-by: Jianmin Lv <lvjianmin@loongson.cn>
>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>> ---
>>> drivers/acpi/irq.c | 8 ++++++--
>>> include/linux/acpi.h | 1 +
>>> 2 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
>>> index 6e1633ac1756..66c5f01995d0 100644
>>> --- a/drivers/acpi/irq.c
>>> +++ b/drivers/acpi/irq.c
>>> @@ -13,6 +13,7 @@
>>> enum acpi_irq_model_id acpi_irq_model;
>>> static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
>>> +static int (*acpi_gsi_to_irq_fallback)(u32 gsi);
>>> /**
>>> * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI
>>> @@ -33,9 +34,12 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq)
>>> *irq = irq_find_mapping(d, gsi);
>>> /*
>>> - * *irq == 0 means no mapping, that should
>>> - * be reported as a failure
>>> + * *irq == 0 means no mapping, that should be reported as a
>>> + * failure, unless there is an arch-specific fallback handler.
>>> */
>>> + if (!*irq && acpi_gsi_to_irq_fallback)
>>> + *irq = acpi_gsi_to_irq_fallback(gsi);
>>> +
>>> return (*irq > 0) ? 0 : -EINVAL;
>>> }
>>> EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
>>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>>> index 957e23f727ea..71d3719e3ec4 100644
>>> --- a/include/linux/acpi.h
>>> +++ b/include/linux/acpi.h
>>> @@ -357,6 +357,7 @@ 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 *(*)(u32));
>>> +void acpi_set_gsi_to_irq_fallback(int (*)(u32));
>>>
>>
>> Hi, Marc
>>
>> I want to make sure that if acpi_set_gsi_to_irq_fallback should be
>> implemented in driver/acpi/irq.c as acpi_set_irq_model, e.g.:
>>
>> void __init acpi_set_gsi_to_irq_fallback(int (*fn)(u32))
>> {
>> acpi_gsi_to_irq_fallback = fn;
>> }
>>
>> And then, arch related code can call acpi_set_gsi_to_irq_fallback
>> to register a callback.
>
> Yes. I had something like that, but forgot to add it to the patch,
> apparently.
>
Ok, I'll add that to the patch, please check the change in next version.
> \x17 M.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver
2022-06-27 7:34 ` Marc Zyngier
@ 2022-06-27 8:04 ` Jianmin Lv
0 siblings, 0 replies; 27+ messages in thread
From: Jianmin Lv @ 2022-06-27 8:04 UTC (permalink / raw)
To: Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Hanjun Guo, Lorenzo Pieralisi,
Jiaxun Yang, Huacai Chen
On 2022/6/27 下午3:34, Marc Zyngier wrote:
> On Mon, 20 Jun 2022 04:12:10 +0100,
> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>
>>
>>
>> On 2022/6/19 上午1:22, Marc Zyngier wrote:
>>> On Wed, 15 Jun 2022 07:07:24 +0100,
>>> Jianmin Lv <lvjianmin@loongson.cn> wrote:
>>>>
>>>> The library file contains following content:
>>>> - Implement acpi_get_gsi_domain_id callback.
>>>> - Implement initialization of vector group entries and APIs
>>>> for building hierachy irqdomains.
>>>>
>>>> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
>>>> ---
>>>> drivers/irqchip/Makefile | 2 +-
>>>> drivers/irqchip/irq-loongarch-pic-common.c | 122 +++++++++++++++++++++++++++++
>>>> drivers/irqchip/irq-loongarch-pic-common.h | 39 +++++++++
>>>> 3 files changed, 162 insertions(+), 1 deletion(-)
>>>> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c
>>>> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h
>>>>
>>>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>>>> index 6894a13..2d0d871 100644
>>>> --- a/drivers/irqchip/Makefile
>>>> +++ b/drivers/irqchip/Makefile
>>>> @@ -103,7 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
>>>> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
>>>> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
>>>> obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
>>>> -obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
>>>> +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-pic-common.o
>>>> obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o
>>>> obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
>>>> obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
>>>> diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
>>>> new file mode 100644
>>>> index 0000000..2f75362
>>>> --- /dev/null
>>>> +++ b/drivers/irqchip/irq-loongarch-pic-common.c
>>>> @@ -0,0 +1,122 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
>>>> + */
>>>> +
>>>> +#include <linux/irq.h>
>>>> +#include <linux/pci.h>
>>>> +#include <linux/acpi.h>
>>>> +#include "irq-loongarch-pic-common.h"
>>>> +
>>>> +static struct acpi_vector_group vector_group[MAX_IO_PICS];
>>>> +
>>>> +struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
>>>> +
>>>> +struct fwnode_handle *liointc_handle;
>>>> +struct fwnode_handle *pch_lpc_handle;
>>>> +struct fwnode_handle *pch_msi_handle[MAX_IO_PICS];
>>>> +struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
>>>
>>> Why aren't these in individual drivers, and then have accessors to
>>> retrieve them?
>>>
>>
>> Ok, I'll try to put them in individual drivers.
>>
>>
>>>> +
>>>> +static int find_pch_pic(u32 gsi)
>>>> +{
>>>> + int i, start, end;
>>>> +
>>>> + /* Find the PCH_PIC that manages this GSI. */
>>>> + for (i = 0; i < MAX_IO_PICS; i++) {
>>>> + struct acpi_madt_bio_pic *irq_cfg = acpi_pchpic[i];
>>>> +
>>>> + if (!irq_cfg)
>>>> + return -1;
>>>> +
>>>> + start = irq_cfg->gsi_base;
>>>> + end = irq_cfg->gsi_base + irq_cfg->size;
>>>> + if (gsi >= start && gsi < end)
>>>> + return i;
>>>> + }
>>>> +
>>>> + pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
>>>> + return -1;
>>>> +}
>>>
>>> Same thing. This really should be in the PCH driver, and be called by
>>> lpic_get_gsi_domain().
>>>
>>
>> Ok, I'll try to put them in PCH driver.
>>
>>
>>>> +
>>>> +struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi)
>>>> +{
>>>> + int id;
>>>> + struct fwnode_handle *domain_handle = NULL;
>>>> +
>>>> + switch (gsi) {
>>>> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
>>>> + if (liointc_handle)
>>>> + domain_handle = liointc_handle;
>>>> + break;
>>>> +
>>>> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
>>>> + if (pch_lpc_handle)
>>>> + domain_handle = pch_lpc_handle;
>>>> + break;
>>>> +
>>>> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
>>>> + id = find_pch_pic(gsi);
>>>> + if (id >= 0 && pch_pic_handle[id])
>>>> + domain_handle = pch_pic_handle[id];
>>>> +
>>>> + break;
>>>> + }
>>>> +
>>>> + return domain_handle;
>>>> +}
>>>> +
>>>> +static int pci_mcfg_parse(struct acpi_table_header *header)
>>>> +{
>>>> + struct acpi_table_mcfg *mcfg;
>>>> + struct acpi_mcfg_allocation *mptr;
>>>> + int i, n;
>>>> +
>>>> + if (header->length < sizeof(struct acpi_table_mcfg))
>>>> + return -EINVAL;
>>>> +
>>>> + n = (header->length - sizeof(struct acpi_table_mcfg)) /
>>>> + sizeof(struct acpi_mcfg_allocation);
>>>> + mcfg = (struct acpi_table_mcfg *)header;
>>>> + mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
>>>> +
>>>> + for (i = 0; i < n; i++, mptr++)
>>>> + vector_group[mptr->pci_segment].node = (mptr->address >> 44) & 0xf;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +void __init init_vector_parent_group(void)
>>>> +{
>>>> + acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
>>>> +}
>>>
>>> I really don't think the PCI code should be anywhere near
>>> this. Frankly, this file looks like a dumping ground for totally
>>> unrelated stuff.
>>>
>>
>>
>> For ARM, the msi domain of a pci device is matched by comparing PCI
>> segment of it and PCI segment of msi domain's parent domain(its
>> domain).
>> The PCI segment number here is as in MCFG and as returned by _SEG in
>> the namespace.
>>
>> For LoongArch, a similar way is used, but we don't use
>> IORT-like table, rather, we directly used PCI segment and Base
>> address(node information is in bit44-47 of the address) in MCFG for
>> it, so we need to get them by early parsing MCFG before creating MSI
>> and PCH irqdomain(MSI and PCH irqdomain have the same node as their
>> parent
>> irqdomain).
>>
>> If the related code is not suitable to be here, Maybe I should put
>> them in arch/loongarch/kernel/irq.c and call init_vector_parent_group
>> before irqchip_init().
>
> That'd probably be better. This really is arch-specific stuff, and not
> interrupt-controller related.
>
Ok, I'll do that in next version.
> M.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-18 10:36 ` Marc Zyngier
2022-06-20 2:46 ` Jianmin Lv
2022-06-25 9:34 ` Jianmin Lv
@ 2022-06-28 7:42 ` Hanjun Guo
2022-06-28 8:45 ` Jianmin Lv
2 siblings, 1 reply; 27+ messages in thread
From: Hanjun Guo @ 2022-06-28 7:42 UTC (permalink / raw)
To: Marc Zyngier, Jianmin Lv
Cc: Thomas Gleixner, linux-kernel, Lorenzo Pieralisi, Jiaxun Yang,
Huacai Chen
On 2022/6/18 18:36, Marc Zyngier wrote:
[...]
>>>> --- 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,10 +26,7 @@
>>>> */
>>>> 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);
>>>> -
>>>> - *irq = irq_find_mapping(d, gsi);
>>>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
>>>
>>> What is this?
>>>
>>> - This wasn't part of my initial patch, and randomly changing patches
>>> without mentioning it isn't acceptable
>>>
>>> - you *cannot* trigger a registration here, as this isn't what the API
>>> advertises
>>>
>>> - what makes you think that passing random values (NULL, -1... )to
>>> acpi_register_gsi() is an acceptable thing to do?
>>>
>>> The original patch had:
>>>
>>> @@ -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);
>>> /*
>>>
>>> and I don't think it needs anything else. If something breaks, let's
>>> discuss it, but don't abuse the API nor the fact that I usually don't
>>> review my own patches to sneak things in...
>>>
>>
>> Sorry, Marc, I don't know how to communicate with you for my change
>> here before submitting the patch, maybe I should mention it in the
>> patch commit or code.
>
> It should at least be discussed first, like you are doing it here.
>
>> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
>> only handle existed mapping and will return -EINVAL if mapping not
>> found. When I test on my machine, a calling stack is as following:
>>
>>
>> acpi_bus_init
>> ->acpi_enable_subsystem
>> ->acpi_ev_install_xrupt_handlers
>> ->acpi_ev_install_sci_handler
>> ->acpi_os_install_interrupt_handler
>> ->acpi_gsi_to_irq
>>
>>
>> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
>> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it
>> so that new mapping for gsi is created if no mapping is found.
>
> So it looks like we have a discrepancy between the x86 and ARM on that
> front.
>
> Lorenzo, Hanjun, can you please have a look at this and shed some
> light on what the expected behaviour is? It looks like we never
> encountered an issue with this on arm64, which tends to indicate that
> we don't usually use the above path.
Sorry for the late reply, I just noticed this tomorrow.
As you said, we never encountered Jianmin's issue on ARM64 hardware,
for the call stack which Jianmin shows, acpi_ev_install_xrupt_handlers()
is only called for non-reduced ACPI hardware, but ARM64 is always
defined as reduced ACPI hardware in the ACPI spec, from the first
supported version of ACPI spec for ARM.
Jianmin, is the LoongArch using the redunced hardware mode in ACPI?
if it's using SCI interrupt, I think not, correct me if I'm wrong.
>
>> I looked into generic acpi_register_gsi, the existed mapping will be
>> checked first by calling irq_find_mapping, so I think calling
>> acpi_register_gsi in acpi_gsi_to_irq can address the problem.
>>
>> But you're right, I'm wrong that I passed random value of -1 to
>> acpi_register_gsi. I don't find a right way to address the problem
>> without changing acpi_gsi_to_irq. I'll continue to work for the
>> problem.
>
> At the very least, this should be indirected so that the existing
> behaviour isn't affected, no matter how badly broken arm64 may or may
> not be here. Please have a look at the patch below that should help
> you with this.
Looks good to me, I will review and test the v13 patch set.
Thanks
Hanjun
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-28 7:42 ` Hanjun Guo
@ 2022-06-28 8:45 ` Jianmin Lv
2022-06-28 9:12 ` Hanjun Guo
0 siblings, 1 reply; 27+ messages in thread
From: Jianmin Lv @ 2022-06-28 8:45 UTC (permalink / raw)
To: Hanjun Guo, Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Lorenzo Pieralisi, Jiaxun Yang,
Huacai Chen
On 2022/6/28 下午3:42, Hanjun Guo wrote:
> On 2022/6/18 18:36, Marc Zyngier wrote:
> [...]
>>>>> --- 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,10 +26,7 @@
>>>>> */
>>>>> 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);
>>>>> -
>>>>> - *irq = irq_find_mapping(d, gsi);
>>>>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
>>>>
>>>> What is this?
>>>>
>>>> - This wasn't part of my initial patch, and randomly changing patches
>>>> without mentioning it isn't acceptable
>>>>
>>>> - you *cannot* trigger a registration here, as this isn't what the API
>>>> advertises
>>>>
>>>> - what makes you think that passing random values (NULL, -1... )to
>>>> acpi_register_gsi() is an acceptable thing to do?
>>>>
>>>> The original patch had:
>>>>
>>>> @@ -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);
>>>> /*
>>>>
>>>> and I don't think it needs anything else. If something breaks, let's
>>>> discuss it, but don't abuse the API nor the fact that I usually don't
>>>> review my own patches to sneak things in...
>>>>
>>>
>>> Sorry, Marc, I don't know how to communicate with you for my change
>>> here before submitting the patch, maybe I should mention it in the
>>> patch commit or code.
>>
>> It should at least be discussed first, like you are doing it here.
>>
>>> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
>>> only handle existed mapping and will return -EINVAL if mapping not
>>> found. When I test on my machine, a calling stack is as following:
>>>
>>>
>>> acpi_bus_init
>>> ->acpi_enable_subsystem
>>> ->acpi_ev_install_xrupt_handlers
>>> ->acpi_ev_install_sci_handler
>>> ->acpi_os_install_interrupt_handler
>>> ->acpi_gsi_to_irq
>>>
>>>
>>> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
>>> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it
>>> so that new mapping for gsi is created if no mapping is found.
>>
>> So it looks like we have a discrepancy between the x86 and ARM on that
>> front.
>>
>> Lorenzo, Hanjun, can you please have a look at this and shed some
>> light on what the expected behaviour is? It looks like we never
>> encountered an issue with this on arm64, which tends to indicate that
>> we don't usually use the above path.
>
> Sorry for the late reply, I just noticed this tomorrow.
>
> As you said, we never encountered Jianmin's issue on ARM64 hardware,
> for the call stack which Jianmin shows, acpi_ev_install_xrupt_handlers()
> is only called for non-reduced ACPI hardware, but ARM64 is always
> defined as reduced ACPI hardware in the ACPI spec, from the first
> supported version of ACPI spec for ARM.
>
> Jianmin, is the LoongArch using the redunced hardware mode in ACPI?
> if it's using SCI interrupt, I think not, correct me if I'm wrong.
>
Thanks for your reply, Hanjun, LoongArch uses non-reduced ACPI hardware,
so SCI interrupt is used, which is different from ARM using reduced
hardware.
>>
>>> I looked into generic acpi_register_gsi, the existed mapping will be
>>> checked first by calling irq_find_mapping, so I think calling
>>> acpi_register_gsi in acpi_gsi_to_irq can address the problem.
>>>
>>> But you're right, I'm wrong that I passed random value of -1 to
>>> acpi_register_gsi. I don't find a right way to address the problem
>>> without changing acpi_gsi_to_irq. I'll continue to work for the
>>> problem.
>>
>> At the very least, this should be indirected so that the existing
>> behaviour isn't affected, no matter how badly broken arm64 may or may
>> not be here. Please have a look at the patch below that should help
>> you with this.
>
> Looks good to me, I will review and test the v13 patch set.
>
> Thanks
> Hanjun
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains
2022-06-28 8:45 ` Jianmin Lv
@ 2022-06-28 9:12 ` Hanjun Guo
0 siblings, 0 replies; 27+ messages in thread
From: Hanjun Guo @ 2022-06-28 9:12 UTC (permalink / raw)
To: Jianmin Lv, Marc Zyngier
Cc: Thomas Gleixner, linux-kernel, Lorenzo Pieralisi, Jiaxun Yang,
Huacai Chen
On 2022/6/28 16:45, Jianmin Lv wrote:
>
>
> On 2022/6/28 下午3:42, Hanjun Guo wrote:
>> On 2022/6/18 18:36, Marc Zyngier wrote:
>> [...]
>>>>>> --- 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,10 +26,7 @@
>>>>>> */
>>>>>> 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);
>>>>>> -
>>>>>> - *irq = irq_find_mapping(d, gsi);
>>>>>> + *irq = acpi_register_gsi(NULL, gsi, -1, -1);
>>>>>
>>>>> What is this?
>>>>>
>>>>> - This wasn't part of my initial patch, and randomly changing patches
>>>>> without mentioning it isn't acceptable
>>>>>
>>>>> - you *cannot* trigger a registration here, as this isn't what the API
>>>>> advertises
>>>>>
>>>>> - what makes you think that passing random values (NULL, -1... )to
>>>>> acpi_register_gsi() is an acceptable thing to do?
>>>>>
>>>>> The original patch had:
>>>>>
>>>>> @@ -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);
>>>>> /*
>>>>>
>>>>> and I don't think it needs anything else. If something breaks, let's
>>>>> discuss it, but don't abuse the API nor the fact that I usually don't
>>>>> review my own patches to sneak things in...
>>>>>
>>>>
>>>> Sorry, Marc, I don't know how to communicate with you for my change
>>>> here before submitting the patch, maybe I should mention it in the
>>>> patch commit or code.
>>>
>>> It should at least be discussed first, like you are doing it here.
>>>
>>>> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c
>>>> only handle existed mapping and will return -EINVAL if mapping not
>>>> found. When I test on my machine, a calling stack is as following:
>>>>
>>>>
>>>> acpi_bus_init
>>>> ->acpi_enable_subsystem
>>>> ->acpi_ev_install_xrupt_handlers
>>>> ->acpi_ev_install_sci_handler
>>>> ->acpi_os_install_interrupt_handler
>>>> ->acpi_gsi_to_irq
>>>>
>>>>
>>>> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I
>>>> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it
>>>> so that new mapping for gsi is created if no mapping is found.
>>>
>>> So it looks like we have a discrepancy between the x86 and ARM on that
>>> front.
>>>
>>> Lorenzo, Hanjun, can you please have a look at this and shed some
>>> light on what the expected behaviour is? It looks like we never
>>> encountered an issue with this on arm64, which tends to indicate that
>>> we don't usually use the above path.
>>
>> Sorry for the late reply, I just noticed this tomorrow.
What? Tomorrow? more coffee is needed... it's yesterday...
>>
>> As you said, we never encountered Jianmin's issue on ARM64 hardware,
>> for the call stack which Jianmin shows, acpi_ev_install_xrupt_handlers()
>> is only called for non-reduced ACPI hardware, but ARM64 is always
>> defined as reduced ACPI hardware in the ACPI spec, from the first
>> supported version of ACPI spec for ARM.
>>
>> Jianmin, is the LoongArch using the redunced hardware mode in ACPI?
>> if it's using SCI interrupt, I think not, correct me if I'm wrong.
>>
>
> Thanks for your reply, Hanjun, LoongArch uses non-reduced ACPI hardware,
> so SCI interrupt is used, which is different from ARM using reduced
> hardware.
OK, so for ARM64, it will not call acpi_gsi_to_irq() before the
irqdomain and mapping created.
Thanks
Hanjun
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2022-06-28 9:12 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-15 6:07 [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 01/10] APCI: irq: Add support for multiple GSI domains Jianmin Lv
2022-06-15 7:14 ` Marc Zyngier
2022-06-15 9:28 ` Jianmin Lv
2022-06-18 10:36 ` Marc Zyngier
2022-06-20 2:46 ` Jianmin Lv
2022-06-25 9:34 ` Jianmin Lv
2022-06-27 7:32 ` Marc Zyngier
2022-06-27 8:00 ` Jianmin Lv
2022-06-28 7:42 ` Hanjun Guo
2022-06-28 8:45 ` Jianmin Lv
2022-06-28 9:12 ` Hanjun Guo
2022-06-15 9:43 ` Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 02/10] genirq/generic_chip: export irq_unmap_generic_chip Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 03/10] irqchip: Add LoongArch CPU interrupt controller support Jianmin Lv
2022-06-18 10:59 ` Marc Zyngier
2022-06-20 3:06 ` Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver Jianmin Lv
2022-06-18 17:22 ` Marc Zyngier
2022-06-20 3:12 ` Jianmin Lv
2022-06-27 7:34 ` Marc Zyngier
2022-06-27 8:04 ` Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 05/10] irqchip/loongson-pch-pic: Add ACPI init support Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 06/10] irqchip/loongson-pch-msi: " Jianmin Lv
2022-06-15 6:07 ` [PATCH V12 07/10] irqchip/loongson-htvec: " Jianmin Lv
2022-06-18 10:39 ` [PATCH V12 00/10] irqchip: Add LoongArch-related irqchip drivers Marc Zyngier
2022-06-20 2:28 ` Jianmin Lv
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.