| Message ID | 20210320181610.680870-9-j.neuschaefer@gmx.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initial support for Nuvoton WPCM450 BMC SoC | expand |
On Sat, 20 Mar 2021 18:16:04 +0000, Jonathan Neuschäfer <j.neuschaefer@gmx.net> wrote: > > The WPCM450 AIC ("Advanced Interrupt Controller") is the interrupt > controller found in the Nuvoton WPCM450 SoC and other Winbond/Nuvoton > SoCs. > > The list of registers if based on the AMI vendor kernel and the > Nuvoton W90N745 datasheet. > > Although the hardware supports other interrupt modes, the driver only > supports high-level interrupts at the moment, because other modes could > not be tested so far. > > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> > --- > arch/arm/mach-npcm/Kconfig | 1 + > drivers/irqchip/Kconfig | 6 ++ > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-wpcm450-aic.c | 162 ++++++++++++++++++++++++++++++ > 4 files changed, 170 insertions(+) > create mode 100644 drivers/irqchip/irq-wpcm450-aic.c > > diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig > index 658c8efb4ca14..a71cf1d189ae5 100644 > --- a/arch/arm/mach-npcm/Kconfig > +++ b/arch/arm/mach-npcm/Kconfig > @@ -10,6 +10,7 @@ config ARCH_WPCM450 > bool "Support for WPCM450 BMC (Hermon)" > depends on ARCH_MULTI_V5 > select CPU_ARM926T > + select WPCM450_AIC > select NPCM7XX_TIMER > help > General support for WPCM450 BMC (Hermon). > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index e74fa206240a1..baf4efec31c67 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -586,4 +586,10 @@ config MST_IRQ > help > Support MStar Interrupt Controller. > > +config WPCM450_AIC > + bool "Nuvoton WPCM450 Advanced Interrupt Controller" > + depends on ARCH_WPCM450 || COMPILE_TEST > + help > + Support for the interrupt controller in the Nuvoton WPCM450 BMC SoC. > + > endmenu > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index c59b95a0532c9..bef57937e7296 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -113,3 +113,4 @@ obj-$(CONFIG_LOONGSON_PCH_MSI) += irq-loongson-pch-msi.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 > +obj-$(CONFIG_WPCM450_AIC) += irq-wpcm450-aic.o > diff --git a/drivers/irqchip/irq-wpcm450-aic.c b/drivers/irqchip/irq-wpcm450-aic.c > new file mode 100644 > index 0000000000000..0d6dd8b1fc824 > --- /dev/null > +++ b/drivers/irqchip/irq-wpcm450-aic.c > @@ -0,0 +1,162 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// Copyright 2021 Jonathan Neuschäfer > + > +#include <linux/console.h> That's unexpected. Why do you need this? > +#include <linux/irqchip.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > + > +#include <asm/exception.h> > + > +#define AIC_SCR(x) ((x)*4) /* Source control registers */ > +#define AIC_GEN 0x84 > +#define AIC_GRSR 0x88 > +#define AIC_IRSR 0x100 /* Interrupt raw status register */ > +#define AIC_IASR 0x104 /* Interrupt active status register */ > +#define AIC_ISR 0x108 /* Interrupt status register */ > +#define AIC_IPER 0x10c /* Interrupt priority encoding register */ > +#define AIC_ISNR 0x110 /* Interrupt source number register */ > +#define AIC_IMR 0x114 /* Interrupt mask register */ > +#define AIC_OISR 0x118 /* Output interrupt status register */ > +#define AIC_MECR 0x120 /* Mask enable command register */ > +#define AIC_MDCR 0x124 /* Mask disable command register */ > +#define AIC_SSCR 0x128 /* Source set command register */ > +#define AIC_SCCR 0x12c /* Source clear command register */ > +#define AIC_EOSCR 0x130 /* End of service command register */ > + > +#define AIC_SCR_SRCTYPE_LOW_LEVEL (0 << 6) > +#define AIC_SCR_SRCTYPE_HIGH_LEVEL (1 << 6) > +#define AIC_SCR_SRCTYPE_NEG_EDGE (2 << 6) > +#define AIC_SCR_SRCTYPE_POS_EDGE (3 << 6) > +#define AIC_SCR_PRIORITY(x) (x) A mask would be welcomed for this field. > + > +#define IRQS 32 Please use something a bit less generic. > + > +struct wpcm450_aic { > + void __iomem *regs; > + struct irq_domain *domain; > +}; > + > +static struct wpcm450_aic *aic; > + > +static void wpcm450_aic_init_hw(void) > +{ > + int i; > + > + /* Disable (mask) all interrupts */ > + writel(0xffffffff, aic->regs + AIC_MDCR); Consider using relaxed accessors throughout this driver. > + > + /* > + * Make sure the interrupt controller is ready to serve new interrupts. > + * Reading from IPER indicates that the nIRQ signal may be deasserted, > + * and writing to EOSCR indicates that interrupt handling has finished. > + */ > + readl(aic->regs + AIC_IPER); > + writel(0, aic->regs + AIC_EOSCR); > + > + /* Initialize trigger mode and priority of each interrupt source */ > + for (i = 0; i < IRQS; i++) > + writel(AIC_SCR_SRCTYPE_HIGH_LEVEL | AIC_SCR_PRIORITY(7), > + aic->regs + AIC_SCR(i)); > +} > + > +static void __exception_irq_entry wpcm450_aic_handle_irq(struct pt_regs *regs) > +{ > + int hwirq; > + > + /* Determine the interrupt source */ > + /* Read IPER to signal that nIRQ can be de-asserted */ > + hwirq = readl(aic->regs + AIC_IPER) / 4; > + > + handle_domain_irq(aic->domain, hwirq, regs); > +} > + > +static void wpcm450_aic_ack(struct irq_data *d) > +{ > + /* Signal end-of-service */ > + writel(0, aic->regs + AIC_EOSCR); Is that an Ack or an EOI? My gut feeling is that the above read is the Ack, and that this write should actually be an EOI callback. > +} > + > +static void wpcm450_aic_mask(struct irq_data *d) > +{ > + unsigned int mask = 1U << d->hwirq; Consider using BIT(). > + > + /* Disable (mask) the interrupt */ > + writel(mask, aic->regs + AIC_MDCR); > +} > + > +static void wpcm450_aic_unmask(struct irq_data *d) > +{ > + unsigned int mask = 1U << d->hwirq; > + > + /* Enable (unmask) the interrupt */ > + writel(mask, aic->regs + AIC_MECR); > +} > + > +static int wpcm450_aic_set_type(struct irq_data *d, unsigned int flow_type) > +{ > + /* > + * The hardware supports high/low level, as well as rising/falling edge > + * modes, and the DT binding accommodates for that, but as long as > + * other modes than high level mode are not used and can't be tested, > + * they are rejected in this driver. > + */ > + if ((flow_type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_HIGH) { > + pr_err("IRQ mode %#x is not supported\n", flow_type); The core kernel shouts loudly enough, no need for extra messages. > + return -EINVAL; > + } > + > + return 0; > +} > + > +static struct irq_chip wpcm450_aic_chip = { > + .name = "wpcm450-aic", > + .irq_ack = wpcm450_aic_ack, > + .irq_mask = wpcm450_aic_mask, > + .irq_unmask = wpcm450_aic_unmask, > + .irq_set_type = wpcm450_aic_set_type, > +}; > + > +static int wpcm450_aic_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) > +{ > + if (hwirq > IRQS) > + return -EPERM; > + > + irq_set_chip_and_handler(irq, &wpcm450_aic_chip, handle_level_irq); > + irq_set_chip_data(irq, aic); > + irq_set_probe(irq); > + > + return 0; > +} > + > +static const struct irq_domain_ops wpcm450_aic_ops = { > + .map = wpcm450_aic_map, > + .xlate = irq_domain_xlate_twocell, > +}; > + > +static int __init wpcm450_aic_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + if (parent) > + return -EINVAL; > + > + aic = kzalloc(sizeof(*aic), GFP_KERNEL); > + if (!aic) > + return -ENOMEM; > + > + aic->regs = of_iomap(node, 0); > + if (!aic->regs) { > + pr_err("Failed to map WPCM450 AIC registers\n"); > + return -ENOMEM; > + } > + > + wpcm450_aic_init_hw(); > + > + set_handle_irq(wpcm450_aic_handle_irq); > + > + aic->domain = irq_domain_add_linear(node, IRQS, &wpcm450_aic_ops, aic); > + > + return 0; > +} > + > +IRQCHIP_DECLARE(wpcm450_aic, "nuvoton,wpcm450-aic", wpcm450_aic_of_init); Otherwise, looks good. M.
On Wed, Mar 24, 2021 at 05:16:35PM +0000, Marc Zyngier wrote: > On Sat, 20 Mar 2021 18:16:04 +0000, > Jonathan Neuschäfer <j.neuschaefer@gmx.net> wrote: > > > > The WPCM450 AIC ("Advanced Interrupt Controller") is the interrupt > > controller found in the Nuvoton WPCM450 SoC and other Winbond/Nuvoton > > SoCs. > > > > The list of registers if based on the AMI vendor kernel and the > > Nuvoton W90N745 datasheet. > > > > Although the hardware supports other interrupt modes, the driver only > > supports high-level interrupts at the moment, because other modes could > > not be tested so far. > > > > Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> > > --- [...] > > +// SPDX-License-Identifier: GPL-2.0-only > > +// Copyright 2021 Jonathan Neuschäfer > > + > > +#include <linux/console.h> > > That's unexpected. Why do you need this? I forgot about linux/printk.h. > > +#define AIC_SCR_SRCTYPE_LOW_LEVEL (0 << 6) > > +#define AIC_SCR_SRCTYPE_HIGH_LEVEL (1 << 6) > > +#define AIC_SCR_SRCTYPE_NEG_EDGE (2 << 6) > > +#define AIC_SCR_SRCTYPE_POS_EDGE (3 << 6) > > +#define AIC_SCR_PRIORITY(x) (x) > > A mask would be welcomed for this field. Ok, I'll add +#define AIC_SCR_PRIORITY_MASK 0x7 Should I apply it in AIC_SCR_PRIORITY(x), too? > > + > > +#define IRQS 32 > > Please use something a bit less generic. Ok, I'll rename it to AIC_NUM_IRQS. > > +static void wpcm450_aic_init_hw(void) > > +{ > > + int i; > > + > > + /* Disable (mask) all interrupts */ > > + writel(0xffffffff, aic->regs + AIC_MDCR); > > Consider using relaxed accessors throughout this driver. I'll read up on how to use them correctly. > > +static void __exception_irq_entry wpcm450_aic_handle_irq(struct pt_regs *regs) > > +{ > > + int hwirq; > > + > > + /* Determine the interrupt source */ > > + /* Read IPER to signal that nIRQ can be de-asserted */ > > + hwirq = readl(aic->regs + AIC_IPER) / 4; > > + > > + handle_domain_irq(aic->domain, hwirq, regs); > > +} > > + > > +static void wpcm450_aic_ack(struct irq_data *d) > > +{ > > + /* Signal end-of-service */ > > + writel(0, aic->regs + AIC_EOSCR); > > Is that an Ack or an EOI? My gut feeling is that the above read is the > Ack, and that this write should actually be an EOI callback. I agree that EOSCR (End of Service Command Register) matches the description of EOI. The reading IPER serves a dual purpose, as indicated above. I could move the IPER read to a separate irq_ack function and use ISNR (Interrupt source number register) in the IRQ handler instead. This should work (haven't tested it yet), but I'm not sure it's strictly better. > > +static void wpcm450_aic_mask(struct irq_data *d) > > +{ > > + unsigned int mask = 1U << d->hwirq; > > Consider using BIT(). Will do. > > +static int wpcm450_aic_set_type(struct irq_data *d, unsigned int flow_type) > > +{ > > + /* > > + * The hardware supports high/low level, as well as rising/falling edge > > + * modes, and the DT binding accommodates for that, but as long as > > + * other modes than high level mode are not used and can't be tested, > > + * they are rejected in this driver. > > + */ > > + if ((flow_type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_HIGH) { > > + pr_err("IRQ mode %#x is not supported\n", flow_type); > > The core kernel shouts loudly enough, no need for extra messages. Ok. > Otherwise, looks good. > > M. Thanks for your review! Jonathan Neuschäfer
On Fri, Mar 26, 2021 at 07:52:07PM +0100, Jonathan Neuschäfer wrote: > On Wed, Mar 24, 2021 at 05:16:35PM +0000, Marc Zyngier wrote: > > On Sat, 20 Mar 2021 18:16:04 +0000, Jonathan Neuschäfer <j.neuschaefer@gmx.net> wrote: [...] > > > + /* Disable (mask) all interrupts */ > > > + writel(0xffffffff, aic->regs + AIC_MDCR); > > > > Consider using relaxed accessors throughout this driver. > > I'll read up on how to use them correctly. Upon further consideration, I decided not to bother with relaxed MMIO access, because it doesn't make a difference on the target platform — Nuvoton WPCM450 with ARM926 (ARMv5). Best regards, Jonathan Neuschäfer
diff --git a/arch/arm/mach-npcm/Kconfig b/arch/arm/mach-npcm/Kconfig index 658c8efb4ca14..a71cf1d189ae5 100644 --- a/arch/arm/mach-npcm/Kconfig +++ b/arch/arm/mach-npcm/Kconfig @@ -10,6 +10,7 @@ config ARCH_WPCM450 bool "Support for WPCM450 BMC (Hermon)" depends on ARCH_MULTI_V5 select CPU_ARM926T + select WPCM450_AIC select NPCM7XX_TIMER help General support for WPCM450 BMC (Hermon). diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index e74fa206240a1..baf4efec31c67 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -586,4 +586,10 @@ config MST_IRQ help Support MStar Interrupt Controller. +config WPCM450_AIC + bool "Nuvoton WPCM450 Advanced Interrupt Controller" + depends on ARCH_WPCM450 || COMPILE_TEST + help + Support for the interrupt controller in the Nuvoton WPCM450 BMC SoC. + endmenu diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index c59b95a0532c9..bef57937e7296 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -113,3 +113,4 @@ obj-$(CONFIG_LOONGSON_PCH_MSI) += irq-loongson-pch-msi.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 +obj-$(CONFIG_WPCM450_AIC) += irq-wpcm450-aic.o diff --git a/drivers/irqchip/irq-wpcm450-aic.c b/drivers/irqchip/irq-wpcm450-aic.c new file mode 100644 index 0000000000000..0d6dd8b1fc824 --- /dev/null +++ b/drivers/irqchip/irq-wpcm450-aic.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0-only +// Copyright 2021 Jonathan Neuschäfer + +#include <linux/console.h> +#include <linux/irqchip.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> + +#include <asm/exception.h> + +#define AIC_SCR(x) ((x)*4) /* Source control registers */ +#define AIC_GEN 0x84 +#define AIC_GRSR 0x88 +#define AIC_IRSR 0x100 /* Interrupt raw status register */ +#define AIC_IASR 0x104 /* Interrupt active status register */ +#define AIC_ISR 0x108 /* Interrupt status register */ +#define AIC_IPER 0x10c /* Interrupt priority encoding register */ +#define AIC_ISNR 0x110 /* Interrupt source number register */ +#define AIC_IMR 0x114 /* Interrupt mask register */ +#define AIC_OISR 0x118 /* Output interrupt status register */ +#define AIC_MECR 0x120 /* Mask enable command register */ +#define AIC_MDCR 0x124 /* Mask disable command register */ +#define AIC_SSCR 0x128 /* Source set command register */ +#define AIC_SCCR 0x12c /* Source clear command register */ +#define AIC_EOSCR 0x130 /* End of service command register */ + +#define AIC_SCR_SRCTYPE_LOW_LEVEL (0 << 6) +#define AIC_SCR_SRCTYPE_HIGH_LEVEL (1 << 6) +#define AIC_SCR_SRCTYPE_NEG_EDGE (2 << 6) +#define AIC_SCR_SRCTYPE_POS_EDGE (3 << 6) +#define AIC_SCR_PRIORITY(x) (x) + +#define IRQS 32 + +struct wpcm450_aic { + void __iomem *regs; + struct irq_domain *domain; +}; + +static struct wpcm450_aic *aic; + +static void wpcm450_aic_init_hw(void) +{ + int i; + + /* Disable (mask) all interrupts */ + writel(0xffffffff, aic->regs + AIC_MDCR); + + /* + * Make sure the interrupt controller is ready to serve new interrupts. + * Reading from IPER indicates that the nIRQ signal may be deasserted, + * and writing to EOSCR indicates that interrupt handling has finished. + */ + readl(aic->regs + AIC_IPER); + writel(0, aic->regs + AIC_EOSCR); + + /* Initialize trigger mode and priority of each interrupt source */ + for (i = 0; i < IRQS; i++) + writel(AIC_SCR_SRCTYPE_HIGH_LEVEL | AIC_SCR_PRIORITY(7), + aic->regs + AIC_SCR(i)); +} + +static void __exception_irq_entry wpcm450_aic_handle_irq(struct pt_regs *regs) +{ + int hwirq; + + /* Determine the interrupt source */ + /* Read IPER to signal that nIRQ can be de-asserted */ + hwirq = readl(aic->regs + AIC_IPER) / 4; + + handle_domain_irq(aic->domain, hwirq, regs); +} + +static void wpcm450_aic_ack(struct irq_data *d) +{ + /* Signal end-of-service */ + writel(0, aic->regs + AIC_EOSCR); +} + +static void wpcm450_aic_mask(struct irq_data *d) +{ + unsigned int mask = 1U << d->hwirq; + + /* Disable (mask) the interrupt */ + writel(mask, aic->regs + AIC_MDCR); +} + +static void wpcm450_aic_unmask(struct irq_data *d) +{ + unsigned int mask = 1U << d->hwirq; + + /* Enable (unmask) the interrupt */ + writel(mask, aic->regs + AIC_MECR); +} + +static int wpcm450_aic_set_type(struct irq_data *d, unsigned int flow_type) +{ + /* + * The hardware supports high/low level, as well as rising/falling edge + * modes, and the DT binding accommodates for that, but as long as + * other modes than high level mode are not used and can't be tested, + * they are rejected in this driver. + */ + if ((flow_type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_HIGH) { + pr_err("IRQ mode %#x is not supported\n", flow_type); + return -EINVAL; + } + + return 0; +} + +static struct irq_chip wpcm450_aic_chip = { + .name = "wpcm450-aic", + .irq_ack = wpcm450_aic_ack, + .irq_mask = wpcm450_aic_mask, + .irq_unmask = wpcm450_aic_unmask, + .irq_set_type = wpcm450_aic_set_type, +}; + +static int wpcm450_aic_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) +{ + if (hwirq > IRQS) + return -EPERM; + + irq_set_chip_and_handler(irq, &wpcm450_aic_chip, handle_level_irq); + irq_set_chip_data(irq, aic); + irq_set_probe(irq); + + return 0; +} + +static const struct irq_domain_ops wpcm450_aic_ops = { + .map = wpcm450_aic_map, + .xlate = irq_domain_xlate_twocell, +}; + +static int __init wpcm450_aic_of_init(struct device_node *node, + struct device_node *parent) +{ + if (parent) + return -EINVAL; + + aic = kzalloc(sizeof(*aic), GFP_KERNEL); + if (!aic) + return -ENOMEM; + + aic->regs = of_iomap(node, 0); + if (!aic->regs) { + pr_err("Failed to map WPCM450 AIC registers\n"); + return -ENOMEM; + } + + wpcm450_aic_init_hw(); + + set_handle_irq(wpcm450_aic_handle_irq); + + aic->domain = irq_domain_add_linear(node, IRQS, &wpcm450_aic_ops, aic); + + return 0; +} + +IRQCHIP_DECLARE(wpcm450_aic, "nuvoton,wpcm450-aic", wpcm450_aic_of_init);
The WPCM450 AIC ("Advanced Interrupt Controller") is the interrupt controller found in the Nuvoton WPCM450 SoC and other Winbond/Nuvoton SoCs. The list of registers if based on the AMI vendor kernel and the Nuvoton W90N745 datasheet. Although the hardware supports other interrupt modes, the driver only supports high-level interrupts at the moment, because other modes could not be tested so far. Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> --- arch/arm/mach-npcm/Kconfig | 1 + drivers/irqchip/Kconfig | 6 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-wpcm450-aic.c | 162 ++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+) create mode 100644 drivers/irqchip/irq-wpcm450-aic.c -- 2.30.2