2022-06-27 12:28:03

by 吕建民

[permalink] [raw]
Subject: [PATCH V13 05/13] irqchip: Add Loongson PCH LPC controller support

From: Huacai Chen <[email protected]>

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 <[email protected]>
Signed-off-by: Jianmin Lv <[email protected]>
Signed-off-by: Huacai Chen <[email protected]>
---
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 = {
+ .name = "PCH LPC",
+ .irq_mask = mask_lpc_irq,
+ .irq_unmask = unmask_lpc_irq,
+ .irq_ack = ack_lpc_irq,
+ .irq_set_type = lpc_set_type,
+ .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);
+ 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);
+ writel(0, priv->base + LPC_INT_ENA);
+ /* Clear all 18-bit interrpt bit */
+ writel(0x3ffff, priv->base + LPC_INT_CLR);
+}
+
+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;
+ }
+ 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


2022-06-29 11:05:35

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH V13 05/13] irqchip: Add Loongson PCH LPC controller support

On Mon, 27 Jun 2022 12:39:49 +0100,
Jianmin Lv <[email protected]> wrote:
>
> From: Huacai Chen <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Jianmin Lv <[email protected]>
> Signed-off-by: Huacai Chen <[email protected]>
> ---
> 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.

2022-06-30 05:06:01

by 吕建民

[permalink] [raw]
Subject: Re: [PATCH V13 05/13] irqchip: Add Loongson PCH LPC controller support



On 2022/6/29 下午6:58, Marc Zyngier wrote:
> On Mon, 27 Jun 2022 12:39:49 +0100,
> Jianmin Lv <[email protected]> wrote:
>>
>> From: Huacai Chen <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Jianmin Lv <[email protected]>
>> Signed-off-by: Huacai Chen <[email protected]>
>> ---
>> 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.
>

Ok, thanks.


>> + .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.
>

Ok.


>> + .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().
>

Ok, thanks, got it.


>> + 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.
>

Ok.


>> + 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.
>

Ok.


>> +}
>> +
>> +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.
>

Yes, thanks.

>> + }
>> + 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.
>