On 2020-08-03 16:03, Mark-PK Tsai wrote:
From: Marc Zyngier <maz@xxxxxxxxxx>
On 2020-08-03 07:22, Mark-PK Tsai wrote:
> Add mt58xx interrupt controller support using hierarchy irq
> domain.
>
> Signed-off-by: Mark-PK Tsai <mark-pk.tsai@xxxxxxxxxxxx>
> ---
> drivers/irqchip/Kconfig | 7 ++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-mt58xx.c | 196 +++++++++++++++++++++++++++++++++++
> 3 files changed, 204 insertions(+)
> create mode 100644 drivers/irqchip/irq-mt58xx.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 216b3b8392b5..00453af78be0 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -572,4 +572,11 @@ config LOONGSON_PCH_MSI
> help
> Support for the Loongson PCH MSI Controller.
>
> +config MT58XX_IRQ
> + bool "MT58XX IRQ"
> + select IRQ_DOMAIN
> + select IRQ_DOMAIN_HIERARCHY
> + help
> + Support Mediatek MT58XX Interrupt Controller.
> +
> endmenu
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 133f9c45744a..5062e9bfa92d 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -111,3 +111,4 @@ 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_MT58XX_IRQ) += irq-mt58xx.o
> diff --git a/drivers/irqchip/irq-mt58xx.c
> b/drivers/irqchip/irq-mt58xx.c
> new file mode 100644
> index 000000000000..e45ad023afa6
> --- /dev/null
> +++ b/drivers/irqchip/irq-mt58xx.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
> + * Author Mark-PK Tsai <mark-pk.tsai@xxxxxxxxxxxx>
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>
> +
> +#define INTC_MASK 0x0
> +#define INTC_EOI 0x20
> +
> +struct mtk_intc_chip_data {
> + char *name;
> + struct irq_chip chip;
There is no need to embed a full struct irqchip per controller, see
below.
We want to distinguish which controller the device interrupts are
belong to
by "cat /proc/interrupts".
And if all the controller share the same struct, the name field will be
the
same.
Do you have suggestion for this?
Yes. /proc/interrupts is not a debug tool, and doesn't need to
show the interrupt routing. We have a debugfs option for this
purpose, and that is what you should use. If it is missing
any information, just say so and I will see what we can do.
[...]
> +static struct irq_chip mtk_intc_chip = {
> + .irq_mask = mtk_intc_mask_irq,
> + .irq_unmask = mtk_intc_unmask_irq,
> + .irq_eoi = mtk_intc_eoi_irq,
> + .irq_get_irqchip_state = irq_chip_get_parent_state,
> + .irq_set_irqchip_state = irq_chip_set_parent_state,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> + .irq_set_vcpu_affinity = irq_chip_set_vcpu_affinity_parent,
How about retrigger?
What is retrigger means?
It allows the HW to regenerate an interrupt. Set irq_retrigger
to irq_chip_retrigger_hierarchy, and you'll be fine. It is
going to be implemented shortly in the GIC driver.
To be honest, I just try to direct all the irqchip ops implemented in
/drivers/irqchip/irq-gic.c to gic driver.
But "irq_set_vcpu_affinity" is not used in our projects now.
I assume you are not contributing this code just for your
own project...
Should I remove ".irq_set_vcpu_affinity" here?
No, just leave it. Who knows, just in case virtualization becomes
a thing...
M.
--
Jazz is not dead. It just smells funny...
