[Patch v7 0/7] microblaze/PowerPC: Move irq-xilinx to irqchip

[Patch v7 0/7] microblaze/PowerPC: Move irq-xilinx to irqchip

[Zubair Lutfullah Kakakhel]

Hi,

This patch series moves the Xilinx interrupt controller driver out
of arch/microblaze to drivers/irqchip and then cleans it up a bit.
And then removes another implementation of the driver in arch/powerpc.

This effort results in one common driver usable by mips,microblaze
and powerpc.

Compile tested on microblaze-el.
Tested using qemu-system-ppc using virtix440-ml507
Tested on MIPSfpga platform.

Based on v4.9-rc5

Thanks,
ZubairLK

V6 -> V7
Rebase to v4.5-rc5
Split print messages cleanup into a separate patch
Use jump label api to restructure read/write handling in driver.

V5 -> V6
Split patch series. Patches for arch/mips can go separately
Rebase to v4.9-rc3
Added chained_irq_enter/exit
Removed __func__ used in pr_err

V4 -> V5
Added a new patch that removes the PPC driver
Rebase to v4.9-rc1
Better error handling

V3 -> V4
Better error handling
Some minor fixups

V2 -> V3
Cleanup the interrupt controller driver a bit based on feedback
Rebase to v4.8-rc4

V1 -> V2
Resubmitting without truncating the diff output for file moves
Removed accidental local mac address entry
Individual logs have more detail


Zubair Lutfullah Kakakhel (7):
  microblaze: irqchip: Move intc driver to irqchip
  irqchip: xilinx: clean up print messages
  irqchip: xilinx: restructure and use jump label api
  irqchip: xilinx: Rename get_irq to xintc_get_irq
  irqchip: xilinx: Add support for parent intc
  irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided
  powerpc/virtex: Use generic xilinx irqchip driver

 arch/microblaze/Kconfig                |   1 +
 arch/microblaze/include/asm/irq.h      |   2 +-
 arch/microblaze/kernel/Makefile        |   2 +-
 arch/microblaze/kernel/intc.c          | 196 ---------------------------
 arch/microblaze/kernel/irq.c           |   4 +-
 arch/powerpc/include/asm/xilinx_intc.h |   2 +-
 arch/powerpc/platforms/40x/Kconfig     |   1 +
 arch/powerpc/platforms/40x/virtex.c    |   2 +-
 arch/powerpc/platforms/44x/Kconfig     |   1 +
 arch/powerpc/platforms/44x/virtex.c    |   2 +-
 arch/powerpc/sysdev/xilinx_intc.c      | 211 +----------------------------
 drivers/irqchip/Kconfig                |   4 +
 drivers/irqchip/Makefile               |   1 +
 drivers/irqchip/irq-xilinx-intc.c      | 241 +++++++++++++++++++++++++++++++++
 14 files changed, 258 insertions(+), 412 deletions(-)
 delete mode 100644 arch/microblaze/kernel/intc.c
 create mode 100644 drivers/irqchip/irq-xilinx-intc.c

-- 
2.10.2

[Patch v7 1/7] microblaze: irqchip: Move intc driver to irqchip

[Zubair Lutfullah Kakakhel]

The Xilinx AXI Interrupt Controller IP block is used by the MIPS
based xilfpga platform and a few PowerPC based platforms.

Move the interrupt controller code out of arch/microblaze so that
it can be used by everyone

Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---
V6 -> V7
Rebase to v4.9-rc5

V5 -> V6
Rebase to v4.9-rc3

V4 -> V5
Rebase to v4.9-rc1
Renamed back to irq-xilinx-intc.c

V3 -> V4
No change

V2 -> V3
No change here. Cleanup patches follow after this patch.
Its debatable to cleanup before/after move. Decided to place cleanup
after move to put history in new place.

V1 -> V2

Renamed irq-xilinx to irq-axi-intc
Renamed CONFIG_XILINX_INTC to CONFIG_XILINX_AXI_INTC
Patch is now without rename flag so as to facilitate review
---
 arch/microblaze/Kconfig                                            | 1 +
 arch/microblaze/kernel/Makefile                                    | 2 +-
 drivers/irqchip/Kconfig                                            | 4 ++++
 drivers/irqchip/Makefile                                           | 1 +
 arch/microblaze/kernel/intc.c => drivers/irqchip/irq-xilinx-intc.c | 0
 5 files changed, 7 insertions(+), 1 deletion(-)
 rename arch/microblaze/kernel/intc.c => drivers/irqchip/irq-xilinx-intc.c (100%)

diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index 86f6572..85885a5 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -27,6 +27,7 @@ config MICROBLAZE
 	select HAVE_MEMBLOCK_NODE_MAP
 	select HAVE_OPROFILE
 	select IRQ_DOMAIN
+	select XILINX_INTC
 	select MODULES_USE_ELF_RELA
 	select OF
 	select OF_EARLY_FLATTREE
diff --git a/arch/microblaze/kernel/Makefile b/arch/microblaze/kernel/Makefile
index f08baca..e098381 100644
--- a/arch/microblaze/kernel/Makefile
+++ b/arch/microblaze/kernel/Makefile
@@ -15,7 +15,7 @@ endif
 extra-y := head.o vmlinux.lds
 
 obj-y += dma.o exceptions.o \
-	hw_exception_handler.o intc.o irq.o \
+	hw_exception_handler.o irq.o \
 	platform.o process.o prom.o ptrace.o \
 	reset.o setup.o signal.o sys_microblaze.o timer.o traps.o unwind.o
 
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index bc0af33..ae96731 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -211,6 +211,10 @@ config XTENSA_MX
 	bool
 	select IRQ_DOMAIN
 
+config XILINX_INTC
+	bool
+	select IRQ_DOMAIN
+
 config IRQ_CROSSBAR
 	bool
 	help
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index e4dbfc8..0e55d94 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
 obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
 obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
 obj-$(CONFIG_XTENSA_MX)			+= irq-xtensa-mx.o
+obj-$(CONFIG_XILINX_INTC)		+= irq-xilinx-intc.o
 obj-$(CONFIG_IRQ_CROSSBAR)		+= irq-crossbar.o
 obj-$(CONFIG_SOC_VF610)			+= irq-vf610-mscm-ir.o
 obj-$(CONFIG_BCM6345_L1_IRQ)		+= irq-bcm6345-l1.o
diff --git a/arch/microblaze/kernel/intc.c b/drivers/irqchip/irq-xilinx-intc.c
similarity index 100%
rename from arch/microblaze/kernel/intc.c
rename to drivers/irqchip/irq-xilinx-intc.c
-- 
2.10.2

[Patch v7 2/7] irqchip: xilinx: clean up print messages

[Zubair Lutfullah Kakakhel]

Remove __func__ and prefix irq-xilinx in various debug prints

Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---
V6 -> V7
New patch
This diff was squashed into another patch. Split it up for cleanliness
---
 drivers/irqchip/irq-xilinx-intc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 90bec7d..096c1ed 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -58,7 +58,7 @@ static void intc_enable_or_unmask(struct irq_data *d)
 {
 	unsigned long mask = 1 << d->hwirq;
 
-	pr_debug("enable_or_unmask: %ld\n", d->hwirq);
+	pr_debug("irq-xilinx: enable_or_unmask: %ld\n", d->hwirq);
 
 	/* ack level irqs because they can't be acked during
 	 * ack function since the handle_level_irq function
@@ -72,13 +72,13 @@ static void intc_enable_or_unmask(struct irq_data *d)
 
 static void intc_disable_or_mask(struct irq_data *d)
 {
-	pr_debug("disable: %ld\n", d->hwirq);
+	pr_debug("irq-xilinx: disable: %ld\n", d->hwirq);
 	write_fn(1 << d->hwirq, intc_baseaddr + CIE);
 }
 
 static void intc_ack(struct irq_data *d)
 {
-	pr_debug("ack: %ld\n", d->hwirq);
+	pr_debug("irq-xilinx: ack: %ld\n", d->hwirq);
 	write_fn(1 << d->hwirq, intc_baseaddr + IAR);
 }
 
@@ -86,7 +86,7 @@ static void intc_mask_ack(struct irq_data *d)
 {
 	unsigned long mask = 1 << d->hwirq;
 
-	pr_debug("disable_and_ack: %ld\n", d->hwirq);
+	pr_debug("irq-xilinx: disable_and_ack: %ld\n", d->hwirq);
 	write_fn(mask, intc_baseaddr + CIE);
 	write_fn(mask, intc_baseaddr + IAR);
 }
@@ -109,7 +109,7 @@ unsigned int get_irq(void)
 	if (hwirq != -1U)
 		irq = irq_find_mapping(root_domain, hwirq);
 
-	pr_debug("get_irq: hwirq=%d, irq=%d\n", hwirq, irq);
+	pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);
 
 	return irq;
 }
@@ -146,20 +146,20 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
 
 	ret = of_property_read_u32(intc, "xlnx,num-intr-inputs", &nr_irq);
 	if (ret < 0) {
-		pr_err("%s: unable to read xlnx,num-intr-inputs\n", __func__);
+		pr_err("irq-xilinx: unable to read xlnx,num-intr-inputs\n");
 		return ret;
 	}
 
 	ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &intr_mask);
 	if (ret < 0) {
-		pr_err("%s: unable to read xlnx,kind-of-intr\n", __func__);
+		pr_err("irq-xilinx: unable to read xlnx,kind-of-intr\n");
 		return ret;
 	}
 
 	if (intr_mask >> nr_irq)
-		pr_warn("%s: mismatch in kind-of-intr param\n", __func__);
+		pr_warn("irq-xilinx: mismatch in kind-of-intr param\n");
 
-	pr_info("%s: num_irq=%d, edge=0x%x\n",
+	pr_info("irq-xilinx: %s: num_irq=%d, edge=0x%x\n",
 		intc->full_name, nr_irq, intr_mask);
 
 	write_fn = intc_write32;
-- 
2.10.2

[Patch v7 3/7] irqchip: xilinx: restructure and use jump label api

[Zubair Lutfullah Kakakhel]

Add a global structure to house various variables.
And cleanup read/write handling by using jump label api.

Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---
V6 -> V7
Restructure and use jump label api
Better commit log

V5 -> V6
Removed __func__ from printk
Rebase to v4.9-rc3

V4 -> V5
Rebased to v4.9-rc1
Better error handling

V3 -> V4
Better error handling for kzalloc
Erroring out if the axi intc is probed twice as that isn't
supported

V2 -> V3
New patch. Cleans up driver structure
---
 drivers/irqchip/irq-xilinx-intc.c | 118 +++++++++++++++++++++-----------------
 1 file changed, 66 insertions(+), 52 deletions(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 096c1ed..7331d8c 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -14,10 +14,9 @@
 #include <linux/irqchip.h>
 #include <linux/of_address.h>
 #include <linux/io.h>
+#include <linux/jump_label.h>
 #include <linux/bug.h>
 
-static void __iomem *intc_baseaddr;
-
 /* No one else should require these constants, so define them locally here. */
 #define ISR 0x00			/* Interrupt Status Register */
 #define IPR 0x04			/* Interrupt Pending Register */
@@ -31,27 +30,30 @@ static void __iomem *intc_baseaddr;
 #define MER_ME (1<<0)
 #define MER_HIE (1<<1)
 
-static unsigned int (*read_fn)(void __iomem *);
-static void (*write_fn)(u32, void __iomem *);
+static DEFINE_STATIC_KEY_FALSE(xintc_is_be);
 
-static void intc_write32(u32 val, void __iomem *addr)
-{
-	iowrite32(val, addr);
-}
+struct xintc_irq_chip {
+	void		__iomem *base;
+	struct		irq_domain *root_domain;
+	u32		intr_mask;
+};
 
-static unsigned int intc_read32(void __iomem *addr)
-{
-	return ioread32(addr);
-}
+static struct xintc_irq_chip *xintc_irqc;
 
-static void intc_write32_be(u32 val, void __iomem *addr)
+static void xintc_write(int reg, u32 data)
 {
-	iowrite32be(val, addr);
+	if (static_branch_unlikely(&xintc_is_be))
+		iowrite32be(data, xintc_irqc->base + reg);
+	else
+		iowrite32(data, xintc_irqc->base + reg);
 }
 
-static unsigned int intc_read32_be(void __iomem *addr)
+static unsigned int xintc_read(int reg)
 {
-	return ioread32be(addr);
+	if (static_branch_unlikely(&xintc_is_be))
+		return ioread32be(xintc_irqc->base + reg);
+	else
+		return ioread32(xintc_irqc->base + reg);
 }
 
 static void intc_enable_or_unmask(struct irq_data *d)
@@ -65,21 +67,21 @@ static void intc_enable_or_unmask(struct irq_data *d)
 	 * acks the irq before calling the interrupt handler
 	 */
 	if (irqd_is_level_type(d))
-		write_fn(mask, intc_baseaddr + IAR);
+		xintc_write(IAR, mask);
 
-	write_fn(mask, intc_baseaddr + SIE);
+	xintc_write(SIE, mask);
 }
 
 static void intc_disable_or_mask(struct irq_data *d)
 {
 	pr_debug("irq-xilinx: disable: %ld\n", d->hwirq);
-	write_fn(1 << d->hwirq, intc_baseaddr + CIE);
+	xintc_write(CIE, 1 << d->hwirq);
 }
 
 static void intc_ack(struct irq_data *d)
 {
 	pr_debug("irq-xilinx: ack: %ld\n", d->hwirq);
-	write_fn(1 << d->hwirq, intc_baseaddr + IAR);
+	xintc_write(IAR, 1 << d->hwirq);
 }
 
 static void intc_mask_ack(struct irq_data *d)
@@ -87,8 +89,8 @@ static void intc_mask_ack(struct irq_data *d)
 	unsigned long mask = 1 << d->hwirq;
 
 	pr_debug("irq-xilinx: disable_and_ack: %ld\n", d->hwirq);
-	write_fn(mask, intc_baseaddr + CIE);
-	write_fn(mask, intc_baseaddr + IAR);
+	xintc_write(CIE, mask);
+	xintc_write(IAR, mask);
 }
 
 static struct irq_chip intc_dev = {
@@ -99,15 +101,13 @@ static struct irq_chip intc_dev = {
 	.irq_mask_ack = intc_mask_ack,
 };
 
-static struct irq_domain *root_domain;
-
 unsigned int get_irq(void)
 {
 	unsigned int hwirq, irq = -1;
 
-	hwirq = read_fn(intc_baseaddr + IVR);
+	hwirq = xintc_read(IVR);
 	if (hwirq != -1U)
-		irq = irq_find_mapping(root_domain, hwirq);
+		irq = irq_find_mapping(xintc_irqc->root_domain, hwirq);
 
 	pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);
 
@@ -116,9 +116,7 @@ unsigned int get_irq(void)
 
 static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
 {
-	u32 intr_mask = (u32)d->host_data;
-
-	if (intr_mask & (1 << hw)) {
+	if (xintc_irqc->intr_mask & (1 << hw)) {
 		irq_set_chip_and_handler_name(irq, &intc_dev,
 						handle_edge_irq, "edge");
 		irq_clear_status_flags(irq, IRQ_LEVEL);
@@ -138,59 +136,75 @@ static const struct irq_domain_ops xintc_irq_domain_ops = {
 static int __init xilinx_intc_of_init(struct device_node *intc,
 					     struct device_node *parent)
 {
-	u32 nr_irq, intr_mask;
+	u32 nr_irq;
 	int ret;
+	struct xintc_irq_chip *irqc;
 
-	intc_baseaddr = of_iomap(intc, 0);
-	BUG_ON(!intc_baseaddr);
+	if (xintc_irqc) {
+		pr_err("irq-xilinx: Multiple instances aren't supported\n");
+		return -EINVAL;
+	}
+
+	irqc = kzalloc(sizeof(*irqc), GFP_KERNEL);
+	if (!irqc)
+		return -ENOMEM;
+
+	xintc_irqc = irqc;
+
+	irqc->base = of_iomap(intc, 0);
+	BUG_ON(!irqc->base);
 
 	ret = of_property_read_u32(intc, "xlnx,num-intr-inputs", &nr_irq);
 	if (ret < 0) {
 		pr_err("irq-xilinx: unable to read xlnx,num-intr-inputs\n");
-		return ret;
+		goto err_alloc;
 	}
 
-	ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &intr_mask);
+	ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &irqc->intr_mask);
 	if (ret < 0) {
 		pr_err("irq-xilinx: unable to read xlnx,kind-of-intr\n");
-		return ret;
+		goto err_alloc;
 	}
 
-	if (intr_mask >> nr_irq)
+	if (irqc->intr_mask >> nr_irq)
 		pr_warn("irq-xilinx: mismatch in kind-of-intr param\n");
 
 	pr_info("irq-xilinx: %s: num_irq=%d, edge=0x%x\n",
-		intc->full_name, nr_irq, intr_mask);
+		intc->full_name, nr_irq, irqc->intr_mask);
 
-	write_fn = intc_write32;
-	read_fn = intc_read32;
 
 	/*
 	 * Disable all external interrupts until they are
 	 * explicity requested.
 	 */
-	write_fn(0, intc_baseaddr + IER);
+	xintc_write(IER, 0);
 
 	/* Acknowledge any pending interrupts just in case. */
-	write_fn(0xffffffff, intc_baseaddr + IAR);
+	xintc_write(IAR, 0xffffffff);
 
 	/* Turn on the Master Enable. */
-	write_fn(MER_HIE | MER_ME, intc_baseaddr + MER);
-	if (!(read_fn(intc_baseaddr + MER) & (MER_HIE | MER_ME))) {
-		write_fn = intc_write32_be;
-		read_fn = intc_read32_be;
-		write_fn(MER_HIE | MER_ME, intc_baseaddr + MER);
+	xintc_write(MER, MER_HIE | MER_ME);
+	if (!(xintc_read(MER) & (MER_HIE | MER_ME))) {
+		static_branch_enable(&xintc_is_be);
+		xintc_write(MER, MER_HIE | MER_ME);
 	}
 
-	/* Yeah, okay, casting the intr_mask to a void* is butt-ugly, but I'm
-	 * lazy and Michal can clean it up to something nicer when he tests
-	 * and commits this patch.  ~~gcl */
-	root_domain = irq_domain_add_linear(intc, nr_irq, &xintc_irq_domain_ops,
-							(void *)intr_mask);
+	irqc->root_domain = irq_domain_add_linear(intc, nr_irq,
+						  &xintc_irq_domain_ops, irqc);
+	if (!irqc->root_domain) {
+		pr_err("irq-xilinx: Unable to create IRQ domain\n");
+		goto err_alloc;
+	}
 
-	irq_set_default_host(root_domain);
+	irq_set_default_host(irqc->root_domain);
 
 	return 0;
+
+err_alloc:
+	xintc_irqc = NULL;
+	kfree(irqc);
+	return ret;
+
 }
 
 IRQCHIP_DECLARE(xilinx_intc, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
-- 
2.10.2

[Patch v7 4/7] irqchip: xilinx: Rename get_irq to xintc_get_irq

[Zubair Lutfullah Kakakhel]

Now that the driver is generic and used by multiple archs,
get_irq is too generic.

Rename get_irq to xintc_get_irq to avoid any conflicts

Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---
V6 -> V7
Rebase to v4.9-rc5

V5 -> V6
Removed __func__ in printk
Rebase to v4.9-rc3

V4 -> V5
Rebased to v4.9-rc1
Use __func__ in pr_err

V3 -> V4
New patch.
---
 arch/microblaze/include/asm/irq.h | 2 +-
 arch/microblaze/kernel/irq.c      | 4 ++--
 drivers/irqchip/irq-xilinx-intc.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/microblaze/include/asm/irq.h b/arch/microblaze/include/asm/irq.h
index bab3b13..d785def 100644
--- a/arch/microblaze/include/asm/irq.h
+++ b/arch/microblaze/include/asm/irq.h
@@ -16,6 +16,6 @@ struct pt_regs;
 extern void do_IRQ(struct pt_regs *regs);
 
 /* should be defined in each interrupt controller driver */
-extern unsigned int get_irq(void);
+extern unsigned int xintc_get_irq(void);
 
 #endif /* _ASM_MICROBLAZE_IRQ_H */
diff --git a/arch/microblaze/kernel/irq.c b/arch/microblaze/kernel/irq.c
index 11e24de..903dad8 100644
--- a/arch/microblaze/kernel/irq.c
+++ b/arch/microblaze/kernel/irq.c
@@ -29,12 +29,12 @@ void __irq_entry do_IRQ(struct pt_regs *regs)
 	trace_hardirqs_off();
 
 	irq_enter();
-	irq = get_irq();
+	irq = xintc_get_irq();
 next_irq:
 	BUG_ON(!irq);
 	generic_handle_irq(irq);
 
-	irq = get_irq();
+	irq = xintc_get_irq();
 	if (irq != -1U) {
 		pr_debug("next irq: %d\n", irq);
 		++concurrent_irq;
diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 7331d8c..34ec609 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -101,7 +101,7 @@ static struct irq_chip intc_dev = {
 	.irq_mask_ack = intc_mask_ack,
 };
 
-unsigned int get_irq(void)
+unsigned int xintc_get_irq(void)
 {
 	unsigned int hwirq, irq = -1;
 
-- 
2.10.2

[Patch v7 5/7] irqchip: xilinx: Add support for parent intc

[Zubair Lutfullah Kakakhel]

The MIPS based xilfpga platform has the following IRQ structure

Peripherals --> xilinx_intcontroller -> mips_cpu_int controller

Add support for the driver to chain the irq handler

Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---
V6 -> V7
Rebase to v4.9-rc5

V5 -> V6
Use chained_irq_enter and chained_irq_exit
Add error check for irq_of_parse_and_map
Rebase to v4.9-rc3

V4 -> V5
Rebased to v4.9-rc1
Missing curly braces

V3 -> V4
Clean up if/else when a parent is found
Pass irqchip structure to handler as data

V2 -> V3
Reused existing parent node instead of finding again.
Cleanup up handler based on review

V1 -> V2

No change
---
 drivers/irqchip/irq-xilinx-intc.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 34ec609..971c141 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -12,10 +12,12 @@
 #include <linux/irqdomain.h>
 #include <linux/irq.h>
 #include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/of_address.h>
 #include <linux/io.h>
 #include <linux/jump_label.h>
 #include <linux/bug.h>
+#include <linux/of_irq.h>
 
 /* No one else should require these constants, so define them locally here. */
 #define ISR 0x00			/* Interrupt Status Register */
@@ -133,11 +135,26 @@ static const struct irq_domain_ops xintc_irq_domain_ops = {
 	.map = xintc_map,
 };
 
+static void xil_intc_irq_handler(struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	u32 pending;
+
+	chained_irq_enter(chip, desc);
+	do {
+		pending = xintc_get_irq();
+		if (pending == -1U)
+			break;
+		generic_handle_irq(pending);
+	} while (true);
+	chained_irq_exit(chip, desc);
+}
+
 static int __init xilinx_intc_of_init(struct device_node *intc,
 					     struct device_node *parent)
 {
 	u32 nr_irq;
-	int ret;
+	int ret, irq;
 	struct xintc_irq_chip *irqc;
 
 	if (xintc_irqc) {
@@ -196,7 +213,20 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
 		goto err_alloc;
 	}
 
-	irq_set_default_host(irqc->root_domain);
+	if (parent) {
+		irq = irq_of_parse_and_map(intc, 0);
+		if (irq) {
+			irq_set_chained_handler_and_data(irq,
+							 xil_intc_irq_handler,
+							 irqc);
+		} else {
+			pr_err("irq-xilinx: interrupts property not in DT\n");
+			ret = -EINVAL;
+			goto err_alloc;
+		}
+	} else {
+		irq_set_default_host(irqc->root_domain);
+	}
 
 	return 0;
 
-- 
2.10.2

[Patch v7 6/7] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided

[Zubair Lutfullah Kakakhel]

The powerpc dts file does not have the xlnx,kind-of-intr property.
Instead of erroring out, give a warning instead. And attempt to
continue to probe the interrupt controller while assuming
kind-of-intr is 0x0 as a fall back.

Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---
V6 -> V7
Rebase to v4.9-rc5

V5 -> V6
Rebase to v4.9-rc3

V5 new patch
---
 drivers/irqchip/irq-xilinx-intc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 971c141..d330917 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -179,8 +179,8 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
 
 	ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &irqc->intr_mask);
 	if (ret < 0) {
-		pr_err("irq-xilinx: unable to read xlnx,kind-of-intr\n");
-		goto err_alloc;
+		pr_warn("irq-xilinx: unable to read xlnx,kind-of-intr\n");
+		irqc->intr_mask = 0;
 	}
 
 	if (irqc->intr_mask >> nr_irq)
-- 
2.10.2

[Patch v7 7/7] powerpc/virtex: Use generic xilinx irqchip driver

[Zubair Lutfullah Kakakhel]

The Xilinx interrupt controller driver is now available in drivers/irqchip.
Switch to using that driver.

Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

---
V6 - V7
Rebase to v4.9-rc5

V5 -> V6 Added Acked-by Micheal Ellerman

V5 New patch

Tested on virtex440-ml507 using qemu
---
 arch/powerpc/include/asm/xilinx_intc.h |   2 +-
 arch/powerpc/platforms/40x/Kconfig     |   1 +
 arch/powerpc/platforms/40x/virtex.c    |   2 +-
 arch/powerpc/platforms/44x/Kconfig     |   1 +
 arch/powerpc/platforms/44x/virtex.c    |   2 +-
 arch/powerpc/sysdev/xilinx_intc.c      | 211 +--------------------------------
 drivers/irqchip/irq-xilinx-intc.c      |   3 +-
 7 files changed, 9 insertions(+), 213 deletions(-)

diff --git a/arch/powerpc/include/asm/xilinx_intc.h b/arch/powerpc/include/asm/xilinx_intc.h
index 343612f..3192d7f 100644
--- a/arch/powerpc/include/asm/xilinx_intc.h
+++ b/arch/powerpc/include/asm/xilinx_intc.h
@@ -14,7 +14,7 @@
 #ifdef __KERNEL__
 
 extern void __init xilinx_intc_init_tree(void);
-extern unsigned int xilinx_intc_get_irq(void);
+extern unsigned int xintc_get_irq(void);
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_XILINX_INTC_H */
diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index e3257f2..1d7c1b1 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -64,6 +64,7 @@ config XILINX_VIRTEX_GENERIC_BOARD
 	default n
 	select XILINX_VIRTEX_II_PRO
 	select XILINX_VIRTEX_4_FX
+	select XILINX_INTC
 	help
 	  This option enables generic support for Xilinx Virtex based boards.
 
diff --git a/arch/powerpc/platforms/40x/virtex.c b/arch/powerpc/platforms/40x/virtex.c
index 91a08ea..e3d5e09 100644
--- a/arch/powerpc/platforms/40x/virtex.c
+++ b/arch/powerpc/platforms/40x/virtex.c
@@ -48,7 +48,7 @@ define_machine(virtex) {
 	.probe			= virtex_probe,
 	.setup_arch		= xilinx_pci_init,
 	.init_IRQ		= xilinx_intc_init_tree,
-	.get_irq		= xilinx_intc_get_irq,
+	.get_irq		= xintc_get_irq,
 	.restart		= ppc4xx_reset_system,
 	.calibrate_decr		= generic_calibrate_decr,
 };
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index 48fc180..25b8d64 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -241,6 +241,7 @@ config XILINX_VIRTEX440_GENERIC_BOARD
 	depends on 44x
 	default n
 	select XILINX_VIRTEX_5_FXT
+	select XILINX_INTC
 	help
 	  This option enables generic support for Xilinx Virtex based boards
 	  that use a 440 based processor in the Virtex 5 FXT FPGA architecture.
diff --git a/arch/powerpc/platforms/44x/virtex.c b/arch/powerpc/platforms/44x/virtex.c
index a7e0802..3eb13ed 100644
--- a/arch/powerpc/platforms/44x/virtex.c
+++ b/arch/powerpc/platforms/44x/virtex.c
@@ -54,7 +54,7 @@ define_machine(virtex) {
 	.probe			= virtex_probe,
 	.setup_arch		= xilinx_pci_init,
 	.init_IRQ		= xilinx_intc_init_tree,
-	.get_irq		= xilinx_intc_get_irq,
+	.get_irq		= xintc_get_irq,
 	.calibrate_decr		= generic_calibrate_decr,
 	.restart		= ppc4xx_reset_system,
 };
diff --git a/arch/powerpc/sysdev/xilinx_intc.c b/arch/powerpc/sysdev/xilinx_intc.c
index 0f52d79..4a86dcf 100644
--- a/arch/powerpc/sysdev/xilinx_intc.c
+++ b/arch/powerpc/sysdev/xilinx_intc.c
@@ -29,194 +29,7 @@
 #include <asm/processor.h>
 #include <asm/i8259.h>
 #include <asm/irq.h>
-
-/*
- * INTC Registers
- */
-#define XINTC_ISR	0	/* Interrupt Status */
-#define XINTC_IPR	4	/* Interrupt Pending */
-#define XINTC_IER	8	/* Interrupt Enable */
-#define XINTC_IAR	12	/* Interrupt Acknowledge */
-#define XINTC_SIE	16	/* Set Interrupt Enable bits */
-#define XINTC_CIE	20	/* Clear Interrupt Enable bits */
-#define XINTC_IVR	24	/* Interrupt Vector */
-#define XINTC_MER	28	/* Master Enable */
-
-static struct irq_domain *master_irqhost;
-
-#define XILINX_INTC_MAXIRQS	(32)
-
-/* The following table allows the interrupt type, edge or level,
- * to be cached after being read from the device tree until the interrupt
- * is mapped
- */
-static int xilinx_intc_typetable[XILINX_INTC_MAXIRQS];
-
-/* Map the interrupt type from the device tree to the interrupt types
- * used by the interrupt subsystem
- */
-static unsigned char xilinx_intc_map_senses[] = {
-	IRQ_TYPE_EDGE_RISING,
-	IRQ_TYPE_EDGE_FALLING,
-	IRQ_TYPE_LEVEL_HIGH,
-	IRQ_TYPE_LEVEL_LOW,
-};
-
-/*
- * The interrupt controller is setup such that it doesn't work well with
- * the level interrupt handler in the kernel because the handler acks the
- * interrupt before calling the application interrupt handler. To deal with
- * that, we use 2 different irq chips so that different functions can be
- * used for level and edge type interrupts.
- *
- * IRQ Chip common (across level and edge) operations
- */
-static void xilinx_intc_mask(struct irq_data *d)
-{
-	int irq = irqd_to_hwirq(d);
-	void * regs = irq_data_get_irq_chip_data(d);
-	pr_debug("mask: %d\n", irq);
-	out_be32(regs + XINTC_CIE, 1 << irq);
-}
-
-static int xilinx_intc_set_type(struct irq_data *d, unsigned int flow_type)
-{
-	return 0;
-}
-
-/*
- * IRQ Chip level operations
- */
-static void xilinx_intc_level_unmask(struct irq_data *d)
-{
-	int irq = irqd_to_hwirq(d);
-	void * regs = irq_data_get_irq_chip_data(d);
-	pr_debug("unmask: %d\n", irq);
-	out_be32(regs + XINTC_SIE, 1 << irq);
-
-	/* ack level irqs because they can't be acked during
-	 * ack function since the handle_level_irq function
-	 * acks the irq before calling the inerrupt handler
-	 */
-	out_be32(regs + XINTC_IAR, 1 << irq);
-}
-
-static struct irq_chip xilinx_intc_level_irqchip = {
-	.name = "Xilinx Level INTC",
-	.irq_mask = xilinx_intc_mask,
-	.irq_mask_ack = xilinx_intc_mask,
-	.irq_unmask = xilinx_intc_level_unmask,
-	.irq_set_type = xilinx_intc_set_type,
-};
-
-/*
- * IRQ Chip edge operations
- */
-static void xilinx_intc_edge_unmask(struct irq_data *d)
-{
-	int irq = irqd_to_hwirq(d);
-	void *regs = irq_data_get_irq_chip_data(d);
-	pr_debug("unmask: %d\n", irq);
-	out_be32(regs + XINTC_SIE, 1 << irq);
-}
-
-static void xilinx_intc_edge_ack(struct irq_data *d)
-{
-	int irq = irqd_to_hwirq(d);
-	void * regs = irq_data_get_irq_chip_data(d);
-	pr_debug("ack: %d\n", irq);
-	out_be32(regs + XINTC_IAR, 1 << irq);
-}
-
-static struct irq_chip xilinx_intc_edge_irqchip = {
-	.name = "Xilinx Edge  INTC",
-	.irq_mask = xilinx_intc_mask,
-	.irq_unmask = xilinx_intc_edge_unmask,
-	.irq_ack = xilinx_intc_edge_ack,
-	.irq_set_type = xilinx_intc_set_type,
-};
-
-/*
- * IRQ Host operations
- */
-
-/**
- * xilinx_intc_xlate - translate virq# from device tree interrupts property
- */
-static int xilinx_intc_xlate(struct irq_domain *h, struct device_node *ct,
-				const u32 *intspec, unsigned int intsize,
-				irq_hw_number_t *out_hwirq,
-				unsigned int *out_flags)
-{
-	if ((intsize < 2) || (intspec[0] >= XILINX_INTC_MAXIRQS))
-		return -EINVAL;
-
-	/* keep a copy of the interrupt type til the interrupt is mapped
-	 */
-	xilinx_intc_typetable[intspec[0]] = xilinx_intc_map_senses[intspec[1]];
-
-	/* Xilinx uses 2 interrupt entries, the 1st being the h/w
-	 * interrupt number, the 2nd being the interrupt type, edge or level
-	 */
-	*out_hwirq = intspec[0];
-	*out_flags = xilinx_intc_map_senses[intspec[1]];
-
-	return 0;
-}
-static int xilinx_intc_map(struct irq_domain *h, unsigned int virq,
-				  irq_hw_number_t irq)
-{
-	irq_set_chip_data(virq, h->host_data);
-
-	if (xilinx_intc_typetable[irq] == IRQ_TYPE_LEVEL_HIGH ||
-	    xilinx_intc_typetable[irq] == IRQ_TYPE_LEVEL_LOW) {
-		irq_set_chip_and_handler(virq, &xilinx_intc_level_irqchip,
-					 handle_level_irq);
-	} else {
-		irq_set_chip_and_handler(virq, &xilinx_intc_edge_irqchip,
-					 handle_edge_irq);
-	}
-	return 0;
-}
-
-static const struct irq_domain_ops xilinx_intc_ops = {
-	.map = xilinx_intc_map,
-	.xlate = xilinx_intc_xlate,
-};
-
-struct irq_domain * __init
-xilinx_intc_init(struct device_node *np)
-{
-	struct irq_domain * irq;
-	void * regs;
-
-	/* Find and map the intc registers */
-	regs = of_iomap(np, 0);
-	if (!regs) {
-		pr_err("xilinx_intc: could not map registers\n");
-		return NULL;
-	}
-
-	/* Setup interrupt controller */
-	out_be32(regs + XINTC_IER, 0); /* disable all irqs */
-	out_be32(regs + XINTC_IAR, ~(u32) 0); /* Acknowledge pending irqs */
-	out_be32(regs + XINTC_MER, 0x3UL); /* Turn on the Master Enable. */
-
-	/* Allocate and initialize an irq_domain structure. */
-	irq = irq_domain_add_linear(np, XILINX_INTC_MAXIRQS, &xilinx_intc_ops,
-				    regs);
-	if (!irq)
-		panic(__FILE__ ": Cannot allocate IRQ host\n");
-
-	return irq;
-}
-
-int xilinx_intc_get_irq(void)
-{
-	void * regs = master_irqhost->host_data;
-	pr_debug("get_irq:\n");
-	return irq_linear_revmap(master_irqhost, in_be32(regs + XINTC_IVR));
-}
+#include <linux/irqchip.h>
 
 #if defined(CONFIG_PPC_I8259)
 /*
@@ -265,31 +78,11 @@ static void __init xilinx_i8259_setup_cascade(void)
 static inline void xilinx_i8259_setup_cascade(void) { return; }
 #endif /* defined(CONFIG_PPC_I8259) */
 
-static const struct of_device_id xilinx_intc_match[] __initconst = {
-	{ .compatible = "xlnx,opb-intc-1.00.c", },
-	{ .compatible = "xlnx,xps-intc-1.00.a", },
-	{}
-};
-
 /*
  * Initialize master Xilinx interrupt controller
  */
 void __init xilinx_intc_init_tree(void)
 {
-	struct device_node *np;
-
-	/* find top level interrupt controller */
-	for_each_matching_node(np, xilinx_intc_match) {
-		if (!of_get_property(np, "interrupts", NULL))
-			break;
-	}
-	BUG_ON(!np);
-
-	master_irqhost = xilinx_intc_init(np);
-	BUG_ON(!master_irqhost);
-
-	irq_set_default_host(master_irqhost);
-	of_node_put(np);
-
+	irqchip_init();
 	xilinx_i8259_setup_cascade();
 }
diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index d330917..3db7ab1 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -237,4 +237,5 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
 
 }
 
-IRQCHIP_DECLARE(xilinx_intc, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
+IRQCHIP_DECLARE(xilinx_intc_xps, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
+IRQCHIP_DECLARE(xilinx_intc_opb, "xlnx,opb-intc-1.00.c", xilinx_intc_of_init);
-- 
2.10.2

Re: [Patch v7 1/7] microblaze: irqchip: Move intc driver to irqchip

[Michal Simek]

On 14.11.2016 13:13, Zubair Lutfullah Kakakhel wrote:
> The Xilinx AXI Interrupt Controller IP block is used by the MIPS
> based xilfpga platform and a few PowerPC based platforms.
> 
> Move the interrupt controller code out of arch/microblaze so that
> it can be used by everyone
> 
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> 
> ---
> V6 -> V7
> Rebase to v4.9-rc5
> 
> V5 -> V6
> Rebase to v4.9-rc3
> 
> V4 -> V5
> Rebase to v4.9-rc1
> Renamed back to irq-xilinx-intc.c
> 
> V3 -> V4
> No change
> 
> V2 -> V3
> No change here. Cleanup patches follow after this patch.
> Its debatable to cleanup before/after move. Decided to place cleanup
> after move to put history in new place.
> 
> V1 -> V2
> 
> Renamed irq-xilinx to irq-axi-intc
> Renamed CONFIG_XILINX_INTC to CONFIG_XILINX_AXI_INTC
> Patch is now without rename flag so as to facilitate review
> ---
>  arch/microblaze/Kconfig                                            | 1 +
>  arch/microblaze/kernel/Makefile                                    | 2 +-
>  drivers/irqchip/Kconfig                                            | 4 ++++
>  drivers/irqchip/Makefile                                           | 1 +
>  arch/microblaze/kernel/intc.c => drivers/irqchip/irq-xilinx-intc.c | 0
>  5 files changed, 7 insertions(+), 1 deletion(-)
>  rename arch/microblaze/kernel/intc.c => drivers/irqchip/irq-xilinx-intc.c (100%)
> 
> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
> index 86f6572..85885a5 100644
> --- a/arch/microblaze/Kconfig
> +++ b/arch/microblaze/Kconfig
> @@ -27,6 +27,7 @@ config MICROBLAZE
>  	select HAVE_MEMBLOCK_NODE_MAP
>  	select HAVE_OPROFILE
>  	select IRQ_DOMAIN
> +	select XILINX_INTC
>  	select MODULES_USE_ELF_RELA
>  	select OF
>  	select OF_EARLY_FLATTREE
> diff --git a/arch/microblaze/kernel/Makefile b/arch/microblaze/kernel/Makefile
> index f08baca..e098381 100644
> --- a/arch/microblaze/kernel/Makefile
> +++ b/arch/microblaze/kernel/Makefile
> @@ -15,7 +15,7 @@ endif
>  extra-y := head.o vmlinux.lds
>  
>  obj-y += dma.o exceptions.o \
> -	hw_exception_handler.o intc.o irq.o \
> +	hw_exception_handler.o irq.o \
>  	platform.o process.o prom.o ptrace.o \
>  	reset.o setup.o signal.o sys_microblaze.o timer.o traps.o unwind.o
>  
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index bc0af33..ae96731 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -211,6 +211,10 @@ config XTENSA_MX
>  	bool
>  	select IRQ_DOMAIN
>  
> +config XILINX_INTC
> +	bool
> +	select IRQ_DOMAIN
> +
>  config IRQ_CROSSBAR
>  	bool
>  	help
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index e4dbfc8..0e55d94 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_TB10X_IRQC)		+= irq-tb10x.o
>  obj-$(CONFIG_TS4800_IRQ)		+= irq-ts4800.o
>  obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
>  obj-$(CONFIG_XTENSA_MX)			+= irq-xtensa-mx.o
> +obj-$(CONFIG_XILINX_INTC)		+= irq-xilinx-intc.o
>  obj-$(CONFIG_IRQ_CROSSBAR)		+= irq-crossbar.o
>  obj-$(CONFIG_SOC_VF610)			+= irq-vf610-mscm-ir.o
>  obj-$(CONFIG_BCM6345_L1_IRQ)		+= irq-bcm6345-l1.o
> diff --git a/arch/microblaze/kernel/intc.c b/drivers/irqchip/irq-xilinx-intc.c
> similarity index 100%
> rename from arch/microblaze/kernel/intc.c
> rename to drivers/irqchip/irq-xilinx-intc.c
> 

Tested-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [Patch v7 2/7] irqchip: xilinx: clean up print messages

[Michal Simek]

On 14.11.2016 13:13, Zubair Lutfullah Kakakhel wrote:
> Remove __func__ and prefix irq-xilinx in various debug prints
> 
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> 
> ---
> V6 -> V7
> New patch
> This diff was squashed into another patch. Split it up for cleanliness
> ---
>  drivers/irqchip/irq-xilinx-intc.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index 90bec7d..096c1ed 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -58,7 +58,7 @@ static void intc_enable_or_unmask(struct irq_data *d)
>  {
>  	unsigned long mask = 1 << d->hwirq;
>  
> -	pr_debug("enable_or_unmask: %ld\n", d->hwirq);
> +	pr_debug("irq-xilinx: enable_or_unmask: %ld\n", d->hwirq);
>  
>  	/* ack level irqs because they can't be acked during
>  	 * ack function since the handle_level_irq function
> @@ -72,13 +72,13 @@ static void intc_enable_or_unmask(struct irq_data *d)
>  
>  static void intc_disable_or_mask(struct irq_data *d)
>  {
> -	pr_debug("disable: %ld\n", d->hwirq);
> +	pr_debug("irq-xilinx: disable: %ld\n", d->hwirq);
>  	write_fn(1 << d->hwirq, intc_baseaddr + CIE);
>  }
>  
>  static void intc_ack(struct irq_data *d)
>  {
> -	pr_debug("ack: %ld\n", d->hwirq);
> +	pr_debug("irq-xilinx: ack: %ld\n", d->hwirq);
>  	write_fn(1 << d->hwirq, intc_baseaddr + IAR);
>  }
>  
> @@ -86,7 +86,7 @@ static void intc_mask_ack(struct irq_data *d)
>  {
>  	unsigned long mask = 1 << d->hwirq;
>  
> -	pr_debug("disable_and_ack: %ld\n", d->hwirq);
> +	pr_debug("irq-xilinx: disable_and_ack: %ld\n", d->hwirq);
>  	write_fn(mask, intc_baseaddr + CIE);
>  	write_fn(mask, intc_baseaddr + IAR);
>  }
> @@ -109,7 +109,7 @@ unsigned int get_irq(void)
>  	if (hwirq != -1U)
>  		irq = irq_find_mapping(root_domain, hwirq);
>  
> -	pr_debug("get_irq: hwirq=%d, irq=%d\n", hwirq, irq);
> +	pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);
>  
>  	return irq;
>  }
> @@ -146,20 +146,20 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
>  
>  	ret = of_property_read_u32(intc, "xlnx,num-intr-inputs", &nr_irq);
>  	if (ret < 0) {
> -		pr_err("%s: unable to read xlnx,num-intr-inputs\n", __func__);
> +		pr_err("irq-xilinx: unable to read xlnx,num-intr-inputs\n");
>  		return ret;
>  	}
>  
>  	ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &intr_mask);
>  	if (ret < 0) {
> -		pr_err("%s: unable to read xlnx,kind-of-intr\n", __func__);
> +		pr_err("irq-xilinx: unable to read xlnx,kind-of-intr\n");
>  		return ret;
>  	}
>  
>  	if (intr_mask >> nr_irq)
> -		pr_warn("%s: mismatch in kind-of-intr param\n", __func__);
> +		pr_warn("irq-xilinx: mismatch in kind-of-intr param\n");
>  
> -	pr_info("%s: num_irq=%d, edge=0x%x\n",
> +	pr_info("irq-xilinx: %s: num_irq=%d, edge=0x%x\n",
>  		intc->full_name, nr_irq, intr_mask);
>  
>  	write_fn = intc_write32;
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [Patch v7 4/7] irqchip: xilinx: Rename get_irq to xintc_get_irq

[Michal Simek]

On 14.11.2016 13:13, Zubair Lutfullah Kakakhel wrote:
> Now that the driver is generic and used by multiple archs,
> get_irq is too generic.
> 
> Rename get_irq to xintc_get_irq to avoid any conflicts
> 
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> 
> ---
> V6 -> V7
> Rebase to v4.9-rc5
> 
> V5 -> V6
> Removed __func__ in printk
> Rebase to v4.9-rc3
> 
> V4 -> V5
> Rebased to v4.9-rc1
> Use __func__ in pr_err
> 
> V3 -> V4
> New patch.
> ---
>  arch/microblaze/include/asm/irq.h | 2 +-
>  arch/microblaze/kernel/irq.c      | 4 ++--
>  drivers/irqchip/irq-xilinx-intc.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/microblaze/include/asm/irq.h b/arch/microblaze/include/asm/irq.h
> index bab3b13..d785def 100644
> --- a/arch/microblaze/include/asm/irq.h
> +++ b/arch/microblaze/include/asm/irq.h
> @@ -16,6 +16,6 @@ struct pt_regs;
>  extern void do_IRQ(struct pt_regs *regs);
>  
>  /* should be defined in each interrupt controller driver */
> -extern unsigned int get_irq(void);
> +extern unsigned int xintc_get_irq(void);
>  
>  #endif /* _ASM_MICROBLAZE_IRQ_H */
> diff --git a/arch/microblaze/kernel/irq.c b/arch/microblaze/kernel/irq.c
> index 11e24de..903dad8 100644
> --- a/arch/microblaze/kernel/irq.c
> +++ b/arch/microblaze/kernel/irq.c
> @@ -29,12 +29,12 @@ void __irq_entry do_IRQ(struct pt_regs *regs)
>  	trace_hardirqs_off();
>  
>  	irq_enter();
> -	irq = get_irq();
> +	irq = xintc_get_irq();
>  next_irq:
>  	BUG_ON(!irq);
>  	generic_handle_irq(irq);
>  
> -	irq = get_irq();
> +	irq = xintc_get_irq();
>  	if (irq != -1U) {
>  		pr_debug("next irq: %d\n", irq);
>  		++concurrent_irq;
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index 7331d8c..34ec609 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -101,7 +101,7 @@ static struct irq_chip intc_dev = {
>  	.irq_mask_ack = intc_mask_ack,
>  };
>  
> -unsigned int get_irq(void)
> +unsigned int xintc_get_irq(void)
>  {
>  	unsigned int hwirq, irq = -1;
>  
> 

Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [Patch v7 6/7] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided

[Michal Simek]

On 14.11.2016 13:13, Zubair Lutfullah Kakakhel wrote:
> The powerpc dts file does not have the xlnx,kind-of-intr property.
> Instead of erroring out, give a warning instead. And attempt to
> continue to probe the interrupt controller while assuming
> kind-of-intr is 0x0 as a fall back.
> 
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> 
> ---
> V6 -> V7
> Rebase to v4.9-rc5
> 
> V5 -> V6
> Rebase to v4.9-rc3
> 
> V5 new patch
> ---
>  drivers/irqchip/irq-xilinx-intc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index 971c141..d330917 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -179,8 +179,8 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
>  
>  	ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &irqc->intr_mask);
>  	if (ret < 0) {
> -		pr_err("irq-xilinx: unable to read xlnx,kind-of-intr\n");
> -		goto err_alloc;
> +		pr_warn("irq-xilinx: unable to read xlnx,kind-of-intr\n");
> +		irqc->intr_mask = 0;
>  	}
>  
>  	if (irqc->intr_mask >> nr_irq)
> 


Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [Patch v7 7/7] powerpc/virtex: Use generic xilinx irqchip driver

[Michal Simek]

On 14.11.2016 13:13, Zubair Lutfullah Kakakhel wrote:
> The Xilinx interrupt controller driver is now available in drivers/irqchip.
> Switch to using that driver.
> 
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> 
> ---
> V6 - V7
> Rebase to v4.9-rc5
> 
> V5 -> V6 Added Acked-by Micheal Ellerman
> 
> V5 New patch
> 
> Tested on virtex440-ml507 using qemu
> ---
>  arch/powerpc/include/asm/xilinx_intc.h |   2 +-
>  arch/powerpc/platforms/40x/Kconfig     |   1 +
>  arch/powerpc/platforms/40x/virtex.c    |   2 +-
>  arch/powerpc/platforms/44x/Kconfig     |   1 +
>  arch/powerpc/platforms/44x/virtex.c    |   2 +-
>  arch/powerpc/sysdev/xilinx_intc.c      | 211 +--------------------------------
>  drivers/irqchip/irq-xilinx-intc.c      |   3 +-
>  7 files changed, 9 insertions(+), 213 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/xilinx_intc.h b/arch/powerpc/include/asm/xilinx_intc.h
> index 343612f..3192d7f 100644
> --- a/arch/powerpc/include/asm/xilinx_intc.h
> +++ b/arch/powerpc/include/asm/xilinx_intc.h
> @@ -14,7 +14,7 @@
>  #ifdef __KERNEL__
>  
>  extern void __init xilinx_intc_init_tree(void);
> -extern unsigned int xilinx_intc_get_irq(void);
> +extern unsigned int xintc_get_irq(void);
>  
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_XILINX_INTC_H */
> diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
> index e3257f2..1d7c1b1 100644
> --- a/arch/powerpc/platforms/40x/Kconfig
> +++ b/arch/powerpc/platforms/40x/Kconfig
> @@ -64,6 +64,7 @@ config XILINX_VIRTEX_GENERIC_BOARD
>  	default n
>  	select XILINX_VIRTEX_II_PRO
>  	select XILINX_VIRTEX_4_FX
> +	select XILINX_INTC
>  	help
>  	  This option enables generic support for Xilinx Virtex based boards.
>  
> diff --git a/arch/powerpc/platforms/40x/virtex.c b/arch/powerpc/platforms/40x/virtex.c
> index 91a08ea..e3d5e09 100644
> --- a/arch/powerpc/platforms/40x/virtex.c
> +++ b/arch/powerpc/platforms/40x/virtex.c
> @@ -48,7 +48,7 @@ define_machine(virtex) {
>  	.probe			= virtex_probe,
>  	.setup_arch		= xilinx_pci_init,
>  	.init_IRQ		= xilinx_intc_init_tree,
> -	.get_irq		= xilinx_intc_get_irq,
> +	.get_irq		= xintc_get_irq,
>  	.restart		= ppc4xx_reset_system,
>  	.calibrate_decr		= generic_calibrate_decr,
>  };
> diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
> index 48fc180..25b8d64 100644
> --- a/arch/powerpc/platforms/44x/Kconfig
> +++ b/arch/powerpc/platforms/44x/Kconfig
> @@ -241,6 +241,7 @@ config XILINX_VIRTEX440_GENERIC_BOARD
>  	depends on 44x
>  	default n
>  	select XILINX_VIRTEX_5_FXT
> +	select XILINX_INTC
>  	help
>  	  This option enables generic support for Xilinx Virtex based boards
>  	  that use a 440 based processor in the Virtex 5 FXT FPGA architecture.
> diff --git a/arch/powerpc/platforms/44x/virtex.c b/arch/powerpc/platforms/44x/virtex.c
> index a7e0802..3eb13ed 100644
> --- a/arch/powerpc/platforms/44x/virtex.c
> +++ b/arch/powerpc/platforms/44x/virtex.c
> @@ -54,7 +54,7 @@ define_machine(virtex) {
>  	.probe			= virtex_probe,
>  	.setup_arch		= xilinx_pci_init,
>  	.init_IRQ		= xilinx_intc_init_tree,
> -	.get_irq		= xilinx_intc_get_irq,
> +	.get_irq		= xintc_get_irq,
>  	.calibrate_decr		= generic_calibrate_decr,
>  	.restart		= ppc4xx_reset_system,
>  };
> diff --git a/arch/powerpc/sysdev/xilinx_intc.c b/arch/powerpc/sysdev/xilinx_intc.c
> index 0f52d79..4a86dcf 100644
> --- a/arch/powerpc/sysdev/xilinx_intc.c
> +++ b/arch/powerpc/sysdev/xilinx_intc.c
> @@ -29,194 +29,7 @@
>  #include <asm/processor.h>
>  #include <asm/i8259.h>
>  #include <asm/irq.h>
> -
> -/*
> - * INTC Registers
> - */
> -#define XINTC_ISR	0	/* Interrupt Status */
> -#define XINTC_IPR	4	/* Interrupt Pending */
> -#define XINTC_IER	8	/* Interrupt Enable */
> -#define XINTC_IAR	12	/* Interrupt Acknowledge */
> -#define XINTC_SIE	16	/* Set Interrupt Enable bits */
> -#define XINTC_CIE	20	/* Clear Interrupt Enable bits */
> -#define XINTC_IVR	24	/* Interrupt Vector */
> -#define XINTC_MER	28	/* Master Enable */
> -
> -static struct irq_domain *master_irqhost;
> -
> -#define XILINX_INTC_MAXIRQS	(32)
> -
> -/* The following table allows the interrupt type, edge or level,
> - * to be cached after being read from the device tree until the interrupt
> - * is mapped
> - */
> -static int xilinx_intc_typetable[XILINX_INTC_MAXIRQS];
> -
> -/* Map the interrupt type from the device tree to the interrupt types
> - * used by the interrupt subsystem
> - */
> -static unsigned char xilinx_intc_map_senses[] = {
> -	IRQ_TYPE_EDGE_RISING,
> -	IRQ_TYPE_EDGE_FALLING,
> -	IRQ_TYPE_LEVEL_HIGH,
> -	IRQ_TYPE_LEVEL_LOW,
> -};
> -
> -/*
> - * The interrupt controller is setup such that it doesn't work well with
> - * the level interrupt handler in the kernel because the handler acks the
> - * interrupt before calling the application interrupt handler. To deal with
> - * that, we use 2 different irq chips so that different functions can be
> - * used for level and edge type interrupts.
> - *
> - * IRQ Chip common (across level and edge) operations
> - */
> -static void xilinx_intc_mask(struct irq_data *d)
> -{
> -	int irq = irqd_to_hwirq(d);
> -	void * regs = irq_data_get_irq_chip_data(d);
> -	pr_debug("mask: %d\n", irq);
> -	out_be32(regs + XINTC_CIE, 1 << irq);
> -}
> -
> -static int xilinx_intc_set_type(struct irq_data *d, unsigned int flow_type)
> -{
> -	return 0;
> -}
> -
> -/*
> - * IRQ Chip level operations
> - */
> -static void xilinx_intc_level_unmask(struct irq_data *d)
> -{
> -	int irq = irqd_to_hwirq(d);
> -	void * regs = irq_data_get_irq_chip_data(d);
> -	pr_debug("unmask: %d\n", irq);
> -	out_be32(regs + XINTC_SIE, 1 << irq);
> -
> -	/* ack level irqs because they can't be acked during
> -	 * ack function since the handle_level_irq function
> -	 * acks the irq before calling the inerrupt handler
> -	 */
> -	out_be32(regs + XINTC_IAR, 1 << irq);
> -}
> -
> -static struct irq_chip xilinx_intc_level_irqchip = {
> -	.name = "Xilinx Level INTC",
> -	.irq_mask = xilinx_intc_mask,
> -	.irq_mask_ack = xilinx_intc_mask,
> -	.irq_unmask = xilinx_intc_level_unmask,
> -	.irq_set_type = xilinx_intc_set_type,
> -};
> -
> -/*
> - * IRQ Chip edge operations
> - */
> -static void xilinx_intc_edge_unmask(struct irq_data *d)
> -{
> -	int irq = irqd_to_hwirq(d);
> -	void *regs = irq_data_get_irq_chip_data(d);
> -	pr_debug("unmask: %d\n", irq);
> -	out_be32(regs + XINTC_SIE, 1 << irq);
> -}
> -
> -static void xilinx_intc_edge_ack(struct irq_data *d)
> -{
> -	int irq = irqd_to_hwirq(d);
> -	void * regs = irq_data_get_irq_chip_data(d);
> -	pr_debug("ack: %d\n", irq);
> -	out_be32(regs + XINTC_IAR, 1 << irq);
> -}
> -
> -static struct irq_chip xilinx_intc_edge_irqchip = {
> -	.name = "Xilinx Edge  INTC",
> -	.irq_mask = xilinx_intc_mask,
> -	.irq_unmask = xilinx_intc_edge_unmask,
> -	.irq_ack = xilinx_intc_edge_ack,
> -	.irq_set_type = xilinx_intc_set_type,
> -};
> -
> -/*
> - * IRQ Host operations
> - */
> -
> -/**
> - * xilinx_intc_xlate - translate virq# from device tree interrupts property
> - */
> -static int xilinx_intc_xlate(struct irq_domain *h, struct device_node *ct,
> -				const u32 *intspec, unsigned int intsize,
> -				irq_hw_number_t *out_hwirq,
> -				unsigned int *out_flags)
> -{
> -	if ((intsize < 2) || (intspec[0] >= XILINX_INTC_MAXIRQS))
> -		return -EINVAL;
> -
> -	/* keep a copy of the interrupt type til the interrupt is mapped
> -	 */
> -	xilinx_intc_typetable[intspec[0]] = xilinx_intc_map_senses[intspec[1]];
> -
> -	/* Xilinx uses 2 interrupt entries, the 1st being the h/w
> -	 * interrupt number, the 2nd being the interrupt type, edge or level
> -	 */
> -	*out_hwirq = intspec[0];
> -	*out_flags = xilinx_intc_map_senses[intspec[1]];
> -
> -	return 0;
> -}
> -static int xilinx_intc_map(struct irq_domain *h, unsigned int virq,
> -				  irq_hw_number_t irq)
> -{
> -	irq_set_chip_data(virq, h->host_data);
> -
> -	if (xilinx_intc_typetable[irq] == IRQ_TYPE_LEVEL_HIGH ||
> -	    xilinx_intc_typetable[irq] == IRQ_TYPE_LEVEL_LOW) {
> -		irq_set_chip_and_handler(virq, &xilinx_intc_level_irqchip,
> -					 handle_level_irq);
> -	} else {
> -		irq_set_chip_and_handler(virq, &xilinx_intc_edge_irqchip,
> -					 handle_edge_irq);
> -	}
> -	return 0;
> -}
> -
> -static const struct irq_domain_ops xilinx_intc_ops = {
> -	.map = xilinx_intc_map,
> -	.xlate = xilinx_intc_xlate,
> -};
> -
> -struct irq_domain * __init
> -xilinx_intc_init(struct device_node *np)
> -{
> -	struct irq_domain * irq;
> -	void * regs;
> -
> -	/* Find and map the intc registers */
> -	regs = of_iomap(np, 0);
> -	if (!regs) {
> -		pr_err("xilinx_intc: could not map registers\n");
> -		return NULL;
> -	}
> -
> -	/* Setup interrupt controller */
> -	out_be32(regs + XINTC_IER, 0); /* disable all irqs */
> -	out_be32(regs + XINTC_IAR, ~(u32) 0); /* Acknowledge pending irqs */
> -	out_be32(regs + XINTC_MER, 0x3UL); /* Turn on the Master Enable. */
> -
> -	/* Allocate and initialize an irq_domain structure. */
> -	irq = irq_domain_add_linear(np, XILINX_INTC_MAXIRQS, &xilinx_intc_ops,
> -				    regs);
> -	if (!irq)
> -		panic(__FILE__ ": Cannot allocate IRQ host\n");
> -
> -	return irq;
> -}
> -
> -int xilinx_intc_get_irq(void)
> -{
> -	void * regs = master_irqhost->host_data;
> -	pr_debug("get_irq:\n");
> -	return irq_linear_revmap(master_irqhost, in_be32(regs + XINTC_IVR));
> -}
> +#include <linux/irqchip.h>
>  
>  #if defined(CONFIG_PPC_I8259)
>  /*
> @@ -265,31 +78,11 @@ static void __init xilinx_i8259_setup_cascade(void)
>  static inline void xilinx_i8259_setup_cascade(void) { return; }
>  #endif /* defined(CONFIG_PPC_I8259) */
>  
> -static const struct of_device_id xilinx_intc_match[] __initconst = {
> -	{ .compatible = "xlnx,opb-intc-1.00.c", },
> -	{ .compatible = "xlnx,xps-intc-1.00.a", },
> -	{}
> -};
> -
>  /*
>   * Initialize master Xilinx interrupt controller
>   */
>  void __init xilinx_intc_init_tree(void)
>  {
> -	struct device_node *np;
> -
> -	/* find top level interrupt controller */
> -	for_each_matching_node(np, xilinx_intc_match) {
> -		if (!of_get_property(np, "interrupts", NULL))
> -			break;
> -	}
> -	BUG_ON(!np);
> -
> -	master_irqhost = xilinx_intc_init(np);
> -	BUG_ON(!master_irqhost);
> -
> -	irq_set_default_host(master_irqhost);
> -	of_node_put(np);
> -
> +	irqchip_init();
>  	xilinx_i8259_setup_cascade();
>  }
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index d330917..3db7ab1 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -237,4 +237,5 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
>  
>  }
>  
> -IRQCHIP_DECLARE(xilinx_intc, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
> +IRQCHIP_DECLARE(xilinx_intc_xps, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
> +IRQCHIP_DECLARE(xilinx_intc_opb, "xlnx,opb-intc-1.00.c", xilinx_intc_of_init);
> 


Looks reasonable.
Acked-by: Michal Simek <michal.simek@xilinx.com>

Thanks,
Michal

Re: [Patch v7 3/7] irqchip: xilinx: restructure and use jump label api

[Michal Simek]

On 14.11.2016 13:13, Zubair Lutfullah Kakakhel wrote:
> Add a global structure to house various variables.
> And cleanup read/write handling by using jump label api.
> 
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> 
> ---
> V6 -> V7
> Restructure and use jump label api
> Better commit log
> 
> V5 -> V6
> Removed __func__ from printk
> Rebase to v4.9-rc3
> 
> V4 -> V5
> Rebased to v4.9-rc1
> Better error handling
> 
> V3 -> V4
> Better error handling for kzalloc
> Erroring out if the axi intc is probed twice as that isn't
> supported
> 
> V2 -> V3
> New patch. Cleans up driver structure
> ---
>  drivers/irqchip/irq-xilinx-intc.c | 118 +++++++++++++++++++++-----------------
>  1 file changed, 66 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
> index 096c1ed..7331d8c 100644
> --- a/drivers/irqchip/irq-xilinx-intc.c
> +++ b/drivers/irqchip/irq-xilinx-intc.c
> @@ -14,10 +14,9 @@
>  #include <linux/irqchip.h>
>  #include <linux/of_address.h>
>  #include <linux/io.h>
> +#include <linux/jump_label.h>
>  #include <linux/bug.h>
>  
> -static void __iomem *intc_baseaddr;
> -
>  /* No one else should require these constants, so define them locally here. */
>  #define ISR 0x00			/* Interrupt Status Register */
>  #define IPR 0x04			/* Interrupt Pending Register */
> @@ -31,27 +30,30 @@ static void __iomem *intc_baseaddr;
>  #define MER_ME (1<<0)
>  #define MER_HIE (1<<1)
>  
> -static unsigned int (*read_fn)(void __iomem *);
> -static void (*write_fn)(u32, void __iomem *);
> +static DEFINE_STATIC_KEY_FALSE(xintc_is_be);
>  
> -static void intc_write32(u32 val, void __iomem *addr)
> -{
> -	iowrite32(val, addr);
> -}
> +struct xintc_irq_chip {
> +	void		__iomem *base;
> +	struct		irq_domain *root_domain;
> +	u32		intr_mask;
> +};
>  
> -static unsigned int intc_read32(void __iomem *addr)
> -{
> -	return ioread32(addr);
> -}
> +static struct xintc_irq_chip *xintc_irqc;
>  
> -static void intc_write32_be(u32 val, void __iomem *addr)
> +static void xintc_write(int reg, u32 data)
>  {
> -	iowrite32be(val, addr);
> +	if (static_branch_unlikely(&xintc_is_be))
> +		iowrite32be(data, xintc_irqc->base + reg);
> +	else
> +		iowrite32(data, xintc_irqc->base + reg);
>  }
>  
> -static unsigned int intc_read32_be(void __iomem *addr)
> +static unsigned int xintc_read(int reg)
>  {
> -	return ioread32be(addr);
> +	if (static_branch_unlikely(&xintc_is_be))
> +		return ioread32be(xintc_irqc->base + reg);
> +	else
> +		return ioread32(xintc_irqc->base + reg);
>  }
>  
>  static void intc_enable_or_unmask(struct irq_data *d)
> @@ -65,21 +67,21 @@ static void intc_enable_or_unmask(struct irq_data *d)
>  	 * acks the irq before calling the interrupt handler
>  	 */
>  	if (irqd_is_level_type(d))
> -		write_fn(mask, intc_baseaddr + IAR);
> +		xintc_write(IAR, mask);
>  
> -	write_fn(mask, intc_baseaddr + SIE);
> +	xintc_write(SIE, mask);
>  }
>  
>  static void intc_disable_or_mask(struct irq_data *d)
>  {
>  	pr_debug("irq-xilinx: disable: %ld\n", d->hwirq);
> -	write_fn(1 << d->hwirq, intc_baseaddr + CIE);
> +	xintc_write(CIE, 1 << d->hwirq);
>  }
>  
>  static void intc_ack(struct irq_data *d)
>  {
>  	pr_debug("irq-xilinx: ack: %ld\n", d->hwirq);
> -	write_fn(1 << d->hwirq, intc_baseaddr + IAR);
> +	xintc_write(IAR, 1 << d->hwirq);
>  }
>  
>  static void intc_mask_ack(struct irq_data *d)
> @@ -87,8 +89,8 @@ static void intc_mask_ack(struct irq_data *d)
>  	unsigned long mask = 1 << d->hwirq;
>  
>  	pr_debug("irq-xilinx: disable_and_ack: %ld\n", d->hwirq);
> -	write_fn(mask, intc_baseaddr + CIE);
> -	write_fn(mask, intc_baseaddr + IAR);
> +	xintc_write(CIE, mask);
> +	xintc_write(IAR, mask);
>  }
>  
>  static struct irq_chip intc_dev = {
> @@ -99,15 +101,13 @@ static struct irq_chip intc_dev = {
>  	.irq_mask_ack = intc_mask_ack,
>  };
>  
> -static struct irq_domain *root_domain;
> -
>  unsigned int get_irq(void)
>  {
>  	unsigned int hwirq, irq = -1;
>  
> -	hwirq = read_fn(intc_baseaddr + IVR);
> +	hwirq = xintc_read(IVR);
>  	if (hwirq != -1U)
> -		irq = irq_find_mapping(root_domain, hwirq);
> +		irq = irq_find_mapping(xintc_irqc->root_domain, hwirq);
>  
>  	pr_debug("irq-xilinx: hwirq=%d, irq=%d\n", hwirq, irq);
>  
> @@ -116,9 +116,7 @@ unsigned int get_irq(void)
>  
>  static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
>  {
> -	u32 intr_mask = (u32)d->host_data;
> -
> -	if (intr_mask & (1 << hw)) {
> +	if (xintc_irqc->intr_mask & (1 << hw)) {
>  		irq_set_chip_and_handler_name(irq, &intc_dev,
>  						handle_edge_irq, "edge");
>  		irq_clear_status_flags(irq, IRQ_LEVEL);
> @@ -138,59 +136,75 @@ static const struct irq_domain_ops xintc_irq_domain_ops = {
>  static int __init xilinx_intc_of_init(struct device_node *intc,
>  					     struct device_node *parent)
>  {
> -	u32 nr_irq, intr_mask;
> +	u32 nr_irq;
>  	int ret;
> +	struct xintc_irq_chip *irqc;
>  
> -	intc_baseaddr = of_iomap(intc, 0);
> -	BUG_ON(!intc_baseaddr);
> +	if (xintc_irqc) {
> +		pr_err("irq-xilinx: Multiple instances aren't supported\n");
> +		return -EINVAL;
> +	}

I don't agree with this.
Pretty long time ago we were added support for multiple instances in
xilinx private tree.
You can look here.

https://github.com/Xilinx/linux-xlnx/blob/master/drivers/irqchip/irq-xilinx-intc.c

Not sure if this the latest way how to do it but as you can see
we were setting up
irq_set_handler_data(irq, intc);

and then when you need that structure we were calling
struct intc *local_intc = irq_data_get_irq_chip_data(d);

And that should be it to support multiple instance of this driver.

Based on 5/7 you are describing your interrupt subsystem like this.

Peripherals --> xilinx_intcontroller -> mips_cpu_int controller
If mips_cpu_int has more than one input you can connect more xilinx intc
controllers.
If not you still have an option to connect
xilinx_intcontroller(up to 32 peripherals) -> xilinx_intcontroller(one
intc + up to 31 peripherals)  -> mips_cpu_int controller

Thanks,
Michal

Re: [Patch v7 3/7] irqchip: xilinx: restructure and use jump label api

[Zubair Lutfullah Kakakhel]

Hi,

On 11/15/2016 12:49 PM, Michal Simek wrote:
> On 14.11.2016 13:13, Zubair Lutfullah Kakakhel wrote:
>> Add a global structure to house various variables.
>> And cleanup read/write handling by using jump label api.
>>
>> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>>

...

>> @@ -138,59 +136,75 @@ static const struct irq_domain_ops xintc_irq_domain_ops = {
>>  static int __init xilinx_intc_of_init(struct device_node *intc,
>>  					     struct device_node *parent)
>>  {
>> -	u32 nr_irq, intr_mask;
>> +	u32 nr_irq;
>>  	int ret;
>> +	struct xintc_irq_chip *irqc;
>>
>> -	intc_baseaddr = of_iomap(intc, 0);
>> -	BUG_ON(!intc_baseaddr);
>> +	if (xintc_irqc) {
>> +		pr_err("irq-xilinx: Multiple instances aren't supported\n");
>> +		return -EINVAL;
>> +	}
>
> I don't agree with this.
> Pretty long time ago we were added support for multiple instances in
> xilinx private tree.
> You can look here.
>
> https://github.com/Xilinx/linux-xlnx/blob/master/drivers/irqchip/irq-xilinx-intc.c
>
> Not sure if this the latest way how to do it but as you can see
> we were setting up
> irq_set_handler_data(irq, intc);
>
> and then when you need that structure we were calling
> struct intc *local_intc = irq_data_get_irq_chip_data(d);
>
> And that should be it to support multiple instance of this driver.
>
> Based on 5/7 you are describing your interrupt subsystem like this.
>
> Peripherals --> xilinx_intcontroller -> mips_cpu_int controller
> If mips_cpu_int has more than one input you can connect more xilinx intc
> controllers.
> If not you still have an option to connect
> xilinx_intcontroller(up to 32 peripherals) -> xilinx_intcontroller(one
> intc + up to 31 peripherals)  -> mips_cpu_int controller

That configuration in FPGA is technically possible. Although not done/needed in the
way we use the Xilinx Interrupt Controller IP block in MIPSfpga.

This series takes the drivers out of arch code and makes it accessible.
Any further development on the driver would be common to all architectures.
Support for multiple instances would be a 'new feature'.

I say this as this series keeps growing and mutating in terms of its scope
and work.

Would it be possible to ack this so that the restructure out of arch code
can move forward?

Regards,
ZubairLK

>
> Thanks,
> Michal
>

Re: [Patch v7 3/7] irqchip: xilinx: restructure and use jump label api

[Michal Simek]

On 15.11.2016 17:03, Zubair Lutfullah Kakakhel wrote:
> Hi,
> 
> On 11/15/2016 12:49 PM, Michal Simek wrote:
>> On 14.11.2016 13:13, Zubair Lutfullah Kakakhel wrote:
>>> Add a global structure to house various variables.
>>> And cleanup read/write handling by using jump label api.
>>>
>>> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>>>
> 
> ...
> 
>>> @@ -138,59 +136,75 @@ static const struct irq_domain_ops
>>> xintc_irq_domain_ops = {
>>>  static int __init xilinx_intc_of_init(struct device_node *intc,
>>>                           struct device_node *parent)
>>>  {
>>> -    u32 nr_irq, intr_mask;
>>> +    u32 nr_irq;
>>>      int ret;
>>> +    struct xintc_irq_chip *irqc;
>>>
>>> -    intc_baseaddr = of_iomap(intc, 0);
>>> -    BUG_ON(!intc_baseaddr);
>>> +    if (xintc_irqc) {
>>> +        pr_err("irq-xilinx: Multiple instances aren't supported\n");
>>> +        return -EINVAL;
>>> +    }
>>
>> I don't agree with this.
>> Pretty long time ago we were added support for multiple instances in
>> xilinx private tree.
>> You can look here.
>>
>> https://github.com/Xilinx/linux-xlnx/blob/master/drivers/irqchip/irq-xilinx-intc.c
>>
>>
>> Not sure if this the latest way how to do it but as you can see
>> we were setting up
>> irq_set_handler_data(irq, intc);
>>
>> and then when you need that structure we were calling
>> struct intc *local_intc = irq_data_get_irq_chip_data(d);
>>
>> And that should be it to support multiple instance of this driver.
>>
>> Based on 5/7 you are describing your interrupt subsystem like this.
>>
>> Peripherals --> xilinx_intcontroller -> mips_cpu_int controller
>> If mips_cpu_int has more than one input you can connect more xilinx intc
>> controllers.
>> If not you still have an option to connect
>> xilinx_intcontroller(up to 32 peripherals) -> xilinx_intcontroller(one
>> intc + up to 31 peripherals)  -> mips_cpu_int controller
> 
> That configuration in FPGA is technically possible. Although not
> done/needed in the
> way we use the Xilinx Interrupt Controller IP block in MIPSfpga.
> 
> This series takes the drivers out of arch code and makes it accessible.
> Any further development on the driver would be common to all architectures.
> Support for multiple instances would be a 'new feature'.
> 
> I say this as this series keeps growing and mutating in terms of its scope
> and work.

fair enough - it can be added in separate patch.

> 
> Would it be possible to ack this so that the restructure out of arch code
> can move forward?

I have tested the whole series on Microblaze and I can't see any problem
in running it there.

That's why
Tested-by; Michal Simek <michal.simek@xilinx.com>

If everything is right needs to be checked by irqchip experts.

Thanks,
Michal

Re: [Patch v7 6/7] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided

[Thomas Gleixner]

On Mon, 14 Nov 2016, Zubair Lutfullah Kakakhel wrote:

> The powerpc dts file does not have the xlnx,kind-of-intr property.
> Instead of erroring out, give a warning instead. And attempt to
> continue to probe the interrupt controller while assuming
> kind-of-intr is 0x0 as a fall back.

This is broken, really. On multiplatform kernels this will try to probe the
chip no matter what.
 
Powerpc already has:

static const struct of_device_id xilinx_intc_match[] __initconst = {
       { .compatible = "xlnx,opb-intc-1.00.c", },
       { .compatible = "xlnx,xps-intc-1.00.a", },
       {}
};

Unless I'm missing something important, then adding those compatible
strings to the driver will just keep stuff working as expected instead of
adding unsafe and broken heuristics.

Thanks,

	tglx

Re: [Patch v7 6/7] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided

[Zubair Lutfullah Kakakhel]

Hi,

On 11/18/2016 01:29 PM, Thomas Gleixner wrote:
> On Mon, 14 Nov 2016, Zubair Lutfullah Kakakhel wrote:
>
>> The powerpc dts file does not have the xlnx,kind-of-intr property.
>> Instead of erroring out, give a warning instead. And attempt to
>> continue to probe the interrupt controller while assuming
>> kind-of-intr is 0x0 as a fall back.
>
> This is broken, really. On multiplatform kernels this will try to probe the
> chip no matter what.

I'm not sure I understand why this driver will probe on multi-platform kernels
if the compatible string isn't in the DT?

>
> Powerpc already has:
>
> static const struct of_device_id xilinx_intc_match[] __initconst = {
>        { .compatible = "xlnx,opb-intc-1.00.c", },
>        { .compatible = "xlnx,xps-intc-1.00.a", },
>        {}
> };
>
> Unless I'm missing something important, then adding those compatible
> strings to the driver will just keep stuff working as expected instead of
> adding unsafe and broken heuristics.
>

The last two lines of the driver already specify the compatible strings.

"
IRQCHIP_DECLARE(xilinx_intc_xps, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
IRQCHIP_DECLARE(xilinx_intc_opb, "xlnx,opb-intc-1.00.c", xilinx_intc_of_init);
"

I'll elaborate on the commit message.

The DT node in arch/powerpc for this driver is

intc_0: interrupt-controller@81800000 {
			#interrupt-cells = <0x2>;
			compatible = "xlnx,xps-intc-1.00.a";
			interrupt-controller ;
			reg = < 0x81800000 0x10000 >;
			xlnx,num-intr-inputs = <0xc>;
		} ;

The DT node in arch/microblaze for this driver is

intc_0: interrupt-controller@81800000 {
			#interrupt-cells = <0x2>;
			compatible = "xlnx,xps-intc-1.00.a";
			interrupt-controller ;
			reg = < 0x81800000 0x10000 >;
			xlnx,kind-of-intr = <0x100>;	//<Missing from ppc>!
			xlnx,num-intr-inputs = <0x9>;
		} ;

The PPC driver assumes the kind-of-intr value be 0x0 and doesn't specify it in DT.
This patch makes that a fall back case. Instead of completely error-ing out.

Regards,
ZubairLK

> Thanks,
>
> 	tglx
>
>

Re: [Patch v7 6/7] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided

[Marc Zyngier]

On 21/11/16 14:05, Zubair Lutfullah Kakakhel wrote:
> Hi,
> 
> On 11/18/2016 01:29 PM, Thomas Gleixner wrote:
>> On Mon, 14 Nov 2016, Zubair Lutfullah Kakakhel wrote:
>>
>>> The powerpc dts file does not have the xlnx,kind-of-intr property.
>>> Instead of erroring out, give a warning instead. And attempt to
>>> continue to probe the interrupt controller while assuming
>>> kind-of-intr is 0x0 as a fall back.
>>
>> This is broken, really. On multiplatform kernels this will try to probe the
>> chip no matter what.
> 
> I'm not sure I understand why this driver will probe on multi-platform kernels
> if the compatible string isn't in the DT?
> 
>>
>> Powerpc already has:
>>
>> static const struct of_device_id xilinx_intc_match[] __initconst = {
>>        { .compatible = "xlnx,opb-intc-1.00.c", },
>>        { .compatible = "xlnx,xps-intc-1.00.a", },
>>        {}
>> };
>>
>> Unless I'm missing something important, then adding those compatible
>> strings to the driver will just keep stuff working as expected instead of
>> adding unsafe and broken heuristics.
>>
> 
> The last two lines of the driver already specify the compatible strings.
> 
> "
> IRQCHIP_DECLARE(xilinx_intc_xps, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
> IRQCHIP_DECLARE(xilinx_intc_opb, "xlnx,opb-intc-1.00.c", xilinx_intc_of_init);
> "

Is PPC actually using this infrastructure? It predates the whole
IRQCHIP_DECLARE business by about a decade. You seem to have tested it
using QEMU, so I assume it "just works", but I'd feel more reassured it
you stated so...

Thanks,

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

Re: [Patch v7 6/7] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided

[Zubair Lutfullah Kakakhel]

Hi,

On 11/21/2016 02:17 PM, Marc Zyngier wrote:
> On 21/11/16 14:05, Zubair Lutfullah Kakakhel wrote:
>> Hi,
>>
>> On 11/18/2016 01:29 PM, Thomas Gleixner wrote:
>>> On Mon, 14 Nov 2016, Zubair Lutfullah Kakakhel wrote:
>>>
>>>> The powerpc dts file does not have the xlnx,kind-of-intr property.
>>>> Instead of erroring out, give a warning instead. And attempt to
>>>> continue to probe the interrupt controller while assuming
>>>> kind-of-intr is 0x0 as a fall back.
>>>
>>> This is broken, really. On multiplatform kernels this will try to probe the
>>> chip no matter what.
>>
>> I'm not sure I understand why this driver will probe on multi-platform kernels
>> if the compatible string isn't in the DT?
>>
>>>
>>> Powerpc already has:
>>>
>>> static const struct of_device_id xilinx_intc_match[] __initconst = {
>>>        { .compatible = "xlnx,opb-intc-1.00.c", },
>>>        { .compatible = "xlnx,xps-intc-1.00.a", },
>>>        {}
>>> };
>>>
>>> Unless I'm missing something important, then adding those compatible
>>> strings to the driver will just keep stuff working as expected instead of
>>> adding unsafe and broken heuristics.
>>>
>>
>> The last two lines of the driver already specify the compatible strings.
>>
>> "
>> IRQCHIP_DECLARE(xilinx_intc_xps, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
>> IRQCHIP_DECLARE(xilinx_intc_opb, "xlnx,opb-intc-1.00.c", xilinx_intc_of_init);
>> "
>
> Is PPC actually using this infrastructure? It predates the whole
> IRQCHIP_DECLARE business by about a decade. You seem to have tested it
> using QEMU, so I assume it "just works", but I'd feel more reassured it
> you stated so...

I didn't realize that it could have been an issue.
I simply included <linux/irqchip.h> and called irqchip_init() in the platform code
instead of the previous initialization. Patch 7/7 in this series does that.

And yes I tested it on QEMU. And it does look like it 'just works'.
Without this patch, the UART driver would revert to polling and there would be various
error messages about no irq domain found. With this patch, the 'no irq domain found'
messages disappeared and the uart driver did get an irq.

Regards,
ZubairLK

>
> Thanks,
>
> 	M.
>

Re: [Patch v7 6/7] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided

[Thomas Gleixner]

On Mon, 21 Nov 2016, Zubair Lutfullah Kakakhel wrote:
> On 11/18/2016 01:29 PM, Thomas Gleixner wrote:
> I'll elaborate on the commit message.
> 
> The DT node in arch/powerpc for this driver is
> 
> intc_0: interrupt-controller@81800000 {
> 			#interrupt-cells = <0x2>;
> 			compatible = "xlnx,xps-intc-1.00.a";
> 			interrupt-controller ;
> 			reg = < 0x81800000 0x10000 >;
> 			xlnx,num-intr-inputs = <0xc>;
> 		} ;
> 
> The DT node in arch/microblaze for this driver is
> 
> intc_0: interrupt-controller@81800000 {
> 			#interrupt-cells = <0x2>;
> 			compatible = "xlnx,xps-intc-1.00.a";
> 			interrupt-controller ;
> 			reg = < 0x81800000 0x10000 >;
> 			xlnx,kind-of-intr = <0x100>;	//<Missing from ppc>!
> 			xlnx,num-intr-inputs = <0x9>;
> 		} ;
> 
> The PPC driver assumes the kind-of-intr value be 0x0 and doesn't specify it in
> DT.
> This patch makes that a fall back case. Instead of completely error-ing out.

Ok. makes sense. I misread the changelog/patch.

Thanks,

	tglx

Re: [Patch v7 0/7] microblaze/PowerPC: Move irq-xilinx to irqchip

[Marc Zyngier]

On 14/11/16 12:13, Zubair Lutfullah Kakakhel wrote:
> Hi,
> 
> This patch series moves the Xilinx interrupt controller driver out
> of arch/microblaze to drivers/irqchip and then cleans it up a bit.
> And then removes another implementation of the driver in arch/powerpc.
> 
> This effort results in one common driver usable by mips,microblaze
> and powerpc.
> 
> Compile tested on microblaze-el.
> Tested using qemu-system-ppc using virtix440-ml507
> Tested on MIPSfpga platform.
> 
> Based on v4.9-rc5

I've queued this for 4.10 with Michal's Acks and TBs.

Thanks,

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

Re: [Patch v7 0/7] microblaze/PowerPC: Move irq-xilinx to irqchip

[Zubair Lutfullah Kakakhel]

Hi,

On 11/22/2016 10:55 AM, Marc Zyngier wrote:
> On 14/11/16 12:13, Zubair Lutfullah Kakakhel wrote:
>> Hi,
>>
>> This patch series moves the Xilinx interrupt controller driver out
>> of arch/microblaze to drivers/irqchip and then cleans it up a bit.
>> And then removes another implementation of the driver in arch/powerpc.
>>
>> This effort results in one common driver usable by mips,microblaze
>> and powerpc.
>>
>> Compile tested on microblaze-el.
>> Tested using qemu-system-ppc using virtix440-ml507
>> Tested on MIPSfpga platform.
>>
>> Based on v4.9-rc5
>
> I've queued this for 4.10 with Michal's Acks and TBs.

Thank you very much! :)

ZubairLK

>
> Thanks,
>
> 	M.
>