Messages in this thread | | | Date | Wed, 29 Jun 2022 11:58:58 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH V13 05/13] irqchip: Add Loongson PCH LPC controller support |
| |
On Mon, 27 Jun 2022 12:39:49 +0100, Jianmin Lv <lvjianmin@loongson.cn> wrote: > > From: Huacai Chen <chenhuacai@loongson.cn> > > PCH-LPC stands for "LPC Interrupts" that described in Section 24.3 of > "Loongson 7A1000 Bridge User Manual". For more information please refer > Documentation/loongarch/irq-chip-model.rst. > > Co-developed-by: Jianmin Lv <lvjianmin@loongson.cn> > Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > --- > arch/loongarch/include/asm/irq.h | 4 +- > arch/loongarch/kernel/irq.c | 1 - > drivers/irqchip/Kconfig | 8 ++ > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-loongson-pch-lpc.c | 203 +++++++++++++++++++++++++++++++++ > 5 files changed, 214 insertions(+), 3 deletions(-) > create mode 100644 drivers/irqchip/irq-loongson-pch-lpc.c > > diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h > index ace3ea6..48c0ce4 100644 > --- a/arch/loongarch/include/asm/irq.h > +++ b/arch/loongarch/include/asm/irq.h > @@ -104,7 +104,7 @@ struct irq_domain *eiointc_acpi_init(struct irq_domain *parent, > > struct irq_domain *htvec_acpi_init(struct irq_domain *parent, > struct acpi_madt_ht_pic *acpi_htvec); > -struct irq_domain *pch_lpc_acpi_init(struct irq_domain *parent, > +int pch_lpc_acpi_init(struct irq_domain *parent, > struct acpi_madt_lpc_pic *acpi_pchlpc); > struct irq_domain *pch_msi_acpi_init(struct irq_domain *parent, > struct acpi_madt_msi_pic *acpi_pchmsi); > @@ -121,7 +121,7 @@ struct irq_domain *pch_pic_acpi_init(struct irq_domain *parent, > > extern struct irq_domain *cpu_domain; > extern struct irq_domain *liointc_domain; > -extern struct irq_domain *pch_lpc_domain; > +extern struct fwnode_handle *pch_lpc_handle; > extern struct irq_domain *pch_msi_domain[MAX_IO_PICS]; > extern struct irq_domain *pch_pic_domain[MAX_IO_PICS]; > > diff --git a/arch/loongarch/kernel/irq.c b/arch/loongarch/kernel/irq.c > index b34b8d7..07d6059 100644 > --- a/arch/loongarch/kernel/irq.c > +++ b/arch/loongarch/kernel/irq.c > @@ -27,7 +27,6 @@ > > struct irq_domain *cpu_domain; > struct irq_domain *liointc_domain; > -struct irq_domain *pch_lpc_domain; > struct irq_domain *pch_msi_domain[MAX_IO_PICS]; > struct irq_domain *pch_pic_domain[MAX_IO_PICS]; > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index 1f23a6b..c1d527f 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -591,6 +591,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 && LOONGARCH) > + 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 5b67450..242b8b3 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -108,6 +108,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 0000000..7e4d89a > --- /dev/null > +++ b/drivers/irqchip/irq-loongson-pch-lpc.c > @@ -0,0 +1,203 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Loongson LPC Interrupt Controller support > + * > + * Copyright (C) 2020-2022 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> > + > +/* 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; > + raw_spinlock_t lpc_lock; > + u32 saved_reg_ctl; > + u32 saved_reg_ena; > + u32 saved_reg_pol; > +}; > + > +struct fwnode_handle *pch_lpc_handle; > + > +static void ack_lpc_irq(struct irq_data *d) > +{ > + unsigned long flags; > + struct pch_lpc *priv = d->domain->host_data; > + > + raw_spin_lock_irqsave(&priv->lpc_lock, flags); > + writel(0x1 << d->hwirq, priv->base + LPC_INT_CLR); > + raw_spin_unlock_irqrestore(&priv->lpc_lock, flags); > +} > +static void mask_lpc_irq(struct irq_data *d) > +{ > + unsigned long flags; > + struct pch_lpc *priv = d->domain->host_data; > + > + raw_spin_lock_irqsave(&priv->lpc_lock, flags); > + writel(readl(priv->base + LPC_INT_ENA) & (~(0x1 << (d->hwirq))), > + priv->base + LPC_INT_ENA); > + raw_spin_unlock_irqrestore(&priv->lpc_lock, flags); > +} > + > +static void unmask_lpc_irq(struct irq_data *d) > +{ > + unsigned long flags; > + struct pch_lpc *priv = d->domain->host_data; > + > + raw_spin_lock_irqsave(&priv->lpc_lock, flags); > + writel(readl(priv->base + LPC_INT_ENA) | (0x1 << (d->hwirq)), > + priv->base + LPC_INT_ENA); > + raw_spin_unlock_irqrestore(&priv->lpc_lock, flags); > +} > + > +static int lpc_set_type(struct irq_data *d, unsigned int type) > +{ > + u32 val; > + u32 mask = 0x1 << (d->hwirq); > + struct pch_lpc *priv = d->domain->host_data; > + > + if (!(type & IRQ_TYPE_LEVEL_MASK)) > + return 0; > + > + val = readl(priv->base + LPC_INT_POL); > + > + if (type == IRQ_TYPE_LEVEL_HIGH) > + val |= mask; > + else > + val &= ~mask; > + > + writel(val, priv->base + LPC_INT_POL); > + > + return 0; > +} > + > +static struct irq_chip pch_lpc_irq_chip = {
Make this const.
> + .name = "PCH LPC", > + .irq_mask = mask_lpc_irq, > + .irq_unmask = unmask_lpc_irq, > + .irq_ack = ack_lpc_irq, > + .irq_set_type = lpc_set_type,
nit: please use the same format for all callbacks: lpc_irq_OPERATION (lpc_irq_mask, lpc_irq_set_type...), which makes it much easier to read.
> + .flags = IRQCHIP_SKIP_SET_WAKE, > +}; > + > +static void lpc_irq_dispatch(struct irq_desc *desc) > +{ > + u32 pending, bit, virq; > + struct irq_chip *chip = irq_desc_get_chip(desc); > + struct pch_lpc *priv = irq_desc_get_handler_data(desc); > + > + chained_irq_enter(chip, desc); > + > + pending = readl(priv->base + LPC_INT_ENA); > + pending &= readl(priv->base + LPC_INT_STS); > + if (!pending) > + spurious_interrupt(); > + > + while (pending) { > + bit = __ffs(pending); > + virq = irq_linear_revmap(priv->lpc_domain, bit); > + > + generic_handle_irq(bit);
Replace the whole thing with a call to generic_handle_domain_irq(). No new code should ever have to use generic_handle_irq().
> + 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, > + .translate = irq_domain_translate_twocell, > +}; > + > +static void pch_lpc_reset(struct pch_lpc *priv) > +{ > + /* Enable the LPC interrupt, bit31: en bit30: edge */ > + writel(0x80000000, priv->base + LPC_INT_CTL);
Please don't hardcode numbers. Add the proper bit definitions.
> + writel(0, priv->base + LPC_INT_ENA); > + /* Clear all 18-bit interrpt bit */ > + writel(0x3ffff, priv->base + LPC_INT_CLR);
If that's just a mask, then use GENMASK(17, 0) instead.
> +} > + > +static int pch_lpc_disabled(struct pch_lpc *priv) > +{ > + return (readl(priv->base + LPC_INT_ENA) == 0xffffffff) && > + (readl(priv->base + LPC_INT_STS) == 0xffffffff); > +} > + > +int __init pch_lpc_acpi_init(struct irq_domain *parent, > + struct acpi_madt_lpc_pic *acpi_pchlpc) > +{ > + int parent_irq; > + struct pch_lpc *priv; > + struct irq_fwspec fwspec; > + struct fwnode_handle *irq_handle; > + > + if (!acpi_pchlpc) > + return -EINVAL; > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + 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; > + } > + > + irq_handle = irq_domain_alloc_named_fwnode("lpcintc"); > + if (!irq_handle) { > + pr_err("Unable to allocate domain handle\n"); > + goto iounmap_base; > + } > + > + priv->lpc_domain = irq_domain_create_linear(irq_handle, LPC_COUNT, > + &pch_lpc_domain_ops, priv); > + if (!priv->lpc_domain) { > + pr_err("Failed to create IRQ domain\n"); > + goto iounmap_base;
You also need to free the fwnode.
> + } > + pch_lpc_reset(priv); > + > + fwspec.fwnode = parent->fwnode; > + fwspec.param[0] = 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_handle = irq_handle; > + return 0; > + > +iounmap_base: > + iounmap(priv->base); > +free_priv: > + kfree(priv); > + > + return -ENOMEM; > +} > -- > 1.8.3.1 > >
Thanks,
M.
-- Without deviation from the norm, progress is not possible.
| |