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 next revision of ACPI Specification (current
revision is 6.4).
This patchset adds some irqchip drivers for LoongArch, it is preparing
to add LoongArch support in mainline kernel, we can see a snapshot here:
https://github.com/loongson/linux/tree/loongarch-next
Cross-compile tool chain to build kernel:
https://github.com/loongson/build-tools/releases
Loongson and LoongArch documentations:
https://github.com/loongson/LoongArch-Documentation
LoongArch-specific interrupt controllers:
https://mantis.uefi.org/mantis/view.php?id=2203
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;
Huacai Chen:
irqchip: Adjust Kconfig for Loongson.
irqchip/loongson-pch-pic: Add ACPI init support.
irqchip/loongson-pch-pic: Add suspend/resume support.
irqchip/loongson-pch-msi: Add ACPI init support.
irqchip/loongson-htvec: Add ACPI init support.
irqchip/loongson-htvec: Add suspend/resume support.
irqchip/loongson-liointc: Add ACPI init support.
irqchip: Add LoongArch CPU interrupt controller support.
irqchip: Add Loongson Extended I/O interrupt controller.
irqchip: Add Loongson PCH LPC controller support.
Signed-off-by: Huacai Chen <[email protected]>
---
drivers/irqchip/Kconfig | 37 +++-
drivers/irqchip/Makefile | 3 +
drivers/irqchip/irq-loongarch-cpu.c | 76 ++++++++
drivers/irqchip/irq-loongson-eiointc.c | 326 +++++++++++++++++++++++++++++++++
drivers/irqchip/irq-loongson-htvec.c | 147 +++++++++++----
drivers/irqchip/irq-loongson-liointc.c | 197 ++++++++++++--------
drivers/irqchip/irq-loongson-pch-lpc.c | 204 +++++++++++++++++++++
drivers/irqchip/irq-loongson-pch-msi.c | 126 ++++++++-----
drivers/irqchip/irq-loongson-pch-pic.c | 159 +++++++++++++---
include/linux/cpuhotplug.h | 1 +
10 files changed, 1088 insertions(+), 188 deletions(-)
create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
create mode 100644 drivers/irqchip/irq-loongson-eiointc.c
create mode 100644 drivers/irqchip/irq-loongson-pch-lpc.c
--
2.27.0
We are preparing to add new Loongson (based on LoongArch, not MIPS)
support. LoongArch use ACPI other than DT as its boot protocol, so
add ACPI init support.
Signed-off-by: Huacai Chen <[email protected]>
---
drivers/irqchip/irq-loongson-pch-msi.c | 126 ++++++++++++++++---------
1 file changed, 84 insertions(+), 42 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-pch-msi.c b/drivers/irqchip/irq-loongson-pch-msi.c
index 32562b7e681b..1ecb15f8745b 100644
--- a/drivers/irqchip/irq-loongson-pch-msi.c
+++ b/drivers/irqchip/irq-loongson-pch-msi.c
@@ -21,6 +21,7 @@ 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 void pch_msi_mask_msi_irq(struct irq_data *d)
@@ -154,12 +155,14 @@ static const struct irq_domain_ops pch_msi_middle_domain_ops = {
};
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 +174,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 +186,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,38 +198,18 @@ 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;
@@ -244,7 +219,74 @@ static int pch_msi_init(struct device_node *node,
kfree(priv->msi_map);
err_priv:
kfree(priv);
- return ret;
+
+ return -EINVAL;
+}
+
+#ifdef CONFIG_OF
+
+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
+
+struct fwnode_handle *pch_msi_acpi_init(struct fwnode_handle *parent,
+ struct acpi_madt_msi_pic *acpi_pchmsi)
+{
+ int ret;
+ struct irq_domain *parent_domain;
+ struct fwnode_handle *domain_handle;
+
+ parent_domain = irq_find_matching_fwnode(parent, DOMAIN_BUS_ANY);
+ if (!parent_domain) {
+ pr_err("Failed to find the parent domain\n");
+ return NULL;
+ }
+
+ domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_pchmsi);
+
+ ret = pch_msi_init(acpi_pchmsi->msg_address, acpi_pchmsi->start,
+ acpi_pchmsi->count, parent_domain, domain_handle);
+ if (ret < 0)
+ return NULL;
+
+ return domain_handle;
}
-IRQCHIP_DECLARE(pch_msi, "loongson,pch-msi-1.0", pch_msi_init);
+#endif
--
2.27.0
We are preparing to add new Loongson (based on LoongArch, not MIPS)
support. This patch add Loongson PCH LPC interrupt controller support.
Signed-off-by: Huacai Chen <[email protected]>
---
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-loongson-pch-lpc.c | 204 +++++++++++++++++++++++++
3 files changed, 213 insertions(+)
create mode 100644 drivers/irqchip/irq-loongson-pch-lpc.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 895b19fcea59..2ba0f341d976 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -592,6 +592,14 @@ config LOONGSON_PCH_MSI
help
Support for the Loongson PCH MSI Controller.
+config LOONGSON_PCH_LPC
+ bool "Loongson PCH LPC Controller"
+ depends on MACH_LOONGSON64
+ default MACH_LOONGSON64
+ select IRQ_DOMAIN_HIERARCHY
+ help
+ Support for the Loongson PCH LPC Controller.
+
config MST_IRQ
bool "MStar Interrupt Controller"
depends on ARCH_MEDIATEK || ARCH_MSTARV7 || COMPILE_TEST
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index eb3fdc6fe808..6fd07980fa47 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -112,6 +112,7 @@ obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
obj-$(CONFIG_LOONGSON_PCH_PIC) += irq-loongson-pch-pic.o
obj-$(CONFIG_LOONGSON_PCH_MSI) += irq-loongson-pch-msi.o
+obj-$(CONFIG_LOONGSON_PCH_LPC) += irq-loongson-pch-lpc.o
obj-$(CONFIG_MST_IRQ) += irq-mst-intc.o
obj-$(CONFIG_SL28CPLD_INTC) += irq-sl28cpld.o
obj-$(CONFIG_MACH_REALTEK_RTL) += irq-realtek-rtl.o
diff --git a/drivers/irqchip/irq-loongson-pch-lpc.c b/drivers/irqchip/irq-loongson-pch-lpc.c
new file mode 100644
index 000000000000..14e927548610
--- /dev/null
+++ b/drivers/irqchip/irq-loongson-pch-lpc.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Loongson LPC Interrupt Controller support
+ *
+ * Copyright (C) 2020-2021 Loongson Technology Corporation Limited
+ */
+
+#define pr_fmt(fmt) "lpc: " fmt
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/syscore_ops.h>
+
+/* Registers */
+#define LPC_INT_CTL 0x00
+#define LPC_INT_ENA 0x04
+#define LPC_INT_STS 0x08
+#define LPC_INT_CLR 0x0c
+#define LPC_INT_POL 0x10
+#define LPC_COUNT 16
+
+struct pch_lpc {
+ void __iomem *base;
+ struct irq_domain *lpc_domain;
+ struct fwnode_handle *domain_handle;
+ raw_spinlock_t lpc_lock;
+ u32 saved_reg_ctl;
+ u32 saved_reg_ena;
+ u32 saved_reg_pol;
+};
+
+struct pch_lpc *pch_lpc_priv;
+
+static void ack_lpc_irq(struct irq_data *d)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&pch_lpc_priv->lpc_lock, flags);
+ writel(0x1 << d->irq, pch_lpc_priv->base + LPC_INT_CLR);
+ raw_spin_unlock_irqrestore(&pch_lpc_priv->lpc_lock, flags);
+}
+static void mask_lpc_irq(struct irq_data *d)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&pch_lpc_priv->lpc_lock, flags);
+ writel(readl(pch_lpc_priv->base + LPC_INT_ENA) & (~(0x1 << (d->irq))),
+ pch_lpc_priv->base + LPC_INT_ENA);
+ raw_spin_unlock_irqrestore(&pch_lpc_priv->lpc_lock, flags);
+}
+
+static void mask_ack_lpc_irq(struct irq_data *d)
+{
+}
+
+static void unmask_lpc_irq(struct irq_data *d)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&pch_lpc_priv->lpc_lock, flags);
+ writel(readl(pch_lpc_priv->base + LPC_INT_ENA) | (0x1 << (d->irq)),
+ pch_lpc_priv->base + LPC_INT_ENA);
+ raw_spin_unlock_irqrestore(&pch_lpc_priv->lpc_lock, flags);
+}
+
+static struct irq_chip pch_lpc_irq_chip = {
+ .name = "PCH LPC",
+ .irq_mask = mask_lpc_irq,
+ .irq_unmask = unmask_lpc_irq,
+ .irq_ack = ack_lpc_irq,
+ .irq_mask_ack = mask_ack_lpc_irq,
+ .irq_eoi = unmask_lpc_irq,
+ .flags = IRQCHIP_SKIP_SET_WAKE,
+};
+
+static void lpc_irq_dispatch(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ u32 pending;
+
+ chained_irq_enter(chip, desc);
+
+ pending = readl(pch_lpc_priv->base + LPC_INT_ENA);
+ pending &= readl(pch_lpc_priv->base + LPC_INT_STS);
+ if (!pending)
+ spurious_interrupt();
+
+ while (pending) {
+ int bit = __ffs(pending);
+
+ generic_handle_irq(bit);
+ pending &= ~BIT(bit);
+ }
+ chained_irq_exit(chip, desc);
+}
+
+static int pch_lpc_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hw)
+{
+ irq_set_chip_and_handler(irq, &pch_lpc_irq_chip, handle_level_irq);
+ return 0;
+}
+
+static const struct irq_domain_ops pch_lpc_domain_ops = {
+ .map = pch_lpc_map,
+ .xlate = irq_domain_xlate_onecell,
+};
+
+static void pch_lpc_reset(struct pch_lpc *priv)
+{
+ /* Enable the LPC interrupt, bit31: en bit30: edge */
+ writel(0x80000000, priv->base + LPC_INT_CTL);
+ writel(0, priv->base + LPC_INT_ENA);
+ /* Clear all 18-bit interrpt bit */
+ writel(0x3ffff, priv->base + LPC_INT_CLR);
+}
+
+static int pch_lpc_disabled(struct pch_lpc *priv)
+{
+ return (readl(priv->base + LPC_INT_ENA) == 0xffffffff) &&
+ (readl(priv->base + LPC_INT_STS) == 0xffffffff);
+}
+
+static int pch_lpc_suspend(void)
+{
+ pch_lpc_priv->saved_reg_ctl = readl(pch_lpc_priv->base + LPC_INT_CTL);
+ pch_lpc_priv->saved_reg_ena = readl(pch_lpc_priv->base + LPC_INT_ENA);
+ pch_lpc_priv->saved_reg_pol = readl(pch_lpc_priv->base + LPC_INT_POL);
+ return 0;
+}
+
+static void pch_lpc_resume(void)
+{
+ writel(pch_lpc_priv->saved_reg_ctl, pch_lpc_priv->base + LPC_INT_CTL);
+ writel(pch_lpc_priv->saved_reg_ena, pch_lpc_priv->base + LPC_INT_ENA);
+ writel(pch_lpc_priv->saved_reg_pol, pch_lpc_priv->base + LPC_INT_POL);
+}
+
+static struct syscore_ops pch_lpc_syscore_ops = {
+ .suspend = pch_lpc_suspend,
+ .resume = pch_lpc_resume,
+};
+
+struct fwnode_handle *pch_lpc_acpi_init(struct fwnode_handle *parent,
+ struct acpi_madt_lpc_pic *acpi_pchlpc)
+{
+ int parent_irq;
+ struct pch_lpc *priv;
+ struct irq_fwspec fwspec;
+
+ if (!acpi_pchlpc)
+ return NULL;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return NULL;
+
+ raw_spin_lock_init(&priv->lpc_lock);
+
+ priv->base = ioremap(acpi_pchlpc->address, acpi_pchlpc->size);
+ if (!priv->base)
+ goto free_priv;
+
+ if (pch_lpc_disabled(priv)) {
+ pr_err("Failed to get LPC status\n");
+ goto iounmap_base;
+ }
+
+ priv->domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_pchlpc);
+ if (!priv->domain_handle) {
+ pr_err("Unable to allocate domain handle\n");
+ goto iounmap_base;
+ }
+ priv->lpc_domain = irq_domain_add_legacy(NULL, LPC_COUNT, 0, 0,
+ &pch_lpc_domain_ops, priv);
+ if (!priv->lpc_domain) {
+ pr_err("Failed to create IRQ domain\n");
+ goto iounmap_base;
+ }
+ pch_lpc_reset(priv);
+
+ fwspec.fwnode = parent;
+ fwspec.param[0] = GSI_MIN_PCH_IRQ + acpi_pchlpc->cascade;
+ fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
+ fwspec.param_count = 2;
+ parent_irq = irq_create_fwspec_mapping(&fwspec);
+ irq_set_chained_handler_and_data(parent_irq, lpc_irq_dispatch, priv);
+ pch_lpc_priv = priv;
+
+ register_syscore_ops(&pch_lpc_syscore_ops);
+
+ return priv->domain_handle;
+
+iounmap_base:
+ iounmap(priv->base);
+free_priv:
+ kfree(priv);
+
+ return NULL;
+}
--
2.27.0
We are preparing to add new Loongson (based on LoongArch, not MIPS)
support. LoongArch use ACPI other than DT as its boot protocol, so
add ACPI init support.
Signed-off-by: Huacai Chen <[email protected]>
---
drivers/irqchip/irq-loongson-liointc.c | 197 +++++++++++++++----------
1 file changed, 117 insertions(+), 80 deletions(-)
diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
index 649c58391618..8b9b0dc325bc 100644
--- a/drivers/irqchip/irq-loongson-liointc.c
+++ b/drivers/irqchip/irq-loongson-liointc.c
@@ -16,10 +16,10 @@
#include <linux/smp.h>
#include <linux/irqchip/chained_irq.h>
-#include <loongson.h>
+#include <asm/loongson.h>
#define LIOINTC_CHIP_IRQ 32
-#define LIOINTC_NUM_PARENT 4
+#define LIOINTC_NUM_PARENT 4
#define LIOINTC_NUM_CORES 4
#define LIOINTC_INTC_CHIP_START 0x20
@@ -41,6 +41,7 @@ struct liointc_handler_data {
};
struct liointc_priv {
+ struct fwnode_handle *domain_handle;
struct irq_chip_generic *gc;
struct liointc_handler_data handler[LIOINTC_NUM_PARENT];
void __iomem *core_isr[LIOINTC_NUM_CORES];
@@ -53,7 +54,7 @@ static void liointc_chained_handle_irq(struct irq_desc *desc)
struct liointc_handler_data *handler = irq_desc_get_handler_data(desc);
struct irq_chip *chip = irq_desc_get_chip(desc);
struct irq_chip_generic *gc = handler->priv->gc;
- int core = get_ebase_cpunum() % LIOINTC_NUM_CORES;
+ int core = cpu_logical_map(smp_processor_id()) % LIOINTC_NUM_CORES;
u32 pending;
chained_irq_enter(chip, desc);
@@ -143,97 +144,61 @@ static void liointc_resume(struct irq_chip_generic *gc)
irq_gc_unlock_irqrestore(gc, flags);
}
-static const char * const parent_names[] = {"int0", "int1", "int2", "int3"};
-static const char * const core_reg_names[] = {"isr0", "isr1", "isr2", "isr3"};
+static int parent_irq[LIOINTC_NUM_PARENT];
+static u32 parent_int_map[LIOINTC_NUM_PARENT];
+static const char *const parent_names[] = {"int0", "int1", "int2", "int3"};
+static const char *const core_reg_names[] = {"isr0", "isr1", "isr2", "isr3"};
-static void __iomem *liointc_get_reg_byname(struct device_node *node,
- const char *name)
-{
- int index = of_property_match_string(node, "reg-names", name);
-
- if (index < 0)
- return NULL;
-
- return of_iomap(node, index);
-}
-
-static int __init liointc_of_init(struct device_node *node,
- struct device_node *parent)
+static int liointc_init(phys_addr_t addr, unsigned long size, int revision,
+ struct fwnode_handle *domain_handle, struct device_node *node)
{
+ int i, index, err;
+ void __iomem *base;
+ struct irq_chip_type *ct;
struct irq_chip_generic *gc;
struct irq_domain *domain;
- struct irq_chip_type *ct;
struct liointc_priv *priv;
- void __iomem *base;
- u32 of_parent_int_map[LIOINTC_NUM_PARENT];
- int parent_irq[LIOINTC_NUM_PARENT];
- bool have_parent = FALSE;
- int sz, i, err = 0;
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
- if (of_device_is_compatible(node, "loongson,liointc-2.0")) {
- base = liointc_get_reg_byname(node, "main");
- if (!base) {
- err = -ENODEV;
- goto out_free_priv;
- }
+ base = ioremap(addr, size);
+ if (!base)
+ goto out_free_priv;
- for (i = 0; i < LIOINTC_NUM_CORES; i++)
- priv->core_isr[i] = liointc_get_reg_byname(node, core_reg_names[i]);
- if (!priv->core_isr[0]) {
- err = -ENODEV;
- goto out_iounmap_base;
- }
- } else {
- base = of_iomap(node, 0);
- if (!base) {
- err = -ENODEV;
- goto out_free_priv;
- }
+ priv->domain_handle = domain_handle;
- for (i = 0; i < LIOINTC_NUM_CORES; i++)
- priv->core_isr[i] = base + LIOINTC_REG_INTC_STATUS;
- }
+ for (i = 0; i < LIOINTC_NUM_CORES; i++)
+ priv->core_isr[i] = base + LIOINTC_REG_INTC_STATUS;
- for (i = 0; i < LIOINTC_NUM_PARENT; i++) {
- parent_irq[i] = of_irq_get_byname(node, parent_names[i]);
- if (parent_irq[i] > 0)
- have_parent = TRUE;
- }
- if (!have_parent) {
- err = -ENODEV;
- goto out_iounmap_isr;
- }
+ for (i = 0; i < LIOINTC_NUM_PARENT; i++)
+ priv->handler[i].parent_int_map = parent_int_map[i];
- sz = of_property_read_variable_u32_array(node,
- "loongson,parent_int_map",
- &of_parent_int_map[0],
- LIOINTC_NUM_PARENT,
- LIOINTC_NUM_PARENT);
- if (sz < 4) {
- pr_err("loongson-liointc: No parent_int_map\n");
- err = -ENODEV;
- goto out_iounmap_isr;
- }
+#ifdef CONFIG_OF
+ if (revision > 1) {
+ for (i = 0; i < LIOINTC_NUM_CORES; i++) {
+ index = of_property_match_string(node, "reg-names", core_reg_names[i]);
- for (i = 0; i < LIOINTC_NUM_PARENT; i++)
- priv->handler[i].parent_int_map = of_parent_int_map[i];
+ if (index < 0)
+ return -EINVAL;
+
+ priv->core_isr[i] = of_iomap(node, index);
+ }
+ }
+#endif
/* Setup IRQ domain */
- domain = irq_domain_add_linear(node, 32,
+ domain = irq_domain_create_linear(domain_handle, LIOINTC_CHIP_IRQ,
&irq_generic_chip_ops, priv);
if (!domain) {
pr_err("loongson-liointc: cannot add IRQ domain\n");
- err = -EINVAL;
- goto out_iounmap_isr;
+ goto out_iounmap;
}
- err = irq_alloc_domain_generic_chips(domain, 32, 1,
- node->full_name, handle_level_irq,
- IRQ_NOPROBE, 0, 0);
+ err = irq_alloc_domain_generic_chips(domain, LIOINTC_CHIP_IRQ, 1,
+ (node ? node->full_name : "LIOINTC"),
+ handle_level_irq, 0, IRQ_NOPROBE, 0);
if (err) {
pr_err("loongson-liointc: unable to register IRQ domain\n");
goto out_free_domain;
@@ -293,20 +258,92 @@ static int __init liointc_of_init(struct device_node *node,
out_free_domain:
irq_domain_remove(domain);
-out_iounmap_isr:
- for (i = 0; i < LIOINTC_NUM_CORES; i++) {
- if (!priv->core_isr[i])
- continue;
- iounmap(priv->core_isr[i]);
- }
-out_iounmap_base:
+out_iounmap:
iounmap(base);
out_free_priv:
kfree(priv);
- return err;
+ return -EINVAL;
+}
+
+#ifdef CONFIG_OF
+
+static int __init liointc_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ bool have_parent = FALSE;
+ int sz, i, index, revision, err = 0;
+ struct resource res;
+
+ if (!of_device_is_compatible(node, "loongson,liointc-2.0")) {
+ index = 0;
+ revision = 1;
+ } else {
+ index = of_property_match_string(node, "reg-names", "main");
+ revision = 2;
+ }
+
+ if (of_address_to_resource(node, index, &res))
+ return -EINVAL;
+
+ for (i = 0; i < LIOINTC_NUM_PARENT; i++) {
+ parent_irq[i] = of_irq_get_byname(node, parent_names[i]);
+ if (parent_irq[i] > 0)
+ have_parent = TRUE;
+ }
+ if (!have_parent)
+ return -ENODEV;
+
+ sz = of_property_read_variable_u32_array(node,
+ "loongson,parent_int_map",
+ &parent_int_map[0],
+ LIOINTC_NUM_PARENT,
+ LIOINTC_NUM_PARENT);
+ if (sz < 4) {
+ pr_err("loongson-liointc: No parent_int_map\n");
+ return -ENODEV;
+ }
+
+ err = liointc_init(res.start, resource_size(&res),
+ revision, of_node_to_fwnode(node), node);
+ if (err < 0)
+ return err;
+
+ return 0;
}
IRQCHIP_DECLARE(loongson_liointc_1_0, "loongson,liointc-1.0", liointc_of_init);
IRQCHIP_DECLARE(loongson_liointc_1_0a, "loongson,liointc-1.0a", liointc_of_init);
IRQCHIP_DECLARE(loongson_liointc_2_0, "loongson,liointc-2.0", liointc_of_init);
+
+#endif
+
+#ifdef CONFIG_ACPI
+
+struct fwnode_handle *liointc_acpi_init(struct acpi_madt_lio_pic *acpi_liointc)
+{
+ int ret;
+ struct fwnode_handle *domain_handle;
+
+ parent_int_map[0] = acpi_liointc->cascade_map[0];
+ parent_int_map[1] = acpi_liointc->cascade_map[1];
+
+ parent_irq[0] = LOONGSON_CPU_IRQ_BASE + acpi_liointc->cascade[0];
+ if (!cpu_has_extioi)
+ parent_irq[1] = LOONGSON_CPU_IRQ_BASE + acpi_liointc->cascade[1];
+
+ domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_liointc);
+ if (!domain_handle) {
+ pr_err("Unable to allocate domain handle\n");
+ return NULL;
+ }
+
+ ret = liointc_init(acpi_liointc->address, acpi_liointc->size,
+ 1, domain_handle, NULL);
+ if (ret < 0)
+ return NULL;
+
+ return domain_handle;
+}
+
+#endif
--
2.27.0
We are preparing to add new Loongson (based on LoongArch, not MIPS)
support. This patch add LoongArch CPU interrupt controller support.
Signed-off-by: Huacai Chen <[email protected]>
---
drivers/irqchip/Kconfig | 10 ++++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-loongarch-cpu.c | 76 +++++++++++++++++++++++++++++
3 files changed, 87 insertions(+)
create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 084bc4c2eebd..443c3a7a0cc1 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -528,6 +528,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 f88cbf36a9d2..4e34eebe180b 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
+obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
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 000000000000..8e9e8d39cb22
--- /dev/null
+++ b/drivers/irqchip/irq-loongarch-cpu.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020-2021 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>
+
+static struct irq_domain *irq_domain;
+
+static inline void enable_loongarch_irq(struct irq_data *d)
+{
+ set_csr_ecfg(ECFGF(d->hwirq));
+}
+
+#define eoi_loongarch_irq enable_loongarch_irq
+
+static inline void disable_loongarch_irq(struct irq_data *d)
+{
+ clear_csr_ecfg(ECFGF(d->hwirq));
+}
+
+#define ack_loongarch_irq disable_loongarch_irq
+
+static struct irq_chip loongarch_cpu_irq_controller = {
+ .name = "LoongArch",
+ .irq_ack = ack_loongarch_irq,
+ .irq_eoi = eoi_loongarch_irq,
+ .irq_enable = enable_loongarch_irq,
+ .irq_disable = disable_loongarch_irq,
+};
+
+asmlinkage void default_handle_irq(int irq)
+{
+ do_IRQ(irq_linear_revmap(irq_domain, irq));
+}
+
+static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ struct irq_chip *chip;
+
+ irq_set_noprobe(irq);
+ chip = &loongarch_cpu_irq_controller;
+ set_vi_handler(EXCCODE_INT_START + hwirq, default_handle_irq);
+ irq_set_chip_and_handler(irq, chip, 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,
+};
+
+int __init loongarch_cpu_irq_init(void)
+{
+ /* Mask interrupts. */
+ clear_csr_ecfg(ECFG0_IM);
+ clear_csr_estat(ESTATF_IP);
+
+ irq_domain = irq_domain_add_simple(NULL, EXCCODE_INT_NUM,
+ LOONGSON_CPU_IRQ_BASE, &loongarch_cpu_intc_irq_domain_ops, NULL);
+
+ if (!irq_domain)
+ panic("Failed to add irqdomain for LoongArch CPU");
+
+ return 0;
+}
--
2.27.0
We are preparing to add new Loongson (based on LoongArch, not MIPS)
support. This patch add Loongson Extended I/O CPU interrupt controller
support.
Signed-off-by: Huacai Chen <[email protected]>
---
drivers/irqchip/Kconfig | 9 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-loongson-eiointc.c | 326 +++++++++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
4 files changed, 337 insertions(+)
create mode 100644 drivers/irqchip/irq-loongson-eiointc.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 443c3a7a0cc1..895b19fcea59 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -547,6 +547,15 @@ config LOONGSON_LIOINTC
help
Support for the Loongson Local I/O Interrupt Controller.
+config LOONGSON_EIOINTC
+ bool "Loongson Extend I/O Interrupt Controller"
+ depends on MACH_LOONGSON64
+ default MACH_LOONGSON64
+ select IRQ_DOMAIN_HIERARCHY
+ select GENERIC_IRQ_CHIP
+ help
+ Support for the Loongson3 Extend I/O Interrupt Vector Controller.
+
config LOONGSON_HTPIC
bool "Loongson3 HyperTransport PIC Controller"
depends on (MACH_LOONGSON64 && MIPS)
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 4e34eebe180b..eb3fdc6fe808 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -107,6 +107,7 @@ 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_EIOINTC) += irq-loongson-eiointc.o
obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
obj-$(CONFIG_LOONGSON_PCH_PIC) += irq-loongson-pch-pic.o
diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
new file mode 100644
index 000000000000..184e5f845400
--- /dev/null
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -0,0 +1,326 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Loongson Extend I/O Interrupt Controller support
+ *
+ * Copyright (C) 2020-2021 Loongson Technology Corporation Limited
+ */
+
+#define pr_fmt(fmt) "eiointc: " fmt
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/syscore_ops.h>
+
+#define EIOINTC_REG_NODEMAP 0x14a0
+#define EIOINTC_REG_IPMAP 0x14c0
+#define EIOINTC_REG_ENABLE 0x1600
+#define EIOINTC_REG_BOUNCE 0x1680
+#define EIOINTC_REG_ISR 0x1800
+#define EIOINTC_REG_ROUTE 0x1c00
+
+#define VEC_REG_COUNT 4
+#define VEC_COUNT_PER_REG 64
+#define VEC_COUNT (VEC_REG_COUNT * VEC_COUNT_PER_REG)
+#define VEC_REG_IDX(irq_id) ((irq_id) / VEC_COUNT_PER_REG)
+#define VEC_REG_BIT(irq_id) ((irq_id) % VEC_COUNT_PER_REG)
+
+struct eiointc_priv {
+ struct fwnode_handle *domain_handle;
+ struct irq_domain *eiointc_domain;
+ u32 eiointc_en[VEC_COUNT/32];
+};
+
+struct eiointc_priv *eiointc_priv;
+
+static void eiointc_set_irq_route(int pos, unsigned int cpu)
+{
+ int node, cpu_node, route_node;
+ unsigned char coremap[MAX_NUMNODES];
+ uint32_t pos_off, data, data_byte, data_mask;
+
+ pos_off = pos & ~3;
+ data_byte = pos & 3;
+ data_mask = ~BIT_MASK(data_byte) & 0xf;
+
+ memset(coremap, 0, sizeof(unsigned char) * MAX_NUMNODES);
+
+ /* Calculate node and coremap of target irq */
+ cpu_node = cpu_to_node(cpu);
+ coremap[cpu_node] |= (1 << (topology_core_id(cpu)));
+
+ for_each_online_node(node) {
+ data = 0ULL;
+
+ /* Node 0 is in charge of inter-node interrupt dispatch */
+ route_node = (node == 0) ? cpu_node : node;
+ data |= ((coremap[node] | (route_node << 4)) << (data_byte * 8));
+
+ csr_any_send(EIOINTC_REG_ROUTE + pos_off, data, data_mask, node);
+ }
+}
+
+static DEFINE_SPINLOCK(affinity_lock);
+
+static int eiointc_set_irq_affinity(struct irq_data *d, const struct cpumask *affinity,
+ bool force)
+{
+ unsigned int cpu;
+ unsigned long flags;
+ uint32_t vector, pos_off;
+
+ if (!IS_ENABLED(CONFIG_SMP))
+ return -EPERM;
+
+ spin_lock_irqsave(&affinity_lock, flags);
+
+ if (!cpumask_intersects(affinity, cpu_online_mask)) {
+ spin_unlock_irqrestore(&affinity_lock, flags);
+ return -EINVAL;
+ }
+ cpu = cpumask_first_and(affinity, cpu_online_mask);
+
+ /*
+ * Control interrupt enable or disalbe through cpu 0
+ * which is reponsible for dispatching interrupts.
+ */
+ vector = d->hwirq;
+ pos_off = vector >> 5;
+
+ csr_any_send(EIOINTC_REG_ENABLE + (pos_off << 2),
+ eiointc_priv->eiointc_en[pos_off] & (~((1 << (vector & 0x1F)))), 0x0, 0);
+
+ eiointc_set_irq_route(vector, cpu);
+ csr_any_send(EIOINTC_REG_ENABLE + (pos_off << 2),
+ eiointc_priv->eiointc_en[pos_off], 0x0, 0);
+ irq_data_update_effective_affinity(d, cpumask_of(cpu));
+
+ spin_unlock_irqrestore(&affinity_lock, flags);
+
+ return IRQ_SET_MASK_OK;
+}
+
+static int eiointc_router_init(unsigned int cpu)
+{
+ int i, bit;
+ uint32_t data;
+ uint32_t node = cpu_to_node(cpu);
+
+ if (cpu == cpumask_first(cpumask_of_node(node))) {
+ eiointc_enable();
+
+ for (i = 0; i < VEC_COUNT / 32; i++) {
+ data = (((1 << (i * 2 + 1)) << 16) | (1 << (i * 2)));
+ iocsr_writel(data, EIOINTC_REG_NODEMAP + i * 4);
+ }
+
+ for (i = 0; i < VEC_COUNT / 32 / 4; i++) {
+ data = 0x02020202; /* Route to IP3 */
+ iocsr_writel(data, EIOINTC_REG_IPMAP + i * 4);
+ }
+
+ for (i = 0; i < VEC_COUNT / 4; i++) {
+ /* Route to Node-0 Core-0 */
+ bit = BIT(cpu_logical_map(0));
+ data = bit | (bit << 8) | (bit << 16) | (bit << 24);
+ iocsr_writel(data, EIOINTC_REG_ROUTE + i * 4);
+ }
+
+ for (i = 0; i < VEC_COUNT / 32; i++) {
+ data = 0xffffffff;
+ iocsr_writel(data, EIOINTC_REG_ENABLE + i * 4);
+ iocsr_writel(data, EIOINTC_REG_BOUNCE + i * 4);
+ }
+ }
+
+ return 0;
+}
+
+static void eiointc_irq_dispatch(struct irq_desc *desc)
+{
+ int i;
+ u64 pending;
+ bool handled = false;
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct eiointc_priv *priv = irq_desc_get_handler_data(desc);
+
+ chained_irq_enter(chip, desc);
+
+ for (i = 0; i < VEC_REG_COUNT; i++) {
+ pending = iocsr_readq(EIOINTC_REG_ISR + (i << 3));
+ iocsr_writeq(pending, EIOINTC_REG_ISR + (i << 3));
+ while (pending) {
+ int bit = __ffs(pending);
+ int virq = irq_linear_revmap(priv->eiointc_domain, bit + VEC_COUNT_PER_REG * i);
+
+ generic_handle_irq(virq);
+ pending &= ~BIT(bit);
+ handled = true;
+ }
+ }
+
+ if (!handled)
+ spurious_interrupt();
+
+ chained_irq_exit(chip, desc);
+}
+
+static void eiointc_ack_irq(struct irq_data *d)
+{
+}
+
+static void eiointc_mask_irq(struct irq_data *d)
+{
+}
+
+static void eiointc_unmask_irq(struct irq_data *d)
+{
+}
+
+static struct irq_chip eiointc_irq_chip = {
+ .name = "EIOINTC",
+ .irq_ack = eiointc_ack_irq,
+ .irq_mask = eiointc_mask_irq,
+ .irq_unmask = eiointc_unmask_irq,
+ .irq_set_affinity = eiointc_set_irq_affinity,
+};
+
+static int eiointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ int ret;
+ unsigned int i, type;
+ unsigned long hwirq = 0;
+ struct eiointc *priv = domain->host_data;
+
+ ret = irq_domain_translate_onecell(domain, arg, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++) {
+ irq_domain_set_info(domain, virq + i, hwirq + i, &eiointc_irq_chip,
+ priv, handle_edge_irq, NULL, NULL);
+ }
+
+ return 0;
+}
+
+static void eiointc_domain_free(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs)
+{
+ int i;
+
+ for (i = 0; i < nr_irqs; i++) {
+ struct irq_data *d = irq_domain_get_irq_data(domain, virq + i);
+
+ irq_set_handler(virq + i, NULL);
+ irq_domain_reset_irq_data(d);
+ }
+}
+
+static const struct irq_domain_ops eiointc_domain_ops = {
+ .translate = irq_domain_translate_onecell,
+ .alloc = eiointc_domain_alloc,
+ .free = eiointc_domain_free,
+};
+
+static int eiointc_suspend(void)
+{
+ return 0;
+}
+
+static bool is_eiointc_irq(struct irq_data *irq_data)
+{
+ struct irq_domain *parent;
+
+ for (parent = irq_data->domain; parent; parent = parent->parent) {
+ if (parent == eiointc_priv->eiointc_domain)
+ return true;
+ }
+
+ return false;
+}
+
+static void eiointc_resume(void)
+{
+ int i;
+ struct irq_desc *desc;
+ struct irq_data *irq_data;
+
+ /* init irq en bitmap */
+ for (i = 0; i < VEC_COUNT / 32; i++)
+ eiointc_priv->eiointc_en[i] = 0xffffffff;
+
+ eiointc_router_init(0);
+
+ for (i = 0; i < NR_IRQS; i++) {
+ desc = irq_to_desc(i);
+ if (desc && desc->handle_irq && desc->handle_irq != handle_bad_irq) {
+ irq_data = &desc->irq_data;
+ if (!is_eiointc_irq(irq_data))
+ continue;
+
+ eiointc_set_irq_affinity(irq_data, irq_data->common->affinity, 0);
+ }
+ }
+}
+
+static struct syscore_ops eiointc_syscore_ops = {
+ .suspend = eiointc_suspend,
+ .resume = eiointc_resume,
+};
+
+struct fwnode_handle *eiointc_acpi_init(struct acpi_madt_eio_pic *acpi_eiointc)
+{
+ int i, parent_irq;
+ struct eiointc_priv *priv;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return NULL;
+
+ priv->domain_handle = irq_domain_alloc_fwnode((phys_addr_t *)acpi_eiointc);
+ if (!priv->domain_handle) {
+ pr_err("Unable to allocate domain handle\n");
+ goto out_free_priv;
+ }
+
+ /* Setup IRQ domain */
+ priv->eiointc_domain = irq_domain_create_linear(priv->domain_handle, VEC_COUNT,
+ &eiointc_domain_ops, priv);
+ if (!priv->eiointc_domain) {
+ pr_err("loongson-eiointc: cannot add IRQ domain\n");
+ goto out_free_priv;
+ }
+
+ /* init irq en bitmap */
+ for (i = 0; i < VEC_COUNT/32; i++)
+ priv->eiointc_en[i] = 0xffffffff;
+
+ eiointc_priv = priv;
+
+ eiointc_router_init(0);
+
+ parent_irq = LOONGSON_CPU_IRQ_BASE + acpi_eiointc->cascade;
+ irq_set_chained_handler_and_data(parent_irq, eiointc_irq_dispatch, priv);
+
+ register_syscore_ops(&eiointc_syscore_ops);
+ cpuhp_setup_state_nocalls(CPUHP_AP_IRQ_LOONGARCH_STARTING,
+ "irqchip/loongarch/intc:starting",
+ eiointc_router_init, NULL);
+
+ return eiointc_priv->domain_handle;
+
+out_free_priv:
+ priv->domain_handle = NULL;
+ kfree(priv);
+
+ return NULL;
+}
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index f39b34b13871..34599ce0b18b 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -105,6 +105,7 @@ enum cpuhp_state {
CPUHP_AP_IRQ_BCM2836_STARTING,
CPUHP_AP_IRQ_MIPS_GIC_STARTING,
CPUHP_AP_IRQ_RISCV_STARTING,
+ CPUHP_AP_IRQ_LOONGARCH_STARTING,
CPUHP_AP_IRQ_SIFIVE_PLIC_STARTING,
CPUHP_AP_ARM_MVEBU_COHERENCY,
CPUHP_AP_MICROCODE_LOADER,
--
2.27.0
On Wed, 25 Aug 2021 07:11:50 +0100,
Huacai Chen <[email protected]> wrote:
>
> We are preparing to add new Loongson (based on LoongArch, not MIPS)
You keep saying "not MIPS", and yet all I see is a blind copy of the
MIPS code.
> support. This patch add LoongArch CPU interrupt controller support.
>
> Signed-off-by: Huacai Chen <[email protected]>
> ---
> drivers/irqchip/Kconfig | 10 ++++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-loongarch-cpu.c | 76 +++++++++++++++++++++++++++++
> 3 files changed, 87 insertions(+)
> create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 084bc4c2eebd..443c3a7a0cc1 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -528,6 +528,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 f88cbf36a9d2..4e34eebe180b 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -105,6 +105,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> 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 000000000000..8e9e8d39cb22
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongarch-cpu.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020-2021 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>
> +
> +static struct irq_domain *irq_domain;
> +
> +static inline void enable_loongarch_irq(struct irq_data *d)
Why 'inline' given that it is used as a function pointer?
> +{
> + set_csr_ecfg(ECFGF(d->hwirq));
> +}
> +
> +#define eoi_loongarch_irq enable_loongarch_irq
NAK. EOI and enable cannot be the same operation.
> +
> +static inline void disable_loongarch_irq(struct irq_data *d)
> +{
> + clear_csr_ecfg(ECFGF(d->hwirq));
> +}
> +
> +#define ack_loongarch_irq disable_loongarch_irq
Same thing. Either you have different operations, or this only
supports mask/unmask.
> +
> +static struct irq_chip loongarch_cpu_irq_controller = {
> + .name = "LoongArch",
> + .irq_ack = ack_loongarch_irq,
> + .irq_eoi = eoi_loongarch_irq,
> + .irq_enable = enable_loongarch_irq,
> + .irq_disable = disable_loongarch_irq,
> +};
> +
> +asmlinkage void default_handle_irq(int irq)
> +{
> + do_IRQ(irq_linear_revmap(irq_domain, irq));
This looks both wrong and short sighted:
- irq_linear_revmap() is now another name for irq_find_mapping().
Which means it uses a RCU read critical section. If, as I expect,
this is just a blind copy of the MIPS code, do_IRQ() will not do
anything with respect to irq_enter()/irq_exit(), which will result
in something pretty bad on the exit from idle path. Lockdep will
probably shout at you pretty loudly.
- A single root interrupt controller is, in my modest experience,
something that rarely happen. You will eventually have a variety of
them, and you will have to join the other arches such as arm, arm64,
riscv and csky that use CONFIG_GENERIC_IRQ_MULTI_HANDLER instead of
following the existing MIPS model.
You can solve this by:
- Move over to CONFIG_GENERIC_IRQ_MULTI_HANDLER so that the interrupt
controller can register itself with the core, rather than being
defined at compile time.
- Drop the do_IRQ() madness. Perform whenever stuff you need to do in
the arch code *before* calling into the interrupt controller code.
- Use generic_handle_irq() to call into the irq stack. It will handle
all the irq_enter()/irq_exit() correctly. It will also avoid the
silly double lookup of the irq_desc on interrupt handling.
> +}
> +
> +static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + struct irq_chip *chip;
> +
> + irq_set_noprobe(irq);
> + chip = &loongarch_cpu_irq_controller;
> + set_vi_handler(EXCCODE_INT_START + hwirq, default_handle_irq);
What is that? Yet another MIPS legacy? Why does it have to be per
interrupt if it obviously apply to each and every root interrupt?
Given that 'vi' probably stands for "vectored interrupt", why isn't
that the irq_enable() code?
> + irq_set_chip_and_handler(irq, chip, 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,
> +};
> +
> +int __init loongarch_cpu_irq_init(void)
> +{
> + /* Mask interrupts. */
> + clear_csr_ecfg(ECFG0_IM);
> + clear_csr_estat(ESTATF_IP);
> +
> + irq_domain = irq_domain_add_simple(NULL, EXCCODE_INT_NUM,
> + LOONGSON_CPU_IRQ_BASE, &loongarch_cpu_intc_irq_domain_ops, NULL);
NAK. You still obviously have some static partitioning of the
interrupt space, which is not acceptable for a new architecture.
> +
> + if (!irq_domain)
> + panic("Failed to add irqdomain for LoongArch CPU");
> +
> + return 0;
> +}
I haven't seen much progress from the first version I reviewed. This
is still the same antiquated, broken MIPS code, only with a different
name.
M.
--
Without deviation from the norm, progress is not possible.
Hi, Marc,
On Wed, Aug 25, 2021 at 4:40 PM Marc Zyngier <[email protected]> wrote:
>
> On Wed, 25 Aug 2021 07:11:50 +0100,
> Huacai Chen <[email protected]> wrote:
> >
> > We are preparing to add new Loongson (based on LoongArch, not MIPS)
>
> You keep saying "not MIPS", and yet all I see is a blind copy of the
> MIPS code.
I say "not MIPS" means it isn't compatible with MIPS, but it is
similar to MIPS, so I copy something from MIPS. Of course MIPS is not
a good example (though it works), so I will rework this patch again.
>
> > support. This patch add LoongArch CPU interrupt controller support.
> >
> > Signed-off-by: Huacai Chen <[email protected]>
> > ---
> > drivers/irqchip/Kconfig | 10 ++++
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-loongarch-cpu.c | 76 +++++++++++++++++++++++++++++
> > 3 files changed, 87 insertions(+)
> > create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 084bc4c2eebd..443c3a7a0cc1 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -528,6 +528,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 f88cbf36a9d2..4e34eebe180b 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -105,6 +105,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> > obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> > obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> > obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> > +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> > 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 000000000000..8e9e8d39cb22
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-loongarch-cpu.c
> > @@ -0,0 +1,76 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020-2021 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>
> > +
> > +static struct irq_domain *irq_domain;
> > +
> > +static inline void enable_loongarch_irq(struct irq_data *d)
>
> Why 'inline' given that it is used as a function pointer?
>
> > +{
> > + set_csr_ecfg(ECFGF(d->hwirq));
> > +}
> > +
> > +#define eoi_loongarch_irq enable_loongarch_irq
>
> NAK. EOI and enable cannot be the same operation.
>
> > +
> > +static inline void disable_loongarch_irq(struct irq_data *d)
> > +{
> > + clear_csr_ecfg(ECFGF(d->hwirq));
> > +}
> > +
> > +#define ack_loongarch_irq disable_loongarch_irq
>
> Same thing. Either you have different operations, or this only
> supports mask/unmask.
>
> > +
> > +static struct irq_chip loongarch_cpu_irq_controller = {
> > + .name = "LoongArch",
> > + .irq_ack = ack_loongarch_irq,
> > + .irq_eoi = eoi_loongarch_irq,
> > + .irq_enable = enable_loongarch_irq,
> > + .irq_disable = disable_loongarch_irq,
> > +};
> > +
> > +asmlinkage void default_handle_irq(int irq)
> > +{
> > + do_IRQ(irq_linear_revmap(irq_domain, irq));
>
> This looks both wrong and short sighted:
>
> - irq_linear_revmap() is now another name for irq_find_mapping().
> Which means it uses a RCU read critical section. If, as I expect,
> this is just a blind copy of the MIPS code, do_IRQ() will not do
> anything with respect to irq_enter()/irq_exit(), which will result
> in something pretty bad on the exit from idle path. Lockdep will
> probably shout at you pretty loudly.
>
> - A single root interrupt controller is, in my modest experience,
> something that rarely happen. You will eventually have a variety of
> them, and you will have to join the other arches such as arm, arm64,
> riscv and csky that use CONFIG_GENERIC_IRQ_MULTI_HANDLER instead of
> following the existing MIPS model.
>
> You can solve this by:
>
> - Move over to CONFIG_GENERIC_IRQ_MULTI_HANDLER so that the interrupt
> controller can register itself with the core, rather than being
> defined at compile time.
>
> - Drop the do_IRQ() madness. Perform whenever stuff you need to do in
> the arch code *before* calling into the interrupt controller code.
>
> - Use generic_handle_irq() to call into the irq stack. It will handle
> all the irq_enter()/irq_exit() correctly. It will also avoid the
> silly double lookup of the irq_desc on interrupt handling.
Thank you for your suggestions, this is very helpful.
Huacai
>
> > +}
> > +
> > +static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
> > + irq_hw_number_t hwirq)
> > +{
> > + struct irq_chip *chip;
> > +
> > + irq_set_noprobe(irq);
> > + chip = &loongarch_cpu_irq_controller;
> > + set_vi_handler(EXCCODE_INT_START + hwirq, default_handle_irq);
>
> What is that? Yet another MIPS legacy? Why does it have to be per
> interrupt if it obviously apply to each and every root interrupt?
>
> Given that 'vi' probably stands for "vectored interrupt", why isn't
> that the irq_enable() code?
>
> > + irq_set_chip_and_handler(irq, chip, 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,
> > +};
> > +
> > +int __init loongarch_cpu_irq_init(void)
> > +{
> > + /* Mask interrupts. */
> > + clear_csr_ecfg(ECFG0_IM);
> > + clear_csr_estat(ESTATF_IP);
> > +
> > + irq_domain = irq_domain_add_simple(NULL, EXCCODE_INT_NUM,
> > + LOONGSON_CPU_IRQ_BASE, &loongarch_cpu_intc_irq_domain_ops, NULL);
>
> NAK. You still obviously have some static partitioning of the
> interrupt space, which is not acceptable for a new architecture.
>
> > +
> > + if (!irq_domain)
> > + panic("Failed to add irqdomain for LoongArch CPU");
> > +
> > + return 0;
> > +}
>
> I haven't seen much progress from the first version I reviewed. This
> is still the same antiquated, broken MIPS code, only with a different
> name.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
Hi, Marc,
On Wed, Aug 25, 2021 at 4:40 PM Marc Zyngier <[email protected]> wrote:
>
> On Wed, 25 Aug 2021 07:11:50 +0100,
> Huacai Chen <[email protected]> wrote:
> >
> > We are preparing to add new Loongson (based on LoongArch, not MIPS)
>
> You keep saying "not MIPS", and yet all I see is a blind copy of the
> MIPS code.
>
> > support. This patch add LoongArch CPU interrupt controller support.
> >
> > Signed-off-by: Huacai Chen <[email protected]>
> > ---
> > drivers/irqchip/Kconfig | 10 ++++
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-loongarch-cpu.c | 76 +++++++++++++++++++++++++++++
> > 3 files changed, 87 insertions(+)
> > create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
> >
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 084bc4c2eebd..443c3a7a0cc1 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -528,6 +528,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 f88cbf36a9d2..4e34eebe180b 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -105,6 +105,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> > obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> > obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> > obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> > +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> > 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 000000000000..8e9e8d39cb22
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-loongarch-cpu.c
> > @@ -0,0 +1,76 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2020-2021 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>
> > +
> > +static struct irq_domain *irq_domain;
> > +
> > +static inline void enable_loongarch_irq(struct irq_data *d)
>
> Why 'inline' given that it is used as a function pointer?
>
> > +{
> > + set_csr_ecfg(ECFGF(d->hwirq));
> > +}
> > +
> > +#define eoi_loongarch_irq enable_loongarch_irq
>
> NAK. EOI and enable cannot be the same operation.
>
> > +
> > +static inline void disable_loongarch_irq(struct irq_data *d)
> > +{
> > + clear_csr_ecfg(ECFGF(d->hwirq));
> > +}
> > +
> > +#define ack_loongarch_irq disable_loongarch_irq
>
> Same thing. Either you have different operations, or this only
> supports mask/unmask.
>
> > +
> > +static struct irq_chip loongarch_cpu_irq_controller = {
> > + .name = "LoongArch",
> > + .irq_ack = ack_loongarch_irq,
> > + .irq_eoi = eoi_loongarch_irq,
> > + .irq_enable = enable_loongarch_irq,
> > + .irq_disable = disable_loongarch_irq,
> > +};
> > +
> > +asmlinkage void default_handle_irq(int irq)
> > +{
> > + do_IRQ(irq_linear_revmap(irq_domain, irq));
>
> This looks both wrong and short sighted:
>
> - irq_linear_revmap() is now another name for irq_find_mapping().
> Which means it uses a RCU read critical section. If, as I expect,
> this is just a blind copy of the MIPS code, do_IRQ() will not do
> anything with respect to irq_enter()/irq_exit(), which will result
> in something pretty bad on the exit from idle path. Lockdep will
> probably shout at you pretty loudly.
>
> - A single root interrupt controller is, in my modest experience,
> something that rarely happen. You will eventually have a variety of
> them, and you will have to join the other arches such as arm, arm64,
> riscv and csky that use CONFIG_GENERIC_IRQ_MULTI_HANDLER instead of
> following the existing MIPS model.
I try to use CONFIG_GENERIC_IRQ_MULTI_HANDLER and
set_handle_irq()/handle_arch_irq() as arm64, riscv and csky do. But I
found a problem:
The main handler (e.g., handle_arch_irq()) take only one argument
(i.e., struct pt_regs *regs) and polling all interrupts, but we want
to use vectored interrupts which take a "irq" argument (as
default_handle_irq() does) which can directly handle it.
This seems that if I want to use vectored interrupts, then I will fall
to the MIPS model.
Huacai
>
> You can solve this by:
>
> - Move over to CONFIG_GENERIC_IRQ_MULTI_HANDLER so that the interrupt
> controller can register itself with the core, rather than being
> defined at compile time.
>
> - Drop the do_IRQ() madness. Perform whenever stuff you need to do in
> the arch code *before* calling into the interrupt controller code.
>
> - Use generic_handle_irq() to call into the irq stack. It will handle
> all the irq_enter()/irq_exit() correctly. It will also avoid the
> silly double lookup of the irq_desc on interrupt handling.
>
> > +}
> > +
> > +static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
> > + irq_hw_number_t hwirq)
> > +{
> > + struct irq_chip *chip;
> > +
> > + irq_set_noprobe(irq);
> > + chip = &loongarch_cpu_irq_controller;
> > + set_vi_handler(EXCCODE_INT_START + hwirq, default_handle_irq);
>
> What is that? Yet another MIPS legacy? Why does it have to be per
> interrupt if it obviously apply to each and every root interrupt?
>
> Given that 'vi' probably stands for "vectored interrupt", why isn't
> that the irq_enable() code?
>
> > + irq_set_chip_and_handler(irq, chip, 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,
> > +};
> > +
> > +int __init loongarch_cpu_irq_init(void)
> > +{
> > + /* Mask interrupts. */
> > + clear_csr_ecfg(ECFG0_IM);
> > + clear_csr_estat(ESTATF_IP);
> > +
> > + irq_domain = irq_domain_add_simple(NULL, EXCCODE_INT_NUM,
> > + LOONGSON_CPU_IRQ_BASE, &loongarch_cpu_intc_irq_domain_ops, NULL);
>
> NAK. You still obviously have some static partitioning of the
> interrupt space, which is not acceptable for a new architecture.
>
> > +
> > + if (!irq_domain)
> > + panic("Failed to add irqdomain for LoongArch CPU");
> > +
> > + return 0;
> > +}
>
> I haven't seen much progress from the first version I reviewed. This
> is still the same antiquated, broken MIPS code, only with a different
> name.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
Huacai,
On Sat, 28 Aug 2021 11:07:16 +0100,
Huacai Chen <[email protected]> wrote:
>
> Hi, Marc,
>
> On Wed, Aug 25, 2021 at 4:40 PM Marc Zyngier <[email protected]> wrote:
> >
> > On Wed, 25 Aug 2021 07:11:50 +0100,
> > Huacai Chen <[email protected]> wrote:
> > >
> > > We are preparing to add new Loongson (based on LoongArch, not MIPS)
> >
> > You keep saying "not MIPS", and yet all I see is a blind copy of the
> > MIPS code.
> >
> > > support. This patch add LoongArch CPU interrupt controller support.
> > >
> > > Signed-off-by: Huacai Chen <[email protected]>
> > > ---
> > > drivers/irqchip/Kconfig | 10 ++++
> > > drivers/irqchip/Makefile | 1 +
> > > drivers/irqchip/irq-loongarch-cpu.c | 76 +++++++++++++++++++++++++++++
> > > 3 files changed, 87 insertions(+)
> > > create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
> > >
> > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > > index 084bc4c2eebd..443c3a7a0cc1 100644
> > > --- a/drivers/irqchip/Kconfig
> > > +++ b/drivers/irqchip/Kconfig
> > > @@ -528,6 +528,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 f88cbf36a9d2..4e34eebe180b 100644
> > > --- a/drivers/irqchip/Makefile
> > > +++ b/drivers/irqchip/Makefile
> > > @@ -105,6 +105,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> > > obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> > > obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> > > obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> > > +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> > > 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 000000000000..8e9e8d39cb22
> > > --- /dev/null
> > > +++ b/drivers/irqchip/irq-loongarch-cpu.c
> > > @@ -0,0 +1,76 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2020-2021 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>
> > > +
> > > +static struct irq_domain *irq_domain;
> > > +
> > > +static inline void enable_loongarch_irq(struct irq_data *d)
> >
> > Why 'inline' given that it is used as a function pointer?
> >
> > > +{
> > > + set_csr_ecfg(ECFGF(d->hwirq));
> > > +}
> > > +
> > > +#define eoi_loongarch_irq enable_loongarch_irq
> >
> > NAK. EOI and enable cannot be the same operation.
> >
> > > +
> > > +static inline void disable_loongarch_irq(struct irq_data *d)
> > > +{
> > > + clear_csr_ecfg(ECFGF(d->hwirq));
> > > +}
> > > +
> > > +#define ack_loongarch_irq disable_loongarch_irq
> >
> > Same thing. Either you have different operations, or this only
> > supports mask/unmask.
> >
> > > +
> > > +static struct irq_chip loongarch_cpu_irq_controller = {
> > > + .name = "LoongArch",
> > > + .irq_ack = ack_loongarch_irq,
> > > + .irq_eoi = eoi_loongarch_irq,
> > > + .irq_enable = enable_loongarch_irq,
> > > + .irq_disable = disable_loongarch_irq,
> > > +};
> > > +
> > > +asmlinkage void default_handle_irq(int irq)
> > > +{
> > > + do_IRQ(irq_linear_revmap(irq_domain, irq));
> >
> > This looks both wrong and short sighted:
> >
> > - irq_linear_revmap() is now another name for irq_find_mapping().
> > Which means it uses a RCU read critical section. If, as I expect,
> > this is just a blind copy of the MIPS code, do_IRQ() will not do
> > anything with respect to irq_enter()/irq_exit(), which will result
> > in something pretty bad on the exit from idle path. Lockdep will
> > probably shout at you pretty loudly.
> >
> > - A single root interrupt controller is, in my modest experience,
> > something that rarely happen. You will eventually have a variety of
> > them, and you will have to join the other arches such as arm, arm64,
> > riscv and csky that use CONFIG_GENERIC_IRQ_MULTI_HANDLER instead of
> > following the existing MIPS model.
> I try to use CONFIG_GENERIC_IRQ_MULTI_HANDLER and
> set_handle_irq()/handle_arch_irq() as arm64, riscv and csky do. But I
> found a problem:
> The main handler (e.g., handle_arch_irq()) take only one argument
> (i.e., struct pt_regs *regs) and polling all interrupts, but we want
> to use vectored interrupts which take a "irq" argument (as
> default_handle_irq() does) which can directly handle it.
Are you saying that there is no way for the interrupt controller
driver to figure out the hwirq number on its own? That would seem
pretty odd (even the MIPS GIC has that). Worse case, you can provide
an arch-specific helper that exposes the current hwirq based on the
vector that triggered.
do_IRQ() is a terrible abstraction, and only outlines that your arch
code is badly structured. What does the arch code have to do with a
Linux irq number? It shouldn't care at all, because as a value it has
no significance to the arch code at all. You just go back there
because the management of your interrupt context is upside down, and
it really shouldn't matter *what interrupt fired*.
> This seems that if I want to use vectored interrupts, then I will fall
> to the MIPS model.
Not happening, I'm afraid.
M.
--
Without deviation from the norm, progress is not possible.
Hi, Marc,
On Sat, Aug 28, 2021 at 7:07 PM Marc Zyngier <[email protected]> wrote:
>
> Huacai,
>
> On Sat, 28 Aug 2021 11:07:16 +0100,
> Huacai Chen <[email protected]> wrote:
> >
> > Hi, Marc,
> >
> > On Wed, Aug 25, 2021 at 4:40 PM Marc Zyngier <[email protected]> wrote:
> > >
> > > On Wed, 25 Aug 2021 07:11:50 +0100,
> > > Huacai Chen <[email protected]> wrote:
> > > >
> > > > We are preparing to add new Loongson (based on LoongArch, not MIPS)
> > >
> > > You keep saying "not MIPS", and yet all I see is a blind copy of the
> > > MIPS code.
> > >
> > > > support. This patch add LoongArch CPU interrupt controller support.
> > > >
> > > > Signed-off-by: Huacai Chen <[email protected]>
> > > > ---
> > > > drivers/irqchip/Kconfig | 10 ++++
> > > > drivers/irqchip/Makefile | 1 +
> > > > drivers/irqchip/irq-loongarch-cpu.c | 76 +++++++++++++++++++++++++++++
> > > > 3 files changed, 87 insertions(+)
> > > > create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
> > > >
> > > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > > > index 084bc4c2eebd..443c3a7a0cc1 100644
> > > > --- a/drivers/irqchip/Kconfig
> > > > +++ b/drivers/irqchip/Kconfig
> > > > @@ -528,6 +528,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 f88cbf36a9d2..4e34eebe180b 100644
> > > > --- a/drivers/irqchip/Makefile
> > > > +++ b/drivers/irqchip/Makefile
> > > > @@ -105,6 +105,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> > > > obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> > > > obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> > > > obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> > > > +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> > > > 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 000000000000..8e9e8d39cb22
> > > > --- /dev/null
> > > > +++ b/drivers/irqchip/irq-loongarch-cpu.c
> > > > @@ -0,0 +1,76 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2020-2021 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>
> > > > +
> > > > +static struct irq_domain *irq_domain;
> > > > +
> > > > +static inline void enable_loongarch_irq(struct irq_data *d)
> > >
> > > Why 'inline' given that it is used as a function pointer?
> > >
> > > > +{
> > > > + set_csr_ecfg(ECFGF(d->hwirq));
> > > > +}
> > > > +
> > > > +#define eoi_loongarch_irq enable_loongarch_irq
> > >
> > > NAK. EOI and enable cannot be the same operation.
> > >
> > > > +
> > > > +static inline void disable_loongarch_irq(struct irq_data *d)
> > > > +{
> > > > + clear_csr_ecfg(ECFGF(d->hwirq));
> > > > +}
> > > > +
> > > > +#define ack_loongarch_irq disable_loongarch_irq
> > >
> > > Same thing. Either you have different operations, or this only
> > > supports mask/unmask.
> > >
> > > > +
> > > > +static struct irq_chip loongarch_cpu_irq_controller = {
> > > > + .name = "LoongArch",
> > > > + .irq_ack = ack_loongarch_irq,
> > > > + .irq_eoi = eoi_loongarch_irq,
> > > > + .irq_enable = enable_loongarch_irq,
> > > > + .irq_disable = disable_loongarch_irq,
> > > > +};
> > > > +
> > > > +asmlinkage void default_handle_irq(int irq)
> > > > +{
> > > > + do_IRQ(irq_linear_revmap(irq_domain, irq));
> > >
> > > This looks both wrong and short sighted:
> > >
> > > - irq_linear_revmap() is now another name for irq_find_mapping().
> > > Which means it uses a RCU read critical section. If, as I expect,
> > > this is just a blind copy of the MIPS code, do_IRQ() will not do
> > > anything with respect to irq_enter()/irq_exit(), which will result
> > > in something pretty bad on the exit from idle path. Lockdep will
> > > probably shout at you pretty loudly.
> > >
> > > - A single root interrupt controller is, in my modest experience,
> > > something that rarely happen. You will eventually have a variety of
> > > them, and you will have to join the other arches such as arm, arm64,
> > > riscv and csky that use CONFIG_GENERIC_IRQ_MULTI_HANDLER instead of
> > > following the existing MIPS model.
> > I try to use CONFIG_GENERIC_IRQ_MULTI_HANDLER and
> > set_handle_irq()/handle_arch_irq() as arm64, riscv and csky do. But I
> > found a problem:
> > The main handler (e.g., handle_arch_irq()) take only one argument
> > (i.e., struct pt_regs *regs) and polling all interrupts, but we want
> > to use vectored interrupts which take a "irq" argument (as
> > default_handle_irq() does) which can directly handle it.
>
> Are you saying that there is no way for the interrupt controller
> driver to figure out the hwirq number on its own? That would seem
> pretty odd (even the MIPS GIC has that). Worse case, you can provide
> an arch-specific helper that exposes the current hwirq based on the
> vector that triggered.
We can get the hwirq number by reading CSR.ESTAT register, but in this
way "vectored interrupts" is meaningless.
Huacai
>
> do_IRQ() is a terrible abstraction, and only outlines that your arch
> code is badly structured. What does the arch code have to do with a
> Linux irq number? It shouldn't care at all, because as a value it has
> no significance to the arch code at all. You just go back there
> because the management of your interrupt context is upside down, and
> it really shouldn't matter *what interrupt fired*.
>
> > This seems that if I want to use vectored interrupts, then I will fall
> > to the MIPS model.
>
> Not happening, I'm afraid.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
On Sun, 29 Aug 2021 10:37:48 +0100,
Huacai Chen <[email protected]> wrote:
> > Are you saying that there is no way for the interrupt controller
> > driver to figure out the hwirq number on its own? That would seem
> > pretty odd (even the MIPS GIC has that). Worse case, you can provide
> > an arch-specific helper that exposes the current hwirq based on the
> > vector that triggered.
> We can get the hwirq number by reading CSR.ESTAT register, but in this
> way "vectored interrupts" is meaningless.
Let's face it, the way you use vectored interrupts makes zero sense
already. The whole point of vectored interrupts is that the CPU can
branch to the handler directly, making the interrupt handling cheaper
as there should be no additional decoding and you can run the final
handler immediately. Here, all your interrupts point to the same
"default handler"...
What do vectored interrupts bring? "Absolutely Nothing! (say it again!)"
M.
--
Without deviation from the norm, progress is not possible.
Hi, Marc,
On Sun, Aug 29, 2021 at 6:10 PM Marc Zyngier <[email protected]> wrote:
>
> On Sun, 29 Aug 2021 10:37:48 +0100,
> Huacai Chen <[email protected]> wrote:
>
> > > Are you saying that there is no way for the interrupt controller
> > > driver to figure out the hwirq number on its own? That would seem
> > > pretty odd (even the MIPS GIC has that). Worse case, you can provide
> > > an arch-specific helper that exposes the current hwirq based on the
> > > vector that triggered.
> > We can get the hwirq number by reading CSR.ESTAT register, but in this
> > way "vectored interrupts" is meaningless.
>
> Let's face it, the way you use vectored interrupts makes zero sense
> already. The whole point of vectored interrupts is that the CPU can
> branch to the handler directly, making the interrupt handling cheaper
> as there should be no additional decoding and you can run the final
> handler immediately. Here, all your interrupts point to the same
> "default handler"...
The default handler can be overridden by arch code.
Huacai
>
> What do vectored interrupts bring? "Absolutely Nothing! (say it again!)"
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
On Sun, 29 Aug 2021 11:34:21 +0100,
Huacai Chen <[email protected]> wrote:
>
> Hi, Marc,
>
> On Sun, Aug 29, 2021 at 6:10 PM Marc Zyngier <[email protected]> wrote:
> >
> > On Sun, 29 Aug 2021 10:37:48 +0100,
> > Huacai Chen <[email protected]> wrote:
> >
> > > > Are you saying that there is no way for the interrupt controller
> > > > driver to figure out the hwirq number on its own? That would seem
> > > > pretty odd (even the MIPS GIC has that). Worse case, you can provide
> > > > an arch-specific helper that exposes the current hwirq based on the
> > > > vector that triggered.
> > > We can get the hwirq number by reading CSR.ESTAT register, but in this
> > > way "vectored interrupts" is meaningless.
> >
> > Let's face it, the way you use vectored interrupts makes zero sense
> > already. The whole point of vectored interrupts is that the CPU can
> > branch to the handler directly, making the interrupt handling cheaper
> > as there should be no additional decoding and you can run the final
> > handler immediately. Here, all your interrupts point to the same
> > "default handler"...
> The default handler can be overridden by arch code.
How? Do you plan to bypass the whole of the Linux interrupt stack and
jump straight to the function provided by a driver via request_irq()?
Because that's the *only* way for vectored interrupts to make any
difference. They otherwise are an antiquated leftover from a time when
shaving every single instructions was an absolute requirement.
Vectored interrupts also tend to confuse vectors and priorities (yet
another bad move).
So let's be serious, the whole vectored interrupts is utter rubbish,
and you haven't given *any* argument as to why you can't make your
interrupt handling behave sanely and be maintainable.
Anyhow, we have both wasted enough time on this. I have suggested a
number of ways you can rework your interrupt handling to be more
acceptable. You can take or leave my suggestions, but I have no
intention to give my blessing to patches that have the current level
of quality.
M.
--
Without deviation from the norm, progress is not possible.
Hi, Marc,
On Sun, Aug 29, 2021 at 7:01 PM Marc Zyngier <[email protected]> wrote:
>
> On Sun, 29 Aug 2021 11:34:21 +0100,
> Huacai Chen <[email protected]> wrote:
> >
> > Hi, Marc,
> >
> > On Sun, Aug 29, 2021 at 6:10 PM Marc Zyngier <[email protected]> wrote:
> > >
> > > On Sun, 29 Aug 2021 10:37:48 +0100,
> > > Huacai Chen <[email protected]> wrote:
> > >
> > > > > Are you saying that there is no way for the interrupt controller
> > > > > driver to figure out the hwirq number on its own? That would seem
> > > > > pretty odd (even the MIPS GIC has that). Worse case, you can provide
> > > > > an arch-specific helper that exposes the current hwirq based on the
> > > > > vector that triggered.
> > > > We can get the hwirq number by reading CSR.ESTAT register, but in this
> > > > way "vectored interrupts" is meaningless.
> > >
> > > Let's face it, the way you use vectored interrupts makes zero sense
> > > already. The whole point of vectored interrupts is that the CPU can
> > > branch to the handler directly, making the interrupt handling cheaper
> > > as there should be no additional decoding and you can run the final
> > > handler immediately. Here, all your interrupts point to the same
> > > "default handler"...
> > The default handler can be overridden by arch code.
>
> How? Do you plan to bypass the whole of the Linux interrupt stack and
> jump straight to the function provided by a driver via request_irq()?
>
> Because that's the *only* way for vectored interrupts to make any
> difference. They otherwise are an antiquated leftover from a time when
> shaving every single instructions was an absolute requirement.
> Vectored interrupts also tend to confuse vectors and priorities (yet
> another bad move).
>
> So let's be serious, the whole vectored interrupts is utter rubbish,
> and you haven't given *any* argument as to why you can't make your
> interrupt handling behave sanely and be maintainable.
>
> Anyhow, we have both wasted enough time on this. I have suggested a
> number of ways you can rework your interrupt handling to be more
> acceptable. You can take or leave my suggestions, but I have no
> intention to give my blessing to patches that have the current level
> of quality.
OK, now I know that vectored interrupts make no sense in the current
linux kernel irq framework. I will take all your suggestions, thanks
for your patience in teaching me so much!
Huacai
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.