[permalink] [raw]

Subject: [PATCH 8/9] irqchip: Add Loongson Extended I/O interrupt controller support

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 | 308 +++++++++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
4 files changed, 319 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..e1590a128284
--- /dev/null
+++ b/drivers/irqchip/irq-loongson-eiointc.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020, Jianmin Lv <[email protected]>
+ * Loongson Extend I/O Interrupt Vector support
+ */
+
+#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);
+
+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 void eiointc_resume(void)
+{
+ int i;
+ struct irq_desc *desc;
+
+ /* 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)
+ eiointc_set_irq_affinity(&desc->irq_data,
+ desc->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 *)priv);
+ 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 47e13582d9fc..45244c1ded6a 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

2021-07-06 03:13:47

by Huacai Chen

[permalink] [raw]

Subject: [PATCH 3/9] irqchip/loongson-pch-pic: Add ACPI init support

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-pic.c | 120 ++++++++++++++++++++++++-
1 file changed, 119 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-loongson-pch-pic.c b/drivers/irqchip/irq-loongson-pch-pic.c
index a4eb8a2181c7..62382611995b 100644
--- a/drivers/irqchip/irq-loongson-pch-pic.c
+++ b/drivers/irqchip/irq-loongson-pch-pic.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2020, Jiaxun Yang <[email protected]>
+ * Jianmin Lv <[email protected]>
+ * Huacai Chen <[email protected]>
* Loongson PCH PIC support
*/

@@ -15,6 +17,7 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>
#include <linux/of_platform.h>
+#include <linux/syscore_ops.h>

/* Registers */
#define PCH_PIC_MASK 0x20
@@ -32,14 +35,23 @@
#define PIC_COUNT (PIC_COUNT_PER_REG * PIC_REG_COUNT)
#define PIC_REG_IDX(irq_id) ((irq_id) / PIC_COUNT_PER_REG)
#define PIC_REG_BIT(irq_id) ((irq_id) % PIC_COUNT_PER_REG)
+#define PCH_PIC_SIZE 0x400
+
+static int nr_pch_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 saved_vec_en[PIC_REG_COUNT];
+ u32 saved_vec_pol[PIC_REG_COUNT];
+ u32 saved_vec_edge[PIC_REG_COUNT];
};

+struct pch_pic *pch_pic_priv[4];
+
static void pch_pic_bitset(struct pch_pic *priv, int offset, int bit)
{
u32 reg;
@@ -137,6 +149,7 @@ static struct irq_chip pch_pic_irq_chip = {
.irq_ack = pch_pic_ack_irq,
.irq_set_affinity = irq_chip_set_affinity_parent,
.irq_set_type = pch_pic_set_type,
+ .flags = IRQCHIP_SKIP_SET_WAKE,
};

static int pch_pic_alloc(struct irq_domain *domain, unsigned int virq,
@@ -180,7 +193,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,6 +211,48 @@ static void pch_pic_reset(struct pch_pic *priv)
}
}

+static int pch_pic_suspend(void)
+{
+ int i, j;
+
+ for (i = 0; i < nr_pch_pics; i++) {
+ for (j = 0; j < PIC_REG_COUNT; j++) {
+ pch_pic_priv[i]->saved_vec_pol[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_POL + 4 * j);
+ pch_pic_priv[i]->saved_vec_edge[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_EDGE + 4 * j);
+ pch_pic_priv[i]->saved_vec_en[j] =
+ readl(pch_pic_priv[i]->base + PCH_PIC_MASK + 4 * j);
+ }
+ }
+
+ return 0;
+}
+
+static void pch_pic_resume(void)
+{
+ int i, j;
+
+ for (i = 0; i < nr_pch_pics; i++) {
+ pch_pic_reset(pch_pic_priv[i]);
+ for (j = 0; j < PIC_REG_COUNT; j++) {
+ writel(pch_pic_priv[i]->saved_vec_pol[j],
+ pch_pic_priv[i]->base + PCH_PIC_POL + 4 * j);
+ writel(pch_pic_priv[i]->saved_vec_edge[j],
+ pch_pic_priv[i]->base + PCH_PIC_EDGE + 4 * j);
+ writel(pch_pic_priv[i]->saved_vec_en[j],
+ pch_pic_priv[i]->base + PCH_PIC_MASK + 4 * j);
+ }
+ }
+}
+
+static struct syscore_ops pch_pic_syscore_ops = {
+ .suspend = pch_pic_suspend,
+ .resume = pch_pic_resume,
+};
+
+#ifdef CONFIG_OF
+
static int pch_pic_of_init(struct device_node *node,
struct device_node *parent)
{
@@ -242,6 +297,9 @@ static int pch_pic_of_init(struct device_node *node,
}

pch_pic_reset(priv);
+ pch_pic_priv[0] = priv;
+
+ register_syscore_ops(&pch_pic_syscore_ops);

return 0;

@@ -254,3 +312,63 @@ static int pch_pic_of_init(struct device_node *node,
}

IRQCHIP_DECLARE(pch_pic, "loongson,pch-pic-1.0", pch_pic_of_init);
+
+#endif
+
+#ifdef CONFIG_ACPI
+
+struct fwnode_handle *pch_pic_acpi_init(struct fwnode_handle *parent,
+ struct acpi_madt_bio_pic *acpi_pchpic)
+{
+ int count;
+ struct pch_pic *priv;
+ struct irq_domain *parent_domain;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return NULL;
+
+ raw_spin_lock_init(&priv->pic_lock);
+ priv->base = ioremap(acpi_pchpic->address, acpi_pchpic->size);
+ if (!priv->base)
+ goto free_priv;
+
+ priv->domain_handle = irq_domain_alloc_fwnode(priv->base);
+ if (!priv->domain_handle) {
+ pr_err("Unable to allocate domain handle\n");
+ goto iounmap_base;
+ }
+
+ priv->ht_vec_base = acpi_pchpic->gsi_base;
+ count = ((readq(priv->base) >> 48) & 0xff) + 1;
+ parent_domain = irq_find_matching_fwnode(parent, DOMAIN_BUS_ANY);
+ if (!parent_domain) {
+ pr_err("Failed to find the parent domain\n");
+ goto iounmap_base;
+ }
+
+ priv->pic_domain = irq_domain_create_hierarchy(parent_domain, 0,
+ count, priv->domain_handle,
+ &pch_pic_domain_ops, priv);
+
+ if (!priv->pic_domain) {
+ pr_err("Failed to create IRQ domain\n");
+ goto iounmap_base;
+ }
+
+ pch_pic_reset(priv);
+ pch_pic_priv[nr_pch_pics++] = priv;
+
+ register_syscore_ops(&pch_pic_syscore_ops);
+
+ return priv->domain_handle;
+
+iounmap_base:
+ iounmap(priv->base);
+free_priv:
+ kfree(priv);
+
+ return NULL;
+}
+
+#endif
--
2.27.0

2021-07-06 03:13:49

by Huacai Chen

[permalink] [raw]

On Thu, 12 Aug 2021 13:23:27 +0100,
Huacai Chen <[email protected]> wrote:
>
[...]

> > > > +struct fwnode_handle *pch_pic_acpi_init(struct fwnode_handle *parent,
> > > > + struct acpi_madt_bio_pic *acpi_pchpic)
> > > > +{
> > > > + int count;
> > > > + struct pch_pic *priv;
> > > > + struct irq_domain *parent_domain;
> > > > +
> > > > + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > > > + if (!priv)
> > > > + return NULL;
> > > > +
> > > > + raw_spin_lock_init(&priv->pic_lock);
> > > > + priv->base = ioremap(acpi_pchpic->address, acpi_pchpic->size);
> > > > + if (!priv->base)
> > > > + goto free_priv;
> > > > +
> > > > + priv->domain_handle = irq_domain_alloc_fwnode(priv->base);
> > > > + if (!priv->domain_handle) {
> > > > + pr_err("Unable to allocate domain handle\n");
> > > > + goto iounmap_base;
> > > > + }
> > > > +
> > > > + priv->ht_vec_base = acpi_pchpic->gsi_base;
> > > > + count = ((readq(priv->base) >> 48) & 0xff) + 1;
> > > > + parent_domain = irq_find_matching_fwnode(parent, DOMAIN_BUS_ANY);
> > > > + if (!parent_domain) {
> > > > + pr_err("Failed to find the parent domain\n");
> > > > + goto iounmap_base;
> > > > + }
> > > > +
> > > > + priv->pic_domain = irq_domain_create_hierarchy(parent_domain, 0,
> > > > + count, priv->domain_handle,
> > > > + &pch_pic_domain_ops, priv);
> > > > +
> > > > + if (!priv->pic_domain) {
> > > > + pr_err("Failed to create IRQ domain\n");
> > > > + goto iounmap_base;
> > > > + }
> > > > +
> > > > + pch_pic_reset(priv);
> > > > + pch_pic_priv[nr_pch_pics++] = priv;
> > > > +
> > > > + register_syscore_ops(&pch_pic_syscore_ops);
> > > > +
> > > > + return priv->domain_handle;
> > > > +
> > > > +iounmap_base:
> > > > + iounmap(priv->base);
> > > > +free_priv:
> > > > + kfree(priv);
> > > > +
> > > > + return NULL;
> > > > +}
> > > > +
> > > > +#endif
> > >
> > > A lot of this code is common with its OF counterpart. How about making
> > > this logic common?
> > OK, let me think about.
> Though pch_pic_acpi_init() is similar to pch_pic_of_init(), but it is
> difficult to make a common function, because we cannot prepare
> everything before the common function. For example, ioremap() can be
> the common part, but pch_pic_acpi_init() should get the vector count
> after ioremap(). If we use an argument to distinguish the caller in
> the common function, the complexity increases and seems no benefits.

All firmware implementations allocate private data structures, irq
domains, map MMIO regions, etc. All that can be common. We even have
APIs that deal with both firmware interfaces.

As for the 'read the vector count from the HW', what does it have to
do with driving the HW using DT or ACPI? The HW doesn't *know*. If you
are conflating HW changes and firmware interfaces, then you have
already lost.

M.

--
Without deviation from the norm, progress is not possible.

2021-08-16 03:21:11

by Huacai Chen

[permalink] [raw]

Subject: Re: [PATCH 3/9] irqchip/loongson-pch-pic: Add ACPI init support

Hi, Marc,

On Thu, Aug 12, 2021 at 9:28 PM Marc Zyngier <[email protected]> wrote:
>
> On Thu, 12 Aug 2021 13:23:27 +0100,
> Huacai Chen <[email protected]> wrote:
> >
> [...]
>
> > > > > +struct fwnode_handle *pch_pic_acpi_init(struct fwnode_handle *parent,
> > > > > + struct acpi_madt_bio_pic *acpi_pchpic)
> > > > > +{
> > > > > + int count;
> > > > > + struct pch_pic *priv;
> > > > > + struct irq_domain *parent_domain;
> > > > > +
> > > > > + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > > > > + if (!priv)
> > > > > + return NULL;
> > > > > +
> > > > > + raw_spin_lock_init(&priv->pic_lock);
> > > > > + priv->base = ioremap(acpi_pchpic->address, acpi_pchpic->size);
> > > > > + if (!priv->base)
> > > > > + goto free_priv;
> > > > > +
> > > > > + priv->domain_handle = irq_domain_alloc_fwnode(priv->base);
> > > > > + if (!priv->domain_handle) {
> > > > > + pr_err("Unable to allocate domain handle\n");
> > > > > + goto iounmap_base;
> > > > > + }
> > > > > +
> > > > > + priv->ht_vec_base = acpi_pchpic->gsi_base;
> > > > > + count = ((readq(priv->base) >> 48) & 0xff) + 1;
> > > > > + parent_domain = irq_find_matching_fwnode(parent, DOMAIN_BUS_ANY);
> > > > > + if (!parent_domain) {
> > > > > + pr_err("Failed to find the parent domain\n");
> > > > > + goto iounmap_base;
> > > > > + }
> > > > > +
> > > > > + priv->pic_domain = irq_domain_create_hierarchy(parent_domain, 0,
> > > > > + count, priv->domain_handle,
> > > > > + &pch_pic_domain_ops, priv);
> > > > > +
> > > > > + if (!priv->pic_domain) {
> > > > > + pr_err("Failed to create IRQ domain\n");
> > > > > + goto iounmap_base;
> > > > > + }
> > > > > +
> > > > > + pch_pic_reset(priv);
> > > > > + pch_pic_priv[nr_pch_pics++] = priv;
> > > > > +
> > > > > + register_syscore_ops(&pch_pic_syscore_ops);
> > > > > +
> > > > > + return priv->domain_handle;
> > > > > +
> > > > > +iounmap_base:
> > > > > + iounmap(priv->base);
> > > > > +free_priv:
> > > > > + kfree(priv);
> > > > > +
> > > > > + return NULL;
> > > > > +}
> > > > > +
> > > > > +#endif
> > > >
> > > > A lot of this code is common with its OF counterpart. How about making
> > > > this logic common?
> > > OK, let me think about.
> > Though pch_pic_acpi_init() is similar to pch_pic_of_init(), but it is
> > difficult to make a common function, because we cannot prepare
> > everything before the common function. For example, ioremap() can be
> > the common part, but pch_pic_acpi_init() should get the vector count
> > after ioremap(). If we use an argument to distinguish the caller in
> > the common function, the complexity increases and seems no benefits.
>
> All firmware implementations allocate private data structures, irq
> domains, map MMIO regions, etc. All that can be common. We even have
> APIs that deal with both firmware interfaces.
>
> As for the 'read the vector count from the HW', what does it have to
> do with driving the HW using DT or ACPI? The HW doesn't *know*. If you
> are conflating HW changes and firmware interfaces, then you have
> already lost.
OK, I know, thanks.

Huacai
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.