irqchip: Renesas INTC External IRQ pin driver

From: Magnus Damm <[email protected]>

This patch adds a driver for external IRQ pins connected to the INTC block on recent SoCs from Renesas.

The INTC hardware block usually contains a rather wide range of features ranging from external IRQ pin handling to legacy interrupt controller support. On older SoCs the INTC is used as a general purpose interrupt controller both for external IRQ pins and on-chip devices.

On more recent ARM based SoCs with Cortex-A9 the main interrupt controller is the GIC, but IRQ trigger setup still need to happen in the INTC hardware block.

This driver implements the glue code needed to configure IRQ trigger and also handle mask/unmask and demux of external IRQ pins hooked up from the INTC to the GIC.

Tested on sh73a0 and r8a7779. The hardware varies quite a bit with SoC model, for instance register width and bitfield widths vary wildly. The driver requires one GIC SPI per external IRQ pin to operate. Each driver instance will handle up to 8 external IRQ pins.

The SoCs using this driver are currently mainly used together with regular platform devices so this driver allows configuration via platform data to support things like static interrupt base address. DT support will be added incrementally in the not so distant future.

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/irqchip/Kconfig                               |    4 
 drivers/irqchip/Makefile                              |    1 
 drivers/irqchip/irq-renesas-intc-irqpin.c             |  464 +++++++++++++++++
 include/linux/platform_data/irq-renesas-intc-irqpin.h |   10 
 4 files changed, 479 insertions(+)

drivers/irqchip/Kconfig +4 -0

@@ -25,6 +25,10 @@ config ARM_VIC_NR
 	  The maximum number of VICs available in the system, for
 	  power management.
 
+config RENESAS_INTC_IRQPIN
+	bool
+	select IRQ_DOMAIN
+
 config VERSATILE_FPGA_IRQ
 	bool
 	select IRQ_DOMAIN

drivers/irqchip/Makefile +1 -0

@@ -5,4 +5,5 @@ obj-$(CONFIG_ARCH_SUNXI)		+= irq-sunxi.o
 obj-$(CONFIG_ARCH_SPEAR3XX)		+= spear-shirq.o
 obj-$(CONFIG_ARM_GIC)			+= irq-gic.o
 obj-$(CONFIG_ARM_VIC)			+= irq-vic.o
+obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o

drivers/irqchip/irq-renesas-intc-irqpin.c new file +50 -0

@@ -0,0 +1,464 @@
+/*
+ * Renesas INTC External IRQ Pin Driver
+ *
+ *  Copyright (C) 2013 Magnus Damm
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/platform_data/irq-renesas-intc-irqpin.h>
+
+#define INTC_IRQPIN_MAX 8 /* maximum 8 interrupts per driver instance */
+
+#define INTC_IRQPIN_REG_SENSE 0 /* ICRn */
+#define INTC_IRQPIN_REG_PRIO 1 /* INTPRInn */
+#define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
+#define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
+#define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
+#define INTC_IRQPIN_REG_NR 5
+
+/* INTC external IRQ PIN hardware register access:
+ *
+ * SENSE is read-write 32-bit with 2-bits or 4-bits per IRQ (*)
+ * PRIO is read-write 32-bit with 4-bits per IRQ (**)
+ * SOURCE is read-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ * MASK is write-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ * CLEAR is write-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ *
+ * (*) May be accessed by more than one driver instance - lock needed

drivers/irqchip/irq-renesas-intc-irqpin.c new file +332 -0

+
+struct intc_irqpin_priv {
+	struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
+	struct intc_irqpin_irq irq[INTC_IRQPIN_MAX];
+	struct renesas_intc_irqpin_config config;
+	unsigned int number_of_irqs;
+	struct platform_device *pdev;
+	struct irq_chip irq_chip;
+	struct irq_domain *irq_domain;
+};
+
+static unsigned long intc_irqpin_read32(void __iomem *iomem)
+{
+	return ioread32(iomem);
+}
+
+static unsigned long intc_irqpin_read8(void __iomem *iomem)
+{
+	return ioread8(iomem);
+}
+
+static void intc_irqpin_write32(void __iomem *iomem, unsigned long data)
+{
+	iowrite32(data, iomem);
+}
+
+static void intc_irqpin_write8(void __iomem *iomem, unsigned long data)
+{
+	iowrite8(data, iomem);
+}
+
+static inline unsigned long intc_irqpin_read(struct intc_irqpin_priv *p,
+					     int reg)
+{
+	struct intc_irqpin_iomem *i = &p->iomem[reg];
+	return i->read(i->iomem);
+}
+
+static inline void intc_irqpin_write(struct intc_irqpin_priv *p,
+				     int reg, unsigned long data)
+{
+	struct intc_irqpin_iomem *i = &p->iomem[reg];
+	i->write(i->iomem, data);
+}
+
+static inline unsigned long intc_irqpin_hwirq_mask(struct intc_irqpin_priv *p,
+						   int reg, int hw_irq)
+{
+	return BIT((p->iomem[reg].width - 1) - hw_irq);
+}
+
+static inline void intc_irqpin_irq_write_hwirq(struct intc_irqpin_priv *p,
+					       int reg, int hw_irq)
+{
+	intc_irqpin_write(p, reg, intc_irqpin_hwirq_mask(p, reg, hw_irq));
+}
+
+static DEFINE_RAW_SPINLOCK(intc_irqpin_lock); /* only used by slow path */
+
+static void intc_irqpin_read_modify_write(struct intc_irqpin_priv *p,
+					  int reg, int shift,
+					  int width, int value)
+{
+	unsigned long flags;
+	unsigned long tmp;
+
+	raw_spin_lock_irqsave(&intc_irqpin_lock, flags);
+
+	tmp = intc_irqpin_read(p, reg);
+	tmp &= ~(((1 << width) - 1) << shift);
+	tmp |= value << shift;
+	intc_irqpin_write(p, reg, tmp);
+
+	raw_spin_unlock_irqrestore(&intc_irqpin_lock, flags);
+}
+
+static void intc_irqpin_mask_unmask_prio(struct intc_irqpin_priv *p,
+					 int irq, int do_mask)
+{
+	int bitfield_width = 4; /* PRIO assumed to have fixed bitfield width */
+	int shift = (7 - irq) * bitfield_width; /* PRIO assumed to be 32-bit */
+
+	intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_PRIO,
+				      shift, bitfield_width,
+				      do_mask ? 0 : (1 << bitfield_width) - 1);
+}
+
+static int intc_irqpin_set_sense(struct intc_irqpin_priv *p, int irq, int value)
+{
+	int bitfield_width = p->config.sense_bitfield_width;
+	int shift = (7 - irq) * bitfield_width; /* SENSE assumed to be 32-bit */
+
+	dev_dbg(&p->pdev->dev, "sense irq = %d, mode = %d\n", irq, value);
+
+	if (value >= (1 << bitfield_width))
+		return -EINVAL;
+
+	intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_SENSE, shift,
+				      bitfield_width, value);
+	return 0;
+}
+
+static void intc_irqpin_dbg(struct intc_irqpin_irq *i, char *str)
+{
+	dev_dbg(&i->p->pdev->dev, "%s (%d:%d:%d)\n",
+		str, i->irq, i->hw_irq,
+		irq_find_mapping(i->p->irq_domain, i->hw_irq));
+}
+
+static void intc_irqpin_irq_enable(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int hw_irq = irqd_to_hwirq(d);
+
+	intc_irqpin_dbg(&p->irq[hw_irq], "enable");
+	intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_CLEAR, hw_irq);
+}
+
+static void intc_irqpin_irq_disable(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int hw_irq = irqd_to_hwirq(d);
+
+	intc_irqpin_dbg(&p->irq[hw_irq], "disable");
+	intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_MASK, hw_irq);
+}
+
+static void intc_irqpin_irq_enable_force(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int irq = p->irq[irqd_to_hwirq(d)].irq;
+
+	intc_irqpin_irq_enable(d);
+	irq_get_chip(irq)->irq_unmask(irq_get_irq_data(irq));
+}
+
+static void intc_irqpin_irq_disable_force(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int irq = p->irq[irqd_to_hwirq(d)].irq;
+
+	irq_get_chip(irq)->irq_mask(irq_get_irq_data(irq));
+	intc_irqpin_irq_disable(d);
+}
+
+#define INTC_IRQ_SENSE_VALID 0x10
+#define INTC_IRQ_SENSE(x) (x + INTC_IRQ_SENSE_VALID)
+
+static unsigned char intc_irqpin_sense[IRQ_TYPE_SENSE_MASK + 1] = {
+	[IRQ_TYPE_EDGE_FALLING] = INTC_IRQ_SENSE(0x00),
+	[IRQ_TYPE_EDGE_RISING] = INTC_IRQ_SENSE(0x01),
+	[IRQ_TYPE_LEVEL_LOW] = INTC_IRQ_SENSE(0x02),
+	[IRQ_TYPE_LEVEL_HIGH] = INTC_IRQ_SENSE(0x03),
+	[IRQ_TYPE_EDGE_BOTH] = INTC_IRQ_SENSE(0x04),
+};
+
+static int intc_irqpin_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	unsigned char value = intc_irqpin_sense[type & IRQ_TYPE_SENSE_MASK];
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+
+	if (!(value & INTC_IRQ_SENSE_VALID))
+		return -EINVAL;
+
+	return intc_irqpin_set_sense(p, irqd_to_hwirq(d),
+				     value ^ INTC_IRQ_SENSE_VALID);
+}
+
+static irqreturn_t intc_irqpin_irq_handler(int irq, void *dev_id)
+{
+	struct intc_irqpin_irq *i = dev_id;
+	struct intc_irqpin_priv *p = i->p;
+	unsigned long bit;
+
+	intc_irqpin_dbg(i, "demux1");
+	bit = intc_irqpin_hwirq_mask(p, INTC_IRQPIN_REG_SOURCE, i->hw_irq);
+
+	if (intc_irqpin_read(p, INTC_IRQPIN_REG_SOURCE) & bit) {
+		intc_irqpin_write(p, INTC_IRQPIN_REG_SOURCE, ~bit);
+		intc_irqpin_dbg(i, "demux2");
+		generic_handle_irq(irq_find_mapping(p->irq_domain, i->hw_irq));
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
+}
+
+static int intc_irqpin_irq_domain_map(struct irq_domain *h, unsigned int virq,
+				      irq_hw_number_t hw)
+{
+	struct intc_irqpin_priv *p = h->host_data;
+
+	intc_irqpin_dbg(&p->irq[hw], "map");
+	irq_set_chip_data(virq, h->host_data);
+	irq_set_chip_and_handler(virq, &p->irq_chip, handle_level_irq);
+	set_irq_flags(virq, IRQF_VALID); /* kill me now */
+	return 0;
+}
+
+static struct irq_domain_ops intc_irqpin_irq_domain_ops = {
+	.map	= intc_irqpin_irq_domain_map,
+};
+
+static int intc_irqpin_probe(struct platform_device *pdev)
+{
+	struct renesas_intc_irqpin_config *pdata = pdev->dev.platform_data;
+	struct intc_irqpin_priv *p;
+	struct intc_irqpin_iomem *i;
+	struct resource *io[INTC_IRQPIN_REG_NR];
+	struct resource *irq;
+	struct irq_chip *irq_chip;
+	void (*enable_fn)(struct irq_data *d);
+	void (*disable_fn)(struct irq_data *d);
+	const char *name = dev_name(&pdev->dev);
+	int ret;
+	int k;
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p) {
+		dev_err(&pdev->dev, "failed to allocate driver data\n");
+		ret = -ENOMEM;
+		goto err0;
+	}
+
+	/* deal with driver instance configuration */
+	if (pdata)
+		memcpy(&p->config, pdata, sizeof(*pdata));
+	if (!p->config.sense_bitfield_width)
+		p->config.sense_bitfield_width = 4; /* default to 4 bits */
+
+	p->pdev = pdev;
+	platform_set_drvdata(pdev, p);
+
+	/* get hold of manadatory IOMEM */
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
+		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
+		if (!io[k]) {
+			dev_err(&pdev->dev, "not enough IOMEM resources\n");
+			ret = -EINVAL;
+			goto err1;
+		}
+	}
+
+	/* allow any number of IRQs between 1 and INTC_IRQPIN_MAX */
+	for (k = 0; k < INTC_IRQPIN_MAX; k++) {
+		irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
+		if (!irq)
+			break;
+
+		p->irq[k].hw_irq = k;
+		p->irq[k].p = p;
+		p->irq[k].irq = irq->start;
+	}
+
+	p->number_of_irqs = k;
+	if (p->number_of_irqs < 1) {
+		dev_err(&pdev->dev, "not enough IRQ resources\n");
+		ret = -EINVAL;
+		goto err1;
+	}
+
+	/* ioremap IOMEM and setup read/write callbacks */
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
+		i = &p->iomem[k];
+
+		switch (resource_size(io[k])) {
+		case 1:
+			i->width = 8;
+			i->read = intc_irqpin_read8;
+			i->write = intc_irqpin_write8;
+			break;
+		case 4:
+			i->width = 32;
+			i->read = intc_irqpin_read32;
+			i->write = intc_irqpin_write32;
+			break;
+		default:
+			dev_err(&pdev->dev, "IOMEM size mismatch\n");
+			ret = -EINVAL;
+			goto err2;
+		}
+
+		i->iomem = ioremap_nocache(io[k]->start, resource_size(io[k]));
+		if (!i->iomem) {
+			dev_err(&pdev->dev, "failed to remap IOMEM\n");
+			ret = -ENXIO;
+			goto err2;
+		}
+	}
+
+	/* mask all interrupts using priority */
+	for (k = 0; k < p->number_of_irqs; k++)
+		intc_irqpin_mask_unmask_prio(p, k, 1);
+
+	/* use more severe masking method if requested */
+	if (p->config.control_parent) {
+		enable_fn = intc_irqpin_irq_enable_force;
+		disable_fn = intc_irqpin_irq_disable_force;
+	} else {
+		enable_fn = intc_irqpin_irq_enable;
+		disable_fn = intc_irqpin_irq_disable;
+	}
+
+	irq_chip = &p->irq_chip;
+	irq_chip->name = name;
+	irq_chip->irq_mask = disable_fn;
+	irq_chip->irq_unmask = enable_fn;
+	irq_chip->irq_enable = enable_fn;
+	irq_chip->irq_disable = disable_fn;
+	irq_chip->irq_set_type = intc_irqpin_irq_set_type;
+	irq_chip->flags	= IRQCHIP_SKIP_SET_WAKE;
+
+	p->irq_domain = irq_domain_add_simple(pdev->dev.of_node,
+					      p->number_of_irqs,
+					      p->config.irq_base,
+					      &intc_irqpin_irq_domain_ops, p);
+	if (!p->irq_domain) {
+		ret = -ENXIO;
+		dev_err(&pdev->dev, "cannot initialize irq domain\n");
+		goto err2;
+	}
+
+	/* request and set priority on interrupts one by one */
+	for (k = 0; k < p->number_of_irqs; k++) {
+		if (request_irq(p->irq[k].irq, intc_irqpin_irq_handler,
+				0, name, &p->irq[k])) {
+			dev_err(&pdev->dev, "failed to request low IRQ\n");
+			ret = -ENOENT;
+			goto err3;
+		}
+		intc_irqpin_mask_unmask_prio(p, k, 0);
+	}
+

drivers/irqchip/irq-renesas-intc-irqpin.c new file +56 -0

+	return 0;
+
+err3:
+	for (; k >= 0; k--)
+		free_irq(p->irq[k - 1].irq, &p->irq[k - 1]);
+
+	irq_domain_remove(p->irq_domain);
+err2:
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++)
+		iounmap(p->iomem[k].iomem);
+err1:
+	kfree(p);
+err0:
+	return ret;
+}
+
+static int intc_irqpin_remove(struct platform_device *pdev)
+{
+	struct intc_irqpin_priv *p = platform_get_drvdata(pdev);
+	int k;
+
+	for (k = 0; k < p->number_of_irqs; k++)
+		free_irq(p->irq[k].irq, &p->irq[k]);
+
+	irq_domain_remove(p->irq_domain);
+
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++)
+		iounmap(p->iomem[k].iomem);
+
+	kfree(p);
+	return 0;
+}
+
+static struct platform_driver intc_irqpin_device_driver = {
+	.probe		= intc_irqpin_probe,
+	.remove		= intc_irqpin_remove,
+	.driver		= {
+		.name	= "renesas_intc_irqpin",
+	}
+};
+
+static int __init intc_irqpin_init(void)
+{
+	return platform_driver_register(&intc_irqpin_device_driver);
+}
+postcore_initcall(intc_irqpin_init);
+
+static void __exit intc_irqpin_exit(void)
+{
+	platform_driver_unregister(&intc_irqpin_device_driver);
+}
+module_exit(intc_irqpin_exit);
+
+MODULE_AUTHOR("Magnus Damm");
+MODULE_DESCRIPTION("Renesas INTC External IRQ Pin Driver");
+MODULE_LICENSE("GPL v2");

include/linux/platform_data/irq-renesas-intc-irqpin.h new file +10 -0

@@ -0,0 +1,10 @@
+#ifndef __IRQ_RENESAS_INTC_IRQPIN_H__
+#define __IRQ_RENESAS_INTC_IRQPIN_H__
+
+struct renesas_intc_irqpin_config {
+	unsigned int sense_bitfield_width;
+	unsigned int irq_base;
+	bool control_parent;
+};
+
+#endif /* __IRQ_RENESAS_INTC_IRQPIN_H__ */

Quoted 25 lines · Magnus Damm

From: Magnus Damm <[email protected]>

This patch adds a driver for external IRQ pins connected to the INTC block on recent SoCs from Renesas.

The INTC hardware block usually contains a rather wide range of features ranging from external IRQ pin handling to legacy interrupt controller support. On older SoCs the INTC is used as a general purpose interrupt controller both for external IRQ pins and on-chip devices.

On more recent ARM based SoCs with Cortex-A9 the main interrupt controller is the GIC, but IRQ trigger setup still need to happen in the INTC hardware block.

This driver implements the glue code needed to configure IRQ trigger and also handle mask/unmask and demux of external IRQ pins hooked up from the INTC to the GIC.

Tested on sh73a0 and r8a7779. The hardware varies quite a bit with SoC model, for instance register width and bitfield widths vary wildly. The driver requires one GIC SPI per external IRQ pin to operate. Each driver instance will handle up to 8 external IRQ pins.

Quoted 7 lines

The SoCs using this driver are currently mainly used together with regular platform devices so this driver allows configuration via platform data to support things like static interrupt base address. DT support will be added incrementally in the not so distant future.

Signed-off-by Magnus Damm <[email protected]>

Hi Magnus, Hi all,

I do not expect this code to go through the renesas tree. However, in order to provide a basis for work on renesas SoCs I have added this patch to the topic/intc-external-irq topic branch in the reneas tree on kernel.org and merged it into topic/all+next.

In other words, I am not picking this series up to merge it or add it to linux-next, rather I am storing it for reference.

In the course of adding the branch I noticed a 3 whitespace warnings from git. I have highlighted them below.

drivers/irqchip/irq-renesas-intc-irqpin.c new file +9 -0

+struct intc_irqpin_priv {
+	struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
+	struct intc_irqpin_irq irq[INTC_IRQPIN_MAX];
+	struct renesas_intc_irqpin_config config;
+	unsigned int number_of_irqs;
+	struct platform_device *pdev;
+	struct irq_chip irq_chip;
+	struct irq_domain *irq_domain;
+};

drivers/irqchip/irq-renesas-intc-irqpin.c new file +8 -0

+static DEFINE_RAW_SPINLOCK(intc_irqpin_lock); /* only used by slow path */
+
+static void intc_irqpin_read_modify_write(struct intc_irqpin_priv *p,
+					  int reg, int shift,
+					  int width, int value)
+{
+	unsigned long flags;
+	unsigned long tmp;

drivers/irqchip/irq-renesas-intc-irqpin.c new file +5 -0

+static int intc_irqpin_probe(struct platform_device *pdev)
+{
+	struct renesas_intc_irqpin_config *pdata = pdev->dev.platform_data;
+	struct intc_irqpin_priv *p;
+	struct intc_irqpin_iomem *i;

drivers/irqchip/irq-renesas-intc-irqpin.c new file +55 -0

+	if (!p) {
+		dev_err(&pdev->dev, "failed to allocate driver data\n");
+		ret = -ENOMEM;
+		goto err0;
+	}
+
+	/* deal with driver instance configuration */
+	if (pdata)
+		memcpy(&p->config, pdata, sizeof(*pdata));
+	if (!p->config.sense_bitfield_width)
+		p->config.sense_bitfield_width = 4; /* default to 4 bits */
+
+	p->pdev = pdev;
+	platform_set_drvdata(pdev, p);
+
+	/* get hold of manadatory IOMEM */
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
+		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
+		if (!io[k]) {
+			dev_err(&pdev->dev, "not enough IOMEM resources\n");
+			ret = -EINVAL;
+			goto err1;
+		}
+	}
+
+	/* allow any number of IRQs between 1 and INTC_IRQPIN_MAX */
+	for (k = 0; k < INTC_IRQPIN_MAX; k++) {
+		irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
+		if (!irq)
+			break;
+
+		p->irq[k].hw_irq = k;
+		p->irq[k].p = p;
+		p->irq[k].irq = irq->start;
+	}
+
+	p->number_of_irqs = k;
+	if (p->number_of_irqs < 1) {
+		dev_err(&pdev->dev, "not enough IRQ resources\n");
+		ret = -EINVAL;
+		goto err1;
+	}
+
+	/* ioremap IOMEM and setup read/write callbacks */
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
+		i = &p->iomem[k];
+
+		switch (resource_size(io[k])) {
+		case 1:
+			i->width = 8;
+			i->read = intc_irqpin_read8;
+			i->write = intc_irqpin_write8;
+			break;
+		case 4:
+			i->width = 32;

drivers/irqchip/irq-renesas-intc-irqpin.c new file +32 -0

+		if (!i->iomem) {
+			dev_err(&pdev->dev, "failed to remap IOMEM\n");
+			ret = -ENXIO;
+			goto err2;
+		}
+	}
+
+	/* mask all interrupts using priority */
+	for (k = 0; k < p->number_of_irqs; k++)
+		intc_irqpin_mask_unmask_prio(p, k, 1);
+
+	/* use more severe masking method if requested */
+	if (p->config.control_parent) {
+		enable_fn = intc_irqpin_irq_enable_force;
+		disable_fn = intc_irqpin_irq_disable_force;
+	} else {
+		enable_fn = intc_irqpin_irq_enable;
+		disable_fn = intc_irqpin_irq_disable;
+	}
+
+	irq_chip = &p->irq_chip;
+	irq_chip->name = name;
+	irq_chip->irq_mask = disable_fn;
+	irq_chip->irq_unmask = enable_fn;
+	irq_chip->irq_enable = enable_fn;
+	irq_chip->irq_disable = disable_fn;
+	irq_chip->irq_set_type = intc_irqpin_irq_set_type;
+	irq_chip->flags	= IRQCHIP_SKIP_SET_WAKE;
+
+	p->irq_domain = irq_domain_add_simple(pdev->dev.of_node,
+					      p->number_of_irqs,
+					      p->config.irq_base,

drivers/irqchip/irq-renesas-intc-irqpin.c new file +20 -0

+				0, name, &p->irq[k])) {
+			dev_err(&pdev->dev, "failed to request low IRQ\n");
+			ret = -ENOENT;
+			goto err3;
+		}
+		intc_irqpin_mask_unmask_prio(p, k, 0);
+	}
+
+	dev_info(&pdev->dev, "driving %d irqs\n", p->number_of_irqs);
+
+	/* warn in case of mismatch if irq base is specified */
+	if (p->config.irq_base) {
+		k = irq_find_mapping(p->irq_domain, 0);
+		if (p->config.irq_base != k)
+			dev_warn(&pdev->dev, "irq base mismatch (%d/%d)\n",
+				 p->config.irq_base, k);
+	}
+	
+	return 0;
+

drivers/irqchip/irq-renesas-intc-irqpin.c new file +10 -0

+err0:
+	return ret;
+}
+
+static int intc_irqpin_remove(struct platform_device *pdev)
+{
+	struct intc_irqpin_priv *p = platform_get_drvdata(pdev);
+	int k;
+
+	for (k = 0; k < p->number_of_irqs; k++)

Hi Magnus

Thank you for this patch.
Small comment from me :)

Quoted 9 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +9 -0

+struct intc_irqpin_priv {
+     struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
+     struct intc_irqpin_irq irq[INTC_IRQPIN_MAX];
+     struct renesas_intc_irqpin_config config;
+     unsigned int number_of_irqs;
+     struct platform_device *pdev;
+     struct irq_chip irq_chip;
+     struct irq_domain *irq_domain;
+};

(snip)

Quoted 13 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +13 -0

+static DEFINE_RAW_SPINLOCK(intc_irqpin_lock); /* only used by slow path */
+
+static void intc_irqpin_read_modify_write(struct intc_irqpin_priv *p,
+                                       int reg, int shift,
+                                       int width, int value)
+{
+     unsigned long flags;
+     unsigned long tmp;
+
+     raw_spin_lock_irqsave(&intc_irqpin_lock, flags);
+
+     tmp = intc_irqpin_read(p, reg);
+     tmp &= ~(((1 << width) - 1) << shift);

drivers/irqchip/irq-renesas-intc-irqpin.c line 141

Quoted 5 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +5 -0

+     tmp |= value << shift;
+     intc_irqpin_write(p, reg, tmp);
+
+     raw_spin_unlock_irqrestore(&intc_irqpin_lock, flags);
+}

It is possible to include this spin lock into priv ? This local static spin lock seems not wrong, but looks strange ?

Quoted 12 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +12 -0

+static int intc_irqpin_probe(struct platform_device *pdev)
+{
+     struct renesas_intc_irqpin_config *pdata = pdev->dev.platform_data;
+     struct intc_irqpin_priv *p;
+     struct intc_irqpin_iomem *i;
+     struct resource *io[INTC_IRQPIN_REG_NR];
+     struct resource *irq;
+     struct irq_chip *irq_chip;
+     void (*enable_fn)(struct irq_data *d);
+     void (*disable_fn)(struct irq_data *d);
+     const char *name = dev_name(&pdev->dev);
+     int ret;

drivers/irqchip/irq-renesas-intc-irqpin.c line 283

Quoted 3 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +3 -0

+     int k;
+
+     p = kzalloc(sizeof(*p), GFP_KERNEL);

can you use devm_kzalloc() ?
devm_ioremap_nocache() or devm_request_and_ioremap()
Can you use devm_request_irq()
if you used devm_xxxx, you can remove kfree() / free_irq() / iounmap() here

include/linux/platform_data/irq-renesas-intc-irqpin.h 2013-02-18 18:28:24.000000000 +0900 line 2

Quoted 5 lines

include/linux/platform_data/irq-renesas-intc-irqpin.h 2013-02-18 18:28:24.000000000 +0900 new file +2 -0

@@ -0,0 +1,10 @@
+#ifndef __IRQ_RENESAS_INTC_IRQPIN_H__
+#define __IRQ_RENESAS_INTC_IRQPIN_H__

GPL license signage ?

drivers/irqchip/irq-renesas-intc-irqpin.c new file +13 -0

+	return i->read(i->iomem);
+}
+
+static inline void intc_irqpin_write(struct intc_irqpin_priv *p,
+				     int reg, unsigned long data)
+{
+	struct intc_irqpin_iomem *i = &p->iomem[reg];
+	i->write(i->iomem, data);
+}
+
+static inline unsigned long intc_irqpin_hwirq_mask(struct intc_irqpin_priv *p,
+						   int reg, int hw_irq)
+{

drivers/irqchip/irq-renesas-intc-irqpin.c new file +26 -0

+}
+
+static void intc_irqpin_irq_enable(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int hw_irq = irqd_to_hwirq(d);
+
+	intc_irqpin_dbg(&p->irq[hw_irq], "enable");
+	intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_CLEAR, hw_irq);
+}
+
+static void intc_irqpin_irq_disable(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int hw_irq = irqd_to_hwirq(d);
+
+	intc_irqpin_dbg(&p->irq[hw_irq], "disable");
+	intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_MASK, hw_irq);
+}
+
+static void intc_irqpin_irq_enable_force(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int irq = p->irq[irqd_to_hwirq(d)].irq;
+
+	intc_irqpin_irq_enable(d);

drivers/irqchip/irq-renesas-intc-irqpin.c new file +28 -0

+}
+
+#define INTC_IRQ_SENSE_VALID 0x10
+#define INTC_IRQ_SENSE(x) (x + INTC_IRQ_SENSE_VALID)
+
+static unsigned char intc_irqpin_sense[IRQ_TYPE_SENSE_MASK + 1] = {
+	[IRQ_TYPE_EDGE_FALLING] = INTC_IRQ_SENSE(0x00),
+	[IRQ_TYPE_EDGE_RISING] = INTC_IRQ_SENSE(0x01),
+	[IRQ_TYPE_LEVEL_LOW] = INTC_IRQ_SENSE(0x02),
+	[IRQ_TYPE_LEVEL_HIGH] = INTC_IRQ_SENSE(0x03),
+	[IRQ_TYPE_EDGE_BOTH] = INTC_IRQ_SENSE(0x04),
+};
+
+static int intc_irqpin_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	unsigned char value = intc_irqpin_sense[type & IRQ_TYPE_SENSE_MASK];
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+
+	if (!(value & INTC_IRQ_SENSE_VALID))
+		return -EINVAL;
+
+	return intc_irqpin_set_sense(p, irqd_to_hwirq(d),
+				     value ^ INTC_IRQ_SENSE_VALID);
+}
+
+static irqreturn_t intc_irqpin_irq_handler(int irq, void *dev_id)
+{
+	struct intc_irqpin_irq *i = dev_id;

drivers/irqchip/irq-renesas-intc-irqpin.c new file +4 -0

+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
+}

Magnus,

drivers/irqchip/irq-renesas-intc-irqpin.c line 101

Quoted 4 lines · Magnus Damm

drivers/irqchip/irq-renesas-intc-irqpin.c new file +4 -0

+static inline unsigned long intc_irqpin_read(struct intc_irqpin_priv *p,
+                                          int reg)
+{
+     struct intc_irqpin_iomem *i = &p->iomem[reg];

Newline between variable and code please.

Quoted 17 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +17 -0

+     return i->read(i->iomem);
+}
+
+static inline void intc_irqpin_write(struct intc_irqpin_priv *p,
+                                  int reg, unsigned long data)
+{
+     struct intc_irqpin_iomem *i = &p->iomem[reg];
+     i->write(i->iomem, data);
+}
+
+static inline unsigned long intc_irqpin_hwirq_mask(struct intc_irqpin_priv *p,
+                                                int reg, int hw_irq)
+{
+     return BIT((p->iomem[reg].width - 1) - hw_irq);
+}
+
+static inline void intc_irqpin_irq_write_hwirq(struct intc_irqpin_priv *p,

drivers/irqchip/irq-renesas-intc-irqpin.c line 124

Quoted 6 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +6 -0

+                                            int reg, int hw_irq)
+{
+     intc_irqpin_write(p, reg, intc_irqpin_hwirq_mask(p, reg, hw_irq));
+}
+
+static DEFINE_RAW_SPINLOCK(intc_irqpin_lock); /* only used by slow path */

Shouldn't the lock be part of struct intc_irqpin_priv ?

Quoted 7 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +7 -0

+static void intc_irqpin_read_modify_write(struct intc_irqpin_priv *p,
+                                       int reg, int shift,
+                                       int width, int value)
+{
+     unsigned long flags;
+     unsigned long tmp;
+

drivers/irqchip/irq-renesas-intc-irqpin.c line 173

Quoted 6 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +6 -0

+     raw_spin_lock_irqsave(&intc_irqpin_lock, flags);
+static void intc_irqpin_dbg(struct intc_irqpin_irq *i, char *str)
+{
+     dev_dbg(&i->p->pdev->dev, "%s (%d:%d:%d)\n",
+             str, i->irq, i->hw_irq,
+             irq_find_mapping(i->p->irq_domain, i->hw_irq));

Do you really want to do a lookup for each debug invocation?

Quoted 33 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +33 -0

+}
+
+static void intc_irqpin_irq_enable(struct irq_data *d)
+{
+     struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+     int hw_irq = irqd_to_hwirq(d);
+
+     intc_irqpin_dbg(&p->irq[hw_irq], "enable");
+     intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_CLEAR, hw_irq);
+}
+
+static void intc_irqpin_irq_disable(struct irq_data *d)
+{
+     struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+     int hw_irq = irqd_to_hwirq(d);
+
+     intc_irqpin_dbg(&p->irq[hw_irq], "disable");
+     intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_MASK, hw_irq);
+}
+
+static void intc_irqpin_irq_enable_force(struct irq_data *d)
+{
+     struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+     int irq = p->irq[irqd_to_hwirq(d)].irq;
+
+     intc_irqpin_irq_enable(d);
+     irq_get_chip(irq)->irq_unmask(irq_get_irq_data(irq));
+}
+
+static void intc_irqpin_irq_disable_force(struct irq_data *d)
+{
+     struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+     int irq = p->irq[irqd_to_hwirq(d)].irq;

drivers/irqchip/irq-renesas-intc-irqpin.c line 209

Quoted 3 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +3 -0

+
+     irq_get_chip(irq)->irq_mask(irq_get_irq_data(irq));
+     intc_irqpin_irq_disable(d);

Hmm. If I understand that code correctly, then the _force functions are (un)masking another interrupt line. But this happens without holding irq_desc[irq]->lock. Looks unsafe at least and if correct wants a big fat comment.

Quoted 32 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +32 -0

+}
+
+#define INTC_IRQ_SENSE_VALID 0x10
+#define INTC_IRQ_SENSE(x) (x + INTC_IRQ_SENSE_VALID)
+
+static unsigned char intc_irqpin_sense[IRQ_TYPE_SENSE_MASK + 1] = {
+     [IRQ_TYPE_EDGE_FALLING] = INTC_IRQ_SENSE(0x00),
+     [IRQ_TYPE_EDGE_RISING] = INTC_IRQ_SENSE(0x01),
+     [IRQ_TYPE_LEVEL_LOW] = INTC_IRQ_SENSE(0x02),
+     [IRQ_TYPE_LEVEL_HIGH] = INTC_IRQ_SENSE(0x03),
+     [IRQ_TYPE_EDGE_BOTH] = INTC_IRQ_SENSE(0x04),
+};
+
+static int intc_irqpin_irq_set_type(struct irq_data *d, unsigned int type)
+{
+     unsigned char value = intc_irqpin_sense[type & IRQ_TYPE_SENSE_MASK];
+     struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+
+     if (!(value & INTC_IRQ_SENSE_VALID))
+             return -EINVAL;
+
+     return intc_irqpin_set_sense(p, irqd_to_hwirq(d),
+                                  value ^ INTC_IRQ_SENSE_VALID);
+}
+
+static irqreturn_t intc_irqpin_irq_handler(int irq, void *dev_id)
+{
+     struct intc_irqpin_irq *i = dev_id;
+     struct intc_irqpin_priv *p = i->p;
+     unsigned long bit;
+
+     intc_irqpin_dbg(i, "demux1");

drivers/irqchip/irq-renesas-intc-irqpin.c line 247

Quoted 6 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +6 -0

+     bit = intc_irqpin_hwirq_mask(p, INTC_IRQPIN_REG_SOURCE, i->hw_irq);
+
+     if (intc_irqpin_read(p, INTC_IRQPIN_REG_SOURCE) & bit) {
+             intc_irqpin_write(p, INTC_IRQPIN_REG_SOURCE, ~bit);
+             intc_irqpin_dbg(i, "demux2");
+             generic_handle_irq(irq_find_mapping(p->irq_domain, i->hw_irq));

Shouldn't you cache that mapping value somewhere ?

Quoted 8 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +8 -0

+             return IRQ_HANDLED;
+     }
+     return IRQ_NONE;
+}
+
+static int intc_irqpin_irq_domain_map(struct irq_domain *h, unsigned int virq,
+                                   irq_hw_number_t hw)
+{

drivers/irqchip/irq-renesas-intc-irqpin.c line 261

Quoted 6 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +6 -0

+     struct intc_irqpin_priv *p = h->host_data;
+
+     intc_irqpin_dbg(&p->irq[hw], "map");
+     irq_set_chip_data(virq, h->host_data);
+     irq_set_chip_and_handler(virq, &p->irq_chip, handle_level_irq);
+     set_irq_flags(virq, IRQF_VALID); /* kill me now */

What needs to be killed? -ENOPARSE

Quoted 3 lines · Thomas Gleixner

Quoted 1 line

+static DEFINE_RAW_SPINLOCK(intc_irqpin_lock); /* only used by slow path */

Shouldn't the lock be part of struct intc_irqpin_priv ?

Good idea, but I need to lock access to the SENSE register against multiple driver instances. This is not the case for PRIO. But because both PRIO and SENSE are accessed in the slow path I decided to be lazy and use the same way of locking to reduce the code size.

Quoted 13 lines

Quoted 13 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +13 -0

+static void intc_irqpin_irq_enable_force(struct irq_data *d)
+{
+     struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+     int irq = p->irq[irqd_to_hwirq(d)].irq;
+
+     intc_irqpin_irq_enable(d);
+     irq_get_chip(irq)->irq_unmask(irq_get_irq_data(irq));
+}
+
+static void intc_irqpin_irq_disable_force(struct irq_data *d)
+{
+     struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+     int irq = p->irq[irqd_to_hwirq(d)].irq;

Quoted 8 lines

drivers/irqchip/irq-renesas-intc-irqpin.c line 209

Quoted 3 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +3 -0

+
+     irq_get_chip(irq)->irq_mask(irq_get_irq_data(irq));
+     intc_irqpin_irq_disable(d);

Hmm. If I understand that code correctly, then the _force functions are (un)masking another interrupt line. But this happens without holding irq_desc[irq]->lock. Looks unsafe at least and if correct wants a big fat comment.

On some SoCs the masking for some IRQs seems busted, and the only sane way to work around that is to (un)mask the parent interrupt. The parent happens to be the GIC. I will look into how to add locking.

Quoted 4 lines

drivers/irqchip/irq-renesas-intc-irqpin.c line 261

Quoted 2 lines

drivers/irqchip/irq-renesas-intc-irqpin.c new file +2 -0

+     irq_set_chip_and_handler(virq, &p->irq_chip, handle_level_irq);
+     set_irq_flags(virq, IRQF_VALID); /* kill me now */

What needs to be killed? -ENOPARSE

I'd like to not have to set this flag in my interrupt code.

I've written interrupt code on other architectures before, and from my experience only ARM requires the IRQF_VALID flag to be set because the ARM architecture software believes it is a special case. I may be behind times - I have to admit that I've not checked the latest state
- this flag may not be needed anymore, hurray if so - but at least a couple of years ago it was needed in case of ARM for our shared INTC code (shared between sh and ARM).

What is your opinion about this matter?

From: Magnus Damm <[email protected]>

This patch adds a driver for external IRQ pins connected to the INTC block on recent SoCs from Renesas.

Quoted 5 lines · Magnus Damm

From: Magnus Damm <[email protected]>

This patch adds a driver for external IRQ pins connected to the INTC block on recent SoCs from Renesas.

So how exactly does this interact with the existing sh_intc code? Or is there some reason why you have opted to bypass it in order to implement a simplified reduced-functionality version of INTC support focused only on external pins? If both are used together this is going to be a nightmare for locking, and it's also non-obvious how the IRQ domains on both sides will interact.

This needs a lot more explanation.

Quoted 8 lines · Paul Mundt

So how exactly does this interact with the existing sh_intc code? Or is there some reason why you have opted to bypass it in order to implement a simplified reduced-functionality version of INTC support focused only on external pins? If both are used together this is going to be a nightmare for locking, and it's also non-obvious how the IRQ domains on both sides will interact.

This needs a lot more explanation.

Recent GIC-based SoCs do not make use of INTC for any on-chip I/O devices. This driver is meant to be used as a layer between the actual IRQ pin and the GIC. Anything else needs the full driver. The existing non-DT INTC driver can happily coexist with this driver like it does in the case of sh73a0 here:

[PATCH 02/03] ARM: shmobile: INTC External IRQ pin driver on sh73a0

The driver is not meant to be used with INTC-only based systems like sh7372 and the SH architecture. I would be very happy if someone could get their shit together and fix up DT support for the common INTC code. This has not happened yet though. So if you know anyone with time to spare then feel free to suggest them to work together with Iwamatsu-san to get the DT version of the code reviewed together with Linaro.

Quoted 16 lines · Magnus Damm

Quoted 8 lines · Paul Mundt

So how exactly does this interact with the existing sh_intc code? Or is there some reason why you have opted to bypass it in order to implement a simplified reduced-functionality version of INTC support focused only on external pins? If both are used together this is going to be a nightmare for locking, and it's also non-obvious how the IRQ domains on both sides will interact.

This needs a lot more explanation.

Recent GIC-based SoCs do not make use of INTC for any on-chip I/O devices. This driver is meant to be used as a layer between the actual IRQ pin and the GIC. Anything else needs the full driver. The existing non-DT INTC driver can happily coexist with this driver like it does in the case of sh73a0 here:

Quoted 2 lines

[PATCH 02/03] ARM: shmobile: INTC External IRQ pin driver on sh73a0

Ok, thanks for clarifying.

I suppose the main concern is how quickly this will simply turn in to a deviated partial implementation of the full driver as newer SoCs begin deviating from your simplified case, and we basically end up reimplementing sh_intc anyways.

Quoted 8 lines

The driver is not meant to be used with INTC-only based systems like sh7372 and the SH architecture. I would be very happy if someone could get their shit together and fix up DT support for the common INTC code. This has not happened yet though. So if you know anyone with time to spare then feel free to suggest them to work together with Iwamatsu-san to get the DT version of the code reviewed together with Linaro.

I haven't heard or seen anything new on that in some time, so I assumed the work had stalled. I'm not sure why there wasn't more effort put in to DT support for the INTC code rather than simply coming up with a temporary bypass shim, and I'm not sure why you think this work is blocked by anyone (unless you're just referring to a general lack of resources).

In any event, I'm not sure what the best option for the interim is. I suppose we can merge the irqchips until the INTC stuff catches up, but then we are probaby going to run in to a situation where they either have to co-exist, or the irqchips are removed and the sh_intc code has to carry a compat shim to deal with those DT bindings. Neither of those options seem particularly appealing to me.

As you know, the INTC code that you are referring to is a full interrupt controller designed to work directly with CPU cores like SH and ARM. Newer ARM cores like Cortex-A9 all include the GIC both for IPI purpose in case of SMP and they also implement SPI functionality for I/O device interrupts. So the need for vendor-specific interrupt controller IP is almost down to nothing these days.

With that in mind I do really doubt that we end up reimplementing sh_intc. The upcoming designs that I am aware of do not change their external IRQ pin hardware to force us to update this driver. What happens after that I'm afraid I can't say. =)

I mainly wrote this driver to support the r8a7779 SoC which can't be driven by the existing INTC code base. During development I decided to try to share the driver between multiple GIC-based SoCs like r8a7779 and sh73a0. The reason behind trying to share this driver between multiple SoCs is to remove SoC-specific hacks that can't be dealt with by the existing INTC code.

I don't really understand why you would want to use a generic table driven driver when you have the possibility of using a hardware-specific driver.

I suppose you are wondering why no one seems to be working on INTC DT support at this point. The truth is that we got very little feedback and development support with interrupts in general last autumn and on top of that our developers went their own way instead of following advice.

Quoted 7 lines · Magnus Damm

As you know, the INTC code that you are referring to is a full interrupt controller designed to work directly with CPU cores like SH and ARM. Newer ARM cores like Cortex-A9 all include the GIC both for IPI purpose in case of SMP and they also implement SPI functionality for I/O device interrupts. So the need for vendor-specific interrupt controller IP is almost down to nothing these days.

Yes, I'm aware of that.

Quoted 5 lines

With that in mind I do really doubt that we end up reimplementing sh_intc. The upcoming designs that I am aware of do not change their external IRQ pin hardware to force us to update this driver. What happens after that I'm afraid I can't say. =)

While I don't expect you would end up with a full reimplmentation, my concern is more that it would just be reproducing stuff that's already in place. IOW, perhaps it's less work to put sh_intc on a diet, and tie the core functionality that you need for external IRQ pins in to an irqchip there -- with the core of the old code adapted in to some sort of common base library, rather than coming up with a new lightweight irqchip driver and having to incrementally pile stuff on top of it later.

Quoted 7 lines

I mainly wrote this driver to support the r8a7779 SoC which can't be driven by the existing INTC code base. During development I decided to try to share the driver between multiple GIC-based SoCs like r8a7779 and sh73a0. The reason behind trying to share this driver between multiple SoCs is to remove SoC-specific hacks that can't be dealt with by the existing INTC code.

Ok, fair enough.

Quoted 3 lines

I don't really understand why you would want to use a generic table driven driver when you have the possibility of using a hardware-specific driver.

For the same reason sh_intc was written in the first place, every CPU subtype more or less had a similar set of interrupt controllers with minor deviations. Those deviations were sufficient enough to make the code pretty hairy in places, and what we have now is really more of a subsystem than an irqchip driver.

Having to reproduce 90% of the code when you're adding new CPUs at the rate of 2 a month hardly makes an SoC-specific driver realistic, especially as you just end up with identical features being broken in subtly differnt ways on different SoCs. That being said, a lot of that is legacy stuff and a result of no CPU team talking to any other CPU team ever, and it seems like things have improved enough in that regard that perhaps a hardware-specific driver won't be a complete throw-away disaster like it was before. It's a risk either way, I just hope your lightweight solution remains lightweight and consistent long enough that we don't have multiple copies of slightly different drivers doing exactly same thing spiralling out of control and turning in to a maintenance nightmare.

If sh_intc doesn't deal with the needs of the new GIC-backed SoCs then that's of course something that has to be addressed regardless (whether that be hacking up sh_intc or adding new hardware-specific irqchips).

Quoted 6 lines

I suppose you are wondering why no one seems to be working on INTC DT support at this point. The truth is that we got very little feedback and development support with interrupts in general last autumn and on top of that our developers went their own way instead of following advice.

I assumed it was either being rewritten or had already been merged, so I was simply surprised to hear otherwise. I will dig through the archives a bit later and see about getting caught up.

From: Magnus Damm <[email protected]>

This patch adds a driver for external IRQ pins connected to the INTC block on recent SoCs from Renesas.

The INTC hardware block usually contains a rather wide range of features ranging from external IRQ pin handling to legacy interrupt controller support. On older SoCs the INTC is used as a general purpose interrupt controller both for external IRQ pins and on-chip devices.

On more recent ARM based SoCs with Cortex-A9 the main interrupt controller is the GIC, but IRQ trigger setup still need to happen in the INTC hardware block.

This driver implements the glue code needed to configure IRQ trigger and also handle mask/unmask and demux of external IRQ pins hooked up from the INTC to the GIC.

Tested on sh73a0 and r8a7779. The hardware varies quite a bit with SoC model, for instance register width and bitfield widths vary wildly. The driver requires one GIC SPI per external IRQ pin to operate. Each driver instance will handle up to 8 external IRQ pins.

The SoCs using this driver are currently mainly used together with regular platform devices so this driver allows configuration via platform data to support things like static interrupt base address. DT support will be added incrementally in the not so distant future.

Signed-off-by Magnus Damm <[email protected]>

---

 drivers/irqchip/Kconfig                               |    4 
 drivers/irqchip/Makefile                              |    1 
 drivers/irqchip/irq-renesas-intc-irqpin.c             |  464 +++++++++++++++++
 include/linux/platform_data/irq-renesas-intc-irqpin.h |   10 
 4 files changed, 479 insertions(+)

drivers/irqchip/Kconfig +4 -0

@@ -25,6 +25,10 @@ config ARM_VIC_NR
 	  The maximum number of VICs available in the system, for
 	  power management.
 
+config RENESAS_INTC_IRQPIN
+	bool
+	select IRQ_DOMAIN
+
 config VERSATILE_FPGA_IRQ
 	bool
 	select IRQ_DOMAIN

drivers/irqchip/Makefile +1 -0

@@ -5,4 +5,5 @@ obj-$(CONFIG_ARCH_SUNXI)		+= irq-sunxi.o
 obj-$(CONFIG_ARCH_SPEAR3XX)		+= spear-shirq.o
 obj-$(CONFIG_ARM_GIC)			+= irq-gic.o
 obj-$(CONFIG_ARM_VIC)			+= irq-vic.o
+obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o

drivers/irqchip/irq-renesas-intc-irqpin.c new file +464 -0

@@ -0,0 +1,464 @@
+/*
+ * Renesas INTC External IRQ Pin Driver
+ *
+ *  Copyright (C) 2013 Magnus Damm
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/platform_data/irq-renesas-intc-irqpin.h>
+
+#define INTC_IRQPIN_MAX 8 /* maximum 8 interrupts per driver instance */
+
+#define INTC_IRQPIN_REG_SENSE 0 /* ICRn */
+#define INTC_IRQPIN_REG_PRIO 1 /* INTPRInn */
+#define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
+#define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
+#define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
+#define INTC_IRQPIN_REG_NR 5
+
+/* INTC external IRQ PIN hardware register access:
+ *
+ * SENSE is read-write 32-bit with 2-bits or 4-bits per IRQ (*)
+ * PRIO is read-write 32-bit with 4-bits per IRQ (**)
+ * SOURCE is read-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ * MASK is write-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ * CLEAR is write-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ *
+ * (*) May be accessed by more than one driver instance - lock needed
+ * (**) Read-modify-write access by one driver instance - lock needed
+ * (***) Accessed by one driver instance only - no locking needed
+ */
+
+struct intc_irqpin_iomem {
+	void __iomem *iomem;
+	unsigned long (*read)(void __iomem *iomem);
+	void (*write)(void __iomem *iomem, unsigned long data);
+	int width;
+};  
+
+struct intc_irqpin_irq {
+	int hw_irq;
+	int irq;
+	struct intc_irqpin_priv *p;
+};  
+
+struct intc_irqpin_priv {
+	struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
+	struct intc_irqpin_irq irq[INTC_IRQPIN_MAX];
+	struct renesas_intc_irqpin_config config;
+	unsigned int number_of_irqs;
+	struct platform_device *pdev;
+	struct irq_chip irq_chip;
+	struct irq_domain *irq_domain;
+};
+
+static unsigned long intc_irqpin_read32(void __iomem *iomem)
+{
+	return ioread32(iomem);
+}
+
+static unsigned long intc_irqpin_read8(void __iomem *iomem)
+{
+	return ioread8(iomem);
+}
+
+static void intc_irqpin_write32(void __iomem *iomem, unsigned long data)
+{
+	iowrite32(data, iomem);
+}
+
+static void intc_irqpin_write8(void __iomem *iomem, unsigned long data)
+{
+	iowrite8(data, iomem);
+}
+
+static inline unsigned long intc_irqpin_read(struct intc_irqpin_priv *p,
+					     int reg)
+{
+	struct intc_irqpin_iomem *i = &p->iomem[reg];
+	return i->read(i->iomem);
+}
+
+static inline void intc_irqpin_write(struct intc_irqpin_priv *p,
+				     int reg, unsigned long data)
+{
+	struct intc_irqpin_iomem *i = &p->iomem[reg];
+	i->write(i->iomem, data);
+}
+
+static inline unsigned long intc_irqpin_hwirq_mask(struct intc_irqpin_priv *p,
+						   int reg, int hw_irq)
+{
+	return BIT((p->iomem[reg].width - 1) - hw_irq);
+}
+
+static inline void intc_irqpin_irq_write_hwirq(struct intc_irqpin_priv *p,
+					       int reg, int hw_irq)
+{
+	intc_irqpin_write(p, reg, intc_irqpin_hwirq_mask(p, reg, hw_irq));
+}
+
+static DEFINE_RAW_SPINLOCK(intc_irqpin_lock); /* only used by slow path */
+
+static void intc_irqpin_read_modify_write(struct intc_irqpin_priv *p,
+					  int reg, int shift,
+					  int width, int value)
+{
+	unsigned long flags;
+	unsigned long tmp;
+
+	raw_spin_lock_irqsave(&intc_irqpin_lock, flags);
+
+	tmp = intc_irqpin_read(p, reg);
+	tmp &= ~(((1 << width) - 1) << shift);
+	tmp |= value << shift;
+	intc_irqpin_write(p, reg, tmp);
+
+	raw_spin_unlock_irqrestore(&intc_irqpin_lock, flags);
+}
+
+static void intc_irqpin_mask_unmask_prio(struct intc_irqpin_priv *p,
+					 int irq, int do_mask)
+{
+	int bitfield_width = 4; /* PRIO assumed to have fixed bitfield width */
+	int shift = (7 - irq) * bitfield_width; /* PRIO assumed to be 32-bit */
+
+	intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_PRIO,
+				      shift, bitfield_width,
+				      do_mask ? 0 : (1 << bitfield_width) - 1);
+}
+
+static int intc_irqpin_set_sense(struct intc_irqpin_priv *p, int irq, int value)
+{
+	int bitfield_width = p->config.sense_bitfield_width;
+	int shift = (7 - irq) * bitfield_width; /* SENSE assumed to be 32-bit */
+
+	dev_dbg(&p->pdev->dev, "sense irq = %d, mode = %d\n", irq, value);
+
+	if (value >= (1 << bitfield_width))
+		return -EINVAL;
+
+	intc_irqpin_read_modify_write(p, INTC_IRQPIN_REG_SENSE, shift,
+				      bitfield_width, value);
+	return 0;
+}
+
+static void intc_irqpin_dbg(struct intc_irqpin_irq *i, char *str)
+{
+	dev_dbg(&i->p->pdev->dev, "%s (%d:%d:%d)\n",
+		str, i->irq, i->hw_irq,
+		irq_find_mapping(i->p->irq_domain, i->hw_irq));
+}
+
+static void intc_irqpin_irq_enable(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int hw_irq = irqd_to_hwirq(d);
+
+	intc_irqpin_dbg(&p->irq[hw_irq], "enable");
+	intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_CLEAR, hw_irq);
+}
+
+static void intc_irqpin_irq_disable(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int hw_irq = irqd_to_hwirq(d);
+
+	intc_irqpin_dbg(&p->irq[hw_irq], "disable");
+	intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_MASK, hw_irq);
+}
+
+static void intc_irqpin_irq_enable_force(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int irq = p->irq[irqd_to_hwirq(d)].irq;
+
+	intc_irqpin_irq_enable(d);
+	irq_get_chip(irq)->irq_unmask(irq_get_irq_data(irq));
+}
+
+static void intc_irqpin_irq_disable_force(struct irq_data *d)
+{
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+	int irq = p->irq[irqd_to_hwirq(d)].irq;
+
+	irq_get_chip(irq)->irq_mask(irq_get_irq_data(irq));
+	intc_irqpin_irq_disable(d);
+}
+
+#define INTC_IRQ_SENSE_VALID 0x10
+#define INTC_IRQ_SENSE(x) (x + INTC_IRQ_SENSE_VALID)
+
+static unsigned char intc_irqpin_sense[IRQ_TYPE_SENSE_MASK + 1] = {
+	[IRQ_TYPE_EDGE_FALLING] = INTC_IRQ_SENSE(0x00),
+	[IRQ_TYPE_EDGE_RISING] = INTC_IRQ_SENSE(0x01),
+	[IRQ_TYPE_LEVEL_LOW] = INTC_IRQ_SENSE(0x02),
+	[IRQ_TYPE_LEVEL_HIGH] = INTC_IRQ_SENSE(0x03),
+	[IRQ_TYPE_EDGE_BOTH] = INTC_IRQ_SENSE(0x04),
+};
+
+static int intc_irqpin_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	unsigned char value = intc_irqpin_sense[type & IRQ_TYPE_SENSE_MASK];
+	struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
+
+	if (!(value & INTC_IRQ_SENSE_VALID))
+		return -EINVAL;
+
+	return intc_irqpin_set_sense(p, irqd_to_hwirq(d),
+				     value ^ INTC_IRQ_SENSE_VALID);
+}
+
+static irqreturn_t intc_irqpin_irq_handler(int irq, void *dev_id)
+{
+	struct intc_irqpin_irq *i = dev_id;
+	struct intc_irqpin_priv *p = i->p;
+	unsigned long bit;
+
+	intc_irqpin_dbg(i, "demux1");
+	bit = intc_irqpin_hwirq_mask(p, INTC_IRQPIN_REG_SOURCE, i->hw_irq);
+
+	if (intc_irqpin_read(p, INTC_IRQPIN_REG_SOURCE) & bit) {
+		intc_irqpin_write(p, INTC_IRQPIN_REG_SOURCE, ~bit);
+		intc_irqpin_dbg(i, "demux2");
+		generic_handle_irq(irq_find_mapping(p->irq_domain, i->hw_irq));
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
+}
+
+static int intc_irqpin_irq_domain_map(struct irq_domain *h, unsigned int virq,
+				      irq_hw_number_t hw)
+{
+	struct intc_irqpin_priv *p = h->host_data;
+
+	intc_irqpin_dbg(&p->irq[hw], "map");
+	irq_set_chip_data(virq, h->host_data);
+	irq_set_chip_and_handler(virq, &p->irq_chip, handle_level_irq);
+	set_irq_flags(virq, IRQF_VALID); /* kill me now */
+	return 0;
+}
+
+static struct irq_domain_ops intc_irqpin_irq_domain_ops = {
+	.map	= intc_irqpin_irq_domain_map,
+};
+
+static int intc_irqpin_probe(struct platform_device *pdev)
+{
+	struct renesas_intc_irqpin_config *pdata = pdev->dev.platform_data;
+	struct intc_irqpin_priv *p;
+	struct intc_irqpin_iomem *i;
+	struct resource *io[INTC_IRQPIN_REG_NR];
+	struct resource *irq;
+	struct irq_chip *irq_chip;
+	void (*enable_fn)(struct irq_data *d);
+	void (*disable_fn)(struct irq_data *d);
+	const char *name = dev_name(&pdev->dev);
+	int ret;
+	int k;
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p) {
+		dev_err(&pdev->dev, "failed to allocate driver data\n");
+		ret = -ENOMEM;
+		goto err0;
+	}
+
+	/* deal with driver instance configuration */
+	if (pdata)
+		memcpy(&p->config, pdata, sizeof(*pdata));
+	if (!p->config.sense_bitfield_width)
+		p->config.sense_bitfield_width = 4; /* default to 4 bits */
+
+	p->pdev = pdev;
+	platform_set_drvdata(pdev, p);
+
+	/* get hold of manadatory IOMEM */
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
+		io[k] = platform_get_resource(pdev, IORESOURCE_MEM, k);
+		if (!io[k]) {
+			dev_err(&pdev->dev, "not enough IOMEM resources\n");
+			ret = -EINVAL;
+			goto err1;
+		}
+	}
+
+	/* allow any number of IRQs between 1 and INTC_IRQPIN_MAX */
+	for (k = 0; k < INTC_IRQPIN_MAX; k++) {
+		irq = platform_get_resource(pdev, IORESOURCE_IRQ, k);
+		if (!irq)
+			break;
+
+		p->irq[k].hw_irq = k;
+		p->irq[k].p = p;
+		p->irq[k].irq = irq->start;
+	}
+
+	p->number_of_irqs = k;
+	if (p->number_of_irqs < 1) {
+		dev_err(&pdev->dev, "not enough IRQ resources\n");
+		ret = -EINVAL;
+		goto err1;
+	}
+
+	/* ioremap IOMEM and setup read/write callbacks */
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++) {
+		i = &p->iomem[k];
+
+		switch (resource_size(io[k])) {
+		case 1:
+			i->width = 8;
+			i->read = intc_irqpin_read8;
+			i->write = intc_irqpin_write8;
+			break;
+		case 4:
+			i->width = 32;
+			i->read = intc_irqpin_read32;
+			i->write = intc_irqpin_write32;
+			break;
+		default:
+			dev_err(&pdev->dev, "IOMEM size mismatch\n");
+			ret = -EINVAL;
+			goto err2;
+		}
+
+		i->iomem = ioremap_nocache(io[k]->start, resource_size(io[k]));
+		if (!i->iomem) {
+			dev_err(&pdev->dev, "failed to remap IOMEM\n");
+			ret = -ENXIO;
+			goto err2;
+		}
+	}
+
+	/* mask all interrupts using priority */
+	for (k = 0; k < p->number_of_irqs; k++)
+		intc_irqpin_mask_unmask_prio(p, k, 1);
+
+	/* use more severe masking method if requested */
+	if (p->config.control_parent) {
+		enable_fn = intc_irqpin_irq_enable_force;
+		disable_fn = intc_irqpin_irq_disable_force;
+	} else {
+		enable_fn = intc_irqpin_irq_enable;
+		disable_fn = intc_irqpin_irq_disable;
+	}
+
+	irq_chip = &p->irq_chip;
+	irq_chip->name = name;
+	irq_chip->irq_mask = disable_fn;
+	irq_chip->irq_unmask = enable_fn;
+	irq_chip->irq_enable = enable_fn;
+	irq_chip->irq_disable = disable_fn;
+	irq_chip->irq_set_type = intc_irqpin_irq_set_type;
+	irq_chip->flags	= IRQCHIP_SKIP_SET_WAKE;
+
+	p->irq_domain = irq_domain_add_simple(pdev->dev.of_node,
+					      p->number_of_irqs,
+					      p->config.irq_base,
+					      &intc_irqpin_irq_domain_ops, p);
+	if (!p->irq_domain) {
+		ret = -ENXIO;
+		dev_err(&pdev->dev, "cannot initialize irq domain\n");
+		goto err2;
+	}
+
+	/* request and set priority on interrupts one by one */
+	for (k = 0; k < p->number_of_irqs; k++) {
+		if (request_irq(p->irq[k].irq, intc_irqpin_irq_handler,
+				0, name, &p->irq[k])) {
+			dev_err(&pdev->dev, "failed to request low IRQ\n");
+			ret = -ENOENT;
+			goto err3;
+		}
+		intc_irqpin_mask_unmask_prio(p, k, 0);
+	}
+
+	dev_info(&pdev->dev, "driving %d irqs\n", p->number_of_irqs);
+
+	/* warn in case of mismatch if irq base is specified */
+	if (p->config.irq_base) {
+		k = irq_find_mapping(p->irq_domain, 0);
+		if (p->config.irq_base != k)
+			dev_warn(&pdev->dev, "irq base mismatch (%d/%d)\n",
+				 p->config.irq_base, k);
+	}
+	
+	return 0;
+
+err3:
+	for (; k >= 0; k--)
+		free_irq(p->irq[k - 1].irq, &p->irq[k - 1]);
+
+	irq_domain_remove(p->irq_domain);
+err2:
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++)
+		iounmap(p->iomem[k].iomem);
+err1:
+	kfree(p);
+err0:
+	return ret;
+}
+
+static int intc_irqpin_remove(struct platform_device *pdev)
+{
+	struct intc_irqpin_priv *p = platform_get_drvdata(pdev);
+	int k;
+
+	for (k = 0; k < p->number_of_irqs; k++)
+		free_irq(p->irq[k].irq, &p->irq[k]);
+
+	irq_domain_remove(p->irq_domain);
+
+	for (k = 0; k < INTC_IRQPIN_REG_NR; k++)
+		iounmap(p->iomem[k].iomem);
+
+	kfree(p);
+	return 0;
+}
+
+static struct platform_driver intc_irqpin_device_driver = {
+	.probe		= intc_irqpin_probe,
+	.remove		= intc_irqpin_remove,
+	.driver		= {
+		.name	= "renesas_intc_irqpin",
+	}
+};
+
+static int __init intc_irqpin_init(void)
+{
+	return platform_driver_register(&intc_irqpin_device_driver);
+}
+postcore_initcall(intc_irqpin_init);
+
+static void __exit intc_irqpin_exit(void)
+{
+	platform_driver_unregister(&intc_irqpin_device_driver);
+}
+module_exit(intc_irqpin_exit);
+
+MODULE_AUTHOR("Magnus Damm");
+MODULE_DESCRIPTION("Renesas INTC External IRQ Pin Driver");
+MODULE_LICENSE("GPL v2");

include/linux/platform_data/irq-renesas-intc-irqpin.h new file +10 -0

@@ -0,0 +1,10 @@
+#ifndef __IRQ_RENESAS_INTC_IRQPIN_H__
+#define __IRQ_RENESAS_INTC_IRQPIN_H__
+
+struct renesas_intc_irqpin_config {
+	unsigned int sense_bitfield_width;
+	unsigned int irq_base;
+	bool control_parent;
+};
+
+#endif /* __IRQ_RENESAS_INTC_IRQPIN_H__ */

Quoted 5 lines · Magnus Damm

The SoCs using this driver are currently mainly used together with regular platform devices so this driver allows configuration via platform data to support things like static interrupt base address. DT support will be added incrementally in the not so distant future.

Hi Thomas,

I'm wondering how you would like to handle merging this driver. I can think of three options but I'm sure there are others.

* You can take the patches (there is a follow-up series) yourself.
* I can prepare a pull request for you
* I can prepare a pull request for arm-soc with the shmobile patches that enable the driver on the r8a7779 and sh73a0.

The last option is possibly the easiest.

But in that case I'd appreciate an Ack from you on this patch.

Quoted 12 lines · Simon Horman

Quoted 5 lines · Magnus Damm

The SoCs using this driver are currently mainly used together with regular platform devices so this driver allows configuration via platform data to support things like static interrupt base address. DT support will be added incrementally in the not so distant future.

Hi Thomas,

I'm wondering how you would like to handle merging this driver. I can think of three options but I'm sure there are others.

Quoted 6 lines

* You can take the patches (there is a follow-up series) yourself.
* I can prepare a pull request for you
* I can prepare a pull request for arm-soc with the shmobile patches that enable the driver on the r8a7779 and sh73a0.

The last option is possibly the easiest.

Correct.

Quoted 1 line

But in that case I'd appreciate an Ack from you on this patch.

You want to pick the V2 series, which already has my blessing:

    https://lkml.org/lkml/2013/2/26/305

For merging it through arm-soc you have my ack now :)

Quoted 26 lines · Thomas Gleixner

Quoted 12 lines · Simon Horman

Quoted 5 lines · Magnus Damm

The SoCs using this driver are currently mainly used together with regular platform devices so this driver allows configuration via platform data to support things like static interrupt base address. DT support will be added incrementally in the not so distant future.

Hi Thomas,

I'm wondering how you would like to handle merging this driver. I can think of three options but I'm sure there are others.

Quoted 6 lines

* You can take the patches (there is a follow-up series) yourself.
* I can prepare a pull request for you
* I can prepare a pull request for arm-soc with the shmobile patches that enable the driver on the r8a7779 and sh73a0.

The last option is possibly the easiest.

Correct.

Quoted 1 line

But in that case I'd appreciate an Ack from you on this patch.

You want to pick the V2 series, which already has my blessing:

Quoted 3 lines

    https://lkml.org/lkml/2013/2/26/305

For merging it through arm-soc you have my ack now :)

Thanks, I have that.

It seems that V2 adds onto this patch rather than replaces it. So could I also get an Ack for this patch too?

Quoted 29 lines · Simon Horman

Quoted 28 lines · Thomas Gleixner

Quoted 12 lines · Simon Horman

Quoted 5 lines · Magnus Damm

The SoCs using this driver are currently mainly used together with regular platform devices so this driver allows configuration via platform data to support things like static interrupt base address. DT support will be added incrementally in the not so distant future.

Hi Thomas,

I'm wondering how you would like to handle merging this driver. I can think of three options but I'm sure there are others.

Quoted 6 lines

* You can take the patches (there is a follow-up series) yourself.
* I can prepare a pull request for you
* I can prepare a pull request for arm-soc with the shmobile patches that enable the driver on the r8a7779 and sh73a0.

The last option is possibly the easiest.

Correct.

Quoted 1 line

But in that case I'd appreciate an Ack from you on this patch.

You want to pick the V2 series, which already has my blessing:

    https://lkml.org/lkml/2013/2/26/305

Quoted 6 lines

Quoted 1 line

For merging it through arm-soc you have my ack now :)

Thanks, I have that.

It seems that V2 adds onto this patch rather than replaces it. So could I also get an Ack for this patch too?

Ah,. right. V2 is an incremental fix for V1. Yes, please add my Ack.