[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8636ak6wfo.wl-maz@kernel.org>
Date: Sat, 07 Mar 2020 15:01:31 +0000
From: Marc Zyngier <maz@...nel.org>
To: Jiaxun Yang <jiaxun.yang@...goat.com>
Cc: linux-mips@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
Jason Cooper <jason@...edaemon.net>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Ralf Baechle <ralf@...ux-mips.org>,
Paul Burton <paulburton@...nel.org>,
Huacai Chen <chenhc@...ote.com>,
Allison Randal <allison@...utok.net>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 03/10] irqchip: Add driver for Loongson-3 HyperTransport PIC controller
On Fri, 21 Feb 2020 05:09:18 +0000,
Jiaxun Yang <jiaxun.yang@...goat.com> 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@...goat.com>
> ---
> 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@...goat.com>
> + * 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.
Powered by blists - more mailing lists