* [Patch V6 1/6] microblaze: irqchip: Move intc driver to irqchip
2016-10-31 12:16 [Patch V6 0/6] microblaze/PowerPC: Move irq-xilinx Zubair Lutfullah Kakakhel
@ 2016-10-31 12:16 ` Zubair Lutfullah Kakakhel
2016-10-31 12:16 ` [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write Zubair Lutfullah Kakakhel
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Zubair Lutfullah Kakakhel @ 2016-10-31 12:16 UTC (permalink / raw)
To: monstr, tglx, jason, marc.zyngier
Cc: linux-kernel, michal.simek, linuxppc-dev, mpe, Zubair.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>
---
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 +-
arch/microblaze/kernel/intc.c | 196 --------------------------------------
drivers/irqchip/Kconfig | 4 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-xilinx-intc.c | 196 ++++++++++++++++++++++++++++++++++++++
6 files changed, 203 insertions(+), 197 deletions(-)
delete mode 100644 arch/microblaze/kernel/intc.c
create mode 100644 drivers/irqchip/irq-xilinx-intc.c
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/arch/microblaze/kernel/intc.c b/arch/microblaze/kernel/intc.c
deleted file mode 100644
index 90bec7d..0000000
--- a/arch/microblaze/kernel/intc.c
+++ /dev/null
@@ -1,196 +0,0 @@
-/*
- * Copyright (C) 2007-2013 Michal Simek <monstr@monstr.eu>
- * Copyright (C) 2012-2013 Xilinx, Inc.
- * Copyright (C) 2007-2009 PetaLogix
- * Copyright (C) 2006 Atmark Techno, Inc.
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License. See the file "COPYING" in the main directory of this archive
- * for more details.
- */
-
-#include <linux/irqdomain.h>
-#include <linux/irq.h>
-#include <linux/irqchip.h>
-#include <linux/of_address.h>
-#include <linux/io.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 */
-#define IER 0x08 /* Interrupt Enable Register */
-#define IAR 0x0c /* Interrupt Acknowledge Register */
-#define SIE 0x10 /* Set Interrupt Enable bits */
-#define CIE 0x14 /* Clear Interrupt Enable bits */
-#define IVR 0x18 /* Interrupt Vector Register */
-#define MER 0x1c /* Master Enable Register */
-
-#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 void intc_write32(u32 val, void __iomem *addr)
-{
- iowrite32(val, addr);
-}
-
-static unsigned int intc_read32(void __iomem *addr)
-{
- return ioread32(addr);
-}
-
-static void intc_write32_be(u32 val, void __iomem *addr)
-{
- iowrite32be(val, addr);
-}
-
-static unsigned int intc_read32_be(void __iomem *addr)
-{
- return ioread32be(addr);
-}
-
-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);
-
- /* ack level irqs because they can't be acked during
- * ack function since the handle_level_irq function
- * acks the irq before calling the interrupt handler
- */
- if (irqd_is_level_type(d))
- write_fn(mask, intc_baseaddr + IAR);
-
- write_fn(mask, intc_baseaddr + SIE);
-}
-
-static void intc_disable_or_mask(struct irq_data *d)
-{
- pr_debug("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);
- write_fn(1 << d->hwirq, intc_baseaddr + IAR);
-}
-
-static void intc_mask_ack(struct irq_data *d)
-{
- unsigned long mask = 1 << d->hwirq;
-
- pr_debug("disable_and_ack: %ld\n", d->hwirq);
- write_fn(mask, intc_baseaddr + CIE);
- write_fn(mask, intc_baseaddr + IAR);
-}
-
-static struct irq_chip intc_dev = {
- .name = "Xilinx INTC",
- .irq_unmask = intc_enable_or_unmask,
- .irq_mask = intc_disable_or_mask,
- .irq_ack = intc_ack,
- .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);
- if (hwirq != -1U)
- irq = irq_find_mapping(root_domain, hwirq);
-
- pr_debug("get_irq: hwirq=%d, irq=%d\n", hwirq, irq);
-
- return irq;
-}
-
-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)) {
- irq_set_chip_and_handler_name(irq, &intc_dev,
- handle_edge_irq, "edge");
- irq_clear_status_flags(irq, IRQ_LEVEL);
- } else {
- irq_set_chip_and_handler_name(irq, &intc_dev,
- handle_level_irq, "level");
- irq_set_status_flags(irq, IRQ_LEVEL);
- }
- return 0;
-}
-
-static const struct irq_domain_ops xintc_irq_domain_ops = {
- .xlate = irq_domain_xlate_onetwocell,
- .map = xintc_map,
-};
-
-static int __init xilinx_intc_of_init(struct device_node *intc,
- struct device_node *parent)
-{
- u32 nr_irq, intr_mask;
- int ret;
-
- intc_baseaddr = of_iomap(intc, 0);
- BUG_ON(!intc_baseaddr);
-
- 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__);
- 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__);
- return ret;
- }
-
- if (intr_mask >> nr_irq)
- pr_warn("%s: mismatch in kind-of-intr param\n", __func__);
-
- pr_info("%s: num_irq=%d, edge=0x%x\n",
- intc->full_name, nr_irq, 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);
-
- /* Acknowledge any pending interrupts just in case. */
- write_fn(0xffffffff, intc_baseaddr + IAR);
-
- /* 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);
- }
-
- /* 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);
-
- irq_set_default_host(root_domain);
-
- return 0;
-}
-
-IRQCHIP_DECLARE(xilinx_intc, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
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/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
new file mode 100644
index 0000000..90bec7d
--- /dev/null
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -0,0 +1,196 @@
+/*
+ * Copyright (C) 2007-2013 Michal Simek <monstr@monstr.eu>
+ * Copyright (C) 2012-2013 Xilinx, Inc.
+ * Copyright (C) 2007-2009 PetaLogix
+ * Copyright (C) 2006 Atmark Techno, Inc.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include <linux/irqdomain.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/of_address.h>
+#include <linux/io.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 */
+#define IER 0x08 /* Interrupt Enable Register */
+#define IAR 0x0c /* Interrupt Acknowledge Register */
+#define SIE 0x10 /* Set Interrupt Enable bits */
+#define CIE 0x14 /* Clear Interrupt Enable bits */
+#define IVR 0x18 /* Interrupt Vector Register */
+#define MER 0x1c /* Master Enable Register */
+
+#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 void intc_write32(u32 val, void __iomem *addr)
+{
+ iowrite32(val, addr);
+}
+
+static unsigned int intc_read32(void __iomem *addr)
+{
+ return ioread32(addr);
+}
+
+static void intc_write32_be(u32 val, void __iomem *addr)
+{
+ iowrite32be(val, addr);
+}
+
+static unsigned int intc_read32_be(void __iomem *addr)
+{
+ return ioread32be(addr);
+}
+
+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);
+
+ /* ack level irqs because they can't be acked during
+ * ack function since the handle_level_irq function
+ * acks the irq before calling the interrupt handler
+ */
+ if (irqd_is_level_type(d))
+ write_fn(mask, intc_baseaddr + IAR);
+
+ write_fn(mask, intc_baseaddr + SIE);
+}
+
+static void intc_disable_or_mask(struct irq_data *d)
+{
+ pr_debug("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);
+ write_fn(1 << d->hwirq, intc_baseaddr + IAR);
+}
+
+static void intc_mask_ack(struct irq_data *d)
+{
+ unsigned long mask = 1 << d->hwirq;
+
+ pr_debug("disable_and_ack: %ld\n", d->hwirq);
+ write_fn(mask, intc_baseaddr + CIE);
+ write_fn(mask, intc_baseaddr + IAR);
+}
+
+static struct irq_chip intc_dev = {
+ .name = "Xilinx INTC",
+ .irq_unmask = intc_enable_or_unmask,
+ .irq_mask = intc_disable_or_mask,
+ .irq_ack = intc_ack,
+ .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);
+ if (hwirq != -1U)
+ irq = irq_find_mapping(root_domain, hwirq);
+
+ pr_debug("get_irq: hwirq=%d, irq=%d\n", hwirq, irq);
+
+ return irq;
+}
+
+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)) {
+ irq_set_chip_and_handler_name(irq, &intc_dev,
+ handle_edge_irq, "edge");
+ irq_clear_status_flags(irq, IRQ_LEVEL);
+ } else {
+ irq_set_chip_and_handler_name(irq, &intc_dev,
+ handle_level_irq, "level");
+ irq_set_status_flags(irq, IRQ_LEVEL);
+ }
+ return 0;
+}
+
+static const struct irq_domain_ops xintc_irq_domain_ops = {
+ .xlate = irq_domain_xlate_onetwocell,
+ .map = xintc_map,
+};
+
+static int __init xilinx_intc_of_init(struct device_node *intc,
+ struct device_node *parent)
+{
+ u32 nr_irq, intr_mask;
+ int ret;
+
+ intc_baseaddr = of_iomap(intc, 0);
+ BUG_ON(!intc_baseaddr);
+
+ 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__);
+ 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__);
+ return ret;
+ }
+
+ if (intr_mask >> nr_irq)
+ pr_warn("%s: mismatch in kind-of-intr param\n", __func__);
+
+ pr_info("%s: num_irq=%d, edge=0x%x\n",
+ intc->full_name, nr_irq, 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);
+
+ /* Acknowledge any pending interrupts just in case. */
+ write_fn(0xffffffff, intc_baseaddr + IAR);
+
+ /* 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);
+ }
+
+ /* 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);
+
+ irq_set_default_host(root_domain);
+
+ return 0;
+}
+
+IRQCHIP_DECLARE(xilinx_intc, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write
2016-10-31 12:16 [Patch V6 0/6] microblaze/PowerPC: Move irq-xilinx Zubair Lutfullah Kakakhel
2016-10-31 12:16 ` [Patch V6 1/6] microblaze: irqchip: Move intc driver to irqchip Zubair Lutfullah Kakakhel
@ 2016-10-31 12:16 ` Zubair Lutfullah Kakakhel
2016-10-31 19:51 ` Thomas Gleixner
2016-10-31 12:16 ` [Patch V6 3/6] irqchip: xilinx: Rename get_irq to xintc_get_irq Zubair Lutfullah Kakakhel
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Zubair Lutfullah Kakakhel @ 2016-10-31 12:16 UTC (permalink / raw)
To: monstr, tglx, jason, marc.zyngier
Cc: linux-kernel, michal.simek, linuxppc-dev, mpe, Zubair.Kakakhel
The drivers read/write function handling is a bit quirky.
And the irqmask is passed directly to the handler.
Add a new irqchip struct to pass to the handler and
cleanup read/write handling.
Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
---
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 | 110 +++++++++++++++++++++++++-------------
1 file changed, 74 insertions(+), 36 deletions(-)
diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 90bec7d..c2052fb 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -16,8 +16,6 @@
#include <linux/io.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,8 +29,16 @@
#define MER_ME (1<<0)
#define MER_HIE (1<<1)
-static unsigned int (*read_fn)(void __iomem *);
-static void (*write_fn)(u32, void __iomem *);
+struct xintc_irq_chip {
+ void __iomem *base;
+ struct irq_domain *domain;
+ struct irq_chip chip;
+ u32 intr_mask;
+ unsigned int (*read)(void __iomem *iomem);
+ void (*write)(u32 data, void __iomem *iomem);
+};
+
+static struct xintc_irq_chip *xintc_irqc;
static void intc_write32(u32 val, void __iomem *addr)
{
@@ -54,6 +60,18 @@ static unsigned int intc_read32_be(void __iomem *addr)
return ioread32be(addr);
}
+static inline unsigned int xintc_read(struct xintc_irq_chip *xintc_irqc,
+ int reg)
+{
+ return xintc_irqc->read(xintc_irqc->base + reg);
+}
+
+static inline void xintc_write(struct xintc_irq_chip *xintc_irqc,
+ int reg, u32 data)
+{
+ xintc_irqc->write(data, xintc_irqc->base + reg);
+}
+
static void intc_enable_or_unmask(struct irq_data *d)
{
unsigned long mask = 1 << d->hwirq;
@@ -65,21 +83,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(xintc_irqc, IAR, mask);
- write_fn(mask, intc_baseaddr + SIE);
+ xintc_write(xintc_irqc, SIE, mask);
}
static void intc_disable_or_mask(struct irq_data *d)
{
pr_debug("disable: %ld\n", d->hwirq);
- write_fn(1 << d->hwirq, intc_baseaddr + CIE);
+ xintc_write(xintc_irqc, CIE, 1 << d->hwirq);
}
static void intc_ack(struct irq_data *d)
{
pr_debug("ack: %ld\n", d->hwirq);
- write_fn(1 << d->hwirq, intc_baseaddr + IAR);
+ xintc_write(xintc_irqc, IAR, 1 << d->hwirq);
}
static void intc_mask_ack(struct irq_data *d)
@@ -87,8 +105,8 @@ static void intc_mask_ack(struct irq_data *d)
unsigned long mask = 1 << d->hwirq;
pr_debug("disable_and_ack: %ld\n", d->hwirq);
- write_fn(mask, intc_baseaddr + CIE);
- write_fn(mask, intc_baseaddr + IAR);
+ xintc_write(xintc_irqc, CIE, mask);
+ xintc_write(xintc_irqc, IAR, mask);
}
static struct irq_chip intc_dev = {
@@ -105,7 +123,7 @@ unsigned int get_irq(void)
{
unsigned int hwirq, irq = -1;
- hwirq = read_fn(intc_baseaddr + IVR);
+ hwirq = xintc_read(xintc_irqc, IVR);
if (hwirq != -1U)
irq = irq_find_mapping(root_domain, hwirq);
@@ -116,7 +134,8 @@ 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;
+ struct xintc_irq_chip *irqc = d->host_data;
+ u32 intr_mask = irqc->intr_mask;
if (intr_mask & (1 << hw)) {
irq_set_chip_and_handler_name(irq, &intc_dev,
@@ -138,59 +157,78 @@ static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
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;
+
+ 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;
- intc_baseaddr = of_iomap(intc, 0);
- BUG_ON(!intc_baseaddr);
+ 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("%s: unable to read xlnx,num-intr-inputs\n", __func__);
- return ret;
+ pr_err("irq-xilinx: unable to read xlnx,num-intr-inputs\n");
+ 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("%s: unable to read xlnx,kind-of-intr\n", __func__);
- return ret;
+ pr_err("irq-xilinx: unable to read xlnx,kind-of-intr\n");
+ goto err_alloc;
}
- if (intr_mask >> nr_irq)
- pr_warn("%s: mismatch in kind-of-intr param\n", __func__);
+ if (irqc->intr_mask >> nr_irq)
+ pr_warn("irq-xilinx: mismatch in kind-of-intr param\n");
pr_info("%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;
+ irqc->read = intc_read32;
+ irqc->write = intc_write32;
/*
* Disable all external interrupts until they are
* explicity requested.
*/
- write_fn(0, intc_baseaddr + IER);
+ xintc_write(irqc, IER, 0);
/* Acknowledge any pending interrupts just in case. */
- write_fn(0xffffffff, intc_baseaddr + IAR);
+ xintc_write(irqc, 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(irqc, MER, MER_HIE | MER_ME);
+ if (!(xintc_read(irqc, MER) & (MER_HIE | MER_ME))) {
+ irqc->read = intc_read32_be;
+ irqc->write = intc_write32_be;
+ xintc_write(irqc, 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);
+ if (!root_domain) {
+ pr_err("irq-xilinx: Unable to create IRQ domain\n");
+ goto err_alloc;
+ }
irq_set_default_host(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);
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write
2016-10-31 12:16 ` [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write Zubair Lutfullah Kakakhel
@ 2016-10-31 19:51 ` Thomas Gleixner
2016-11-01 11:05 ` Zubair Lutfullah Kakakhel
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Gleixner @ 2016-10-31 19:51 UTC (permalink / raw)
To: Zubair Lutfullah Kakakhel
Cc: monstr, jason, marc.zyngier, linux-kernel, michal.simek,
linuxppc-dev, mpe
On Mon, 31 Oct 2016, Zubair Lutfullah Kakakhel wrote:
> The drivers read/write function handling is a bit quirky.
Can you please explain in more detail what's quirky and why it should be
done differently,
> And the irqmask is passed directly to the handler.
I can't make any sense out of that sentence. Which handler? If you talk
about the write function, then I don't see a change. So what are you
talking about?
> Add a new irqchip struct to pass to the handler and
> cleanup read/write handling.
I still don't see what it cleans up. You move the write function pointer
into a data structure, which is exposed by another pointer. So you create
two levels of indirection in the hotpath. The function prototype is still
the same. So all this does is making things slower unless I'm missing
something.
> -static unsigned int (*read_fn)(void __iomem *);
> -static void (*write_fn)(u32, void __iomem *);
> +struct xintc_irq_chip {
> + void __iomem *base;
> + struct irq_domain *domain;
> + struct irq_chip chip;
The tabs between struct and the structure name are bogus.
> + u32 intr_mask;
> + unsigned int (*read)(void __iomem *iomem);
> + void (*write)(u32 data, void __iomem *iomem);
Please structure that like a table:
void __iomem *base;
struct irq_domain *domain;
struct irq_chip chip;
u32 intr_mask;
unsigned int (*read)(void __iomem *iomem);
void (*write)(u32 data, void __iomem *iomem);
Can you see how that makes parsing the struct simpler, because the data
types are clearly to identify?
> +static struct xintc_irq_chip *xintc_irqc;
>
> static void intc_write32(u32 val, void __iomem *addr)
> {
> @@ -54,6 +60,18 @@ static unsigned int intc_read32_be(void __iomem *addr)
> return ioread32be(addr);
> }
>
> +static inline unsigned int xintc_read(struct xintc_irq_chip *xintc_irqc,
> + int reg)
> +{
> + return xintc_irqc->read(xintc_irqc->base + reg);
> +}
> +
> +static inline void xintc_write(struct xintc_irq_chip *xintc_irqc,
> + int reg, u32 data)
> +{
> + xintc_irqc->write(data, xintc_irqc->base + reg);
> +}
> +
> static void intc_enable_or_unmask(struct irq_data *d)
> {
> unsigned long mask = 1 << d->hwirq;
> @@ -65,21 +83,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(xintc_irqc, IAR, mask);
So this whole thing makes only sense, when you want to support multiple
instances of that chip and then you need to store the xintc_irqc pointer as
irqchip data and retrieve it from there. Unless you do that, this "cleanup"
is just churn for nothing with the effect of making things less efficient.
Thanks,
tglx
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write
2016-10-31 19:51 ` Thomas Gleixner
@ 2016-11-01 11:05 ` Zubair Lutfullah Kakakhel
2016-11-07 9:53 ` Zubair Lutfullah Kakakhel
2016-11-09 15:53 ` Marc Zyngier
0 siblings, 2 replies; 12+ messages in thread
From: Zubair Lutfullah Kakakhel @ 2016-11-01 11:05 UTC (permalink / raw)
To: Thomas Gleixner
Cc: monstr, jason, marc.zyngier, linux-kernel, michal.simek,
linuxppc-dev, mpe
Hi,
Thanks for the review.
On 10/31/2016 07:51 PM, Thomas Gleixner wrote:
> On Mon, 31 Oct 2016, Zubair Lutfullah Kakakhel wrote:
>> The drivers read/write function handling is a bit quirky.
>
> Can you please explain in more detail what's quirky and why it should be
> done differently,
>
>> And the irqmask is passed directly to the handler.
>
> I can't make any sense out of that sentence. Which handler? If you talk
> about the write function, then I don't see a change. So what are you
> talking about?
Thanks. I'll add more detail in v7 if this patch survives.
>
>> Add a new irqchip struct to pass to the handler and
>> cleanup read/write handling.
>
> I still don't see what it cleans up. You move the write function pointer
> into a data structure, which is exposed by another pointer. So you create
> two levels of indirection in the hotpath. The function prototype is still
> the same. So all this does is making things slower unless I'm missing
> something.
I wrote this patch/cleanup based on a review of driver by Marc when I moved the
driver from arch/microblaze to drivers/irqchip
"Marc Zyngier
...
> arch/microblaze/kernel/intc.c | 196 ----------------------------------------
> drivers/irqchip/irq-axi-intc.c | 196 ++++++++++++++++++++++++++++++++++++++++
...
> + /* 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);
Since you're now reworking this driver, how about addressing this
ugliness? You could store the intr_mask together with intc_baseaddr,
and the read/write functions in a global structure, and pass a
pointer to it? That would make the code a bit nicer...
"
https://patchwork.kernel.org/patch/9287933/
>
>> -static unsigned int (*read_fn)(void __iomem *);
>> -static void (*write_fn)(u32, void __iomem *);
>> +struct xintc_irq_chip {
>> + void __iomem *base;
>> + struct irq_domain *domain;
>> + struct irq_chip chip;
>
> The tabs between struct and the structure name are bogus.
>
>> + u32 intr_mask;
>> + unsigned int (*read)(void __iomem *iomem);
>> + void (*write)(u32 data, void __iomem *iomem);
>
> Please structure that like a table:
>
> void __iomem *base;
> struct irq_domain *domain;
> struct irq_chip chip;
> u32 intr_mask;
> unsigned int (*read)(void __iomem *iomem);
> void (*write)(u32 data, void __iomem *iomem);
>
> Can you see how that makes parsing the struct simpler, because the data
> types are clearly to identify?
That does make it look much better.
>
>> +static struct xintc_irq_chip *xintc_irqc;
>>
>> static void intc_write32(u32 val, void __iomem *addr)
>> {
>> @@ -54,6 +60,18 @@ static unsigned int intc_read32_be(void __iomem *addr)
>> return ioread32be(addr);
>> }
>>
>> +static inline unsigned int xintc_read(struct xintc_irq_chip *xintc_irqc,
>> + int reg)
>> +{
>> + return xintc_irqc->read(xintc_irqc->base + reg);
>> +}
>> +
>> +static inline void xintc_write(struct xintc_irq_chip *xintc_irqc,
>> + int reg, u32 data)
>> +{
>> + xintc_irqc->write(data, xintc_irqc->base + reg);
>> +}
>> +
>> static void intc_enable_or_unmask(struct irq_data *d)
>> {
>> unsigned long mask = 1 << d->hwirq;
>> @@ -65,21 +83,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(xintc_irqc, IAR, mask);
>
> So this whole thing makes only sense, when you want to support multiple
> instances of that chip and then you need to store the xintc_irqc pointer as
> irqchip data and retrieve it from there. Unless you do that, this "cleanup"
> is just churn for nothing with the effect of making things less efficient.
>
Indeed the driver doesn't support multiple instances of the Xilinx Interrupt controller.
I don't have a use-case or the hardware for that.
So what would be the recommended course of action?
Regards,
ZubairLK
> Thanks,
>
> tglx
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write
2016-11-01 11:05 ` Zubair Lutfullah Kakakhel
@ 2016-11-07 9:53 ` Zubair Lutfullah Kakakhel
2016-11-09 15:53 ` Marc Zyngier
1 sibling, 0 replies; 12+ messages in thread
From: Zubair Lutfullah Kakakhel @ 2016-11-07 9:53 UTC (permalink / raw)
To: Thomas Gleixner
Cc: monstr, jason, marc.zyngier, linux-kernel, michal.simek,
linuxppc-dev, mpe
Hi,
On 11/01/2016 11:05 AM, Zubair Lutfullah Kakakhel wrote:
> Hi,
>
> Thanks for the review.
>
> On 10/31/2016 07:51 PM, Thomas Gleixner wrote:
>> On Mon, 31 Oct 2016, Zubair Lutfullah Kakakhel wrote:
>>> The drivers read/write function handling is a bit quirky.
>>
>> Can you please explain in more detail what's quirky and why it should be
>> done differently,
>>
>>> And the irqmask is passed directly to the handler.
>>
>> I can't make any sense out of that sentence. Which handler? If you talk
>> about the write function, then I don't see a change. So what are you
>> talking about?
>
> Thanks. I'll add more detail in v7 if this patch survives.
>
>>
>>> Add a new irqchip struct to pass to the handler and
>>> cleanup read/write handling.
>>
>> I still don't see what it cleans up. You move the write function pointer
>> into a data structure, which is exposed by another pointer. So you create
>> two levels of indirection in the hotpath. The function prototype is still
>> the same. So all this does is making things slower unless I'm missing
>> something.
>
> I wrote this patch/cleanup based on a review of driver by Marc when I moved the
> driver from arch/microblaze to drivers/irqchip
>
> "Marc Zyngier
>
> ...
>
>> arch/microblaze/kernel/intc.c | 196 ----------------------------------------
>> drivers/irqchip/irq-axi-intc.c | 196 ++++++++++++++++++++++++++++++++++++++++
>
> ...
>
>> + /* 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);
>
> Since you're now reworking this driver, how about addressing this
> ugliness? You could store the intr_mask together with intc_baseaddr,
> and the read/write functions in a global structure, and pass a
> pointer to it? That would make the code a bit nicer...
> "
>
> https://patchwork.kernel.org/patch/9287933/
>
>>
>>> -static unsigned int (*read_fn)(void __iomem *);
>>> -static void (*write_fn)(u32, void __iomem *);
>>> +struct xintc_irq_chip {
>>> + void __iomem *base;
>>> + struct irq_domain *domain;
>>> + struct irq_chip chip;
>>
>> The tabs between struct and the structure name are bogus.
>>
>>> + u32 intr_mask;
>>> + unsigned int (*read)(void __iomem *iomem);
>>> + void (*write)(u32 data, void __iomem *iomem);
>>
>> Please structure that like a table:
>>
>> void __iomem *base;
>> struct irq_domain *domain;
>> struct irq_chip chip;
>> u32 intr_mask;
>> unsigned int (*read)(void __iomem *iomem);
>> void (*write)(u32 data, void __iomem *iomem);
>>
>> Can you see how that makes parsing the struct simpler, because the data
>> types are clearly to identify?
>
> That does make it look much better.
>
>>
>>> +static struct xintc_irq_chip *xintc_irqc;
>>>
>>> static void intc_write32(u32 val, void __iomem *addr)
>>> {
>>> @@ -54,6 +60,18 @@ static unsigned int intc_read32_be(void __iomem *addr)
>>> return ioread32be(addr);
>>> }
>>>
>>> +static inline unsigned int xintc_read(struct xintc_irq_chip *xintc_irqc,
>>> + int reg)
>>> +{
>>> + return xintc_irqc->read(xintc_irqc->base + reg);
>>> +}
>>> +
>>> +static inline void xintc_write(struct xintc_irq_chip *xintc_irqc,
>>> + int reg, u32 data)
>>> +{
>>> + xintc_irqc->write(data, xintc_irqc->base + reg);
>>> +}
>>> +
>>> static void intc_enable_or_unmask(struct irq_data *d)
>>> {
>>> unsigned long mask = 1 << d->hwirq;
>>> @@ -65,21 +83,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(xintc_irqc, IAR, mask);
>>
>> So this whole thing makes only sense, when you want to support multiple
>> instances of that chip and then you need to store the xintc_irqc pointer as
>> irqchip data and retrieve it from there. Unless you do that, this "cleanup"
>> is just churn for nothing with the effect of making things less efficient.
>>
>
> Indeed the driver doesn't support multiple instances of the Xilinx Interrupt controller.
> I don't have a use-case or the hardware for that.
>
> So what would be the recommended course of action?
I'd really appreciate a consensus here.
I'm hoping that these patches can go in v4.10.
Thanks
ZubairLK
>
> Regards,
> ZubairLK
>
>> Thanks,
>>
>> tglx
>>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write
2016-11-01 11:05 ` Zubair Lutfullah Kakakhel
2016-11-07 9:53 ` Zubair Lutfullah Kakakhel
@ 2016-11-09 15:53 ` Marc Zyngier
2016-11-15 11:06 ` Michal Simek
1 sibling, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2016-11-09 15:53 UTC (permalink / raw)
To: Zubair Lutfullah Kakakhel, Thomas Gleixner
Cc: monstr, jason, linux-kernel, michal.simek, linuxppc-dev, mpe
On 01/11/16 11:05, Zubair Lutfullah Kakakhel wrote:
> Hi,
>
> Thanks for the review.
>
> On 10/31/2016 07:51 PM, Thomas Gleixner wrote:
>> On Mon, 31 Oct 2016, Zubair Lutfullah Kakakhel wrote:
>>> The drivers read/write function handling is a bit quirky.
>>
>> Can you please explain in more detail what's quirky and why it should be
>> done differently,
>>
>>> And the irqmask is passed directly to the handler.
>>
>> I can't make any sense out of that sentence. Which handler? If you talk
>> about the write function, then I don't see a change. So what are you
>> talking about?
>
> Thanks. I'll add more detail in v7 if this patch survives.
>
>>
>>> Add a new irqchip struct to pass to the handler and
>>> cleanup read/write handling.
>>
>> I still don't see what it cleans up. You move the write function pointer
>> into a data structure, which is exposed by another pointer. So you create
>> two levels of indirection in the hotpath. The function prototype is still
>> the same. So all this does is making things slower unless I'm missing
>> something.
>
> I wrote this patch/cleanup based on a review of driver by Marc when I moved the
> driver from arch/microblaze to drivers/irqchip
>
> "Marc Zyngier
>
> ...
>
> > arch/microblaze/kernel/intc.c | 196 ----------------------------------------
> > drivers/irqchip/irq-axi-intc.c | 196 ++++++++++++++++++++++++++++++++++++++++
>
> ...
>
> > + /* 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);
>
> Since you're now reworking this driver, how about addressing this
> ugliness? You could store the intr_mask together with intc_baseaddr,
> and the read/write functions in a global structure, and pass a
> pointer to it? That would make the code a bit nicer...
> "
>
> https://patchwork.kernel.org/patch/9287933/
>
>>
>>> -static unsigned int (*read_fn)(void __iomem *);
>>> -static void (*write_fn)(u32, void __iomem *);
>>> +struct xintc_irq_chip {
>>> + void __iomem *base;
>>> + struct irq_domain *domain;
>>> + struct irq_chip chip;
>>
>> The tabs between struct and the structure name are bogus.
>>
>>> + u32 intr_mask;
>>> + unsigned int (*read)(void __iomem *iomem);
>>> + void (*write)(u32 data, void __iomem *iomem);
>>
>> Please structure that like a table:
>>
>> void __iomem *base;
>> struct irq_domain *domain;
>> struct irq_chip chip;
>> u32 intr_mask;
>> unsigned int (*read)(void __iomem *iomem);
>> void (*write)(u32 data, void __iomem *iomem);
>>
>> Can you see how that makes parsing the struct simpler, because the data
>> types are clearly to identify?
>
> That does make it look much better.
>
>>
>>> +static struct xintc_irq_chip *xintc_irqc;
>>>
>>> static void intc_write32(u32 val, void __iomem *addr)
>>> {
>>> @@ -54,6 +60,18 @@ static unsigned int intc_read32_be(void __iomem *addr)
>>> return ioread32be(addr);
>>> }
>>>
>>> +static inline unsigned int xintc_read(struct xintc_irq_chip *xintc_irqc,
>>> + int reg)
>>> +{
>>> + return xintc_irqc->read(xintc_irqc->base + reg);
>>> +}
>>> +
>>> +static inline void xintc_write(struct xintc_irq_chip *xintc_irqc,
>>> + int reg, u32 data)
>>> +{
>>> + xintc_irqc->write(data, xintc_irqc->base + reg);
>>> +}
>>> +
>>> static void intc_enable_or_unmask(struct irq_data *d)
>>> {
>>> unsigned long mask = 1 << d->hwirq;
>>> @@ -65,21 +83,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(xintc_irqc, IAR, mask);
>>
>> So this whole thing makes only sense, when you want to support multiple
>> instances of that chip and then you need to store the xintc_irqc pointer as
>> irqchip data and retrieve it from there. Unless you do that, this "cleanup"
>> is just churn for nothing with the effect of making things less efficient.
>>
>
> Indeed the driver doesn't support multiple instances of the Xilinx Interrupt controller.
> I don't have a use-case or the hardware for that.
>
> So what would be the recommended course of action?
If you really don't want/need to support multi-instance, then this is
indeed overkill. You're then better off having a simple static key that
deals with the endianess of the peripheral, and have a global structure
that contains the relevant data:
static DEFINE_STATIC_KEY_FALSE(xintc_is_be);
static void xintc_write(int reg, u32 data)
{
if (static_branch_unlikely(&xintc_is_be))
iowrite32be(data, xint_irqc.base + reg);
else
iowrite32(data, xint_irqc.base + reg);
}
and something similar for the write accessor.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write
2016-11-09 15:53 ` Marc Zyngier
@ 2016-11-15 11:06 ` Michal Simek
0 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2016-11-15 11:06 UTC (permalink / raw)
To: Marc Zyngier, Zubair Lutfullah Kakakhel, Thomas Gleixner
Cc: jason, linux-kernel, michal.simek, linuxppc-dev, mpe
On 9.11.2016 16:53, Marc Zyngier wrote:
> On 01/11/16 11:05, Zubair Lutfullah Kakakhel wrote:
>> Hi,
>>
>> Thanks for the review.
>>
>> On 10/31/2016 07:51 PM, Thomas Gleixner wrote:
>>> On Mon, 31 Oct 2016, Zubair Lutfullah Kakakhel wrote:
>>>> The drivers read/write function handling is a bit quirky.
>>>
>>> Can you please explain in more detail what's quirky and why it should be
>>> done differently,
>>>
>>>> And the irqmask is passed directly to the handler.
>>>
>>> I can't make any sense out of that sentence. Which handler? If you talk
>>> about the write function, then I don't see a change. So what are you
>>> talking about?
>>
>> Thanks. I'll add more detail in v7 if this patch survives.
>>
>>>
>>>> Add a new irqchip struct to pass to the handler and
>>>> cleanup read/write handling.
>>>
>>> I still don't see what it cleans up. You move the write function pointer
>>> into a data structure, which is exposed by another pointer. So you create
>>> two levels of indirection in the hotpath. The function prototype is still
>>> the same. So all this does is making things slower unless I'm missing
>>> something.
>>
>> I wrote this patch/cleanup based on a review of driver by Marc when I moved the
>> driver from arch/microblaze to drivers/irqchip
>>
>> "Marc Zyngier
>>
>> ...
>>
>> > arch/microblaze/kernel/intc.c | 196 ----------------------------------------
>> > drivers/irqchip/irq-axi-intc.c | 196 ++++++++++++++++++++++++++++++++++++++++
>>
>> ...
>>
>> > + /* 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);
>>
>> Since you're now reworking this driver, how about addressing this
>> ugliness? You could store the intr_mask together with intc_baseaddr,
>> and the read/write functions in a global structure, and pass a
>> pointer to it? That would make the code a bit nicer...
>> "
>>
>> https://patchwork.kernel.org/patch/9287933/
>>
>>>
>>>> -static unsigned int (*read_fn)(void __iomem *);
>>>> -static void (*write_fn)(u32, void __iomem *);
>>>> +struct xintc_irq_chip {
>>>> + void __iomem *base;
>>>> + struct irq_domain *domain;
>>>> + struct irq_chip chip;
>>>
>>> The tabs between struct and the structure name are bogus.
>>>
>>>> + u32 intr_mask;
>>>> + unsigned int (*read)(void __iomem *iomem);
>>>> + void (*write)(u32 data, void __iomem *iomem);
>>>
>>> Please structure that like a table:
>>>
>>> void __iomem *base;
>>> struct irq_domain *domain;
>>> struct irq_chip chip;
>>> u32 intr_mask;
>>> unsigned int (*read)(void __iomem *iomem);
>>> void (*write)(u32 data, void __iomem *iomem);
>>>
>>> Can you see how that makes parsing the struct simpler, because the data
>>> types are clearly to identify?
>>
>> That does make it look much better.
>>
>>>
>>>> +static struct xintc_irq_chip *xintc_irqc;
>>>>
>>>> static void intc_write32(u32 val, void __iomem *addr)
>>>> {
>>>> @@ -54,6 +60,18 @@ static unsigned int intc_read32_be(void __iomem *addr)
>>>> return ioread32be(addr);
>>>> }
>>>>
>>>> +static inline unsigned int xintc_read(struct xintc_irq_chip *xintc_irqc,
>>>> + int reg)
>>>> +{
>>>> + return xintc_irqc->read(xintc_irqc->base + reg);
>>>> +}
>>>> +
>>>> +static inline void xintc_write(struct xintc_irq_chip *xintc_irqc,
>>>> + int reg, u32 data)
>>>> +{
>>>> + xintc_irqc->write(data, xintc_irqc->base + reg);
>>>> +}
>>>> +
>>>> static void intc_enable_or_unmask(struct irq_data *d)
>>>> {
>>>> unsigned long mask = 1 << d->hwirq;
>>>> @@ -65,21 +83,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(xintc_irqc, IAR, mask);
>>>
>>> So this whole thing makes only sense, when you want to support multiple
>>> instances of that chip and then you need to store the xintc_irqc pointer as
>>> irqchip data and retrieve it from there. Unless you do that, this "cleanup"
>>> is just churn for nothing with the effect of making things less efficient.
>>>
>>
>> Indeed the driver doesn't support multiple instances of the Xilinx Interrupt controller.
>> I don't have a use-case or the hardware for that.
>>
>> So what would be the recommended course of action?
>
> If you really don't want/need to support multi-instance, then this is
> indeed overkill. You're then better off having a simple static key that
> deals with the endianess of the peripheral, and have a global structure
> that contains the relevant data:
By design this is PL IP and multi-instance support should be there.
You are using it with MIPS but you can connect more INTC together.
Thanks,
Michal
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Patch V6 3/6] irqchip: xilinx: Rename get_irq to xintc_get_irq
2016-10-31 12:16 [Patch V6 0/6] microblaze/PowerPC: Move irq-xilinx Zubair Lutfullah Kakakhel
2016-10-31 12:16 ` [Patch V6 1/6] microblaze: irqchip: Move intc driver to irqchip Zubair Lutfullah Kakakhel
2016-10-31 12:16 ` [Patch V6 2/6] irqchip: xilinx: Clean up irqdomain argument and read/write Zubair Lutfullah Kakakhel
@ 2016-10-31 12:16 ` Zubair Lutfullah Kakakhel
2016-10-31 12:16 ` [Patch V6 4/6] irqchip: xilinx: Add support for parent intc Zubair Lutfullah Kakakhel
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Zubair Lutfullah Kakakhel @ 2016-10-31 12:16 UTC (permalink / raw)
To: monstr, tglx, jason, marc.zyngier
Cc: linux-kernel, michal.simek, linuxppc-dev, mpe, Zubair.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>
---
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 | 4 ++--
3 files changed, 5 insertions(+), 5 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 @@
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 c2052fb..1f37089 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -119,7 +119,7 @@ static void intc_mask_ack(struct irq_data *d)
static struct irq_domain *root_domain;
-unsigned int get_irq(void)
+unsigned int xintc_get_irq(void)
{
unsigned int hwirq, irq = -1;
@@ -127,7 +127,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;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Patch V6 4/6] irqchip: xilinx: Add support for parent intc
2016-10-31 12:16 [Patch V6 0/6] microblaze/PowerPC: Move irq-xilinx Zubair Lutfullah Kakakhel
` (2 preceding siblings ...)
2016-10-31 12:16 ` [Patch V6 3/6] irqchip: xilinx: Rename get_irq to xintc_get_irq Zubair Lutfullah Kakakhel
@ 2016-10-31 12:16 ` Zubair Lutfullah Kakakhel
2016-10-31 12:16 ` [Patch V6 5/6] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided Zubair Lutfullah Kakakhel
2016-10-31 12:16 ` [Patch V6 6/6] powerpc/virtex: Use generic xilinx irqchip driver Zubair Lutfullah Kakakhel
5 siblings, 0 replies; 12+ messages in thread
From: Zubair Lutfullah Kakakhel @ 2016-10-31 12:16 UTC (permalink / raw)
To: monstr, tglx, jason, marc.zyngier
Cc: linux-kernel, michal.simek, linuxppc-dev, mpe, Zubair.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>
---
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 | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 1f37089..ea15446 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -12,9 +12,11 @@
#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/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 */
@@ -154,11 +156,26 @@ static int xintc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
.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) {
@@ -220,7 +237,22 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
goto err_alloc;
}
- irq_set_default_host(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(root_domain);
+ }
return 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Patch V6 5/6] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided
2016-10-31 12:16 [Patch V6 0/6] microblaze/PowerPC: Move irq-xilinx Zubair Lutfullah Kakakhel
` (3 preceding siblings ...)
2016-10-31 12:16 ` [Patch V6 4/6] irqchip: xilinx: Add support for parent intc Zubair Lutfullah Kakakhel
@ 2016-10-31 12:16 ` Zubair Lutfullah Kakakhel
2016-10-31 12:16 ` [Patch V6 6/6] powerpc/virtex: Use generic xilinx irqchip driver Zubair Lutfullah Kakakhel
5 siblings, 0 replies; 12+ messages in thread
From: Zubair Lutfullah Kakakhel @ 2016-10-31 12:16 UTC (permalink / raw)
To: monstr, tglx, jason, marc.zyngier
Cc: linux-kernel, michal.simek, linuxppc-dev, mpe, Zubair.Kakakhel
The powerpc dts file upstream 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>
---
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 ea15446..954e9b4 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -200,8 +200,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)
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Patch V6 6/6] powerpc/virtex: Use generic xilinx irqchip driver
2016-10-31 12:16 [Patch V6 0/6] microblaze/PowerPC: Move irq-xilinx Zubair Lutfullah Kakakhel
` (4 preceding siblings ...)
2016-10-31 12:16 ` [Patch V6 5/6] irqchip: xilinx: Try to fall back if xlnx,kind-of-intr not provided Zubair Lutfullah Kakakhel
@ 2016-10-31 12:16 ` Zubair Lutfullah Kakakhel
5 siblings, 0 replies; 12+ messages in thread
From: Zubair Lutfullah Kakakhel @ 2016-10-31 12:16 UTC (permalink / raw)
To: monstr, tglx, jason, marc.zyngier
Cc: linux-kernel, michal.simek, linuxppc-dev, mpe, Zubair.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)
---
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 @@ static int __init virtex_probe(void)
.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 @@ static int __init virtex_probe(void)
.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 954e9b4..4c994c9 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -263,4 +263,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);
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread