On Thu, 22 Mar 2018 13:55:42 +0000, Alexandre Belloni wrote: Hi Alexandre,
> > The Microsemi Ocelot SoC has a pretty simple IRQ controller in its ICPU > block. Add a driver for it. > > Signed-off-by: Alexandre Belloni <alexandre.bell...@bootlin.com> > --- > changes since v1: > - removed license in favor of SPDX tag > - reworked style as suggested by Thomas > > drivers/irqchip/Kconfig | 5 ++ > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-mscc-ocelot.c | 109 > ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 115 insertions(+) > create mode 100644 drivers/irqchip/irq-mscc-ocelot.c > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > index d913aec85109..0270797765f9 100644 > --- a/drivers/irqchip/Kconfig > +++ b/drivers/irqchip/Kconfig > @@ -286,6 +286,11 @@ config IRQ_MXS > select IRQ_DOMAIN > select STMP_DEVICE > > +config MSCC_OCELOT_IRQ > + bool > + select IRQ_DOMAIN > + select GENERIC_IRQ_CHIP > + > config MVEBU_GICP > bool > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile > index d27e3e3619e0..63d7bb434c33 100644 > --- a/drivers/irqchip/Makefile > +++ b/drivers/irqchip/Makefile > @@ -72,6 +72,7 @@ obj-$(CONFIG_ARCH_SA1100) += irq-sa11x0.o > obj-$(CONFIG_INGENIC_IRQ) += irq-ingenic.o > obj-$(CONFIG_IMX_GPCV2) += irq-imx-gpcv2.o > obj-$(CONFIG_PIC32_EVIC) += irq-pic32-evic.o > +obj-$(CONFIG_MSCC_OCELOT_IRQ) += irq-mscc-ocelot.o > obj-$(CONFIG_MVEBU_GICP) += irq-mvebu-gicp.o > obj-$(CONFIG_MVEBU_ICU) += irq-mvebu-icu.o > obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o > diff --git a/drivers/irqchip/irq-mscc-ocelot.c > b/drivers/irqchip/irq-mscc-ocelot.c > new file mode 100644 > index 000000000000..c71094de3cbd > --- /dev/null > +++ b/drivers/irqchip/irq-mscc-ocelot.c > @@ -0,0 +1,109 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > +/* > + * Microsemi Ocelot IRQ controller driver > + * > + * Copyright (c) 2017 Microsemi Corporation > + */ > +#include <linux/bitops.h> > +#include <linux/irq.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/irqchip.h> > +#include <linux/irqchip/chained_irq.h> > +#include <linux/interrupt.h> > + > +#define ICPU_CFG_INTR_INTR_STICKY 0x10 > +#define ICPU_CFG_INTR_INTR_ENA 0x18 > +#define ICPU_CFG_INTR_INTR_ENA_CLR 0x1c > +#define ICPU_CFG_INTR_INTR_ENA_SET 0x20 > +#define ICPU_CFG_INTR_DST_INTR_IDENT(x) (0x38 + 0x4 * (x)) > +#define ICPU_CFG_INTR_INTR_TRIGGER(x) (0x5c + 0x4 * (x)) > + > +#define OCELOT_NR_IRQ 24 > + > +static void ocelot_irq_unmask(struct irq_data *data) > +{ > + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data); > + struct irq_chip_type *ct = irq_data_get_chip_type(data); > + unsigned int mask = data->mask; > + u32 val; > + > + irq_gc_lock(gc); > + val = irq_reg_readl(gc, ICPU_CFG_INTR_INTR_TRIGGER(0)) | > + irq_reg_readl(gc, ICPU_CFG_INTR_INTR_TRIGGER(1)); > + if (!(val & mask)) > + irq_reg_writel(gc, mask, ICPU_CFG_INTR_INTR_STICKY); > + > + *ct->mask_cache &= ~mask; > + irq_reg_writel(gc, mask, ICPU_CFG_INTR_INTR_ENA_SET); > + irq_gc_unlock(gc); > +} > + > +static void ocelot_irq_handler(struct irq_desc *desc) > +{ > + struct irq_chip *chip = irq_desc_get_chip(desc); > + struct irq_domain *d = irq_desc_get_handler_data(desc); > + struct irq_chip_generic *gc = irq_get_domain_generic_chip(d, 0); > + u32 reg = irq_reg_readl(gc, ICPU_CFG_INTR_DST_INTR_IDENT(0)); > + > + chained_irq_enter(chip, desc); > + > + while (reg) { > + u32 hwirq = __fls(reg); > + > + generic_handle_irq(irq_find_mapping(d, hwirq)); > + reg &= ~(BIT(hwirq)); > + } > + > + chained_irq_exit(chip, desc); > +} > + > +static int __init ocelot_irq_init(struct device_node *node, > + struct device_node *parent) > +{ > + struct irq_domain *domain; > + struct irq_chip_generic *gc; > + int parent_irq, ret; > + > + domain = irq_domain_add_linear(node, OCELOT_NR_IRQ, > + &irq_generic_chip_ops, NULL); > + if (!domain) { > + pr_err("%s: unable to add irq domain\n", node->name); > + return -ENOMEM; > + } > + > + ret = irq_alloc_domain_generic_chips(domain, OCELOT_NR_IRQ, 1, > + "icpu", handle_level_irq, > + 0, 0, 0); > + if (ret) { > + pr_err("%s: unable to alloc irq domain gc\n", node->name); > + return ret; You're now leaking the allocated domain. > + } > + > + gc = irq_get_domain_generic_chip(domain, 0); > + gc->reg_base = of_iomap(node, 0); > + if (!gc->reg_base) { > + pr_err("%s: unable to map resource\n", node->name); > + return -ENOMEM; And here the chip. > + } > + > + gc->chip_types[0].regs.ack = ICPU_CFG_INTR_INTR_STICKY; > + gc->chip_types[0].regs.mask = ICPU_CFG_INTR_INTR_ENA_CLR; > + gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit; > + gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit; > + gc->chip_types[0].chip.irq_unmask = ocelot_irq_unmask; > + > + /* Mask and ack all interrupts */ > + irq_reg_writel(gc, 0, ICPU_CFG_INTR_INTR_ENA); > + irq_reg_writel(gc, 0xffffffff, ICPU_CFG_INTR_INTR_STICKY); > + > + parent_irq = irq_of_parse_and_map(node, 0); > + if (!parent_irq) and here the ioremap space. You could also check that parent is non-NULL early on, just in case... > + return -EINVAL; > + > + irq_set_chained_handler_and_data(parent_irq, ocelot_irq_handler, > + domain); > + > + return 0; > +} > +IRQCHIP_DECLARE(ocelot_icpu, "mscc,ocelot-icpu-intr", ocelot_irq_init); > -- > 2.16.2 > If you rework the error handling quickly, I'll queue this for 4.17. It is quite refreshing to see an irqchip with only 3 functions... Thanks, M. -- Jazz is not dead, it just smell funny.