[RFC PATCH v1] irqchip: Add support for tango interrupt router

[RFC PATCH v1] irqchip: Add support for tango interrupt router

[Mason]

The interrupt router supports 128 inputs -> 24 outputs

---
 .../bindings/interrupt-controller/sigma,smp8759-intc.txt  |  48 ++++++
 drivers/irqchip/Makefile                                  |   2 +-
 drivers/irqchip/irq-smp8759.c                             | 204 ++++++++++++++++++++++++
 3 files changed, 253 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..c84ca9315768
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,48 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+- interrupts: list of interrupt lines to parent
+
+Example:
+
+	interrupt-controller@6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+		/*
+		 * There probably is a better way than explicitly listing
+		 * the 24 interrupts?
+		 */
+		interrupts =
+			<GIC_SPI  0 IRQ_TYPE_NONE>,
+			<GIC_SPI  1 IRQ_TYPE_NONE>,
+			<GIC_SPI  2 IRQ_TYPE_NONE>,
+			<GIC_SPI  3 IRQ_TYPE_NONE>,
+			<GIC_SPI  4 IRQ_TYPE_NONE>,
+			<GIC_SPI  5 IRQ_TYPE_NONE>,
+			<GIC_SPI  6 IRQ_TYPE_NONE>,
+			<GIC_SPI  7 IRQ_TYPE_NONE>,
+			<GIC_SPI  8 IRQ_TYPE_NONE>,
+			<GIC_SPI  9 IRQ_TYPE_NONE>,
+			<GIC_SPI 10 IRQ_TYPE_NONE>,
+			<GIC_SPI 11 IRQ_TYPE_NONE>,
+			<GIC_SPI 12 IRQ_TYPE_NONE>,
+			<GIC_SPI 13 IRQ_TYPE_NONE>,
+			<GIC_SPI 14 IRQ_TYPE_NONE>,
+			<GIC_SPI 15 IRQ_TYPE_NONE>,
+			<GIC_SPI 16 IRQ_TYPE_NONE>,
+			<GIC_SPI 17 IRQ_TYPE_NONE>,
+			<GIC_SPI 18 IRQ_TYPE_NONE>,
+			<GIC_SPI 19 IRQ_TYPE_NONE>,
+			<GIC_SPI 20 IRQ_TYPE_NONE>,
+			<GIC_SPI 21 IRQ_TYPE_NONE>,
+			<GIC_SPI 22 IRQ_TYPE_NONE>,
+			<GIC_SPI 23 IRQ_TYPE_NONE>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..b8338e14d399
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,204 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+#define IRQ_MAX			128
+#define OUTPUT_MAX		24
+#define IRQ_ENABLE		BIT(31)
+#define LEVEL_OUTPUT_LINE	0
+
+/*
+ * 128 inputs -> 24 outputs
+ * Group LEVEL_HIGH IRQs on output line 0
+ * Edge interrupts get a dedicated output line
+ * gic_spi_to_tango_hwirq array maps GIC SPI hwirq to tango hwirq
+ */
+struct tango_intc {
+	DECLARE_BITMAP(enabled, IRQ_MAX);
+	spinlock_t lock;
+	void __iomem *config;
+	void __iomem *status;
+	struct irq_domain *dom;
+	u8 gic_spi_to_tango_hwirq[OUTPUT_MAX];
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	unsigned int pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	memcpy_fromio(status, intc->status, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled, IRQ_MAX);
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	unsigned int virq;
+	struct irq_data *d = &desc->irq_data;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+
+	/* I don't know how to get the SPI number, d->hwirq - 32 is a hack */
+	int hwirq = intc->gic_spi_to_tango_hwirq[d->hwirq - 32];
+	//printk("%s: SPI=%lu hwirq=%d\n", __func__, d->hwirq, hwirq);
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, hwirq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	__clear_bit(data->hwirq, intc->enabled);
+	writel_relaxed(0, intc->config + data->hwirq * 4);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+#if 0
+	if (!in_irq() && !in_interrupt()) {
+		printk("HWIRQ=%lu mask=%u\n", data->hwirq, data->mask);
+		dump_stack();
+	}
+#endif
+
+	spin_lock_irqsave(&intc->lock, flags);
+	__set_bit(data->hwirq, intc->enabled);
+	writel_relaxed(IRQ_ENABLE | data->mask, intc->config + data->hwirq * 4);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int find_free_output_line(struct tango_intc *intc)
+{
+	int i;
+
+	/* Start at 1, because 0 is reserved for level interrupts */
+	for (i = 1; i < OUTPUT_MAX; ++i)
+		if (intc->gic_spi_to_tango_hwirq[i] == 0)
+			return i;
+
+	return -ENOSPC;
+}
+
+int tango_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct tango_intc *intc = data->chip_data;
+	printk("%s: IRQ=%lu type=%x\n", __func__, data->hwirq, flow_type);
+	dump_stack();
+	if (flow_type & IRQ_TYPE_LEVEL_HIGH) {
+		data->mask = LEVEL_OUTPUT_LINE;
+		return 0;
+	}
+
+	if (flow_type & IRQ_TYPE_EDGE_RISING) {
+		int res = find_free_output_line(intc);
+		if (res < 0)
+			return res;
+		data->mask = res;
+		intc->gic_spi_to_tango_hwirq[res] = data->hwirq;
+		printk("Map tango hwirq %lu to GIC SPI %d\n", data->hwirq, res);
+		return 0;
+	}
+
+	/* LEVEL_LOW and EDGE_FALLING are not supported */
+	return -ENOSYS;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_map(struct irq_domain *dom, unsigned int virq, irq_hw_number_t hw)
+{
+	struct tango_intc *intc = dom->host_data;
+	struct irq_desc *desc = irq_to_desc(virq);
+	printk("%s: dom=%p virq=%u hwirq=%lu desc=%p\n", __func__, dom, virq, hw, desc);
+	irq_domain_set_info(dom, virq, hw, &tango_chip, intc, handle_simple_irq, NULL, NULL);
+	return 0;
+}
+
+static void tango_unmap(struct irq_domain *d, unsigned int virq)
+{
+	printk("%s: dom=%p virq=%u\n", __func__, d, virq);
+	dump_stack();
+}
+
+struct irq_domain_ops dom_ops =
+{
+	.map	= tango_map,
+	.unmap	= tango_unmap,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int i, virq;
+	struct irq_domain *irq_dom;
+	struct irq_data *irq_data;
+	void __iomem *base;
+	struct tango_intc *intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+
+	base = of_iomap(node, 0);
+	if (!base)
+		panic("%s: of_iomap failed", node->name);
+
+	/*
+	 * Use output line 0 for level high IRQs
+	 * Should we check that index 0 points to SPI 0?
+	 */
+	virq = irq_of_parse_and_map(node, 0);
+	if (!virq)
+		panic("%s: Failed to map IRQ 0\n", node->name);
+
+	irq_data = irq_get_irq_data(virq);
+	irqd_set_trigger_type(irq_data, IRQ_TYPE_LEVEL_HIGH);
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (i = 1; i < OUTPUT_MAX; ++i)
+	{
+		virq = irq_of_parse_and_map(node, i);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, i);
+
+		/*
+		 * I don't think trigger type should appear in DT because
+		 * it is a purely SW/driver decision
+		 */
+		irq_data = irq_get_irq_data(virq);
+		irqd_set_trigger_type(irq_data, IRQ_TYPE_EDGE_RISING);
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	irq_dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+
+	spin_lock_init(&intc->lock);
+	intc->config = base;
+	intc->status = base + 0x420;
+	intc->dom = irq_dom;
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
1.8.3.1

[RFC PATCH v1] irqchip: Add support for tango interrupt router

[Mason]

The interrupt router supports 128 inputs -> 24 outputs

---
 .../bindings/interrupt-controller/sigma,smp8759-intc.txt  |  48 ++++++
 drivers/irqchip/Makefile                                  |   2 +-
 drivers/irqchip/irq-smp8759.c                             | 204 ++++++++++++++++++++++++
 3 files changed, 253 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..c84ca9315768
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,48 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+- interrupts: list of interrupt lines to parent
+
+Example:
+
+	interrupt-controller at 6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+		/*
+		 * There probably is a better way than explicitly listing
+		 * the 24 interrupts?
+		 */
+		interrupts =
+			<GIC_SPI  0 IRQ_TYPE_NONE>,
+			<GIC_SPI  1 IRQ_TYPE_NONE>,
+			<GIC_SPI  2 IRQ_TYPE_NONE>,
+			<GIC_SPI  3 IRQ_TYPE_NONE>,
+			<GIC_SPI  4 IRQ_TYPE_NONE>,
+			<GIC_SPI  5 IRQ_TYPE_NONE>,
+			<GIC_SPI  6 IRQ_TYPE_NONE>,
+			<GIC_SPI  7 IRQ_TYPE_NONE>,
+			<GIC_SPI  8 IRQ_TYPE_NONE>,
+			<GIC_SPI  9 IRQ_TYPE_NONE>,
+			<GIC_SPI 10 IRQ_TYPE_NONE>,
+			<GIC_SPI 11 IRQ_TYPE_NONE>,
+			<GIC_SPI 12 IRQ_TYPE_NONE>,
+			<GIC_SPI 13 IRQ_TYPE_NONE>,
+			<GIC_SPI 14 IRQ_TYPE_NONE>,
+			<GIC_SPI 15 IRQ_TYPE_NONE>,
+			<GIC_SPI 16 IRQ_TYPE_NONE>,
+			<GIC_SPI 17 IRQ_TYPE_NONE>,
+			<GIC_SPI 18 IRQ_TYPE_NONE>,
+			<GIC_SPI 19 IRQ_TYPE_NONE>,
+			<GIC_SPI 20 IRQ_TYPE_NONE>,
+			<GIC_SPI 21 IRQ_TYPE_NONE>,
+			<GIC_SPI 22 IRQ_TYPE_NONE>,
+			<GIC_SPI 23 IRQ_TYPE_NONE>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..b8338e14d399
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,204 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+#define IRQ_MAX			128
+#define OUTPUT_MAX		24
+#define IRQ_ENABLE		BIT(31)
+#define LEVEL_OUTPUT_LINE	0
+
+/*
+ * 128 inputs -> 24 outputs
+ * Group LEVEL_HIGH IRQs on output line 0
+ * Edge interrupts get a dedicated output line
+ * gic_spi_to_tango_hwirq array maps GIC SPI hwirq to tango hwirq
+ */
+struct tango_intc {
+	DECLARE_BITMAP(enabled, IRQ_MAX);
+	spinlock_t lock;
+	void __iomem *config;
+	void __iomem *status;
+	struct irq_domain *dom;
+	u8 gic_spi_to_tango_hwirq[OUTPUT_MAX];
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	unsigned int pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	memcpy_fromio(status, intc->status, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled, IRQ_MAX);
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	unsigned int virq;
+	struct irq_data *d = &desc->irq_data;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+
+	/* I don't know how to get the SPI number, d->hwirq - 32 is a hack */
+	int hwirq = intc->gic_spi_to_tango_hwirq[d->hwirq - 32];
+	//printk("%s: SPI=%lu hwirq=%d\n", __func__, d->hwirq, hwirq);
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, hwirq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	__clear_bit(data->hwirq, intc->enabled);
+	writel_relaxed(0, intc->config + data->hwirq * 4);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+#if 0
+	if (!in_irq() && !in_interrupt()) {
+		printk("HWIRQ=%lu mask=%u\n", data->hwirq, data->mask);
+		dump_stack();
+	}
+#endif
+
+	spin_lock_irqsave(&intc->lock, flags);
+	__set_bit(data->hwirq, intc->enabled);
+	writel_relaxed(IRQ_ENABLE | data->mask, intc->config + data->hwirq * 4);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int find_free_output_line(struct tango_intc *intc)
+{
+	int i;
+
+	/* Start at 1, because 0 is reserved for level interrupts */
+	for (i = 1; i < OUTPUT_MAX; ++i)
+		if (intc->gic_spi_to_tango_hwirq[i] == 0)
+			return i;
+
+	return -ENOSPC;
+}
+
+int tango_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct tango_intc *intc = data->chip_data;
+	printk("%s: IRQ=%lu type=%x\n", __func__, data->hwirq, flow_type);
+	dump_stack();
+	if (flow_type & IRQ_TYPE_LEVEL_HIGH) {
+		data->mask = LEVEL_OUTPUT_LINE;
+		return 0;
+	}
+
+	if (flow_type & IRQ_TYPE_EDGE_RISING) {
+		int res = find_free_output_line(intc);
+		if (res < 0)
+			return res;
+		data->mask = res;
+		intc->gic_spi_to_tango_hwirq[res] = data->hwirq;
+		printk("Map tango hwirq %lu to GIC SPI %d\n", data->hwirq, res);
+		return 0;
+	}
+
+	/* LEVEL_LOW and EDGE_FALLING are not supported */
+	return -ENOSYS;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_map(struct irq_domain *dom, unsigned int virq, irq_hw_number_t hw)
+{
+	struct tango_intc *intc = dom->host_data;
+	struct irq_desc *desc = irq_to_desc(virq);
+	printk("%s: dom=%p virq=%u hwirq=%lu desc=%p\n", __func__, dom, virq, hw, desc);
+	irq_domain_set_info(dom, virq, hw, &tango_chip, intc, handle_simple_irq, NULL, NULL);
+	return 0;
+}
+
+static void tango_unmap(struct irq_domain *d, unsigned int virq)
+{
+	printk("%s: dom=%p virq=%u\n", __func__, d, virq);
+	dump_stack();
+}
+
+struct irq_domain_ops dom_ops =
+{
+	.map	= tango_map,
+	.unmap	= tango_unmap,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int i, virq;
+	struct irq_domain *irq_dom;
+	struct irq_data *irq_data;
+	void __iomem *base;
+	struct tango_intc *intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+
+	base = of_iomap(node, 0);
+	if (!base)
+		panic("%s: of_iomap failed", node->name);
+
+	/*
+	 * Use output line 0 for level high IRQs
+	 * Should we check that index 0 points to SPI 0?
+	 */
+	virq = irq_of_parse_and_map(node, 0);
+	if (!virq)
+		panic("%s: Failed to map IRQ 0\n", node->name);
+
+	irq_data = irq_get_irq_data(virq);
+	irqd_set_trigger_type(irq_data, IRQ_TYPE_LEVEL_HIGH);
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (i = 1; i < OUTPUT_MAX; ++i)
+	{
+		virq = irq_of_parse_and_map(node, i);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, i);
+
+		/*
+		 * I don't think trigger type should appear in DT because
+		 * it is a purely SW/driver decision
+		 */
+		irq_data = irq_get_irq_data(virq);
+		irqd_set_trigger_type(irq_data, IRQ_TYPE_EDGE_RISING);
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	irq_dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+
+	spin_lock_init(&intc->lock);
+	intc->config = base;
+	intc->status = base + 0x420;
+	intc->dom = irq_dom;
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
1.8.3.1

Re: [RFC PATCH v1] irqchip: Add support for tango interrupt router

[Thomas Petazzoni]

Hello,

On Tue, 6 Jun 2017 15:42:36 +0200, Mason wrote:

> +	interrupt-controller@6f800 {
> +		compatible = "sigma,smp8759-intc";
> +		reg = <0x6f800 0x430>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-parent = <&gic>;
> +		/*
> +		 * There probably is a better way than explicitly listing
> +		 * the 24 interrupts?
> +		 */

What we do on Marvell platforms is:

	marvell,spi-base = <128>, <136>, <144>, <152>;

see marvell,odmi-controller.txt.

In another driver I submitted, we're doing:

	marvell,spi-ranges = <64 64>, <288 64>;

Retrospectively, I would have preferred to use marvell,spi-ranges for
the first DT binding as well, since it allows to express both the base
and number of interrupts available in the range.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[RFC PATCH v1] irqchip: Add support for tango interrupt router

[Thomas Petazzoni]

Hello,

On Tue, 6 Jun 2017 15:42:36 +0200, Mason wrote:

> +	interrupt-controller at 6f800 {
> +		compatible = "sigma,smp8759-intc";
> +		reg = <0x6f800 0x430>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-parent = <&gic>;
> +		/*
> +		 * There probably is a better way than explicitly listing
> +		 * the 24 interrupts?
> +		 */

What we do on Marvell platforms is:

	marvell,spi-base = <128>, <136>, <144>, <152>;

see marvell,odmi-controller.txt.

In another driver I submitted, we're doing:

	marvell,spi-ranges = <64 64>, <288 64>;

Retrospectively, I would have preferred to use marvell,spi-ranges for
the first DT binding as well, since it allows to express both the base
and number of interrupts available in the range.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [RFC PATCH v1] irqchip: Add support for tango interrupt router

[Mason]

On 06/06/2017 17:52, Thomas Petazzoni wrote:

> On Tue, 6 Jun 2017 15:42:36 +0200, Mason wrote:
> 
>> +	interrupt-controller@6f800 {
>> +		compatible = "sigma,smp8759-intc";
>> +		reg = <0x6f800 0x430>;
>> +		interrupt-controller;
>> +		#interrupt-cells = <2>;
>> +		interrupt-parent = <&gic>;
>> +		/*
>> +		 * There probably is a better way than explicitly listing
>> +		 * the 24 interrupts?
>> +		 */
> 
> What we do on Marvell platforms is:
> 
> 	marvell,spi-base = <128>, <136>, <144>, <152>;
> 
> see marvell,odmi-controller.txt.
> 
> In another driver I submitted, we're doing:
> 
> 	marvell,spi-ranges = <64 64>, <288 64>;
> 
> Retrospectively, I would have preferred to use marvell,spi-ranges for
> the first DT binding as well, since it allows to express both the base
> and number of interrupts available in the range.

Sorry for the delay, I got distracted by other drivers
(PCIe, clkgen, i2c, infrared).

Thanks for the suggestion.

So, if I remove the "interrupts" property from the controller's
DT node, I can no longer use irq_of_parse_and_map() followed by
irqd_set_trigger_type(), right?

I would have to "emulate" irq_of_parse_and_map() with
something along the lines of:

#include <dt-bindings/interrupt-controller/arm-gic.h>
static int __init map_irq(struct device_node *gic, int irq, int type)
{
	struct of_phandle_args data = { gic, 3, { GIC_SPI, irq, type }};
	return irq_create_of_mapping(&data);
}

Then map all 24 interrupts at init:

	virq = map_irq(gic, 0, IRQ_TYPE_LEVEL_HIGH);
	for (i = 1; i < 24; ++i)
		virq = map_irq(gic, i, IRQ_TYPE_EDGE_RISING);

Is that correct?

Does it make sense to use a separate ISR for the two kinds
of interrupts?

Regards.

[RFC PATCH v1] irqchip: Add support for tango interrupt router

[Mason]

On 06/06/2017 17:52, Thomas Petazzoni wrote:

> On Tue, 6 Jun 2017 15:42:36 +0200, Mason wrote:
> 
>> +	interrupt-controller at 6f800 {
>> +		compatible = "sigma,smp8759-intc";
>> +		reg = <0x6f800 0x430>;
>> +		interrupt-controller;
>> +		#interrupt-cells = <2>;
>> +		interrupt-parent = <&gic>;
>> +		/*
>> +		 * There probably is a better way than explicitly listing
>> +		 * the 24 interrupts?
>> +		 */
> 
> What we do on Marvell platforms is:
> 
> 	marvell,spi-base = <128>, <136>, <144>, <152>;
> 
> see marvell,odmi-controller.txt.
> 
> In another driver I submitted, we're doing:
> 
> 	marvell,spi-ranges = <64 64>, <288 64>;
> 
> Retrospectively, I would have preferred to use marvell,spi-ranges for
> the first DT binding as well, since it allows to express both the base
> and number of interrupts available in the range.

Sorry for the delay, I got distracted by other drivers
(PCIe, clkgen, i2c, infrared).

Thanks for the suggestion.

So, if I remove the "interrupts" property from the controller's
DT node, I can no longer use irq_of_parse_and_map() followed by
irqd_set_trigger_type(), right?

I would have to "emulate" irq_of_parse_and_map() with
something along the lines of:

#include <dt-bindings/interrupt-controller/arm-gic.h>
static int __init map_irq(struct device_node *gic, int irq, int type)
{
	struct of_phandle_args data = { gic, 3, { GIC_SPI, irq, type }};
	return irq_create_of_mapping(&data);
}

Then map all 24 interrupts at init:

	virq = map_irq(gic, 0, IRQ_TYPE_LEVEL_HIGH);
	for (i = 1; i < 24; ++i)
		virq = map_irq(gic, i, IRQ_TYPE_EDGE_RISING);

Is that correct?

Does it make sense to use a separate ISR for the two kinds
of interrupts?

Regards.

[RFC PATCH v2] irqchip: Add support for tango interrupt router

[Mason]

128 inputs, 24 outputs (to GIC SPI 0-23)
---
There might be a few things wrong with this driver.
When I cat /proc/interrupts the interrupt count appears
to be bogus (as if level IRQ counts are added to edge
IRQ counts). Did I mess something up with the IRQ domains?
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 ++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 203 +++++++++++++++++++++
 3 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller@6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..ec7fee4574ef
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,203 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+#define IRQ_MAX		128
+#define SPI_MAX		24 /* 24 output lines routed to SPI 0-23 */
+#define LEVEL_SPI	17
+#define IRQ_ENABLE	BIT(31)
+
+/*
+ * 128 inputs mapped to 24 outputs
+ * LEVEL_HIGH IRQs are muxed on output line 'LEVEL_SPI'
+ * EDGE_RISING IRQs get a dedicated output line
+ * gic_spi_to_tango_hwirq array maps GIC SPI hwirq to tango hwirq
+ */
+struct tango_intc {
+	DECLARE_BITMAP(enabled, IRQ_MAX);
+	spinlock_t lock;
+	void __iomem *config;
+	void __iomem *status;
+	struct irq_domain *dom;
+	u8 gic_spi_to_tango_hwirq[SPI_MAX];
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	unsigned int pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	memcpy_fromio(status, intc->status, IRQ_MAX / BITS_PER_BYTE);
+	spin_lock(&intc->lock);
+	bitmap_and(status, status, intc->enabled, IRQ_MAX);
+	spin_unlock(&intc->lock);
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	unsigned int virq;
+	struct irq_data *d = &desc->irq_data;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+
+	/* I don't know how to get the SPI number, d->hwirq - 32 is a hack */
+	int hwirq = intc->gic_spi_to_tango_hwirq[d->hwirq - 32];
+	//printk("%s: SPI=%lu hwirq=%d\n", __func__, d->hwirq, hwirq);
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, hwirq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	__clear_bit(data->hwirq, intc->enabled);
+	writel_relaxed(0, intc->config + data->hwirq * 4);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+#if 0
+	if (!in_irq() && !in_interrupt()) {
+		printk("HWIRQ=%lu mask=%u\n", data->hwirq, data->mask);
+		dump_stack();
+	}
+#endif
+
+	spin_lock_irqsave(&intc->lock, flags);
+	__set_bit(data->hwirq, intc->enabled);
+	writel_relaxed(IRQ_ENABLE | data->mask, intc->config + data->hwirq * 4);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int find_free_output_line(struct tango_intc *intc)
+{
+	int spi;
+
+	for (spi = 0; spi < SPI_MAX; ++spi)
+		if (intc->gic_spi_to_tango_hwirq[spi] == 0)
+			return spi;
+
+	return -ENOSPC;
+}
+
+int tango_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct tango_intc *intc = data->chip_data;
+	printk("%s: IRQ=%lu type=%x\n", __func__, data->hwirq, flow_type);
+	dump_stack();
+	if (flow_type & IRQ_TYPE_LEVEL_HIGH) {
+		data->mask = LEVEL_SPI;
+		return 0;
+	}
+
+	if (flow_type & IRQ_TYPE_EDGE_RISING) {
+		int res = find_free_output_line(intc);
+		if (res < 0)
+			return res;
+		data->mask = res;
+		intc->gic_spi_to_tango_hwirq[res] = data->hwirq;
+		printk("Map tango hwirq %lu to GIC SPI %d\n", data->hwirq, res);
+		return 0;
+	}
+
+	/* LEVEL_LOW and EDGE_FALLING are not supported */
+	return -ENOSYS;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_map(struct irq_domain *dom, unsigned int virq, irq_hw_number_t hw)
+{
+	struct tango_intc *intc = dom->host_data;
+	struct irq_desc *desc = irq_to_desc(virq);
+	printk("%s: dom=%p virq=%u hwirq=%lu desc=%p\n", __func__, dom, virq, hw, desc);
+	irq_domain_set_info(dom, virq, hw, &tango_chip, intc, handle_simple_irq, NULL, NULL);
+	return 0;
+}
+
+static void tango_unmap(struct irq_domain *d, unsigned int virq)
+{
+	printk("%s: dom=%p virq=%u\n", __func__, d, virq);
+	dump_stack();
+}
+
+struct irq_domain_ops dom_ops =
+{
+	.map	= tango_map,
+	.unmap	= tango_unmap,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+static int __init map_irq(struct device_node *gic, int irq, int type)
+{
+	struct of_phandle_args irq_data = { gic, 3, { GIC_SPI, irq, type }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	struct irq_domain *irq_dom;
+	void __iomem *base;
+	struct tango_intc *intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+
+	base = of_iomap(node, 0);
+	if (!base)
+		panic("%s: of_iomap failed", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 0; spi < SPI_MAX; ++spi) {
+		if (spi == LEVEL_SPI)
+			continue;
+
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	irq_dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+
+	spin_lock_init(&intc->lock);
+	intc->config = base;
+	intc->status = base + 0x420;
+	intc->dom = irq_dom;
+	intc->gic_spi_to_tango_hwirq[LEVEL_SPI] = ~0;
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);

[RFC PATCH v2] irqchip: Add support for tango interrupt router

[Mason]

128 inputs, 24 outputs (to GIC SPI 0-23)
---
There might be a few things wrong with this driver.
When I cat /proc/interrupts the interrupt count appears
to be bogus (as if level IRQ counts are added to edge
IRQ counts). Did I mess something up with the IRQ domains?
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 ++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 203 +++++++++++++++++++++
 3 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller at 6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..ec7fee4574ef
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,203 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+#define IRQ_MAX		128
+#define SPI_MAX		24 /* 24 output lines routed to SPI 0-23 */
+#define LEVEL_SPI	17
+#define IRQ_ENABLE	BIT(31)
+
+/*
+ * 128 inputs mapped to 24 outputs
+ * LEVEL_HIGH IRQs are muxed on output line 'LEVEL_SPI'
+ * EDGE_RISING IRQs get a dedicated output line
+ * gic_spi_to_tango_hwirq array maps GIC SPI hwirq to tango hwirq
+ */
+struct tango_intc {
+	DECLARE_BITMAP(enabled, IRQ_MAX);
+	spinlock_t lock;
+	void __iomem *config;
+	void __iomem *status;
+	struct irq_domain *dom;
+	u8 gic_spi_to_tango_hwirq[SPI_MAX];
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	unsigned int pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	memcpy_fromio(status, intc->status, IRQ_MAX / BITS_PER_BYTE);
+	spin_lock(&intc->lock);
+	bitmap_and(status, status, intc->enabled, IRQ_MAX);
+	spin_unlock(&intc->lock);
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	unsigned int virq;
+	struct irq_data *d = &desc->irq_data;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+
+	/* I don't know how to get the SPI number, d->hwirq - 32 is a hack */
+	int hwirq = intc->gic_spi_to_tango_hwirq[d->hwirq - 32];
+	//printk("%s: SPI=%lu hwirq=%d\n", __func__, d->hwirq, hwirq);
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, hwirq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	__clear_bit(data->hwirq, intc->enabled);
+	writel_relaxed(0, intc->config + data->hwirq * 4);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+#if 0
+	if (!in_irq() && !in_interrupt()) {
+		printk("HWIRQ=%lu mask=%u\n", data->hwirq, data->mask);
+		dump_stack();
+	}
+#endif
+
+	spin_lock_irqsave(&intc->lock, flags);
+	__set_bit(data->hwirq, intc->enabled);
+	writel_relaxed(IRQ_ENABLE | data->mask, intc->config + data->hwirq * 4);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int find_free_output_line(struct tango_intc *intc)
+{
+	int spi;
+
+	for (spi = 0; spi < SPI_MAX; ++spi)
+		if (intc->gic_spi_to_tango_hwirq[spi] == 0)
+			return spi;
+
+	return -ENOSPC;
+}
+
+int tango_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct tango_intc *intc = data->chip_data;
+	printk("%s: IRQ=%lu type=%x\n", __func__, data->hwirq, flow_type);
+	dump_stack();
+	if (flow_type & IRQ_TYPE_LEVEL_HIGH) {
+		data->mask = LEVEL_SPI;
+		return 0;
+	}
+
+	if (flow_type & IRQ_TYPE_EDGE_RISING) {
+		int res = find_free_output_line(intc);
+		if (res < 0)
+			return res;
+		data->mask = res;
+		intc->gic_spi_to_tango_hwirq[res] = data->hwirq;
+		printk("Map tango hwirq %lu to GIC SPI %d\n", data->hwirq, res);
+		return 0;
+	}
+
+	/* LEVEL_LOW and EDGE_FALLING are not supported */
+	return -ENOSYS;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_map(struct irq_domain *dom, unsigned int virq, irq_hw_number_t hw)
+{
+	struct tango_intc *intc = dom->host_data;
+	struct irq_desc *desc = irq_to_desc(virq);
+	printk("%s: dom=%p virq=%u hwirq=%lu desc=%p\n", __func__, dom, virq, hw, desc);
+	irq_domain_set_info(dom, virq, hw, &tango_chip, intc, handle_simple_irq, NULL, NULL);
+	return 0;
+}
+
+static void tango_unmap(struct irq_domain *d, unsigned int virq)
+{
+	printk("%s: dom=%p virq=%u\n", __func__, d, virq);
+	dump_stack();
+}
+
+struct irq_domain_ops dom_ops =
+{
+	.map	= tango_map,
+	.unmap	= tango_unmap,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+static int __init map_irq(struct device_node *gic, int irq, int type)
+{
+	struct of_phandle_args irq_data = { gic, 3, { GIC_SPI, irq, type }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	struct irq_domain *irq_dom;
+	void __iomem *base;
+	struct tango_intc *intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+
+	base = of_iomap(node, 0);
+	if (!base)
+		panic("%s: of_iomap failed", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 0; spi < SPI_MAX; ++spi) {
+		if (spi == LEVEL_SPI)
+			continue;
+
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	irq_dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+
+	spin_lock_init(&intc->lock);
+	intc->config = base;
+	intc->status = base + 0x420;
+	intc->dom = irq_dom;
+	intc->gic_spi_to_tango_hwirq[LEVEL_SPI] = ~0;
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);

Re: [RFC PATCH v2] irqchip: Add support for tango interrupt router

[Mason]

On 12/07/2017 18:39, Mason wrote:

> 128 inputs, 24 outputs (to GIC SPI 0-23)
> 
> There might be a few things wrong with this driver. When I
> cat /proc/interrupts the interrupt count appears to be bogus
> (as if level IRQ counts are added to edge IRQ counts).

OK, I found the issue.

It occurred on the interrupt lines that stay high when
the HW block is idle, so I mishandled them on every
level interrupt.

I have two remaining issues:

1) In the ISR, I get the hwirq from the GIC. What is the
API to translate that to the SPI? I'm currently just
subtracting 32.

2) I'm currently using a single domain, with a
handle_simple_irq domain handler. That's probably
wrong. Should I define two domains, one for edge
IRQs and one for level IRQs, and use the appropriate
handler?  Should both domain have 128 entries?
(I.e. are they indexed by the hwirq?)
And should I use linear or tree?

Regards.

[RFC PATCH v2] irqchip: Add support for tango interrupt router

[Mason]

On 12/07/2017 18:39, Mason wrote:

> 128 inputs, 24 outputs (to GIC SPI 0-23)
> 
> There might be a few things wrong with this driver. When I
> cat /proc/interrupts the interrupt count appears to be bogus
> (as if level IRQ counts are added to edge IRQ counts).

OK, I found the issue.

It occurred on the interrupt lines that stay high when
the HW block is idle, so I mishandled them on every
level interrupt.

I have two remaining issues:

1) In the ISR, I get the hwirq from the GIC. What is the
API to translate that to the SPI? I'm currently just
subtracting 32.

2) I'm currently using a single domain, with a
handle_simple_irq domain handler. That's probably
wrong. Should I define two domains, one for edge
IRQs and one for level IRQs, and use the appropriate
handler?  Should both domain have 128 entries?
(I.e. are they indexed by the hwirq?)
And should I use linear or tree?

Regards.

Re: [RFC PATCH v2] irqchip: Add support for tango interrupt router

[Mason]

On 15/07/2017 15:06, Mason wrote:

> I have two remaining issues:
> 
> 1) In the ISR, I get the hwirq from the GIC. What is the
> API to translate that to the SPI? I'm currently just
> subtracting 32.

gic_set_type() in drivers/irqchip/irq-gic.c

	/* SPIs have restrictions on the supported types */
	if (gicirq >= 32 && type != IRQ_TYPE_LEVEL_HIGH &&
			    type != IRQ_TYPE_EDGE_RISING)
		return -EINVAL;


gic_irq_domain_translate() in the same file:

		/* Get the interrupt number and add 16 to skip over SGIs */
		*hwirq = fwspec->param[1] + 16;

		/*
		 * For SPIs, we need to add 16 more to get the GIC irq
		 * ID number
		 */
		if (!fwspec->param[0])
			*hwirq += 16;


So it seems "acceptable" to compute spi = d->hwirq - 32;


> 2) I'm currently using a single domain, with a
> handle_simple_irq domain handler. That's probably
> wrong. Should I define two domains, one for edge
> IRQs and one for level IRQs, and use the appropriate
> handler?  Should both domain have 128 entries?
> (I.e. are they indexed by the hwirq?)
> And should I use linear or tree?

I will read this again carefully:
https://www.kernel.org/doc/Documentation/IRQ-domain.txt

Regards.

[RFC PATCH v2] irqchip: Add support for tango interrupt router

[Mason]

On 15/07/2017 15:06, Mason wrote:

> I have two remaining issues:
> 
> 1) In the ISR, I get the hwirq from the GIC. What is the
> API to translate that to the SPI? I'm currently just
> subtracting 32.

gic_set_type() in drivers/irqchip/irq-gic.c

	/* SPIs have restrictions on the supported types */
	if (gicirq >= 32 && type != IRQ_TYPE_LEVEL_HIGH &&
			    type != IRQ_TYPE_EDGE_RISING)
		return -EINVAL;


gic_irq_domain_translate() in the same file:

		/* Get the interrupt number and add 16 to skip over SGIs */
		*hwirq = fwspec->param[1] + 16;

		/*
		 * For SPIs, we need to add 16 more to get the GIC irq
		 * ID number
		 */
		if (!fwspec->param[0])
			*hwirq += 16;


So it seems "acceptable" to compute spi = d->hwirq - 32;


> 2) I'm currently using a single domain, with a
> handle_simple_irq domain handler. That's probably
> wrong. Should I define two domains, one for edge
> IRQs and one for level IRQs, and use the appropriate
> handler?  Should both domain have 128 entries?
> (I.e. are they indexed by the hwirq?)
> And should I use linear or tree?

I will read this again carefully:
https://www.kernel.org/doc/Documentation/IRQ-domain.txt

Regards.

[RFC PATCH v3] irqchip: Add support for tango interrupt router

[Mason]

This controller maps 128 input lines to 24 output lines.
The output lines are routed to GIC SPI 0 to 23.
This driver muxes LEVEL_HIGH IRQs onto output line 0,
and gives every EDGE_RISING IRQ a dedicated output line.
---
I think the driver is mostly finished. It works without errors
for my basic use-cases. There is one required feature missing:
suspend/resume support. Should it be a separate patch, or just
squashed into this patch? Regards.
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 +++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 169 +++++++++++++++++++++
 3 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller@6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..b3274779ee7a
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,169 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver muxes LEVEL_HIGH IRQs onto output line 0,
+ * and gives every EDGE_RISING IRQ a dedicated output line.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define LEVEL_SPI	0
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	struct irq_domain *dom;
+	u8 spi_to_tango_irq[SPI_MAX];
+	DECLARE_BITMAP(enabled_level, IRQ_MAX);
+	spinlock_t lock;
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	unsigned int pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&intc->lock);
+	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
+	spin_unlock(&intc->lock);
+
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	unsigned int virq;
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, tango_irq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	if (data->mask == LEVEL_SPI)
+		__clear_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(IRQ_ENABLE | data->mask, intc->base + data->hwirq * 4);
+	if (data->mask == LEVEL_SPI)
+		__set_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int tango_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct tango_intc *intc = data->chip_data;
+	int spi;
+
+	if (flow_type & IRQ_TYPE_LEVEL_HIGH) {
+		data->mask = LEVEL_SPI;
+		return 0;
+	}
+
+	if (flow_type & IRQ_TYPE_EDGE_RISING) {
+		for (spi = 1; spi < SPI_MAX; ++spi) {
+			if (intc->spi_to_tango_irq[spi] == 0) {
+				data->mask = spi;
+				intc->spi_to_tango_irq[spi] = data->hwirq;
+				return 0;
+			}
+		}
+		return -ENOSPC;
+	}
+
+	return -EINVAL;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_map(struct irq_domain *dom, unsigned int virq, irq_hw_number_t hwirq)
+{
+	struct tango_intc *intc = dom->host_data;
+
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+	irq_set_handler(virq, handle_simple_irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.map	= tango_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static int __init map_irq(struct device_node *gic, int spi, int trigger)
+{
+	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	void __iomem *base;
+	struct tango_intc *intc;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	base = of_iomap(node, 0);
+	if (!intc || !base)
+		panic("%s: Failed to kzalloc and iomap\n", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 1; spi < SPI_MAX; ++spi) {
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	intc->base = base;
+	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+	spin_lock_init(&intc->lock);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

[RFC PATCH v3] irqchip: Add support for tango interrupt router

[Mason]

This controller maps 128 input lines to 24 output lines.
The output lines are routed to GIC SPI 0 to 23.
This driver muxes LEVEL_HIGH IRQs onto output line 0,
and gives every EDGE_RISING IRQ a dedicated output line.
---
I think the driver is mostly finished. It works without errors
for my basic use-cases. There is one required feature missing:
suspend/resume support. Should it be a separate patch, or just
squashed into this patch? Regards.
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 +++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 169 +++++++++++++++++++++
 3 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller at 6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..b3274779ee7a
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,169 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver muxes LEVEL_HIGH IRQs onto output line 0,
+ * and gives every EDGE_RISING IRQ a dedicated output line.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define LEVEL_SPI	0
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	struct irq_domain *dom;
+	u8 spi_to_tango_irq[SPI_MAX];
+	DECLARE_BITMAP(enabled_level, IRQ_MAX);
+	spinlock_t lock;
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	unsigned int pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&intc->lock);
+	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
+	spin_unlock(&intc->lock);
+
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	unsigned int virq;
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, tango_irq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	if (data->mask == LEVEL_SPI)
+		__clear_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(IRQ_ENABLE | data->mask, intc->base + data->hwirq * 4);
+	if (data->mask == LEVEL_SPI)
+		__set_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int tango_set_type(struct irq_data *data, unsigned int flow_type)
+{
+	struct tango_intc *intc = data->chip_data;
+	int spi;
+
+	if (flow_type & IRQ_TYPE_LEVEL_HIGH) {
+		data->mask = LEVEL_SPI;
+		return 0;
+	}
+
+	if (flow_type & IRQ_TYPE_EDGE_RISING) {
+		for (spi = 1; spi < SPI_MAX; ++spi) {
+			if (intc->spi_to_tango_irq[spi] == 0) {
+				data->mask = spi;
+				intc->spi_to_tango_irq[spi] = data->hwirq;
+				return 0;
+			}
+		}
+		return -ENOSPC;
+	}
+
+	return -EINVAL;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_map(struct irq_domain *dom, unsigned int virq, irq_hw_number_t hwirq)
+{
+	struct tango_intc *intc = dom->host_data;
+
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+	irq_set_handler(virq, handle_simple_irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.map	= tango_map,
+	.xlate	= irq_domain_xlate_twocell,
+};
+
+static int __init map_irq(struct device_node *gic, int spi, int trigger)
+{
+	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	void __iomem *base;
+	struct tango_intc *intc;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	base = of_iomap(node, 0);
+	if (!intc || !base)
+		panic("%s: Failed to kzalloc and iomap\n", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 1; spi < SPI_MAX; ++spi) {
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	intc->base = base;
+	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+	spin_lock_init(&intc->lock);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

Re: [RFC PATCH v3] irqchip: Add support for tango interrupt router

[Mason]

On 17/07/2017 17:36, Mason wrote:

> This controller maps 128 input lines to 24 output lines.
> The output lines are routed to GIC SPI 0 to 23.
> This driver muxes LEVEL_HIGH IRQs onto output line 0,
> and gives every EDGE_RISING IRQ a dedicated output line.
> ---
> I think the driver is mostly finished. It works without
> errors for my basic use-cases.

I spotted a problem. Output lines are allocated
for edge interrupts in tango_set_type() but they
are never released. So if we insmod/rmmod modules
in a loop, we will run out of output lines.

It seems set_type() is not the right place for an
allocation. Perhaps that should be done in tango_map()?

I'll try to trace the code to figure out when the
unmap function is called, as well.

> There is one required feature missing:
> suspend/resume support.
> Should it be a separate patch, or just squashed
> into this patch?

That question still stands.

Regards.

[RFC PATCH v3] irqchip: Add support for tango interrupt router

[Mason]

On 17/07/2017 17:36, Mason wrote:

> This controller maps 128 input lines to 24 output lines.
> The output lines are routed to GIC SPI 0 to 23.
> This driver muxes LEVEL_HIGH IRQs onto output line 0,
> and gives every EDGE_RISING IRQ a dedicated output line.
> ---
> I think the driver is mostly finished. It works without
> errors for my basic use-cases.

I spotted a problem. Output lines are allocated
for edge interrupts in tango_set_type() but they
are never released. So if we insmod/rmmod modules
in a loop, we will run out of output lines.

It seems set_type() is not the right place for an
allocation. Perhaps that should be done in tango_map()?

I'll try to trace the code to figure out when the
unmap function is called, as well.

> There is one required feature missing:
> suspend/resume support.
> Should it be a separate patch, or just squashed
> into this patch?

That question still stands.

Regards.

[PATCH v4] irqchip: Add support for tango interrupt router

[Mason]

This controller maps 128 input lines to 24 output lines.
The output lines are routed to GIC SPI 0 to 23.
This driver muxes LEVEL_HIGH IRQs onto output line 0,
and gives every EDGE_RISING IRQ a dedicated output line.
---
Changes from v3 to v4:
o Register tango_alloc() instead of tango_map() in order to allocate
the SPI in tango_alloc() rather than in tango_set_type()
o tango_set_type() becomes a NOP
TODO: suspend/resume support
Should it be a separate patch, or just squashed into this one?
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 +++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 172 +++++++++++++++++++++
 3 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller@6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..f0ba6b12b146
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,172 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver muxes LEVEL_HIGH IRQs onto output line 0,
+ * and gives every EDGE_RISING IRQ a dedicated output line.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define LEVEL_SPI	0
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	struct irq_domain *dom;
+	u8 spi_to_tango_irq[SPI_MAX];
+	DECLARE_BITMAP(enabled_level, IRQ_MAX);
+	spinlock_t lock;
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	uint pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&intc->lock);
+	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
+	spin_unlock(&intc->lock);
+
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	uint virq;
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, tango_irq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	if (data->mask == LEVEL_SPI)
+		__clear_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(IRQ_ENABLE | data->mask, intc->base + data->hwirq * 4);
+	if (data->mask == LEVEL_SPI)
+		__set_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int tango_set_type(struct irq_data *data, uint flow_type)
+{
+	return 0;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
+{
+	int spi;
+	struct irq_fwspec *fwspec = arg;
+	struct tango_intc *intc = dom->host_data;
+	struct irq_data *data = irq_get_irq_data(virq);
+	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
+
+	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	if (trigger & IRQ_TYPE_LEVEL_HIGH)
+		data->mask = LEVEL_SPI;
+
+	if (trigger & IRQ_TYPE_EDGE_RISING) {
+		for (spi = 1; spi < SPI_MAX; ++spi) {
+			if (intc->spi_to_tango_irq[spi] == 0) {
+				data->mask = spi;
+				intc->spi_to_tango_irq[spi] = hwirq;
+				break;
+			}
+		}
+		if (spi == SPI_MAX)
+			return -ENOSPC;
+	}
+
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+	irq_set_handler(virq, handle_simple_irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+	.alloc	= tango_alloc,
+};
+
+static int __init map_irq(struct device_node *gic, int spi, int trigger)
+{
+	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	struct tango_intc *intc;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	if (!intc)
+		panic("%s: Failed to kalloc\n", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 1; spi < SPI_MAX; ++spi) {
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	spin_lock_init(&intc->lock);
+	intc->base = of_iomap(node, 0);
+	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+	if (!intc->base || !intc->dom)
+		panic("%s: Failed to setup IRQ controller\n", node->name);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

[PATCH v4] irqchip: Add support for tango interrupt router

[Mason]

This controller maps 128 input lines to 24 output lines.
The output lines are routed to GIC SPI 0 to 23.
This driver muxes LEVEL_HIGH IRQs onto output line 0,
and gives every EDGE_RISING IRQ a dedicated output line.
---
Changes from v3 to v4:
o Register tango_alloc() instead of tango_map() in order to allocate
the SPI in tango_alloc() rather than in tango_set_type()
o tango_set_type() becomes a NOP
TODO: suspend/resume support
Should it be a separate patch, or just squashed into this one?
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 +++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 172 +++++++++++++++++++++
 3 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller at 6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..f0ba6b12b146
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,172 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver muxes LEVEL_HIGH IRQs onto output line 0,
+ * and gives every EDGE_RISING IRQ a dedicated output line.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define LEVEL_SPI	0
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	struct irq_domain *dom;
+	u8 spi_to_tango_irq[SPI_MAX];
+	DECLARE_BITMAP(enabled_level, IRQ_MAX);
+	spinlock_t lock;
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	uint pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&intc->lock);
+	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
+	spin_unlock(&intc->lock);
+
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	uint virq;
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, tango_irq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	if (data->mask == LEVEL_SPI)
+		__clear_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(IRQ_ENABLE | data->mask, intc->base + data->hwirq * 4);
+	if (data->mask == LEVEL_SPI)
+		__set_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int tango_set_type(struct irq_data *data, uint flow_type)
+{
+	return 0;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
+{
+	int spi;
+	struct irq_fwspec *fwspec = arg;
+	struct tango_intc *intc = dom->host_data;
+	struct irq_data *data = irq_get_irq_data(virq);
+	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
+
+	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	if (trigger & IRQ_TYPE_LEVEL_HIGH)
+		data->mask = LEVEL_SPI;
+
+	if (trigger & IRQ_TYPE_EDGE_RISING) {
+		for (spi = 1; spi < SPI_MAX; ++spi) {
+			if (intc->spi_to_tango_irq[spi] == 0) {
+				data->mask = spi;
+				intc->spi_to_tango_irq[spi] = hwirq;
+				break;
+			}
+		}
+		if (spi == SPI_MAX)
+			return -ENOSPC;
+	}
+
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+	irq_set_handler(virq, handle_simple_irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+	.alloc	= tango_alloc,
+};
+
+static int __init map_irq(struct device_node *gic, int spi, int trigger)
+{
+	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	struct tango_intc *intc;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	if (!intc)
+		panic("%s: Failed to kalloc\n", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 1; spi < SPI_MAX; ++spi) {
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	spin_lock_init(&intc->lock);
+	intc->base = of_iomap(node, 0);
+	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+	if (!intc->base || !intc->dom)
+		panic("%s: Failed to setup IRQ controller\n", node->name);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

[PATCH v5] irqchip: Add support for tango interrupt router

[Mason]

This controller maps 128 input lines to 24 output lines.
The output lines are routed to GIC SPI 0 to 23.
This driver muxes LEVEL_HIGH IRQs onto output line 0,
and gives every EDGE_RISING IRQ a dedicated output line.
---
Changes from v4 to v5:
- maz said "data->mask is definitely off limits. it is an internal
data structure for the generic irqchip, and drivers have no business
touching that." Use an array instead.
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 +++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 174 +++++++++++++++++++++
 3 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller@6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..2219c449b29e
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,174 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver muxes LEVEL_HIGH IRQs onto output line 0,
+ * and gives every EDGE_RISING IRQ a dedicated output line.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define LEVEL_SPI	0
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	struct irq_domain *dom;
+	u8 spi_to_tango_irq[SPI_MAX];
+	u8 tango_irq_to_spi[IRQ_MAX];
+	DECLARE_BITMAP(enabled_level, IRQ_MAX);
+	spinlock_t lock;
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	uint pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&intc->lock);
+	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
+	spin_unlock(&intc->lock);
+
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	uint virq;
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, tango_irq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 spi = intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	if (spi == LEVEL_SPI)
+		__clear_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 spi = intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(IRQ_ENABLE | spi, intc->base + data->hwirq * 4);
+	if (spi == LEVEL_SPI)
+		__set_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int tango_set_type(struct irq_data *data, uint flow_type)
+{
+	return 0;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
+{
+	int spi;
+	struct irq_fwspec *fwspec = arg;
+	struct tango_intc *intc = dom->host_data;
+	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
+
+	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	if (trigger & IRQ_TYPE_LEVEL_HIGH)
+		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
+
+	if (trigger & IRQ_TYPE_EDGE_RISING) {
+		for (spi = 1; spi < SPI_MAX; ++spi) {
+			if (intc->spi_to_tango_irq[spi] == 0) {
+				intc->tango_irq_to_spi[hwirq] = spi;
+				intc->spi_to_tango_irq[spi] = hwirq;
+				break;
+			}
+		}
+		if (spi == SPI_MAX)
+			return -ENOSPC;
+	}
+
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+	irq_set_handler(virq, handle_simple_irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+	.alloc	= tango_alloc,
+};
+
+static int __init map_irq(struct device_node *gic, int spi, int trigger)
+{
+	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	struct tango_intc *intc;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	if (!intc)
+		panic("%s: Failed to kalloc\n", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 1; spi < SPI_MAX; ++spi) {
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	spin_lock_init(&intc->lock);
+	intc->base = of_iomap(node, 0);
+	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+	if (!intc->base || !intc->dom)
+		panic("%s: Failed to setup IRQ controller\n", node->name);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

[PATCH v5] irqchip: Add support for tango interrupt router

[Mason]

This controller maps 128 input lines to 24 output lines.
The output lines are routed to GIC SPI 0 to 23.
This driver muxes LEVEL_HIGH IRQs onto output line 0,
and gives every EDGE_RISING IRQ a dedicated output line.
---
Changes from v4 to v5:
- maz said "data->mask is definitely off limits. it is an internal
data structure for the generic irqchip, and drivers have no business
touching that." Use an array instead.
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 +++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 174 +++++++++++++++++++++
 3 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller at 6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..2219c449b29e
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,174 @@
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver muxes LEVEL_HIGH IRQs onto output line 0,
+ * and gives every EDGE_RISING IRQ a dedicated output line.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define LEVEL_SPI	0
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	struct irq_domain *dom;
+	u8 spi_to_tango_irq[SPI_MAX];
+	u8 tango_irq_to_spi[IRQ_MAX];
+	DECLARE_BITMAP(enabled_level, IRQ_MAX);
+	spinlock_t lock;
+};
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	uint pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&intc->lock);
+	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
+	spin_unlock(&intc->lock);
+
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	uint virq;
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, tango_irq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 spi = intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	if (spi == LEVEL_SPI)
+		__clear_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 spi = intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(IRQ_ENABLE | spi, intc->base + data->hwirq * 4);
+	if (spi == LEVEL_SPI)
+		__set_bit(data->hwirq, intc->enabled_level);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int tango_set_type(struct irq_data *data, uint flow_type)
+{
+	return 0;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
+{
+	int spi;
+	struct irq_fwspec *fwspec = arg;
+	struct tango_intc *intc = dom->host_data;
+	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
+
+	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	if (trigger & IRQ_TYPE_LEVEL_HIGH)
+		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
+
+	if (trigger & IRQ_TYPE_EDGE_RISING) {
+		for (spi = 1; spi < SPI_MAX; ++spi) {
+			if (intc->spi_to_tango_irq[spi] == 0) {
+				intc->tango_irq_to_spi[hwirq] = spi;
+				intc->spi_to_tango_irq[spi] = hwirq;
+				break;
+			}
+		}
+		if (spi == SPI_MAX)
+			return -ENOSPC;
+	}
+
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+	irq_set_handler(virq, handle_simple_irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+	.alloc	= tango_alloc,
+};
+
+static int __init map_irq(struct device_node *gic, int spi, int trigger)
+{
+	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	struct tango_intc *intc;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	if (!intc)
+		panic("%s: Failed to kalloc\n", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 1; spi < SPI_MAX; ++spi) {
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	spin_lock_init(&intc->lock);
+	intc->base = of_iomap(node, 0);
+	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+	if (!intc->base || !intc->dom)
+		panic("%s: Failed to setup IRQ controller\n", node->name);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

[PATCH v6] irqchip: Add support for tango interrupt router

[Mason]

This controller maps 128 input lines to 24 output lines.
The output lines are routed to GIC SPI 0 to 23.
This driver muxes LEVEL_HIGH IRQs onto output line 0,
and gives every EDGE_RISING IRQ a dedicated output line.
---
Changes from v5 to v6:
Add suspend/resume support

This driver is now feature complete AFAICT.
Marc, Thomas, Jason, it would be great if you could tell me
if I did something stupid wrt the irqchip API.
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 ++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 204 +++++++++++++++++++++
 3 files changed, 223 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller@6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..a1680a250598
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,204 @@
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/syscore_ops.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver muxes LEVEL_HIGH IRQs onto output line 0,
+ * and gives every EDGE_RISING IRQ a dedicated output line.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define LEVEL_SPI	0
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	struct irq_domain *dom;
+	u8 spi_to_tango_irq[SPI_MAX];
+	u8 tango_irq_to_spi[IRQ_MAX];
+	DECLARE_BITMAP(enabled_level, IRQ_MAX);
+	DECLARE_BITMAP(enabled_edge, IRQ_MAX);
+	spinlock_t lock;
+};
+
+static struct tango_intc *tango_intc;
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	uint pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&intc->lock);
+	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
+	spin_unlock(&intc->lock);
+
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	uint virq;
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, tango_irq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 spi = intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	if (spi == LEVEL_SPI)
+		__clear_bit(data->hwirq, intc->enabled_level);
+	else
+		__clear_bit(data->hwirq, intc->enabled_edge);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 spi = intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(IRQ_ENABLE | spi, intc->base + data->hwirq * 4);
+	if (spi == LEVEL_SPI)
+		__set_bit(data->hwirq, intc->enabled_level);
+	else
+		__set_bit(data->hwirq, intc->enabled_edge);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int tango_set_type(struct irq_data *data, uint flow_type)
+{
+	return 0;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
+{
+	int spi;
+	struct irq_fwspec *fwspec = arg;
+	struct tango_intc *intc = dom->host_data;
+	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
+
+	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	if (trigger & IRQ_TYPE_LEVEL_HIGH)
+		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
+
+	if (trigger & IRQ_TYPE_EDGE_RISING) {
+		for (spi = 1; spi < SPI_MAX; ++spi) {
+			if (intc->spi_to_tango_irq[spi] == 0) {
+				intc->tango_irq_to_spi[hwirq] = spi;
+				intc->spi_to_tango_irq[spi] = hwirq;
+				break;
+			}
+		}
+		if (spi == SPI_MAX)
+			return -ENOSPC;
+	}
+
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+	irq_set_handler(virq, handle_simple_irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+	.alloc	= tango_alloc,
+};
+
+static void tango_resume(void)
+{
+	int hwirq;
+	DECLARE_BITMAP(enabled, IRQ_MAX);
+	struct tango_intc *intc = tango_intc;
+
+	bitmap_or(enabled, intc->enabled_level, intc->enabled_edge, IRQ_MAX);
+	for (hwirq = 0; hwirq < IRQ_MAX; ++hwirq) {
+		u32 val = intc->tango_irq_to_spi[hwirq];
+		if (test_bit(hwirq, enabled))
+			val |= IRQ_ENABLE;
+		writel_relaxed(val, intc->base + hwirq * 4);
+	}
+}
+
+static struct syscore_ops tango_syscore_ops = {
+	.resume		= tango_resume,
+};
+
+static int __init map_irq(struct device_node *gic, int spi, int trigger)
+{
+	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	struct tango_intc *intc;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	if (!intc)
+		panic("%s: Failed to kalloc\n", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 1; spi < SPI_MAX; ++spi) {
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	tango_intc = intc;
+	register_syscore_ops(&tango_syscore_ops);
+
+	spin_lock_init(&intc->lock);
+	intc->base = of_iomap(node, 0);
+	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+	if (!intc->base || !intc->dom)
+		panic("%s: Failed to setup IRQ controller\n", node->name);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

[PATCH v6] irqchip: Add support for tango interrupt router

[Mason]

This controller maps 128 input lines to 24 output lines.
The output lines are routed to GIC SPI 0 to 23.
This driver muxes LEVEL_HIGH IRQs onto output line 0,
and gives every EDGE_RISING IRQ a dedicated output line.
---
Changes from v5 to v6:
Add suspend/resume support

This driver is now feature complete AFAICT.
Marc, Thomas, Jason, it would be great if you could tell me
if I did something stupid wrt the irqchip API.
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 ++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 204 +++++++++++++++++++++
 3 files changed, 223 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..9ec922f3c0a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt router
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller at 6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..a1680a250598
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,204 @@
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/syscore_ops.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver muxes LEVEL_HIGH IRQs onto output line 0,
+ * and gives every EDGE_RISING IRQ a dedicated output line.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define LEVEL_SPI	0
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	struct irq_domain *dom;
+	u8 spi_to_tango_irq[SPI_MAX];
+	u8 tango_irq_to_spi[IRQ_MAX];
+	DECLARE_BITMAP(enabled_level, IRQ_MAX);
+	DECLARE_BITMAP(enabled_edge, IRQ_MAX);
+	spinlock_t lock;
+};
+
+static struct tango_intc *tango_intc;
+
+static void tango_level_isr(struct irq_desc *desc)
+{
+	uint pos, virq;
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	DECLARE_BITMAP(status, IRQ_MAX);
+
+	chained_irq_enter(chip, desc);
+
+	spin_lock(&intc->lock);
+	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
+	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
+	spin_unlock(&intc->lock);
+
+	for_each_set_bit(pos, status, IRQ_MAX) {
+		virq = irq_find_mapping(intc->dom, pos);
+		generic_handle_irq(virq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_edge_isr(struct irq_desc *desc)
+{
+	uint virq;
+	struct irq_data *data = irq_desc_get_irq_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	struct tango_intc *intc = irq_desc_get_handler_data(desc);
+	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
+
+	chained_irq_enter(chip, desc);
+	virq = irq_find_mapping(intc->dom, tango_irq);
+	generic_handle_irq(virq);
+	chained_irq_exit(chip, desc);
+}
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 spi = intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	if (spi == LEVEL_SPI)
+		__clear_bit(data->hwirq, intc->enabled_level);
+	else
+		__clear_bit(data->hwirq, intc->enabled_edge);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 spi = intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(IRQ_ENABLE | spi, intc->base + data->hwirq * 4);
+	if (spi == LEVEL_SPI)
+		__set_bit(data->hwirq, intc->enabled_level);
+	else
+		__set_bit(data->hwirq, intc->enabled_edge);
+	spin_unlock_irqrestore(&intc->lock, flags);
+}
+
+static int tango_set_type(struct irq_data *data, uint flow_type)
+{
+	return 0;
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "tango",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_set_type	= tango_set_type,
+};
+
+static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
+{
+	int spi;
+	struct irq_fwspec *fwspec = arg;
+	struct tango_intc *intc = dom->host_data;
+	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
+
+	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	if (trigger & IRQ_TYPE_LEVEL_HIGH)
+		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
+
+	if (trigger & IRQ_TYPE_EDGE_RISING) {
+		for (spi = 1; spi < SPI_MAX; ++spi) {
+			if (intc->spi_to_tango_irq[spi] == 0) {
+				intc->tango_irq_to_spi[hwirq] = spi;
+				intc->spi_to_tango_irq[spi] = hwirq;
+				break;
+			}
+		}
+		if (spi == SPI_MAX)
+			return -ENOSPC;
+	}
+
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+	irq_set_handler(virq, handle_simple_irq);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+	.alloc	= tango_alloc,
+};
+
+static void tango_resume(void)
+{
+	int hwirq;
+	DECLARE_BITMAP(enabled, IRQ_MAX);
+	struct tango_intc *intc = tango_intc;
+
+	bitmap_or(enabled, intc->enabled_level, intc->enabled_edge, IRQ_MAX);
+	for (hwirq = 0; hwirq < IRQ_MAX; ++hwirq) {
+		u32 val = intc->tango_irq_to_spi[hwirq];
+		if (test_bit(hwirq, enabled))
+			val |= IRQ_ENABLE;
+		writel_relaxed(val, intc->base + hwirq * 4);
+	}
+}
+
+static struct syscore_ops tango_syscore_ops = {
+	.resume		= tango_resume,
+};
+
+static int __init map_irq(struct device_node *gic, int spi, int trigger)
+{
+	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
+	return irq_create_of_mapping(&irq_data);
+}
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	int spi, virq;
+	struct tango_intc *intc;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	if (!intc)
+		panic("%s: Failed to kalloc\n", node->name);
+
+	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
+	if (!virq)
+		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
+
+	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
+
+	for (spi = 1; spi < SPI_MAX; ++spi) {
+		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
+		if (!virq)
+			panic("%s: Failed to map IRQ %d\n", node->name, spi);
+
+		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
+	}
+
+	tango_intc = intc;
+	register_syscore_ops(&tango_syscore_ops);
+
+	spin_lock_init(&intc->lock);
+	intc->base = of_iomap(node, 0);
+	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
+	if (!intc->base || !intc->dom)
+		panic("%s: Failed to setup IRQ controller\n", node->name);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

Re: [PATCH v6] irqchip: Add support for tango interrupt router

[Marc Zyngier]

On 01/08/17 17:56, Mason wrote:
> This controller maps 128 input lines to 24 output lines.
> The output lines are routed to GIC SPI 0 to 23.
> This driver muxes LEVEL_HIGH IRQs onto output line 0,
> and gives every EDGE_RISING IRQ a dedicated output line.
> ---
> Changes from v5 to v6:
> Add suspend/resume support
> 
> This driver is now feature complete AFAICT.
> Marc, Thomas, Jason, it would be great if you could tell me
> if I did something stupid wrt the irqchip API.
> ---
>  .../interrupt-controller/sigma,smp8759-intc.txt    |  18 ++
>  drivers/irqchip/Makefile                           |   2 +-
>  drivers/irqchip/irq-smp8759.c                      | 204 +++++++++++++++++++++
>  3 files changed, 223 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
>  create mode 100644 drivers/irqchip/irq-smp8759.c
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
> new file mode 100644
> index 000000000000..9ec922f3c0a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
> @@ -0,0 +1,18 @@
> +Sigma Designs SMP8759 interrupt router
> +
> +Required properties:
> +- compatible: "sigma,smp8759-intc"
> +- reg: address/size of register area
> +- interrupt-controller
> +- #interrupt-cells: <2>  (hwirq and trigger_type)
> +- interrupt-parent: parent phandle
> +
> +Example:
> +
> +	interrupt-controller@6f800 {
> +		compatible = "sigma,smp8759-intc";
> +		reg = <0x6f800 0x430>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-parent = <&gic>;
> +	};
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index e4dbfc85abdb..013104923b71 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
>  obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
>  obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
>  obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
> -obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
> +obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
>  obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
>  obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
>  obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
> diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
> new file mode 100644
> index 000000000000..a1680a250598
> --- /dev/null
> +++ b/drivers/irqchip/irq-smp8759.c
> @@ -0,0 +1,204 @@
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/syscore_ops.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +/*
> + * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
> + * The output lines are routed to GIC SPI 0 to 23.
> + * This driver muxes LEVEL_HIGH IRQs onto output line 0,
> + * and gives every EDGE_RISING IRQ a dedicated output line.
> + */
> +#define IRQ_MAX		128
> +#define SPI_MAX		24
> +#define LEVEL_SPI	0
> +#define IRQ_ENABLE	BIT(31)
> +#define STATUS		0x420
> +
> +struct tango_intc {
> +	void __iomem *base;
> +	struct irq_domain *dom;
> +	u8 spi_to_tango_irq[SPI_MAX];
> +	u8 tango_irq_to_spi[IRQ_MAX];
> +	DECLARE_BITMAP(enabled_level, IRQ_MAX);
> +	DECLARE_BITMAP(enabled_edge, IRQ_MAX);
> +	spinlock_t lock;
> +};
> +
> +static struct tango_intc *tango_intc;
> +
> +static void tango_level_isr(struct irq_desc *desc)
> +{
> +	uint pos, virq;
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
> +	DECLARE_BITMAP(status, IRQ_MAX);
> +
> +	chained_irq_enter(chip, desc);
> +
> +	spin_lock(&intc->lock);
> +	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);

No. Please don't. Nothing guarantees that your bus can deal with those.
We have readl_relaxed, which is what should be used.

Also, you do know which inputs are level, right? So why do you need to
read the whole register array all the time?

> +	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
> +	spin_unlock(&intc->lock);
> +
> +	for_each_set_bit(pos, status, IRQ_MAX) {
> +		virq = irq_find_mapping(intc->dom, pos);
> +		generic_handle_irq(virq);

Please check for virq==0, just in case you get a spurious interrupt.

> +	}
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +static void tango_edge_isr(struct irq_desc *desc)
> +{
> +	uint virq;
> +	struct irq_data *data = irq_desc_get_irq_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
> +	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
> +
> +	chained_irq_enter(chip, desc);
> +	virq = irq_find_mapping(intc->dom, tango_irq);
> +	generic_handle_irq(virq);
> +	chained_irq_exit(chip, desc);

If you have a 1:1 mapping between an edge input and its output, why do
you with a chained interrupt handler? This should be a hierarchical
setup for these 23 interrupts.

> +}
> +
> +static void tango_mask(struct irq_data *data)
> +{
> +	unsigned long flags;
> +	struct tango_intc *intc = data->chip_data;
> +	u32 spi = intc->tango_irq_to_spi[data->hwirq];
> +
> +	spin_lock_irqsave(&intc->lock, flags);
> +	writel_relaxed(0, intc->base + data->hwirq * 4);
> +	if (spi == LEVEL_SPI)
> +		__clear_bit(data->hwirq, intc->enabled_level);
> +	else
> +		__clear_bit(data->hwirq, intc->enabled_edge);
> +	spin_unlock_irqrestore(&intc->lock, flags);
> +}
> +
> +static void tango_unmask(struct irq_data *data)
> +{
> +	unsigned long flags;
> +	struct tango_intc *intc = data->chip_data;
> +	u32 spi = intc->tango_irq_to_spi[data->hwirq];
> +
> +	spin_lock_irqsave(&intc->lock, flags);
> +	writel_relaxed(IRQ_ENABLE | spi, intc->base + data->hwirq * 4);
> +	if (spi == LEVEL_SPI)
> +		__set_bit(data->hwirq, intc->enabled_level);
> +	else
> +		__set_bit(data->hwirq, intc->enabled_edge);
> +	spin_unlock_irqrestore(&intc->lock, flags);
> +}
> +
> +static int tango_set_type(struct irq_data *data, uint flow_type)
> +{
> +	return 0;

What does this mean? Either you can do a set-type (and you do it), or
you cannot, and you fail. At least you check that what you're asked to
do matches the configuration.

> +}
> +
> +static struct irq_chip tango_chip = {
> +	.name		= "tango",
> +	.irq_mask	= tango_mask,
> +	.irq_unmask	= tango_unmask,
> +	.irq_set_type	= tango_set_type,
> +};
> +
> +static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
> +{
> +	int spi;
> +	struct irq_fwspec *fwspec = arg;
> +	struct tango_intc *intc = dom->host_data;
> +	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
> +
> +	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
> +		return -EINVAL;
> +
> +	if (trigger & IRQ_TYPE_LEVEL_HIGH)
> +		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
> +
> +	if (trigger & IRQ_TYPE_EDGE_RISING) {
> +		for (spi = 1; spi < SPI_MAX; ++spi) {
> +			if (intc->spi_to_tango_irq[spi] == 0) {
> +				intc->tango_irq_to_spi[hwirq] = spi;
> +				intc->spi_to_tango_irq[spi] = hwirq;
> +				break;
> +			}
> +		}
> +		if (spi == SPI_MAX)
> +			return -ENOSPC;
> +	}

What's wrong with haing a bitmap allocation, just like on other drivers?

> +
> +	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
> +	irq_set_handler(virq, handle_simple_irq);
> +
> +	return 0;
> +}
> +
> +static struct irq_domain_ops dom_ops = {
> +	.xlate	= irq_domain_xlate_twocell,
> +	.alloc	= tango_alloc,
> +};
> +
> +static void tango_resume(void)
> +{
> +	int hwirq;
> +	DECLARE_BITMAP(enabled, IRQ_MAX);
> +	struct tango_intc *intc = tango_intc;
> +
> +	bitmap_or(enabled, intc->enabled_level, intc->enabled_edge, IRQ_MAX);
> +	for (hwirq = 0; hwirq < IRQ_MAX; ++hwirq) {
> +		u32 val = intc->tango_irq_to_spi[hwirq];
> +		if (test_bit(hwirq, enabled))
> +			val |= IRQ_ENABLE;
> +		writel_relaxed(val, intc->base + hwirq * 4);
> +	}
> +}
> +
> +static struct syscore_ops tango_syscore_ops = {
> +	.resume		= tango_resume,
> +};
> +
> +static int __init map_irq(struct device_node *gic, int spi, int trigger)
> +{
> +	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
> +	return irq_create_of_mapping(&irq_data);
> +}
> +
> +static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
> +{
> +	int spi, virq;
> +	struct tango_intc *intc;
> +
> +	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
> +	if (!intc)
> +		panic("%s: Failed to kalloc\n", node->name);
> +
> +	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
> +	if (!virq)
> +		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
> +
> +	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
> +
> +	for (spi = 1; spi < SPI_MAX; ++spi) {
> +		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
> +		if (!virq)
> +			panic("%s: Failed to map IRQ %d\n", node->name, spi);
> +
> +		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
> +	}

Calling panic? For a secondary interrupt controller? Don't. We call
panic when we know for sure that the system is in such a state that
we're better off killing it altogether than keeping it running (to avoid
corruption, for example). panic is not a substitute for proper error
handling.

> +
> +	tango_intc = intc;
> +	register_syscore_ops(&tango_syscore_ops);
> +
> +	spin_lock_init(&intc->lock);
> +	intc->base = of_iomap(node, 0);
> +	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
> +	if (!intc->base || !intc->dom)
> +		panic("%s: Failed to setup IRQ controller\n", node->name);
> +
> +	return 0;
> +}
> +IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
> 

Overall, this edge business feels wrong. If you want to mux a single
putput for all level interrupts, fine by me. But edge interrupts that
have a 1:1 mapping with the underlying SPI must be represented as a
hierarchy.

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH v6] irqchip: Add support for tango interrupt router

[Marc Zyngier]

On 01/08/17 17:56, Mason wrote:
> This controller maps 128 input lines to 24 output lines.
> The output lines are routed to GIC SPI 0 to 23.
> This driver muxes LEVEL_HIGH IRQs onto output line 0,
> and gives every EDGE_RISING IRQ a dedicated output line.
> ---
> Changes from v5 to v6:
> Add suspend/resume support
> 
> This driver is now feature complete AFAICT.
> Marc, Thomas, Jason, it would be great if you could tell me
> if I did something stupid wrt the irqchip API.
> ---
>  .../interrupt-controller/sigma,smp8759-intc.txt    |  18 ++
>  drivers/irqchip/Makefile                           |   2 +-
>  drivers/irqchip/irq-smp8759.c                      | 204 +++++++++++++++++++++
>  3 files changed, 223 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
>  create mode 100644 drivers/irqchip/irq-smp8759.c
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
> new file mode 100644
> index 000000000000..9ec922f3c0a4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
> @@ -0,0 +1,18 @@
> +Sigma Designs SMP8759 interrupt router
> +
> +Required properties:
> +- compatible: "sigma,smp8759-intc"
> +- reg: address/size of register area
> +- interrupt-controller
> +- #interrupt-cells: <2>  (hwirq and trigger_type)
> +- interrupt-parent: parent phandle
> +
> +Example:
> +
> +	interrupt-controller at 6f800 {
> +		compatible = "sigma,smp8759-intc";
> +		reg = <0x6f800 0x430>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		interrupt-parent = <&gic>;
> +	};
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index e4dbfc85abdb..013104923b71 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
>  obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
>  obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
>  obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
> -obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
> +obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
>  obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
>  obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
>  obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
> diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
> new file mode 100644
> index 000000000000..a1680a250598
> --- /dev/null
> +++ b/drivers/irqchip/irq-smp8759.c
> @@ -0,0 +1,204 @@
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/syscore_ops.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +/*
> + * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
> + * The output lines are routed to GIC SPI 0 to 23.
> + * This driver muxes LEVEL_HIGH IRQs onto output line 0,
> + * and gives every EDGE_RISING IRQ a dedicated output line.
> + */
> +#define IRQ_MAX		128
> +#define SPI_MAX		24
> +#define LEVEL_SPI	0
> +#define IRQ_ENABLE	BIT(31)
> +#define STATUS		0x420
> +
> +struct tango_intc {
> +	void __iomem *base;
> +	struct irq_domain *dom;
> +	u8 spi_to_tango_irq[SPI_MAX];
> +	u8 tango_irq_to_spi[IRQ_MAX];
> +	DECLARE_BITMAP(enabled_level, IRQ_MAX);
> +	DECLARE_BITMAP(enabled_edge, IRQ_MAX);
> +	spinlock_t lock;
> +};
> +
> +static struct tango_intc *tango_intc;
> +
> +static void tango_level_isr(struct irq_desc *desc)
> +{
> +	uint pos, virq;
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
> +	DECLARE_BITMAP(status, IRQ_MAX);
> +
> +	chained_irq_enter(chip, desc);
> +
> +	spin_lock(&intc->lock);
> +	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);

No. Please don't. Nothing guarantees that your bus can deal with those.
We have readl_relaxed, which is what should be used.

Also, you do know which inputs are level, right? So why do you need to
read the whole register array all the time?

> +	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
> +	spin_unlock(&intc->lock);
> +
> +	for_each_set_bit(pos, status, IRQ_MAX) {
> +		virq = irq_find_mapping(intc->dom, pos);
> +		generic_handle_irq(virq);

Please check for virq==0, just in case you get a spurious interrupt.

> +	}
> +
> +	chained_irq_exit(chip, desc);
> +}
> +
> +static void tango_edge_isr(struct irq_desc *desc)
> +{
> +	uint virq;
> +	struct irq_data *data = irq_desc_get_irq_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
> +	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
> +
> +	chained_irq_enter(chip, desc);
> +	virq = irq_find_mapping(intc->dom, tango_irq);
> +	generic_handle_irq(virq);
> +	chained_irq_exit(chip, desc);

If you have a 1:1 mapping between an edge input and its output, why do
you with a chained interrupt handler? This should be a hierarchical
setup for these 23 interrupts.

> +}
> +
> +static void tango_mask(struct irq_data *data)
> +{
> +	unsigned long flags;
> +	struct tango_intc *intc = data->chip_data;
> +	u32 spi = intc->tango_irq_to_spi[data->hwirq];
> +
> +	spin_lock_irqsave(&intc->lock, flags);
> +	writel_relaxed(0, intc->base + data->hwirq * 4);
> +	if (spi == LEVEL_SPI)
> +		__clear_bit(data->hwirq, intc->enabled_level);
> +	else
> +		__clear_bit(data->hwirq, intc->enabled_edge);
> +	spin_unlock_irqrestore(&intc->lock, flags);
> +}
> +
> +static void tango_unmask(struct irq_data *data)
> +{
> +	unsigned long flags;
> +	struct tango_intc *intc = data->chip_data;
> +	u32 spi = intc->tango_irq_to_spi[data->hwirq];
> +
> +	spin_lock_irqsave(&intc->lock, flags);
> +	writel_relaxed(IRQ_ENABLE | spi, intc->base + data->hwirq * 4);
> +	if (spi == LEVEL_SPI)
> +		__set_bit(data->hwirq, intc->enabled_level);
> +	else
> +		__set_bit(data->hwirq, intc->enabled_edge);
> +	spin_unlock_irqrestore(&intc->lock, flags);
> +}
> +
> +static int tango_set_type(struct irq_data *data, uint flow_type)
> +{
> +	return 0;

What does this mean? Either you can do a set-type (and you do it), or
you cannot, and you fail. At least you check that what you're asked to
do matches the configuration.

> +}
> +
> +static struct irq_chip tango_chip = {
> +	.name		= "tango",
> +	.irq_mask	= tango_mask,
> +	.irq_unmask	= tango_unmask,
> +	.irq_set_type	= tango_set_type,
> +};
> +
> +static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
> +{
> +	int spi;
> +	struct irq_fwspec *fwspec = arg;
> +	struct tango_intc *intc = dom->host_data;
> +	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
> +
> +	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
> +		return -EINVAL;
> +
> +	if (trigger & IRQ_TYPE_LEVEL_HIGH)
> +		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
> +
> +	if (trigger & IRQ_TYPE_EDGE_RISING) {
> +		for (spi = 1; spi < SPI_MAX; ++spi) {
> +			if (intc->spi_to_tango_irq[spi] == 0) {
> +				intc->tango_irq_to_spi[hwirq] = spi;
> +				intc->spi_to_tango_irq[spi] = hwirq;
> +				break;
> +			}
> +		}
> +		if (spi == SPI_MAX)
> +			return -ENOSPC;
> +	}

What's wrong with haing a bitmap allocation, just like on other drivers?

> +
> +	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
> +	irq_set_handler(virq, handle_simple_irq);
> +
> +	return 0;
> +}
> +
> +static struct irq_domain_ops dom_ops = {
> +	.xlate	= irq_domain_xlate_twocell,
> +	.alloc	= tango_alloc,
> +};
> +
> +static void tango_resume(void)
> +{
> +	int hwirq;
> +	DECLARE_BITMAP(enabled, IRQ_MAX);
> +	struct tango_intc *intc = tango_intc;
> +
> +	bitmap_or(enabled, intc->enabled_level, intc->enabled_edge, IRQ_MAX);
> +	for (hwirq = 0; hwirq < IRQ_MAX; ++hwirq) {
> +		u32 val = intc->tango_irq_to_spi[hwirq];
> +		if (test_bit(hwirq, enabled))
> +			val |= IRQ_ENABLE;
> +		writel_relaxed(val, intc->base + hwirq * 4);
> +	}
> +}
> +
> +static struct syscore_ops tango_syscore_ops = {
> +	.resume		= tango_resume,
> +};
> +
> +static int __init map_irq(struct device_node *gic, int spi, int trigger)
> +{
> +	struct of_phandle_args irq_data = { gic, 3, { 0, spi, trigger }};
> +	return irq_create_of_mapping(&irq_data);
> +}
> +
> +static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
> +{
> +	int spi, virq;
> +	struct tango_intc *intc;
> +
> +	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
> +	if (!intc)
> +		panic("%s: Failed to kalloc\n", node->name);
> +
> +	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
> +	if (!virq)
> +		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
> +
> +	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
> +
> +	for (spi = 1; spi < SPI_MAX; ++spi) {
> +		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
> +		if (!virq)
> +			panic("%s: Failed to map IRQ %d\n", node->name, spi);
> +
> +		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
> +	}

Calling panic? For a secondary interrupt controller? Don't. We call
panic when we know for sure that the system is in such a state that
we're better off killing it altogether than keeping it running (to avoid
corruption, for example). panic is not a substitute for proper error
handling.

> +
> +	tango_intc = intc;
> +	register_syscore_ops(&tango_syscore_ops);
> +
> +	spin_lock_init(&intc->lock);
> +	intc->base = of_iomap(node, 0);
> +	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
> +	if (!intc->base || !intc->dom)
> +		panic("%s: Failed to setup IRQ controller\n", node->name);
> +
> +	return 0;
> +}
> +IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
> 

Overall, this edge business feels wrong. If you want to mux a single
putput for all level interrupts, fine by me. But edge interrupts that
have a 1:1 mapping with the underlying SPI must be represented as a
hierarchy.

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v6] irqchip: Add support for tango interrupt router

[Mason]

On 07/08/2017 14:47, Marc Zyngier wrote:

> On 01/08/17 17:56, Mason wrote:
>
>> +/*
>> + * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
>> + * The output lines are routed to GIC SPI 0 to 23.
>> + * This driver muxes LEVEL_HIGH IRQs onto output line 0,
>> + * and gives every EDGE_RISING IRQ a dedicated output line.
>> + */
>> +#define IRQ_MAX		128
>> +#define SPI_MAX		24
>> +#define LEVEL_SPI	0
>> +#define IRQ_ENABLE	BIT(31)
>> +#define STATUS		0x420
>> +
>> +struct tango_intc {
>> +	void __iomem *base;
>> +	struct irq_domain *dom;
>> +	u8 spi_to_tango_irq[SPI_MAX];
>> +	u8 tango_irq_to_spi[IRQ_MAX];
>> +	DECLARE_BITMAP(enabled_level, IRQ_MAX);
>> +	DECLARE_BITMAP(enabled_edge, IRQ_MAX);
>> +	spinlock_t lock;
>> +};
>> +
>> +static struct tango_intc *tango_intc;
>> +
>> +static void tango_level_isr(struct irq_desc *desc)
>> +{
>> +	uint pos, virq;
>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
>> +	DECLARE_BITMAP(status, IRQ_MAX);
>> +
>> +	chained_irq_enter(chip, desc);
>> +
>> +	spin_lock(&intc->lock);
>> +	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
> 
> No. Please don't. Nothing guarantees that your bus can deal with those.
> We have readl_relaxed, which is what should be used.

Is that because readl_relaxed() handles endianness, while
memcpy_fromio() does not?

How do I fill a DECLARE_BITMAP using readl_relaxed() ?

> Also, you do know which inputs are level, right? So why do you need to
> read the whole register array all the time?

AFAIR, interrupts are scattered all over the map, so there's
at least one interrupt per word. I'll double-check.


>> +	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
>> +	spin_unlock(&intc->lock);
>> +
>> +	for_each_set_bit(pos, status, IRQ_MAX) {
>> +		virq = irq_find_mapping(intc->dom, pos);
>> +		generic_handle_irq(virq);
> 
> Please check for virq==0, just in case you get a spurious interrupt.

AFAICT, generic_handle_irq() would handle virq==0
gracefully(?)


>> +static void tango_edge_isr(struct irq_desc *desc)
>> +{
>> +	uint virq;
>> +	struct irq_data *data = irq_desc_get_irq_data(desc);
>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
>> +	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
>> +
>> +	chained_irq_enter(chip, desc);
>> +	virq = irq_find_mapping(intc->dom, tango_irq);
>> +	generic_handle_irq(virq);
>> +	chained_irq_exit(chip, desc);
> 
> If you have a 1:1 mapping between an edge input and its output, why do
> you with a chained interrupt handler? This should be a hierarchical
> setup for these 23 interrupts.

I don't understand what you are suggesting.

I should not call chained_irq_enter/chained_irq_exit
in tango_edge_isr()?


>> +static int tango_set_type(struct irq_data *data, uint flow_type)
>> +{
>> +	return 0;
> 
> What does this mean? Either you can do a set-type (and you do it), or
> you cannot, and you fail. At least you check that what you're asked to
> do matches the configuration.

IIRC, __irq_set_trigger() barfed when I did it differently.

(FWIW, this HW block only routes interrupt signals, it doesn't
latch anything.)


>> +static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
>> +{
>> +	int spi;
>> +	struct irq_fwspec *fwspec = arg;
>> +	struct tango_intc *intc = dom->host_data;
>> +	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
>> +
>> +	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
>> +		return -EINVAL;
>> +
>> +	if (trigger & IRQ_TYPE_LEVEL_HIGH)
>> +		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
>> +
>> +	if (trigger & IRQ_TYPE_EDGE_RISING) {
>> +		for (spi = 1; spi < SPI_MAX; ++spi) {
>> +			if (intc->spi_to_tango_irq[spi] == 0) {
>> +				intc->tango_irq_to_spi[hwirq] = spi;
>> +				intc->spi_to_tango_irq[spi] = hwirq;
>> +				break;
>> +			}
>> +		}
>> +		if (spi == SPI_MAX)
>> +			return -ENOSPC;
>> +	}
> 
> What's wrong with having a bitmap allocation, just like on other drivers?

I don't understand what you are suggesting.

The mapping is set up at run-time, I need to record it
somewhere.


>> +static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
>> +{
>> +	int spi, virq;
>> +	struct tango_intc *intc;
>> +
>> +	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
>> +	if (!intc)
>> +		panic("%s: Failed to kalloc\n", node->name);
>> +
>> +	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
>> +	if (!virq)
>> +		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
>> +
>> +	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
>> +
>> +	for (spi = 1; spi < SPI_MAX; ++spi) {
>> +		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
>> +		if (!virq)
>> +			panic("%s: Failed to map IRQ %d\n", node->name, spi);
>> +
>> +		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
>> +	}
> 
> Calling panic? For a secondary interrupt controller? Don't. We call
> panic when we know for sure that the system is in such a state that
> we're better off killing it altogether than keeping it running (to avoid
> corruption, for example). panic is not a substitute for proper error
> handling.

I handled the setup like irq-tango.c did.

What is the point in bringing up a system where
interrupts do not work? Nothing will work.


>> +	tango_intc = intc;
>> +	register_syscore_ops(&tango_syscore_ops);
>> +
>> +	spin_lock_init(&intc->lock);
>> +	intc->base = of_iomap(node, 0);
>> +	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
>> +	if (!intc->base || !intc->dom)
>> +		panic("%s: Failed to setup IRQ controller\n", node->name);
>> +
>> +	return 0;
>> +}
>> +IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
> 
> Overall, this edge business feels wrong. If you want to mux a single
> output for all level interrupts, fine by me. But edge interrupts that
> have a 1:1 mapping with the underlying SPI must be represented as a
> hierarchy.

I don't understand what you mean by "feels wrong".

There are 128 inputs, and only 24 outputs.
Therefore, I must map some inputs to the same output.
Thomas explained that edge interrupts *cannot* be shared.
So edge interrupts must receive a dedicated output line.
Did I write anything wrong so far?

Therefore, I figured that I must
A) map every edge interrupt to a different output
B) map at least some level interrupts to the same output

Are you saying that I could do things differently?

Do you mean I should have defined two separate domains,
one for level interrupts, one for edge interrupts?

For level interrupts:
irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);

For edge interrupts:
irq_domain_add_hierarchy(...)

Eventually, irq_domain_create_hierarchy() calls
irq_domain_create_linear() anyway.

So maybe you are suggesting a single hierarchical
domain. I will test tomorrow. What differences will
it make? Better performance?

Regards.

[PATCH v6] irqchip: Add support for tango interrupt router

[Mason]

On 07/08/2017 14:47, Marc Zyngier wrote:

> On 01/08/17 17:56, Mason wrote:
>
>> +/*
>> + * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
>> + * The output lines are routed to GIC SPI 0 to 23.
>> + * This driver muxes LEVEL_HIGH IRQs onto output line 0,
>> + * and gives every EDGE_RISING IRQ a dedicated output line.
>> + */
>> +#define IRQ_MAX		128
>> +#define SPI_MAX		24
>> +#define LEVEL_SPI	0
>> +#define IRQ_ENABLE	BIT(31)
>> +#define STATUS		0x420
>> +
>> +struct tango_intc {
>> +	void __iomem *base;
>> +	struct irq_domain *dom;
>> +	u8 spi_to_tango_irq[SPI_MAX];
>> +	u8 tango_irq_to_spi[IRQ_MAX];
>> +	DECLARE_BITMAP(enabled_level, IRQ_MAX);
>> +	DECLARE_BITMAP(enabled_edge, IRQ_MAX);
>> +	spinlock_t lock;
>> +};
>> +
>> +static struct tango_intc *tango_intc;
>> +
>> +static void tango_level_isr(struct irq_desc *desc)
>> +{
>> +	uint pos, virq;
>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
>> +	DECLARE_BITMAP(status, IRQ_MAX);
>> +
>> +	chained_irq_enter(chip, desc);
>> +
>> +	spin_lock(&intc->lock);
>> +	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
> 
> No. Please don't. Nothing guarantees that your bus can deal with those.
> We have readl_relaxed, which is what should be used.

Is that because readl_relaxed() handles endianness, while
memcpy_fromio() does not?

How do I fill a DECLARE_BITMAP using readl_relaxed() ?

> Also, you do know which inputs are level, right? So why do you need to
> read the whole register array all the time?

AFAIR, interrupts are scattered all over the map, so there's
at least one interrupt per word. I'll double-check.


>> +	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
>> +	spin_unlock(&intc->lock);
>> +
>> +	for_each_set_bit(pos, status, IRQ_MAX) {
>> +		virq = irq_find_mapping(intc->dom, pos);
>> +		generic_handle_irq(virq);
> 
> Please check for virq==0, just in case you get a spurious interrupt.

AFAICT, generic_handle_irq() would handle virq==0
gracefully(?)


>> +static void tango_edge_isr(struct irq_desc *desc)
>> +{
>> +	uint virq;
>> +	struct irq_data *data = irq_desc_get_irq_data(desc);
>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
>> +	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
>> +
>> +	chained_irq_enter(chip, desc);
>> +	virq = irq_find_mapping(intc->dom, tango_irq);
>> +	generic_handle_irq(virq);
>> +	chained_irq_exit(chip, desc);
> 
> If you have a 1:1 mapping between an edge input and its output, why do
> you with a chained interrupt handler? This should be a hierarchical
> setup for these 23 interrupts.

I don't understand what you are suggesting.

I should not call chained_irq_enter/chained_irq_exit
in tango_edge_isr()?


>> +static int tango_set_type(struct irq_data *data, uint flow_type)
>> +{
>> +	return 0;
> 
> What does this mean? Either you can do a set-type (and you do it), or
> you cannot, and you fail. At least you check that what you're asked to
> do matches the configuration.

IIRC, __irq_set_trigger() barfed when I did it differently.

(FWIW, this HW block only routes interrupt signals, it doesn't
latch anything.)


>> +static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
>> +{
>> +	int spi;
>> +	struct irq_fwspec *fwspec = arg;
>> +	struct tango_intc *intc = dom->host_data;
>> +	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
>> +
>> +	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
>> +		return -EINVAL;
>> +
>> +	if (trigger & IRQ_TYPE_LEVEL_HIGH)
>> +		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
>> +
>> +	if (trigger & IRQ_TYPE_EDGE_RISING) {
>> +		for (spi = 1; spi < SPI_MAX; ++spi) {
>> +			if (intc->spi_to_tango_irq[spi] == 0) {
>> +				intc->tango_irq_to_spi[hwirq] = spi;
>> +				intc->spi_to_tango_irq[spi] = hwirq;
>> +				break;
>> +			}
>> +		}
>> +		if (spi == SPI_MAX)
>> +			return -ENOSPC;
>> +	}
> 
> What's wrong with having a bitmap allocation, just like on other drivers?

I don't understand what you are suggesting.

The mapping is set up at run-time, I need to record it
somewhere.


>> +static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
>> +{
>> +	int spi, virq;
>> +	struct tango_intc *intc;
>> +
>> +	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
>> +	if (!intc)
>> +		panic("%s: Failed to kalloc\n", node->name);
>> +
>> +	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
>> +	if (!virq)
>> +		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
>> +
>> +	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
>> +
>> +	for (spi = 1; spi < SPI_MAX; ++spi) {
>> +		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
>> +		if (!virq)
>> +			panic("%s: Failed to map IRQ %d\n", node->name, spi);
>> +
>> +		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
>> +	}
> 
> Calling panic? For a secondary interrupt controller? Don't. We call
> panic when we know for sure that the system is in such a state that
> we're better off killing it altogether than keeping it running (to avoid
> corruption, for example). panic is not a substitute for proper error
> handling.

I handled the setup like irq-tango.c did.

What is the point in bringing up a system where
interrupts do not work? Nothing will work.


>> +	tango_intc = intc;
>> +	register_syscore_ops(&tango_syscore_ops);
>> +
>> +	spin_lock_init(&intc->lock);
>> +	intc->base = of_iomap(node, 0);
>> +	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
>> +	if (!intc->base || !intc->dom)
>> +		panic("%s: Failed to setup IRQ controller\n", node->name);
>> +
>> +	return 0;
>> +}
>> +IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
> 
> Overall, this edge business feels wrong. If you want to mux a single
> output for all level interrupts, fine by me. But edge interrupts that
> have a 1:1 mapping with the underlying SPI must be represented as a
> hierarchy.

I don't understand what you mean by "feels wrong".

There are 128 inputs, and only 24 outputs.
Therefore, I must map some inputs to the same output.
Thomas explained that edge interrupts *cannot* be shared.
So edge interrupts must receive a dedicated output line.
Did I write anything wrong so far?

Therefore, I figured that I must
A) map every edge interrupt to a different output
B) map at least some level interrupts to the same output

Are you saying that I could do things differently?

Do you mean I should have defined two separate domains,
one for level interrupts, one for edge interrupts?

For level interrupts:
irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);

For edge interrupts:
irq_domain_add_hierarchy(...)

Eventually, irq_domain_create_hierarchy() calls
irq_domain_create_linear() anyway.

So maybe you are suggesting a single hierarchical
domain. I will test tomorrow. What differences will
it make? Better performance?

Regards.

Re: [PATCH v6] irqchip: Add support for tango interrupt router

[Mason]

On 20/08/2017 19:22, Mason wrote:

> On 07/08/2017 14:47, Marc Zyngier wrote:
> 
>> On 01/08/17 17:56, Mason wrote:
>>
>>> +static int tango_set_type(struct irq_data *data, uint flow_type)
>>> +{
>>> +	return 0;
>>
>> What does this mean? Either you can do a set-type (and you do it), or
>> you cannot, and you fail. At least you check that what you're asked to
>> do matches the configuration.
> 
> IIRC, __irq_set_trigger() barfed when I did it differently.

The way the problem manifests is that /proc/interrupts cannot
determine the trigger types.

(I think the issue might be deeper than this cosmetic aspect.)

# cat /proc/interrupts
           CPU0       CPU1       CPU2       CPU3
 40:       1832        333       2378        471     GIC-0  29 Edge      twd
 41:        487          0          0          0    mapper   1 Edge      serial
 44:         50          0          0          0    mapper  38 Edge      eth0
 51:          3          0          0          0    mapper  37 Edge      phy_interrupt

(1 and 38 are actually level interrupts.)

	ret = sprintf(buf, "%s\n",
		      irqd_is_level_type(&desc->irq_data) ? "level" : "edge");


__irq_set_trigger() is a no-op when irq_set_type is not implemented.

But if the callback returns 0, then __irq_set_trigger() eventually calls

		irqd_clear(&desc->irq_data, IRQD_TRIGGER_MASK);
		irqd_set(&desc->irq_data, flags);


Should I be calling irqd_get_trigger_type() myself earlier,
perhaps in the domain alloc function?

That's what drivers/irqchip/irq-pic32-evic.c seems to do.

The comment seems to state otherwise though:
/*
 * Must only be called inside irq_chip.irq_set_type() functions.
 */
static inline void irqd_set_trigger_type(struct irq_data *d, u32 type)


Regards.

[PATCH v6] irqchip: Add support for tango interrupt router

[Mason]

On 20/08/2017 19:22, Mason wrote:

> On 07/08/2017 14:47, Marc Zyngier wrote:
> 
>> On 01/08/17 17:56, Mason wrote:
>>
>>> +static int tango_set_type(struct irq_data *data, uint flow_type)
>>> +{
>>> +	return 0;
>>
>> What does this mean? Either you can do a set-type (and you do it), or
>> you cannot, and you fail. At least you check that what you're asked to
>> do matches the configuration.
> 
> IIRC, __irq_set_trigger() barfed when I did it differently.

The way the problem manifests is that /proc/interrupts cannot
determine the trigger types.

(I think the issue might be deeper than this cosmetic aspect.)

# cat /proc/interrupts
           CPU0       CPU1       CPU2       CPU3
 40:       1832        333       2378        471     GIC-0  29 Edge      twd
 41:        487          0          0          0    mapper   1 Edge      serial
 44:         50          0          0          0    mapper  38 Edge      eth0
 51:          3          0          0          0    mapper  37 Edge      phy_interrupt

(1 and 38 are actually level interrupts.)

	ret = sprintf(buf, "%s\n",
		      irqd_is_level_type(&desc->irq_data) ? "level" : "edge");


__irq_set_trigger() is a no-op when irq_set_type is not implemented.

But if the callback returns 0, then __irq_set_trigger() eventually calls

		irqd_clear(&desc->irq_data, IRQD_TRIGGER_MASK);
		irqd_set(&desc->irq_data, flags);


Should I be calling irqd_get_trigger_type() myself earlier,
perhaps in the domain alloc function?

That's what drivers/irqchip/irq-pic32-evic.c seems to do.

The comment seems to state otherwise though:
/*
 * Must only be called inside irq_chip.irq_set_type() functions.
 */
static inline void irqd_set_trigger_type(struct irq_data *d, u32 type)


Regards.

Re: [PATCH v6] irqchip: Add support for tango interrupt router

[Marc Zyngier]

On 20/08/17 18:22, Mason wrote:
> On 07/08/2017 14:47, Marc Zyngier wrote:
> 
>> On 01/08/17 17:56, Mason wrote:
>>
>>> +/*
>>> + * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
>>> + * The output lines are routed to GIC SPI 0 to 23.
>>> + * This driver muxes LEVEL_HIGH IRQs onto output line 0,
>>> + * and gives every EDGE_RISING IRQ a dedicated output line.
>>> + */
>>> +#define IRQ_MAX		128
>>> +#define SPI_MAX		24
>>> +#define LEVEL_SPI	0
>>> +#define IRQ_ENABLE	BIT(31)
>>> +#define STATUS		0x420
>>> +
>>> +struct tango_intc {
>>> +	void __iomem *base;
>>> +	struct irq_domain *dom;
>>> +	u8 spi_to_tango_irq[SPI_MAX];
>>> +	u8 tango_irq_to_spi[IRQ_MAX];
>>> +	DECLARE_BITMAP(enabled_level, IRQ_MAX);
>>> +	DECLARE_BITMAP(enabled_edge, IRQ_MAX);
>>> +	spinlock_t lock;
>>> +};
>>> +
>>> +static struct tango_intc *tango_intc;
>>> +
>>> +static void tango_level_isr(struct irq_desc *desc)
>>> +{
>>> +	uint pos, virq;
>>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>>> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
>>> +	DECLARE_BITMAP(status, IRQ_MAX);
>>> +
>>> +	chained_irq_enter(chip, desc);
>>> +
>>> +	spin_lock(&intc->lock);
>>> +	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
>>
>> No. Please don't. Nothing guarantees that your bus can deal with those.
>> We have readl_relaxed, which is what should be used.
> 
> Is that because readl_relaxed() handles endianness, while
> memcpy_fromio() does not?

That's because memcpy_fromio is just that. A memcpy. No guarantees about
the size of the access, no indianness, no barriers.

> How do I fill a DECLARE_BITMAP using readl_relaxed() ?

You don't.

>> Also, you do know which inputs are level, right? So why do you need to
>> read the whole register array all the time?
> 
> AFAIR, interrupts are scattered all over the map, so there's
> at least one interrupt per word. I'll double-check.

And even if that's the case. You just read each word that contains a
level interrupt, deal with that one, and move on to the next.

> 
> 
>>> +	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
>>> +	spin_unlock(&intc->lock);
>>> +
>>> +	for_each_set_bit(pos, status, IRQ_MAX) {
>>> +		virq = irq_find_mapping(intc->dom, pos);
>>> +		generic_handle_irq(virq);
>>
>> Please check for virq==0, just in case you get a spurious interrupt.
> 
> AFAICT, generic_handle_irq() would handle virq==0
> gracefully(?)

Yes, by calling irq_to_desc, which results in a radix_tree_lookup. Just
check for zero here, there is no reason to propagate the crap if we can
avoid it. You can even take that opportunity to dump a splat, because it
means that something is pretty fishy if you're getting spurious interrupts.

>>> +static void tango_edge_isr(struct irq_desc *desc)
>>> +{
>>> +	uint virq;
>>> +	struct irq_data *data = irq_desc_get_irq_data(desc);
>>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>>> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
>>> +	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
>>> +
>>> +	chained_irq_enter(chip, desc);
>>> +	virq = irq_find_mapping(intc->dom, tango_irq);
>>> +	generic_handle_irq(virq);
>>> +	chained_irq_exit(chip, desc);
>>
>> If you have a 1:1 mapping between an edge input and its output, why do
>> you with a chained interrupt handler? This should be a hierarchical
>> setup for these 23 interrupts.
> 
> I don't understand what you are suggesting.
> 
> I should not call chained_irq_enter/chained_irq_exit
> in tango_edge_isr()?

I'm suggesting a hierarchical mapping, and not a multiplexer. I'm sorry,
there is no other words to explain this.

> 
>>> +static int tango_set_type(struct irq_data *data, uint flow_type)
>>> +{
>>> +	return 0;
>>
>> What does this mean? Either you can do a set-type (and you do it), or
>> you cannot, and you fail. At least you check that what you're asked to
>> do matches the configuration.
> 
> IIRC, __irq_set_trigger() barfed when I did it differently.
> 
> (FWIW, this HW block only routes interrupt signals, it doesn't
> latch anything.)

And? You obviously have different ways of dealing with edge and level
interrupts. Also, the parent irqchip deals with probably has its own
views about interrupt triggering.

> 
> 
>>> +static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
>>> +{
>>> +	int spi;
>>> +	struct irq_fwspec *fwspec = arg;
>>> +	struct tango_intc *intc = dom->host_data;
>>> +	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
>>> +
>>> +	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
>>> +		return -EINVAL;
>>> +
>>> +	if (trigger & IRQ_TYPE_LEVEL_HIGH)
>>> +		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
>>> +
>>> +	if (trigger & IRQ_TYPE_EDGE_RISING) {
>>> +		for (spi = 1; spi < SPI_MAX; ++spi) {
>>> +			if (intc->spi_to_tango_irq[spi] == 0) {
>>> +				intc->tango_irq_to_spi[hwirq] = spi;
>>> +				intc->spi_to_tango_irq[spi] = hwirq;
>>> +				break;
>>> +			}
>>> +		}
>>> +		if (spi == SPI_MAX)
>>> +			return -ENOSPC;
>>> +	}
>>
>> What's wrong with having a bitmap allocation, just like on other drivers?
> 
> I don't understand what you are suggesting.
> 
> The mapping is set up at run-time, I need to record it
> somewhere.

Again. All the other drivers in the tree are using a bitmap to deal with
their slot allocation. Why do you have to use a different data structure?

> 
> 
>>> +static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
>>> +{
>>> +	int spi, virq;
>>> +	struct tango_intc *intc;
>>> +
>>> +	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
>>> +	if (!intc)
>>> +		panic("%s: Failed to kalloc\n", node->name);
>>> +
>>> +	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
>>> +	if (!virq)
>>> +		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
>>> +
>>> +	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
>>> +
>>> +	for (spi = 1; spi < SPI_MAX; ++spi) {
>>> +		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
>>> +		if (!virq)
>>> +			panic("%s: Failed to map IRQ %d\n", node->name, spi);
>>> +
>>> +		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
>>> +	}
>>
>> Calling panic? For a secondary interrupt controller? Don't. We call
>> panic when we know for sure that the system is in such a state that
>> we're better off killing it altogether than keeping it running (to avoid
>> corruption, for example). panic is not a substitute for proper error
>> handling.
> 
> I handled the setup like irq-tango.c did.

Doesn't make it less crap.

> What is the point in bringing up a system where
> interrupts do not work? Nothing will work.

This is a secondary interrupt controller. For things that are secondary.
You already have a primary interrupt controller which can be used to
find out about what is going on and diagnostic the platform.

And if the platform is really toasted, then there is no need for calling
panic, the box won't go anywhere.

> 
> 
>>> +	tango_intc = intc;
>>> +	register_syscore_ops(&tango_syscore_ops);
>>> +
>>> +	spin_lock_init(&intc->lock);
>>> +	intc->base = of_iomap(node, 0);
>>> +	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
>>> +	if (!intc->base || !intc->dom)
>>> +		panic("%s: Failed to setup IRQ controller\n", node->name);
>>> +
>>> +	return 0;
>>> +}
>>> +IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
>>
>> Overall, this edge business feels wrong. If you want to mux a single
>> output for all level interrupts, fine by me. But edge interrupts that
>> have a 1:1 mapping with the underlying SPI must be represented as a
>> hierarchy.
> 
> I don't understand what you mean by "feels wrong".
> 
> There are 128 inputs, and only 24 outputs.
> Therefore, I must map some inputs to the same output.
> Thomas explained that edge interrupts *cannot* be shared.
> So edge interrupts must receive a dedicated output line.
> Did I write anything wrong so far?

Let me repeat what Thomas already said:

- you dedicate one line to level interrupts using a multiplexer (chained
interrupts).

- you use the remaining 23 inputs in a hierarchical model, each input
being mapped to one output, no chained handler.

That's what I want to see.

	M.

> 
> Therefore, I figured that I must
> A) map every edge interrupt to a different output
> B) map at least some level interrupts to the same output
> 
> Are you saying that I could do things differently?
> 
> Do you mean I should have defined two separate domains,
> one for level interrupts, one for edge interrupts?
> 
> For level interrupts:
> irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
> 
> For edge interrupts:
> irq_domain_add_hierarchy(...)
> 
> Eventually, irq_domain_create_hierarchy() calls
> irq_domain_create_linear() anyway.
> 
> So maybe you are suggesting a single hierarchical
> domain. I will test tomorrow. What differences will
> it make? Better performance?
> 
> Regards.
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH v6] irqchip: Add support for tango interrupt router

[Marc Zyngier]

On 20/08/17 18:22, Mason wrote:
> On 07/08/2017 14:47, Marc Zyngier wrote:
> 
>> On 01/08/17 17:56, Mason wrote:
>>
>>> +/*
>>> + * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
>>> + * The output lines are routed to GIC SPI 0 to 23.
>>> + * This driver muxes LEVEL_HIGH IRQs onto output line 0,
>>> + * and gives every EDGE_RISING IRQ a dedicated output line.
>>> + */
>>> +#define IRQ_MAX		128
>>> +#define SPI_MAX		24
>>> +#define LEVEL_SPI	0
>>> +#define IRQ_ENABLE	BIT(31)
>>> +#define STATUS		0x420
>>> +
>>> +struct tango_intc {
>>> +	void __iomem *base;
>>> +	struct irq_domain *dom;
>>> +	u8 spi_to_tango_irq[SPI_MAX];
>>> +	u8 tango_irq_to_spi[IRQ_MAX];
>>> +	DECLARE_BITMAP(enabled_level, IRQ_MAX);
>>> +	DECLARE_BITMAP(enabled_edge, IRQ_MAX);
>>> +	spinlock_t lock;
>>> +};
>>> +
>>> +static struct tango_intc *tango_intc;
>>> +
>>> +static void tango_level_isr(struct irq_desc *desc)
>>> +{
>>> +	uint pos, virq;
>>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>>> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
>>> +	DECLARE_BITMAP(status, IRQ_MAX);
>>> +
>>> +	chained_irq_enter(chip, desc);
>>> +
>>> +	spin_lock(&intc->lock);
>>> +	memcpy_fromio(status, intc->base + STATUS, IRQ_MAX / BITS_PER_BYTE);
>>
>> No. Please don't. Nothing guarantees that your bus can deal with those.
>> We have readl_relaxed, which is what should be used.
> 
> Is that because readl_relaxed() handles endianness, while
> memcpy_fromio() does not?

That's because memcpy_fromio is just that. A memcpy. No guarantees about
the size of the access, no indianness, no barriers.

> How do I fill a DECLARE_BITMAP using readl_relaxed() ?

You don't.

>> Also, you do know which inputs are level, right? So why do you need to
>> read the whole register array all the time?
> 
> AFAIR, interrupts are scattered all over the map, so there's
> at least one interrupt per word. I'll double-check.

And even if that's the case. You just read each word that contains a
level interrupt, deal with that one, and move on to the next.

> 
> 
>>> +	bitmap_and(status, status, intc->enabled_level, IRQ_MAX);
>>> +	spin_unlock(&intc->lock);
>>> +
>>> +	for_each_set_bit(pos, status, IRQ_MAX) {
>>> +		virq = irq_find_mapping(intc->dom, pos);
>>> +		generic_handle_irq(virq);
>>
>> Please check for virq==0, just in case you get a spurious interrupt.
> 
> AFAICT, generic_handle_irq() would handle virq==0
> gracefully(?)

Yes, by calling irq_to_desc, which results in a radix_tree_lookup. Just
check for zero here, there is no reason to propagate the crap if we can
avoid it. You can even take that opportunity to dump a splat, because it
means that something is pretty fishy if you're getting spurious interrupts.

>>> +static void tango_edge_isr(struct irq_desc *desc)
>>> +{
>>> +	uint virq;
>>> +	struct irq_data *data = irq_desc_get_irq_data(desc);
>>> +	struct irq_chip *chip = irq_desc_get_chip(desc);
>>> +	struct tango_intc *intc = irq_desc_get_handler_data(desc);
>>> +	int tango_irq = intc->spi_to_tango_irq[data->hwirq - 32];
>>> +
>>> +	chained_irq_enter(chip, desc);
>>> +	virq = irq_find_mapping(intc->dom, tango_irq);
>>> +	generic_handle_irq(virq);
>>> +	chained_irq_exit(chip, desc);
>>
>> If you have a 1:1 mapping between an edge input and its output, why do
>> you with a chained interrupt handler? This should be a hierarchical
>> setup for these 23 interrupts.
> 
> I don't understand what you are suggesting.
> 
> I should not call chained_irq_enter/chained_irq_exit
> in tango_edge_isr()?

I'm suggesting a hierarchical mapping, and not a multiplexer. I'm sorry,
there is no other words to explain this.

> 
>>> +static int tango_set_type(struct irq_data *data, uint flow_type)
>>> +{
>>> +	return 0;
>>
>> What does this mean? Either you can do a set-type (and you do it), or
>> you cannot, and you fail. At least you check that what you're asked to
>> do matches the configuration.
> 
> IIRC, __irq_set_trigger() barfed when I did it differently.
> 
> (FWIW, this HW block only routes interrupt signals, it doesn't
> latch anything.)

And? You obviously have different ways of dealing with edge and level
interrupts. Also, the parent irqchip deals with probably has its own
views about interrupt triggering.

> 
> 
>>> +static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
>>> +{
>>> +	int spi;
>>> +	struct irq_fwspec *fwspec = arg;
>>> +	struct tango_intc *intc = dom->host_data;
>>> +	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
>>> +
>>> +	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
>>> +		return -EINVAL;
>>> +
>>> +	if (trigger & IRQ_TYPE_LEVEL_HIGH)
>>> +		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
>>> +
>>> +	if (trigger & IRQ_TYPE_EDGE_RISING) {
>>> +		for (spi = 1; spi < SPI_MAX; ++spi) {
>>> +			if (intc->spi_to_tango_irq[spi] == 0) {
>>> +				intc->tango_irq_to_spi[hwirq] = spi;
>>> +				intc->spi_to_tango_irq[spi] = hwirq;
>>> +				break;
>>> +			}
>>> +		}
>>> +		if (spi == SPI_MAX)
>>> +			return -ENOSPC;
>>> +	}
>>
>> What's wrong with having a bitmap allocation, just like on other drivers?
> 
> I don't understand what you are suggesting.
> 
> The mapping is set up at run-time, I need to record it
> somewhere.

Again. All the other drivers in the tree are using a bitmap to deal with
their slot allocation. Why do you have to use a different data structure?

> 
> 
>>> +static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
>>> +{
>>> +	int spi, virq;
>>> +	struct tango_intc *intc;
>>> +
>>> +	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
>>> +	if (!intc)
>>> +		panic("%s: Failed to kalloc\n", node->name);
>>> +
>>> +	virq = map_irq(parent, LEVEL_SPI, IRQ_TYPE_LEVEL_HIGH);
>>> +	if (!virq)
>>> +		panic("%s: Failed to map IRQ %d\n", node->name, LEVEL_SPI);
>>> +
>>> +	irq_set_chained_handler_and_data(virq, tango_level_isr, intc);
>>> +
>>> +	for (spi = 1; spi < SPI_MAX; ++spi) {
>>> +		virq = map_irq(parent, spi, IRQ_TYPE_EDGE_RISING);
>>> +		if (!virq)
>>> +			panic("%s: Failed to map IRQ %d\n", node->name, spi);
>>> +
>>> +		irq_set_chained_handler_and_data(virq, tango_edge_isr, intc);
>>> +	}
>>
>> Calling panic? For a secondary interrupt controller? Don't. We call
>> panic when we know for sure that the system is in such a state that
>> we're better off killing it altogether than keeping it running (to avoid
>> corruption, for example). panic is not a substitute for proper error
>> handling.
> 
> I handled the setup like irq-tango.c did.

Doesn't make it less crap.

> What is the point in bringing up a system where
> interrupts do not work? Nothing will work.

This is a secondary interrupt controller. For things that are secondary.
You already have a primary interrupt controller which can be used to
find out about what is going on and diagnostic the platform.

And if the platform is really toasted, then there is no need for calling
panic, the box won't go anywhere.

> 
> 
>>> +	tango_intc = intc;
>>> +	register_syscore_ops(&tango_syscore_ops);
>>> +
>>> +	spin_lock_init(&intc->lock);
>>> +	intc->base = of_iomap(node, 0);
>>> +	intc->dom = irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
>>> +	if (!intc->base || !intc->dom)
>>> +		panic("%s: Failed to setup IRQ controller\n", node->name);
>>> +
>>> +	return 0;
>>> +}
>>> +IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
>>
>> Overall, this edge business feels wrong. If you want to mux a single
>> output for all level interrupts, fine by me. But edge interrupts that
>> have a 1:1 mapping with the underlying SPI must be represented as a
>> hierarchy.
> 
> I don't understand what you mean by "feels wrong".
> 
> There are 128 inputs, and only 24 outputs.
> Therefore, I must map some inputs to the same output.
> Thomas explained that edge interrupts *cannot* be shared.
> So edge interrupts must receive a dedicated output line.
> Did I write anything wrong so far?

Let me repeat what Thomas already said:

- you dedicate one line to level interrupts using a multiplexer (chained
interrupts).

- you use the remaining 23 inputs in a hierarchical model, each input
being mapped to one output, no chained handler.

That's what I want to see.

	M.

> 
> Therefore, I figured that I must
> A) map every edge interrupt to a different output
> B) map at least some level interrupts to the same output
> 
> Are you saying that I could do things differently?
> 
> Do you mean I should have defined two separate domains,
> one for level interrupts, one for edge interrupts?
> 
> For level interrupts:
> irq_domain_add_linear(node, IRQ_MAX, &dom_ops, intc);
> 
> For edge interrupts:
> irq_domain_add_hierarchy(...)
> 
> Eventually, irq_domain_create_hierarchy() calls
> irq_domain_create_linear() anyway.
> 
> So maybe you are suggesting a single hierarchical
> domain. I will test tomorrow. What differences will
> it make? Better performance?
> 
> Regards.
> 


-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v6] irqchip: Add support for tango interrupt router

[Mason]

On 23/08/2017 12:58, Marc Zyngier wrote:

> On 20/08/17 18:22, Mason wrote:
>
>> On 07/08/2017 14:47, Marc Zyngier wrote:
>>
>>> On 01/08/17 17:56, Mason wrote:
>>>
>>>> +static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
>>>> +{
>>>> +	int spi;
>>>> +	struct irq_fwspec *fwspec = arg;
>>>> +	struct tango_intc *intc = dom->host_data;
>>>> +	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
>>>> +
>>>> +	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (trigger & IRQ_TYPE_LEVEL_HIGH)
>>>> +		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
>>>> +
>>>> +	if (trigger & IRQ_TYPE_EDGE_RISING) {
>>>> +		for (spi = 1; spi < SPI_MAX; ++spi) {
>>>> +			if (intc->spi_to_tango_irq[spi] == 0) {
>>>> +				intc->tango_irq_to_spi[hwirq] = spi;
>>>> +				intc->spi_to_tango_irq[spi] = hwirq;
>>>> +				break;
>>>> +			}
>>>> +		}
>>>> +		if (spi == SPI_MAX)
>>>> +			return -ENOSPC;
>>>> +	}
>>>
>>> What's wrong with having a bitmap allocation, just like on other drivers?
>>
>> I don't understand what you are suggesting.
>>
>> The mapping is set up at run-time, I need to record it
>> somewhere.
> 
> Again. All the other drivers in the tree are using a bitmap to deal with
> their slot allocation. Why do you have to use a different data structure?

You appear to be objecting to the spi_to_tango_irq array.

The spi-to-tango-irq mapping has to be stored somewhere.

If I use a hierarchy for edge interrupts, as you have
demanded, then it becomes the core's responsibility to
store the mapping. Thus, I can drop the array, and just
use a bitmap to keep track of which output has already
been allocated.


>>> Calling panic? For a secondary interrupt controller? Don't. We call
>>> panic when we know for sure that the system is in such a state that
>>> we're better off killing it altogether than keeping it running (to avoid
>>> corruption, for example). panic is not a substitute for proper error
>>> handling.
>>
>> I handled the setup like irq-tango.c did.
> 
> Doesn't make it less crap.

Just want to clear something up.

If irq-tango.c were submitted today, would you demand this
issue be fixed, or are some submitters given more leeway
than others?


>>> Overall, this edge business feels wrong. If you want to mux a single
>>> output for all level interrupts, fine by me. But edge interrupts that
>>> have a 1:1 mapping with the underlying SPI must be represented as a
>>> hierarchy.
>>
>> I don't understand what you mean by "feels wrong".
>>
>> There are 128 inputs, and only 24 outputs.
>> Therefore, I must map some inputs to the same output.
>> Thomas explained that edge interrupts *cannot* be shared.
>> So edge interrupts must receive a dedicated output line.
>> Did I write anything wrong so far?
> 
> Let me repeat what Thomas already said:
> 
> - you dedicate one line to level interrupts using a multiplexer (chained
> interrupts).

OK.

> - you use the remaining 23 inputs in a hierarchical model, each input
> being mapped to one output, no chained handler.
> 
> That's what I want to see.

OK.

Can you confirm that this means two separate domains?


One last thing: about generic_handle_irq() and virq==0
I understand your point that irq_to_desc() is an expensive
operation, so it is better to check beforehand. But then,
would it not make sense to add the check in generic_handle_irq()
if all drivers are expected to do it? (Code factoring)

Regards.

[PATCH v6] irqchip: Add support for tango interrupt router

[Mason]

On 23/08/2017 12:58, Marc Zyngier wrote:

> On 20/08/17 18:22, Mason wrote:
>
>> On 07/08/2017 14:47, Marc Zyngier wrote:
>>
>>> On 01/08/17 17:56, Mason wrote:
>>>
>>>> +static int tango_alloc(struct irq_domain *dom, uint virq, uint n, void *arg)
>>>> +{
>>>> +	int spi;
>>>> +	struct irq_fwspec *fwspec = arg;
>>>> +	struct tango_intc *intc = dom->host_data;
>>>> +	u32 hwirq = fwspec->param[0], trigger = fwspec->param[1];
>>>> +
>>>> +	if (trigger & IRQ_TYPE_EDGE_FALLING || trigger & IRQ_TYPE_LEVEL_LOW)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (trigger & IRQ_TYPE_LEVEL_HIGH)
>>>> +		intc->tango_irq_to_spi[hwirq] = LEVEL_SPI;
>>>> +
>>>> +	if (trigger & IRQ_TYPE_EDGE_RISING) {
>>>> +		for (spi = 1; spi < SPI_MAX; ++spi) {
>>>> +			if (intc->spi_to_tango_irq[spi] == 0) {
>>>> +				intc->tango_irq_to_spi[hwirq] = spi;
>>>> +				intc->spi_to_tango_irq[spi] = hwirq;
>>>> +				break;
>>>> +			}
>>>> +		}
>>>> +		if (spi == SPI_MAX)
>>>> +			return -ENOSPC;
>>>> +	}
>>>
>>> What's wrong with having a bitmap allocation, just like on other drivers?
>>
>> I don't understand what you are suggesting.
>>
>> The mapping is set up at run-time, I need to record it
>> somewhere.
> 
> Again. All the other drivers in the tree are using a bitmap to deal with
> their slot allocation. Why do you have to use a different data structure?

You appear to be objecting to the spi_to_tango_irq array.

The spi-to-tango-irq mapping has to be stored somewhere.

If I use a hierarchy for edge interrupts, as you have
demanded, then it becomes the core's responsibility to
store the mapping. Thus, I can drop the array, and just
use a bitmap to keep track of which output has already
been allocated.


>>> Calling panic? For a secondary interrupt controller? Don't. We call
>>> panic when we know for sure that the system is in such a state that
>>> we're better off killing it altogether than keeping it running (to avoid
>>> corruption, for example). panic is not a substitute for proper error
>>> handling.
>>
>> I handled the setup like irq-tango.c did.
> 
> Doesn't make it less crap.

Just want to clear something up.

If irq-tango.c were submitted today, would you demand this
issue be fixed, or are some submitters given more leeway
than others?


>>> Overall, this edge business feels wrong. If you want to mux a single
>>> output for all level interrupts, fine by me. But edge interrupts that
>>> have a 1:1 mapping with the underlying SPI must be represented as a
>>> hierarchy.
>>
>> I don't understand what you mean by "feels wrong".
>>
>> There are 128 inputs, and only 24 outputs.
>> Therefore, I must map some inputs to the same output.
>> Thomas explained that edge interrupts *cannot* be shared.
>> So edge interrupts must receive a dedicated output line.
>> Did I write anything wrong so far?
> 
> Let me repeat what Thomas already said:
> 
> - you dedicate one line to level interrupts using a multiplexer (chained
> interrupts).

OK.

> - you use the remaining 23 inputs in a hierarchical model, each input
> being mapped to one output, no chained handler.
> 
> That's what I want to see.

OK.

Can you confirm that this means two separate domains?


One last thing: about generic_handle_irq() and virq==0
I understand your point that irq_to_desc() is an expensive
operation, so it is better to check beforehand. But then,
would it not make sense to add the check in generic_handle_irq()
if all drivers are expected to do it? (Code factoring)

Regards.

[PATCH v7] irqchip: Add support for tango interrupt mapper

[Mason]

This controller maps 128 input lines to 24 output lines.
The 24 output lines are routed to GIC SPI 0 to 23.
This driver only allocates exclusive output lines (hierarchy).
Let the legacy controller mux latency-insensitive interrupts.
---
Changes from v6 to v7
* Muxing level interrupts leaves performance on the table => Give "important"
interrupts a dedicated output line (hierarchical setup). And let the legacy
interrupt controller mux "less important" interrupts.
* Use a bitmap to manage output line allocation
* Don't panic on error; clean up and return error code
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 +++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 158 +++++++++++++++++++++
 3 files changed, 177 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..f4864979ab44
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt mapper
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller@6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..359ab706f504
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,158 @@
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/syscore_ops.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver only allocates exclusive output lines (hierarchy).
+ * Let the legacy controller mux latency-insensitive interrupts.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	DECLARE_BITMAP(used_spi, SPI_MAX);
+	DECLARE_BITMAP(enabled_irq, IRQ_MAX);
+	u8 tango_irq_to_spi[IRQ_MAX];
+	spinlock_t lock;
+};
+
+static struct tango_intc *tango_intc;
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	__clear_bit(data->hwirq, intc->enabled_irq);
+	spin_unlock_irqrestore(&intc->lock, flags);
+
+	irq_chip_mask_parent(data);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 val = IRQ_ENABLE | intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(val, intc->base + data->hwirq * 4);
+	__set_bit(data->hwirq, intc->enabled_irq);
+	spin_unlock_irqrestore(&intc->lock, flags);
+
+	irq_chip_unmask_parent(data);
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "mapper",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_eoi	= irq_chip_eoi_parent,
+	.irq_set_type	= irq_chip_set_type_parent,
+};
+
+static int alloc_gic_irq(struct irq_domain *dom, uint virq, u32 spi, u32 type)
+{
+	struct fwnode_handle *gic = dom->parent->fwnode;
+	struct irq_fwspec gic_fwspec = { gic, 3, { 0, spi, type }};
+	return irq_domain_alloc_irqs_parent(dom, virq, 1, &gic_fwspec);
+}
+
+static int tango_alloc(struct irq_domain *dom, uint virq, uint nirq, void *arg)
+{
+	int err, spi;
+	unsigned long flags;
+	struct irq_fwspec *fwspec = arg;
+	struct tango_intc *intc = dom->host_data;
+	u32 hwirq = fwspec->param[0], type = fwspec->param[1];
+
+	if (type & IRQ_TYPE_EDGE_FALLING || type & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	spi = find_first_zero_bit(intc->used_spi, SPI_MAX);
+	if (spi >= SPI_MAX) {
+		spin_unlock_irqrestore(&intc->lock, flags);
+		return -ENOSPC;
+	}
+	__set_bit(spi, intc->used_spi);
+	spin_unlock_irqrestore(&intc->lock, flags);
+
+	err = alloc_gic_irq(dom, virq, spi, type);
+	if (err) {
+		spin_lock_irqsave(&intc->lock, flags);
+		__clear_bit(spi, intc->used_spi);
+		spin_unlock_irqrestore(&intc->lock, flags);
+		return err;
+	}
+
+	intc->tango_irq_to_spi[hwirq] = spi;
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+	.alloc	= tango_alloc,
+};
+
+static void tango_resume(void)
+{
+	int hwirq;
+	struct tango_intc *intc = tango_intc;
+
+	for (hwirq = 0; hwirq < IRQ_MAX; ++hwirq) {
+		u32 val = intc->tango_irq_to_spi[hwirq];
+		if (test_bit(hwirq, intc->enabled_irq))
+			val |= IRQ_ENABLE;
+		writel_relaxed(val, intc->base + hwirq * 4);
+	}
+}
+
+static struct syscore_ops tango_syscore_ops = {
+	.resume	= tango_resume,
+};
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	struct tango_intc *intc;
+	struct irq_domain *dom, *gic_dom;
+
+	gic_dom = irq_find_host(parent);
+	if (!gic_dom)
+		return -ENODEV;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	if (!intc)
+		return -ENOMEM;
+
+	intc->base = of_iomap(node, 0);
+	if (!intc->base) {
+		kfree(intc);
+		return -ENXIO;
+	}
+
+	dom = irq_domain_add_hierarchy(gic_dom, 0, IRQ_MAX, node, &dom_ops, intc);
+	if (!dom) {
+		iounmap(intc->base);
+		kfree(intc);
+		return -ENOMEM;
+	}
+
+	tango_intc = intc;
+	register_syscore_ops(&tango_syscore_ops);
+	spin_lock_init(&intc->lock);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0

[PATCH v7] irqchip: Add support for tango interrupt mapper

[Mason]

This controller maps 128 input lines to 24 output lines.
The 24 output lines are routed to GIC SPI 0 to 23.
This driver only allocates exclusive output lines (hierarchy).
Let the legacy controller mux latency-insensitive interrupts.
---
Changes from v6 to v7
* Muxing level interrupts leaves performance on the table => Give "important"
interrupts a dedicated output line (hierarchical setup). And let the legacy
interrupt controller mux "less important" interrupts.
* Use a bitmap to manage output line allocation
* Don't panic on error; clean up and return error code
---
 .../interrupt-controller/sigma,smp8759-intc.txt    |  18 +++
 drivers/irqchip/Makefile                           |   2 +-
 drivers/irqchip/irq-smp8759.c                      | 158 +++++++++++++++++++++
 3 files changed, 177 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
 create mode 100644 drivers/irqchip/irq-smp8759.c

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
new file mode 100644
index 000000000000..f4864979ab44
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sigma,smp8759-intc.txt
@@ -0,0 +1,18 @@
+Sigma Designs SMP8759 interrupt mapper
+
+Required properties:
+- compatible: "sigma,smp8759-intc"
+- reg: address/size of register area
+- interrupt-controller
+- #interrupt-cells: <2>  (hwirq and trigger_type)
+- interrupt-parent: parent phandle
+
+Example:
+
+	interrupt-controller at 6f800 {
+		compatible = "sigma,smp8759-intc";
+		reg = <0x6f800 0x430>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gic>;
+	};
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc85abdb..013104923b71 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -47,7 +47,7 @@ obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= irq-zevio.o
 obj-$(CONFIG_ARCH_VT8500)		+= irq-vt8500.o
 obj-$(CONFIG_ST_IRQCHIP)		+= irq-st.o
-obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o
+obj-$(CONFIG_TANGO_IRQ)			+= irq-tango.o irq-smp8759.o
 obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
diff --git a/drivers/irqchip/irq-smp8759.c b/drivers/irqchip/irq-smp8759.c
new file mode 100644
index 000000000000..359ab706f504
--- /dev/null
+++ b/drivers/irqchip/irq-smp8759.c
@@ -0,0 +1,158 @@
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/syscore_ops.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+/*
+ * This controller maps IRQ_MAX input lines to SPI_MAX output lines.
+ * The output lines are routed to GIC SPI 0 to 23.
+ * This driver only allocates exclusive output lines (hierarchy).
+ * Let the legacy controller mux latency-insensitive interrupts.
+ */
+#define IRQ_MAX		128
+#define SPI_MAX		24
+#define IRQ_ENABLE	BIT(31)
+#define STATUS		0x420
+
+struct tango_intc {
+	void __iomem *base;
+	DECLARE_BITMAP(used_spi, SPI_MAX);
+	DECLARE_BITMAP(enabled_irq, IRQ_MAX);
+	u8 tango_irq_to_spi[IRQ_MAX];
+	spinlock_t lock;
+};
+
+static struct tango_intc *tango_intc;
+
+static void tango_mask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(0, intc->base + data->hwirq * 4);
+	__clear_bit(data->hwirq, intc->enabled_irq);
+	spin_unlock_irqrestore(&intc->lock, flags);
+
+	irq_chip_mask_parent(data);
+}
+
+static void tango_unmask(struct irq_data *data)
+{
+	unsigned long flags;
+	struct tango_intc *intc = data->chip_data;
+	u32 val = IRQ_ENABLE | intc->tango_irq_to_spi[data->hwirq];
+
+	spin_lock_irqsave(&intc->lock, flags);
+	writel_relaxed(val, intc->base + data->hwirq * 4);
+	__set_bit(data->hwirq, intc->enabled_irq);
+	spin_unlock_irqrestore(&intc->lock, flags);
+
+	irq_chip_unmask_parent(data);
+}
+
+static struct irq_chip tango_chip = {
+	.name		= "mapper",
+	.irq_mask	= tango_mask,
+	.irq_unmask	= tango_unmask,
+	.irq_eoi	= irq_chip_eoi_parent,
+	.irq_set_type	= irq_chip_set_type_parent,
+};
+
+static int alloc_gic_irq(struct irq_domain *dom, uint virq, u32 spi, u32 type)
+{
+	struct fwnode_handle *gic = dom->parent->fwnode;
+	struct irq_fwspec gic_fwspec = { gic, 3, { 0, spi, type }};
+	return irq_domain_alloc_irqs_parent(dom, virq, 1, &gic_fwspec);
+}
+
+static int tango_alloc(struct irq_domain *dom, uint virq, uint nirq, void *arg)
+{
+	int err, spi;
+	unsigned long flags;
+	struct irq_fwspec *fwspec = arg;
+	struct tango_intc *intc = dom->host_data;
+	u32 hwirq = fwspec->param[0], type = fwspec->param[1];
+
+	if (type & IRQ_TYPE_EDGE_FALLING || type & IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+
+	spin_lock_irqsave(&intc->lock, flags);
+	spi = find_first_zero_bit(intc->used_spi, SPI_MAX);
+	if (spi >= SPI_MAX) {
+		spin_unlock_irqrestore(&intc->lock, flags);
+		return -ENOSPC;
+	}
+	__set_bit(spi, intc->used_spi);
+	spin_unlock_irqrestore(&intc->lock, flags);
+
+	err = alloc_gic_irq(dom, virq, spi, type);
+	if (err) {
+		spin_lock_irqsave(&intc->lock, flags);
+		__clear_bit(spi, intc->used_spi);
+		spin_unlock_irqrestore(&intc->lock, flags);
+		return err;
+	}
+
+	intc->tango_irq_to_spi[hwirq] = spi;
+	irq_domain_set_hwirq_and_chip(dom, virq, hwirq, &tango_chip, intc);
+
+	return 0;
+}
+
+static struct irq_domain_ops dom_ops = {
+	.xlate	= irq_domain_xlate_twocell,
+	.alloc	= tango_alloc,
+};
+
+static void tango_resume(void)
+{
+	int hwirq;
+	struct tango_intc *intc = tango_intc;
+
+	for (hwirq = 0; hwirq < IRQ_MAX; ++hwirq) {
+		u32 val = intc->tango_irq_to_spi[hwirq];
+		if (test_bit(hwirq, intc->enabled_irq))
+			val |= IRQ_ENABLE;
+		writel_relaxed(val, intc->base + hwirq * 4);
+	}
+}
+
+static struct syscore_ops tango_syscore_ops = {
+	.resume	= tango_resume,
+};
+
+static int __init tango_irq_init(struct device_node *node, struct device_node *parent)
+{
+	struct tango_intc *intc;
+	struct irq_domain *dom, *gic_dom;
+
+	gic_dom = irq_find_host(parent);
+	if (!gic_dom)
+		return -ENODEV;
+
+	intc = kzalloc(sizeof(*intc), GFP_KERNEL);
+	if (!intc)
+		return -ENOMEM;
+
+	intc->base = of_iomap(node, 0);
+	if (!intc->base) {
+		kfree(intc);
+		return -ENXIO;
+	}
+
+	dom = irq_domain_add_hierarchy(gic_dom, 0, IRQ_MAX, node, &dom_ops, intc);
+	if (!dom) {
+		iounmap(intc->base);
+		kfree(intc);
+		return -ENOMEM;
+	}
+
+	tango_intc = intc;
+	register_syscore_ops(&tango_syscore_ops);
+	spin_lock_init(&intc->lock);
+
+	return 0;
+}
+IRQCHIP_DECLARE(tango_intc, "sigma,smp8759-intc", tango_irq_init);
-- 
2.11.0