On Wed, 22 Apr 2020 15:24:21 +0100,
Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote:
>
> This controller appears on Loongson-3 chips for receiving interrupt
> vectors from PCH's PIC and PCH's PCIe MSI interrupts. It usually act
> as the top of irq hierarchy.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>
> ---
> drivers/irqchip/Kconfig | 8 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-loongson-htvec.c | 217 +++++++++++++++++++++++++++
> 3 files changed, 226 insertions(+)
> create mode 100644 drivers/irqchip/irq-loongson-htvec.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index a85aada04a64..de4564e2ea88 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -532,4 +532,12 @@ config LOONGSON_HTPIC
> help
> Support for the Loongson-3 HyperTransport PIC Controller.
>
> +config LOONGSON_HTVEC
> + bool "Loongson3 HyperTransport Interrupt Vector Controller"
> + depends on MACH_LOONGSON64 || COMPILE_TEST
> + default MACH_LOONGSON64
> + select IRQ_DOMAIN_HIERARCHY
> + help
> + Support for the Loongson3 HyperTransport Interrupt Vector Controller.
> +
> endmenu
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 37bbe39bf909..74561879f5a7 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -107,3 +107,4 @@ 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
> +obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
> diff --git a/drivers/irqchip/irq-loongson-htvec.c b/drivers/irqchip/irq-loongson-htvec.c
> new file mode 100644
> index 000000000000..e155ebb99efb
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongson-htvec.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020, Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>
> + * Loongson HyperTransport Interrupt Vector support
> + */
> +
> +#define pr_fmt(fmt) "htvec: " 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>
> +
> +/* Registers */
> +#define HTVEC_EN_OFF 0x20
> +#define HTVEC_MAX_PARENT_IRQ 4
> +
> +#define VEC_COUNT_PER_REG 32
> +#define VEC_REG_COUNT 4
> +#define VEC_COUNT (VEC_COUNT_PER_REG * VEC_REG_COUNT)
> +#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 htvec {
> + void __iomem *base;
> + struct irq_domain *htvec_domain;
> + raw_spinlock_t htvec_lock;
nit: please align member of the structure vertically:
struct htvec {
void __iomem *base;
struct irq_domain *htvec_d
raw_spinlock_t htvec_lock;
};
> +};
> +
> +static void htvec_irq_dispatch(struct irq_desc *desc)
> +{
> + struct htvec *priv = irq_desc_get_handler_data(desc);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + uint32_t pending;
In the kernel, please use u32. uint32_t is reserved for uapi code.
> + bool handled = false;
> + int i;
> +
> + chained_irq_enter(chip, desc);
If this is a chained interrupt controller, it isn't the top-level
interrupt controller, as it obviously feed into another one.
> +
> + for (i = 0; i < VEC_REG_COUNT; i++) {
> + pending = readl(priv->base + 4 * i);
> + /* Ack all IRQs at once, otherwise IRQ flood might happen */
> + writel(pending, priv->base + 4 * i);
> + while (pending) {
> + int bit = __ffs(pending);
> +
> + generic_handle_irq(irq_linear_revmap(priv->htvec_domain,
> + bit + 32 * i));
Isn't this 32 actually VEC_COUNT_PER_REG?
> + pending &= ~BIT(bit);
> + handled = true;
> + }
> + }
> +
> + if (!handled)
> + spurious_interrupt();
> +
> + chained_irq_exit(chip, desc);
> +}
> +
> +static void htvec_bitset(void __iomem *addr, int bit)
> +{
> + u32 reg;
> +
> + addr += VEC_REG_IDX(bit) * 4;
> + reg = readl(addr);
> + reg |= BIT(VEC_REG_BIT(bit));
> + writel(reg, addr);
> +}
> +
> +static void htvec_bitclr(void __iomem *addr, int bit)
> +{
> + u32 reg;
> +
> + addr += VEC_REG_IDX(bit) * 4;
> + reg = readl(addr);
> + reg &= ~BIT(VEC_REG_BIT(bit));
> + writel(reg, addr);
> +}
Given that these two functions have only a single call site, please
move them into their respective caller. At least we won't have to
worry about the locking.
> +
> +static void htvec_mask_irq(struct irq_data *d)
> +{
> + struct htvec *priv = irq_data_get_irq_chip_data(d);
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&priv->htvec_lock, flags);
> + htvec_bitclr(priv->base + HTVEC_EN_OFF, d->hwirq);
> + raw_spin_unlock_irqrestore(&priv->htvec_lock, flags);
> +}
> +
> +static void htvec_unmask_irq(struct irq_data *d)
> +{
> + struct htvec *priv = irq_data_get_irq_chip_data(d);
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&priv->htvec_lock, flags);
> + htvec_bitset(priv->base + HTVEC_EN_OFF, d->hwirq);
> + raw_spin_unlock_irqrestore(&priv->htvec_lock, flags);
> +}
> +
> +static struct irq_chip htvec_irq_chip = {
> + .name = "LOONGSON_HTVEC",
> + .irq_mask = htvec_mask_irq,
> + .irq_unmask = htvec_unmask_irq,
> +};
> +
> +static int htvec_domain_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *arg)
> +{
> + struct htvec *priv = domain->host_data;
> + unsigned long hwirq;
> + unsigned int type;
> +
> + irq_domain_translate_onecell(domain, arg, &hwirq, &type);
> +
> + /* Not much to do, just setup the irqdata */
> + irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> + &htvec_irq_chip, priv);
What sets the flow handler?
Another thing is that you ignore the "nr_irqs" parameter, while you
are handling it in the free callback. You have to be consistent.
> +
> + return 0;
> +}
> +
> +static void htvec_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 htvec_domain_ops = {
> + .translate = irq_domain_translate_onecell,
> + .alloc = htvec_domain_alloc,
> + .free = htvec_domain_free,
> +};
> +
> +static void htvec_reset(struct htvec *priv)
> +{
> + u32 idx;
> +
> + /* Clear IRQ cause registers, mask all interrupts */
> + for (idx = 0; idx < VEC_REG_COUNT; idx++) {
> + writel_relaxed(0x0, priv->base + HTVEC_EN_OFF + 4 * idx);
> + writel_relaxed(0xFFFFFFFF, priv->base);
> + }
> +}
> +
> +static int htvec_of_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + struct htvec *priv;
> + int err, parent_irq[4], num_parents = 0, i;
> +
> + priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + raw_spin_lock_init(&priv->htvec_lock);
> + priv->base = of_iomap(node, 0);
> + if (!priv->base) {
> + err = -ENOMEM;
> + goto free_priv;
> + }
> +
> + /* Interrupt may come from any of the 4 interrupt line */
> + for (i = 0; i < HTVEC_MAX_PARENT_IRQ; i++) {
> + parent_irq[i] = irq_of_parse_and_map(node, i);
> + if (parent_irq[i] <= 0)
> + break;
> +
> + num_parents++;
> + }
> +
> + if (!num_parents) {
> + pr_err("Failed to get parent irqs\n");
> + err = -ENODEV;
> + goto iounmap_base;
> + }
> +
> + priv->htvec_domain = irq_domain_create_linear(of_node_to_fwnode(node),
> + VEC_COUNT,
> + &htvec_domain_ops,
> + priv);
> + if (!priv->htvec_domain) {
> + pr_err("Failed to create IRQ domain\n");
> + err = -ENOMEM;
> + goto iounmap_base;
> + }
> +
> + htvec_reset(priv);
> +
> + for (i = 0; i < num_parents; i++) {
> + irq_set_chained_handler_and_data(parent_irq[i],
> + htvec_irq_dispatch, priv);
> + }
Useless braces.
> +
> + return 0;
> +
> +iounmap_base:
> + iounmap(priv->base);
> +free_priv:
> + kfree(priv);
> +
> + return err;
> +}
> +
> +IRQCHIP_DECLARE(htvec, "loongson,htvec-1.0", htvec_of_init);
Thanks,
M.
--
Jazz is not dead, it just smells funny.
