On Fri, 21 Feb 2020 05:09:18 +0000, Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote: > > This controller appeared on Loongson-3 family of chips to receive interrupts > from PCH PIC. Please explain for us mere mortals what the relationship is between this interrupt controller and the i8259. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> > --- > arch/mips/include/asm/i8259.h | 1 + > drivers/irqchip/Kconfig | 10 ++ > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-i8259.c | 6 +- > drivers/irqchip/irq-loongson-htpic.c | 146 +++++++++++++++++++++++++++ > 5 files changed, 161 insertions(+), 3 deletions(-) > create mode 100644 drivers/irqchip/irq-loongson-htpic.c > > diff --git a/arch/mips/include/asm/i8259.h b/arch/mips/include/asm/i8259.h > index 97a5e41ed1ab..1ec3dbb1588f 100644 > --- a/arch/mips/include/asm/i8259.h > +++ b/arch/mips/include/asm/i8259.h > @@ -36,6 +36,7 @@ extern raw_spinlock_t i8259A_lock; > extern void make_8259A_irq(unsigned int irq); > > extern void init_i8259_irqs(void); > +extern struct irq_domain *of_init_i8259_irqs(struct device_node *node); > > /** > * i8159_set_poll() - Override the i8259 polling function > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index c609eaa319d2..cae6f480c987 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -522,4 +522,14 @@ config LOONGSON_LIOINTC > help > Support for the Loongson Local I/O Interrupt Controller. > > +config LOONGSON_HTPIC > + bool "Loongson3 HyperTransport PIC Controller" > + depends on MACH_LOONGSON64 > + default y > + select IRQ_DOMAIN > + select GENERIC_IRQ_CHIP > + select I8259 > + help > + Support for the Loongson-3 HyperTransport PIC Controller. > + > endmenu > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index 5e7678efdfe6..37bbe39bf909 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -106,3 +106,4 @@ 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_LOONGSON_LIOINTC) += irq-loongson-liointc.o > +obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o > diff --git a/drivers/irqchip/irq-i8259.c b/drivers/irqchip/irq-i8259.c > index d000870d9b6b..9d79acce6c0c 100644 > --- a/drivers/irqchip/irq-i8259.c > +++ b/drivers/irqchip/irq-i8259.c > @@ -309,7 +309,7 @@ static const struct irq_domain_ops i8259A_ops = { > * driver compatibility reasons interrupts 0 - 15 to be the i8259 > * interrupts even if the hardware uses a different interrupt numbering. > */ > -struct irq_domain * __init __init_i8259_irqs(struct device_node *node) > +struct irq_domain * __init of_init_i8259_irqs(struct device_node *node) I don't think this renaming brings much to the table. > { > struct irq_domain *domain; > > @@ -330,7 +330,7 @@ struct irq_domain * __init __init_i8259_irqs(struct device_node *node) > > void __init init_i8259_irqs(void) > { > - __init_i8259_irqs(NULL); > + of_init_i8259_irqs(NULL); > } > > static void i8259_irq_dispatch(struct irq_desc *desc) > @@ -351,7 +351,7 @@ int __init i8259_of_init(struct device_node *node, struct device_node *parent) > struct irq_domain *domain; > unsigned int parent_irq; > > - domain = __init_i8259_irqs(node); > + domain = of_init_i8259_irqs(node); > > parent_irq = irq_of_parse_and_map(node, 0); > if (!parent_irq) { > diff --git a/drivers/irqchip/irq-loongson-htpic.c b/drivers/irqchip/irq-loongson-htpic.c > new file mode 100644 > index 000000000000..a90cf4357285 > --- /dev/null > +++ b/drivers/irqchip/irq-loongson-htpic.c > @@ -0,0 +1,146 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2020, Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> > + * Loongson HTPIC IRQ support > + */ > + > +#include <linux/init.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/irqchip.h> > +#include <linux/irqchip/chained_irq.h> > +#include <linux/irq.h> > +#include <linux/io.h> > +#include <linux/syscore_ops.h> > + > +#include <asm/i8259.h> > + > +#define HTPIC_MAX_PARENT_IRQ 4 > +#define HTINT_NUM_VECTORS 8 > +#define HTINT_EN_OFF 0x20 > + > +struct loongson_htpic { > + void __iomem *base; > + struct irq_domain *domain; > +}; > + > +struct loongson_htpic *htpic; static? > + > +static void htpic_irq_dispatch(struct irq_desc *desc) > +{ > + struct loongson_htpic *priv = irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + uint32_t pending; > + > + chained_irq_enter(chip, desc); > + pending = readl(priv->base); > + /* Ack all IRQs */ > + writel(pending, priv->base); Why isn't this done as part of the irq_ack() callback instead? If the flow handling is different from the 8259, then maybe you shouldn't be hijacking it... > + > + if (!pending) > + spurious_interrupt(); > + > + while (pending) { > + int bit = __ffs(pending); > + > + if (unlikely(bit > 15)) > + spurious_interrupt(); and yet you continue handling it? > + > + generic_handle_irq(irq_linear_revmap(priv->domain, bit)); > + pending &= ~BIT(bit); > + } > + chained_irq_exit(chip, desc); > +} > + > +static void htpic_reg_init(void) > +{ > + int i; > + > + for (i = 0; i < HTINT_NUM_VECTORS; i++) { > + uint32_t val; > + > + /* Disable all HT Vectors */ > + writel(0x0, htpic->base + HTINT_EN_OFF + i * 0x4); > + val = readl(htpic->base + i * 0x4); > + /* Ack all possible pending IRQs */ > + writel(GENMASK(31, 0), htpic->base + i * 0x4); > + } > + > + /* Enable 16 vectors for PIC */ > + writel(0xffff, htpic->base + HTINT_EN_OFF); > +} > + > +static void htpic_resume(void) > +{ > + htpic_reg_init(); > +} > + > +struct syscore_ops htpic_syscore_ops = { > + .resume = htpic_resume, > +}; > + > +int __init htpic_of_init(struct device_node *node, struct device_node *parent) > +{ > + unsigned int parent_irq[4]; > + int i, err; > + int num_parents = 0; > + > + if (htpic) { > + pr_err("loongson-htpic: Only one HTPIC is allowed in the system\n"); > + return -ENODEV; > + } > + > + htpic = kzalloc(sizeof(*htpic), GFP_KERNEL); > + if (!htpic) { > + err = -ENOMEM; > + goto out_free; > + } > + > + htpic->base = of_iomap(node, 0); > + if (!htpic->base) { > + err = -ENODEV; > + goto out_free; > + } > + > + htpic->domain = of_init_i8259_irqs(node); > + if (!htpic->domain) { > + pr_err("loongson-htpic: Failed to initialize i8259 IRQs\n"); > + err = -ENOMEM; > + goto out_iounmap; > + } > + > + for (i = 0; i < HTPIC_MAX_PARENT_IRQ; i++) { > + parent_irq[i] = irq_of_parse_and_map(node, 0); So you only care about the first interrupt in the DT? > + if (parent_irq[i] < 0) 0 is an invalid interrupt. > + break; > + > + num_parents++; > + } > + > + if (!num_parents) { > + pr_err("loongson-htpic: Failed to get parent irqs\n"); > + err = -ENODEV; > + goto out_remove_domain; > + } > + > + htpic_reg_init(); > + > + for (i = 0; i < num_parents; i++) { > + irq_set_chained_handler_and_data(parent_irq[i], > + htpic_irq_dispatch, htpic); > + } What is the mapping between the 16 inputs and the 4 outputs? > + > + register_syscore_ops(&htpic_syscore_ops); > + > + return 0; > + > +out_remove_domain: > + irq_domain_remove(htpic->domain); > +out_iounmap: > + iounmap(htpic->base); > +out_free: > + kfree(htpic); > + return err; > +} > + > +IRQCHIP_DECLARE(loongson_htpic, "loongson,htpic-1.0", htpic_of_init); Thanks, M. -- Jazz is not dead, it just smells funny.