Re: [PATCH V5 08/10] irqchip: Add LoongArch CPU interrupt controller support

[Marc Zyngier]

On Thu, 16 Sep 2021 13:31:36 +0100,
Huacai Chen <chenhuacai@xxxxxxxxxxx> wrote:
> 
> We are preparing to add new Loongson (based on LoongArch, not compatible
> with old MIPS-based Loongson) support. This patch add the LoongArch CPU
> interrupt controller support.
> 
> Signed-off-by: Huacai Chen <chenhuacai@xxxxxxxxxxx>
> ---
>  drivers/irqchip/Kconfig             | 10 ++++
>  drivers/irqchip/Makefile            |  1 +
>  drivers/irqchip/irq-loongarch-cpu.c | 89 +++++++++++++++++++++++++++++
>  3 files changed, 100 insertions(+)
>  create mode 100644 drivers/irqchip/irq-loongarch-cpu.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 084bc4c2eebd..443c3a7a0cc1 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -528,6 +528,16 @@ config EXYNOS_IRQ_COMBINER
>  	  Say yes here to add support for the IRQ combiner devices embedded
>  	  in Samsung Exynos chips.
>  
> +config IRQ_LOONGARCH_CPU
> +	bool
> +	select GENERIC_IRQ_CHIP
> +	select IRQ_DOMAIN
> +	select GENERIC_IRQ_EFFECTIVE_AFF_MASK
> +	help
> +	  Support for the LoongArch CPU Interrupt Controller. For details of
> +	  irq chip hierarchy on LoongArch platforms please read the document
> +	  Documentation/loongarch/irq-chip-model.rst.
> +
>  config LOONGSON_LIOINTC
>  	bool "Loongson Local I/O Interrupt Controller"
>  	depends on MACH_LOONGSON64
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index f88cbf36a9d2..4e34eebe180b 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -105,6 +105,7 @@ 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_TI_PRUSS_INTC)		+= irq-pruss-intc.o
> +obj-$(CONFIG_IRQ_LOONGARCH_CPU)		+= irq-loongarch-cpu.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-loongarch-cpu.c b/drivers/irqchip/irq-loongarch-cpu.c
> new file mode 100644
> index 000000000000..bc15e3cefbd8
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongarch-cpu.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020-2021 Loongson Technology Corporation Limited
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +
> +#include <asm/loongarch.h>
> +#include <asm/setup.h>
> +
> +static struct irq_domain *irq_domain;
> +
> +static void mask_loongarch_irq(struct irq_data *d)
> +{
> +	clear_csr_ecfg(ECFGF(d->hwirq));

Where is this function defined?

> +}
> +
> +static void unmask_loongarch_irq(struct irq_data *d)
> +{
> +	set_csr_ecfg(ECFGF(d->hwirq));
> +}
> +
> +static struct irq_chip cpu_irq_controller = {
> +	.name		= "LoongArch",
> +	.irq_mask	= mask_loongarch_irq,
> +	.irq_unmask	= unmask_loongarch_irq,
> +};
> +
> +static void handle_cpu_irq(struct pt_regs *regs)
> +{
> +	int hwirq;
> +	unsigned int estat = read_csr_estat() & CSR_ESTAT_IS;
> +
> +	while ((hwirq = ffs(estat))) {
> +		estat &= ~BIT(hwirq - 1);
> +		handle_domain_irq(irq_domain, hwirq - 1, regs);
> +	}
> +}
> +
> +int get_ipi_irq(void)
> +{
> +	return irq_create_mapping(irq_domain, EXCCODE_IPI - EXCCODE_INT_START);
> +}
> +
> +int get_pmc_irq(void)
> +{
> +	return irq_create_mapping(irq_domain, EXCCODE_PMC - EXCCODE_INT_START);
> +}
> +
> +int get_timer_irq(void)
> +{
> +	return irq_create_mapping(irq_domain, EXCCODE_TIMER - EXCCODE_INT_START);
> +}

How are these functions used? Why aren't the mappings created using
the normal DT or ACPI flows? Where are these macros?

> +
> +static int loongarch_cpu_intc_map(struct irq_domain *d, unsigned int irq,
> +			     irq_hw_number_t hwirq)
> +{
> +	irq_set_noprobe(irq);
> +	irq_set_chip_and_handler(irq, &cpu_irq_controller, handle_percpu_irq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops loongarch_cpu_intc_irq_domain_ops = {
> +	.map = loongarch_cpu_intc_map,
> +	.xlate = irq_domain_xlate_onecell,
> +};
> +
> +struct irq_domain * __init loongarch_cpu_irq_init(void)
> +{
> +	/* Mask interrupts. */
> +	clear_csr_ecfg(ECFG0_IM);
> +	clear_csr_estat(ESTATF_IP);
> +
> +	irq_domain = irq_domain_add_simple(NULL, EXCCODE_INT_NUM,

This needs to be attached to a firmware node of some sort.

> +		     0, &loongarch_cpu_intc_irq_domain_ops, NULL);
> +
> +	if (!irq_domain)
> +		panic("Failed to add irqdomain for LoongArch CPU");
> +
> +	set_handle_irq(&handle_cpu_irq);
> +
> +	return irq_domain;

Who needs to know anything about this irq domain?

> +}

Given that this refers to all sort of symbols that are not included
here and that there is no reference to another patch series containing
the missing code, I'm not sure where you want to take this.

	M.

-- 
Without deviation from the norm, progress is not possible.