This moves the Versatile FPGA interrupt controller driver, used in
the Integrator/AP, Integrator/CP and some Versatile boards, out
of arch/arm/plat-versatile and down to drivers/irqchip where we
have consensus that such drivers belong. The header file is
consequently moved to <linux/platform_data/irq-versatile-fpga.h>.
Signed-off-by: Linus Walleij <[email protected]>
---
ChangeLog v1->v2:
- Moved include to <linux/irqchip/versatile-fpga.h>
- Deleted merge/moval artifact
---
arch/arm/Kconfig | 4 +-
arch/arm/mach-integrator/integrator_ap.c | 3 +-
arch/arm/mach-integrator/integrator_cp.c | 2 +-
arch/arm/mach-versatile/core.c | 2 +-
arch/arm/plat-versatile/Kconfig | 9 --
arch/arm/plat-versatile/Makefile | 1 -
arch/arm/plat-versatile/fpga-irq.c | 204 ------------------------
arch/arm/plat-versatile/include/plat/fpga-irq.h | 13 --
drivers/irqchip/Kconfig | 9 +-
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-versatile-fpga.c | 204 ++++++++++++++++++++++++
include/linux/irqchip/versatile-fpga.h | 13 ++
12 files changed, 231 insertions(+), 234 deletions(-)
delete mode 100644 arch/arm/plat-versatile/fpga-irq.c
delete mode 100644 arch/arm/plat-versatile/include/plat/fpga-irq.h
create mode 100644 drivers/irqchip/irq-versatile-fpga.c
create mode 100644 include/linux/irqchip/versatile-fpga.h
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 73067ef..2205e3e 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -284,8 +284,8 @@ config ARCH_INTEGRATOR
select MULTI_IRQ_HANDLER
select NEED_MACH_MEMORY_H
select PLAT_VERSATILE
- select PLAT_VERSATILE_FPGA_IRQ
select SPARSE_IRQ
+ select VERSATILE_FPGA_IRQ
help
Support for ARM's Integrator platform.
@@ -318,7 +318,7 @@ config ARCH_VERSATILE
select PLAT_VERSATILE
select PLAT_VERSATILE_CLCD
select PLAT_VERSATILE_CLOCK
- select PLAT_VERSATILE_FPGA_IRQ
+ select VERSATILE_FPGA_IRQ
help
This enables support for ARM Ltd Versatile board.
diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c
index 4f13bc5..e67a9fe 100644
--- a/arch/arm/mach-integrator/integrator_ap.c
+++ b/arch/arm/mach-integrator/integrator_ap.c
@@ -31,6 +31,7 @@
#include <linux/clockchips.h>
#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/irqchip/versatile-fpga.h>
#include <linux/mtd/physmap.h>
#include <linux/clk.h>
#include <linux/platform_data/clk-integrator.h>
@@ -56,8 +57,6 @@
#include <asm/mach/pci.h>
#include <asm/mach/time.h>
-#include <plat/fpga-irq.h>
-
#include "common.h"
/*
diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c
index 4423bc8..acecf04 100644
--- a/arch/arm/mach-integrator/integrator_cp.c
+++ b/arch/arm/mach-integrator/integrator_cp.c
@@ -20,6 +20,7 @@
#include <linux/amba/clcd.h>
#include <linux/amba/mmci.h>
#include <linux/io.h>
+#include <linux/irqchip/versatile-fpga.h>
#include <linux/gfp.h>
#include <linux/mtd/physmap.h>
#include <linux/platform_data/clk-integrator.h>
@@ -46,7 +47,6 @@
#include <asm/hardware/timer-sp.h>
#include <plat/clcd.h>
-#include <plat/fpga-irq.h>
#include <plat/sched_clock.h>
#include "common.h"
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index 5b5c1ee..5d59294 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -32,6 +32,7 @@
#include <linux/amba/mmci.h>
#include <linux/amba/pl022.h>
#include <linux/io.h>
+#include <linux/irqchip/versatile-fpga.h>
#include <linux/gfp.h>
#include <linux/clkdev.h>
#include <linux/mtd/physmap.h>
@@ -51,7 +52,6 @@
#include <asm/hardware/timer-sp.h>
#include <plat/clcd.h>
-#include <plat/fpga-irq.h>
#include <plat/sched_clock.h>
#include "core.h"
diff --git a/arch/arm/plat-versatile/Kconfig b/arch/arm/plat-versatile/Kconfig
index 2a4ae8a..619f0fa 100644
--- a/arch/arm/plat-versatile/Kconfig
+++ b/arch/arm/plat-versatile/Kconfig
@@ -6,15 +6,6 @@ config PLAT_VERSATILE_CLOCK
config PLAT_VERSATILE_CLCD
bool
-config PLAT_VERSATILE_FPGA_IRQ
- bool
- select IRQ_DOMAIN
-
-config PLAT_VERSATILE_FPGA_IRQ_NR
- int
- default 4
- depends on PLAT_VERSATILE_FPGA_IRQ
-
config PLAT_VERSATILE_LEDS
def_bool y if NEW_LEDS
depends on ARCH_REALVIEW || ARCH_VERSATILE
diff --git a/arch/arm/plat-versatile/Makefile b/arch/arm/plat-versatile/Makefile
index 74cfd94..f88d448 100644
--- a/arch/arm/plat-versatile/Makefile
+++ b/arch/arm/plat-versatile/Makefile
@@ -2,7 +2,6 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include
obj-$(CONFIG_PLAT_VERSATILE_CLOCK) += clock.o
obj-$(CONFIG_PLAT_VERSATILE_CLCD) += clcd.o
-obj-$(CONFIG_PLAT_VERSATILE_FPGA_IRQ) += fpga-irq.o
obj-$(CONFIG_PLAT_VERSATILE_LEDS) += leds.o
obj-$(CONFIG_PLAT_VERSATILE_SCHED_CLOCK) += sched-clock.o
obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/plat-versatile/fpga-irq.c b/arch/arm/plat-versatile/fpga-irq.c
deleted file mode 100644
index dfe317c..0000000
--- a/arch/arm/plat-versatile/fpga-irq.c
+++ /dev/null
@@ -1,204 +0,0 @@
-/*
- * Support for Versatile FPGA-based IRQ controllers
- */
-#include <linux/bitops.h>
-#include <linux/irq.h>
-#include <linux/io.h>
-#include <linux/irqdomain.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-
-#include <asm/exception.h>
-#include <asm/mach/irq.h>
-#include <plat/fpga-irq.h>
-
-#define IRQ_STATUS 0x00
-#define IRQ_RAW_STATUS 0x04
-#define IRQ_ENABLE_SET 0x08
-#define IRQ_ENABLE_CLEAR 0x0c
-#define INT_SOFT_SET 0x10
-#define INT_SOFT_CLEAR 0x14
-#define FIQ_STATUS 0x20
-#define FIQ_RAW_STATUS 0x24
-#define FIQ_ENABLE 0x28
-#define FIQ_ENABLE_SET 0x28
-#define FIQ_ENABLE_CLEAR 0x2C
-
-/**
- * struct fpga_irq_data - irq data container for the FPGA IRQ controller
- * @base: memory offset in virtual memory
- * @chip: chip container for this instance
- * @domain: IRQ domain for this instance
- * @valid: mask for valid IRQs on this controller
- * @used_irqs: number of active IRQs on this controller
- */
-struct fpga_irq_data {
- void __iomem *base;
- struct irq_chip chip;
- u32 valid;
- struct irq_domain *domain;
- u8 used_irqs;
-};
-
-/* we cannot allocate memory when the controllers are initially registered */
-static struct fpga_irq_data fpga_irq_devices[CONFIG_PLAT_VERSATILE_FPGA_IRQ_NR];
-static int fpga_irq_id;
-
-static void fpga_irq_mask(struct irq_data *d)
-{
- struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << d->hwirq;
-
- writel(mask, f->base + IRQ_ENABLE_CLEAR);
-}
-
-static void fpga_irq_unmask(struct irq_data *d)
-{
- struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
- u32 mask = 1 << d->hwirq;
-
- writel(mask, f->base + IRQ_ENABLE_SET);
-}
-
-static void fpga_irq_handle(unsigned int irq, struct irq_desc *desc)
-{
- struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
- u32 status = readl(f->base + IRQ_STATUS);
-
- if (status == 0) {
- do_bad_IRQ(irq, desc);
- return;
- }
-
- do {
- irq = ffs(status) - 1;
- status &= ~(1 << irq);
- generic_handle_irq(irq_find_mapping(f->domain, irq));
- } while (status);
-}
-
-/*
- * Handle each interrupt in a single FPGA IRQ controller. Returns non-zero
- * if we've handled at least one interrupt. This does a single read of the
- * status register and handles all interrupts in order from LSB first.
- */
-static int handle_one_fpga(struct fpga_irq_data *f, struct pt_regs *regs)
-{
- int handled = 0;
- int irq;
- u32 status;
-
- while ((status = readl(f->base + IRQ_STATUS))) {
- irq = ffs(status) - 1;
- handle_IRQ(irq_find_mapping(f->domain, irq), regs);
- handled = 1;
- }
-
- return handled;
-}
-
-/*
- * Keep iterating over all registered FPGA IRQ controllers until there are
- * no pending interrupts.
- */
-asmlinkage void __exception_irq_entry fpga_handle_irq(struct pt_regs *regs)
-{
- int i, handled;
-
- do {
- for (i = 0, handled = 0; i < fpga_irq_id; ++i)
- handled |= handle_one_fpga(&fpga_irq_devices[i], regs);
- } while (handled);
-}
-
-static int fpga_irqdomain_map(struct irq_domain *d, unsigned int irq,
- irq_hw_number_t hwirq)
-{
- struct fpga_irq_data *f = d->host_data;
-
- /* Skip invalid IRQs, only register handlers for the real ones */
- if (!(f->valid & BIT(hwirq)))
- return -ENOTSUPP;
- irq_set_chip_data(irq, f);
- irq_set_chip_and_handler(irq, &f->chip,
- handle_level_irq);
- set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
- return 0;
-}
-
-static struct irq_domain_ops fpga_irqdomain_ops = {
- .map = fpga_irqdomain_map,
- .xlate = irq_domain_xlate_onetwocell,
-};
-
-void __init fpga_irq_init(void __iomem *base, const char *name, int irq_start,
- int parent_irq, u32 valid, struct device_node *node)
-{
- struct fpga_irq_data *f;
- int i;
-
- if (fpga_irq_id >= ARRAY_SIZE(fpga_irq_devices)) {
- pr_err("%s: too few FPGA IRQ controllers, increase CONFIG_PLAT_VERSATILE_FPGA_IRQ_NR\n", __func__);
- return;
- }
- f = &fpga_irq_devices[fpga_irq_id];
- f->base = base;
- f->chip.name = name;
- f->chip.irq_ack = fpga_irq_mask;
- f->chip.irq_mask = fpga_irq_mask;
- f->chip.irq_unmask = fpga_irq_unmask;
- f->valid = valid;
-
- if (parent_irq != -1) {
- irq_set_handler_data(parent_irq, f);
- irq_set_chained_handler(parent_irq, fpga_irq_handle);
- }
-
- /* This will also allocate irq descriptors */
- f->domain = irq_domain_add_simple(node, fls(valid), irq_start,
- &fpga_irqdomain_ops, f);
-
- /* This will allocate all valid descriptors in the linear case */
- for (i = 0; i < fls(valid); i++)
- if (valid & BIT(i)) {
- if (!irq_start)
- irq_create_mapping(f->domain, i);
- f->used_irqs++;
- }
-
- pr_info("FPGA IRQ chip %d \"%s\" @ %p, %u irqs\n",
- fpga_irq_id, name, base, f->used_irqs);
-
- fpga_irq_id++;
-}
-
-#ifdef CONFIG_OF
-int __init fpga_irq_of_init(struct device_node *node,
- struct device_node *parent)
-{
- struct fpga_irq_data *f;
- void __iomem *base;
- u32 clear_mask;
- u32 valid_mask;
-
- if (WARN_ON(!node))
- return -ENODEV;
-
- base = of_iomap(node, 0);
- WARN(!base, "unable to map fpga irq registers\n");
-
- if (of_property_read_u32(node, "clear-mask", &clear_mask))
- clear_mask = 0;
-
- if (of_property_read_u32(node, "valid-mask", &valid_mask))
- valid_mask = 0;
-
- fpga_irq_init(base, node->name, 0, -1, valid_mask, node);
-
- writel(clear_mask, base + IRQ_ENABLE_CLEAR);
- writel(clear_mask, base + FIQ_ENABLE_CLEAR);
-
- return 0;
-}
-#endif
diff --git a/arch/arm/plat-versatile/include/plat/fpga-irq.h b/arch/arm/plat-versatile/include/plat/fpga-irq.h
deleted file mode 100644
index 1fac965..0000000
--- a/arch/arm/plat-versatile/include/plat/fpga-irq.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef PLAT_FPGA_IRQ_H
-#define PLAT_FPGA_IRQ_H
-
-struct device_node;
-struct pt_regs;
-
-void fpga_handle_irq(struct pt_regs *regs);
-void fpga_irq_init(void __iomem *, const char *, int, int, u32,
- struct device_node *node);
-int fpga_irq_of_init(struct device_node *node,
- struct device_node *parent);
-
-#endif
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 1bb8bf6..62ca575 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -1 +1,8 @@
-# empty
+config VERSATILE_FPGA_IRQ
+ bool
+ select IRQ_DOMAIN
+
+config VERSATILE_FPGA_IRQ_NR
+ int
+ default 4
+ depends on VERSATILE_FPGA_IRQ
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 054321d..e2e6eb5 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
+obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
diff --git a/drivers/irqchip/irq-versatile-fpga.c b/drivers/irqchip/irq-versatile-fpga.c
new file mode 100644
index 0000000..789b3e5
--- /dev/null
+++ b/drivers/irqchip/irq-versatile-fpga.c
@@ -0,0 +1,204 @@
+/*
+ * Support for Versatile FPGA-based IRQ controllers
+ */
+#include <linux/bitops.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/irqchip/versatile-fpga.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include <asm/exception.h>
+#include <asm/mach/irq.h>
+
+#define IRQ_STATUS 0x00
+#define IRQ_RAW_STATUS 0x04
+#define IRQ_ENABLE_SET 0x08
+#define IRQ_ENABLE_CLEAR 0x0c
+#define INT_SOFT_SET 0x10
+#define INT_SOFT_CLEAR 0x14
+#define FIQ_STATUS 0x20
+#define FIQ_RAW_STATUS 0x24
+#define FIQ_ENABLE 0x28
+#define FIQ_ENABLE_SET 0x28
+#define FIQ_ENABLE_CLEAR 0x2C
+
+/**
+ * struct fpga_irq_data - irq data container for the FPGA IRQ controller
+ * @base: memory offset in virtual memory
+ * @chip: chip container for this instance
+ * @domain: IRQ domain for this instance
+ * @valid: mask for valid IRQs on this controller
+ * @used_irqs: number of active IRQs on this controller
+ */
+struct fpga_irq_data {
+ void __iomem *base;
+ struct irq_chip chip;
+ u32 valid;
+ struct irq_domain *domain;
+ u8 used_irqs;
+};
+
+/* we cannot allocate memory when the controllers are initially registered */
+static struct fpga_irq_data fpga_irq_devices[CONFIG_VERSATILE_FPGA_IRQ_NR];
+static int fpga_irq_id;
+
+static void fpga_irq_mask(struct irq_data *d)
+{
+ struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
+ u32 mask = 1 << d->hwirq;
+
+ writel(mask, f->base + IRQ_ENABLE_CLEAR);
+}
+
+static void fpga_irq_unmask(struct irq_data *d)
+{
+ struct fpga_irq_data *f = irq_data_get_irq_chip_data(d);
+ u32 mask = 1 << d->hwirq;
+
+ writel(mask, f->base + IRQ_ENABLE_SET);
+}
+
+static void fpga_irq_handle(unsigned int irq, struct irq_desc *desc)
+{
+ struct fpga_irq_data *f = irq_desc_get_handler_data(desc);
+ u32 status = readl(f->base + IRQ_STATUS);
+
+ if (status == 0) {
+ do_bad_IRQ(irq, desc);
+ return;
+ }
+
+ do {
+ irq = ffs(status) - 1;
+ status &= ~(1 << irq);
+ generic_handle_irq(irq_find_mapping(f->domain, irq));
+ } while (status);
+}
+
+/*
+ * Handle each interrupt in a single FPGA IRQ controller. Returns non-zero
+ * if we've handled at least one interrupt. This does a single read of the
+ * status register and handles all interrupts in order from LSB first.
+ */
+static int handle_one_fpga(struct fpga_irq_data *f, struct pt_regs *regs)
+{
+ int handled = 0;
+ int irq;
+ u32 status;
+
+ while ((status = readl(f->base + IRQ_STATUS))) {
+ irq = ffs(status) - 1;
+ handle_IRQ(irq_find_mapping(f->domain, irq), regs);
+ handled = 1;
+ }
+
+ return handled;
+}
+
+/*
+ * Keep iterating over all registered FPGA IRQ controllers until there are
+ * no pending interrupts.
+ */
+asmlinkage void __exception_irq_entry fpga_handle_irq(struct pt_regs *regs)
+{
+ int i, handled;
+
+ do {
+ for (i = 0, handled = 0; i < fpga_irq_id; ++i)
+ handled |= handle_one_fpga(&fpga_irq_devices[i], regs);
+ } while (handled);
+}
+
+static int fpga_irqdomain_map(struct irq_domain *d, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ struct fpga_irq_data *f = d->host_data;
+
+ /* Skip invalid IRQs, only register handlers for the real ones */
+ if (!(f->valid & BIT(hwirq)))
+ return -ENOTSUPP;
+ irq_set_chip_data(irq, f);
+ irq_set_chip_and_handler(irq, &f->chip,
+ handle_level_irq);
+ set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
+ return 0;
+}
+
+static struct irq_domain_ops fpga_irqdomain_ops = {
+ .map = fpga_irqdomain_map,
+ .xlate = irq_domain_xlate_onetwocell,
+};
+
+void __init fpga_irq_init(void __iomem *base, const char *name, int irq_start,
+ int parent_irq, u32 valid, struct device_node *node)
+{
+ struct fpga_irq_data *f;
+ int i;
+
+ if (fpga_irq_id >= ARRAY_SIZE(fpga_irq_devices)) {
+ pr_err("%s: too few FPGA IRQ controllers, increase CONFIG_PLAT_VERSATILE_FPGA_IRQ_NR\n", __func__);
+ return;
+ }
+ f = &fpga_irq_devices[fpga_irq_id];
+ f->base = base;
+ f->chip.name = name;
+ f->chip.irq_ack = fpga_irq_mask;
+ f->chip.irq_mask = fpga_irq_mask;
+ f->chip.irq_unmask = fpga_irq_unmask;
+ f->valid = valid;
+
+ if (parent_irq != -1) {
+ irq_set_handler_data(parent_irq, f);
+ irq_set_chained_handler(parent_irq, fpga_irq_handle);
+ }
+
+ /* This will also allocate irq descriptors */
+ f->domain = irq_domain_add_simple(node, fls(valid), irq_start,
+ &fpga_irqdomain_ops, f);
+
+ /* This will allocate all valid descriptors in the linear case */
+ for (i = 0; i < fls(valid); i++)
+ if (valid & BIT(i)) {
+ if (!irq_start)
+ irq_create_mapping(f->domain, i);
+ f->used_irqs++;
+ }
+
+ pr_info("FPGA IRQ chip %d \"%s\" @ %p, %u irqs\n",
+ fpga_irq_id, name, base, f->used_irqs);
+
+ fpga_irq_id++;
+}
+
+#ifdef CONFIG_OF
+int __init fpga_irq_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ struct fpga_irq_data *f;
+ void __iomem *base;
+ u32 clear_mask;
+ u32 valid_mask;
+
+ if (WARN_ON(!node))
+ return -ENODEV;
+
+ base = of_iomap(node, 0);
+ WARN(!base, "unable to map fpga irq registers\n");
+
+ if (of_property_read_u32(node, "clear-mask", &clear_mask))
+ clear_mask = 0;
+
+ if (of_property_read_u32(node, "valid-mask", &valid_mask))
+ valid_mask = 0;
+
+ fpga_irq_init(base, node->name, 0, -1, valid_mask, node);
+
+ writel(clear_mask, base + IRQ_ENABLE_CLEAR);
+ writel(clear_mask, base + FIQ_ENABLE_CLEAR);
+
+ return 0;
+}
+#endif
diff --git a/include/linux/irqchip/versatile-fpga.h b/include/linux/irqchip/versatile-fpga.h
new file mode 100644
index 0000000..1fac965
--- /dev/null
+++ b/include/linux/irqchip/versatile-fpga.h
@@ -0,0 +1,13 @@
+#ifndef PLAT_FPGA_IRQ_H
+#define PLAT_FPGA_IRQ_H
+
+struct device_node;
+struct pt_regs;
+
+void fpga_handle_irq(struct pt_regs *regs);
+void fpga_irq_init(void __iomem *, const char *, int, int, u32,
+ struct device_node *node);
+int fpga_irq_of_init(struct device_node *node,
+ struct device_node *parent);
+
+#endif
--
1.7.11.7
Linus,
On Thu, 1 Nov 2012 22:28:49 +0100, Linus Walleij wrote:
> +void fpga_handle_irq(struct pt_regs *regs);
This function does not need to be exposed in a public header: as
proposed for the bcm2835 and armada-370-xp IRQ controller drivers, the
driver should directly do handle_arch_irq = fpga_handle_irq, and
therefore there is no need for the machine desc structure to reference
fpga_handle_irq anymore.
> +void fpga_irq_init(void __iomem *, const char *, int, int, u32,
> + struct device_node *node);
> +int fpga_irq_of_init(struct device_node *node,
> + struct device_node *parent);
I don't think this function needs to be exported. Just move the
compatible string to drivers/irqchip/irqchip.c and instead of calling
of_irq_init(), call irqchip_init().
This will leave only the fpga_irq_init() function, which we could get
rid of once the non-DT support is removed.
Best regards,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
On Thu, Nov 01, 2012 at 11:20:10PM +0100, Thomas Petazzoni wrote:
> Linus,
>
> On Thu, 1 Nov 2012 22:28:49 +0100, Linus Walleij wrote:
>
> > +void fpga_handle_irq(struct pt_regs *regs);
>
> This function does not need to be exposed in a public header: as
> proposed for the bcm2835 and armada-370-xp IRQ controller drivers, the
> driver should directly do handle_arch_irq = fpga_handle_irq, and
> therefore there is no need for the machine desc structure to reference
> fpga_handle_irq anymore.
Err no, then you don't understand what's going on here. This may or may
not be a top-level IRQ handler. Some ARM platforms have three of these
cascaded, others have one of these cascaded off a VIC or GIC.
To override the top level IRQ handler unconditionally is going to break
platforms.
On 11/02/2012 07:15 AM, Russell King - ARM Linux wrote:
> On Thu, Nov 01, 2012 at 11:20:10PM +0100, Thomas Petazzoni wrote:
>> Linus,
>>
>> On Thu, 1 Nov 2012 22:28:49 +0100, Linus Walleij wrote:
>>
>>> +void fpga_handle_irq(struct pt_regs *regs);
>>
>> This function does not need to be exposed in a public header: as
>> proposed for the bcm2835 and armada-370-xp IRQ controller drivers, the
>> driver should directly do handle_arch_irq = fpga_handle_irq, and
>> therefore there is no need for the machine desc structure to reference
>> fpga_handle_irq anymore.
>
> Err no, then you don't understand what's going on here. This may or may
> not be a top-level IRQ handler. Some ARM platforms have three of these
> cascaded, others have one of these cascaded off a VIC or GIC.
>
> To override the top level IRQ handler unconditionally is going to break
> platforms.
But this should work:
if (!handle_arch_irq)
handle_arch_irq = fpga_handle_irq;
As long as the primary controller is always initialized first, this will
work. This is guaranteed by DT of_irq_init, and you will probably have
other problems if that wasn't the case for non-DT.
Rob
On Monday 05 November 2012, Rob Herring wrote:
> But this should work:
>
> if (!handle_arch_irq)
> handle_arch_irq = fpga_handle_irq;
>
> As long as the primary controller is always initialized first, this will
> work. This is guaranteed by DT of_irq_init, and you will probably have
> other problems if that wasn't the case for non-DT.
How about adding a top-level function in arch/arm that does the assignment
and hides the handle_arch_irq variable:
void set_handle_irq(void (*handle_irq)(struct pt_regs *))
{
if (WARN_ON(handle_arch_irq))
return;
handle_arch_irq = handle_irq;
}
EXPORT_SYMBOL_GPL(set_handle_irq);
Hmm, maybe putting the top-level handler into a loadable module is a bit
far-fetched, but one can hope ;-)
Arnd
On Mon, Nov 05, 2012 at 10:42:26PM +0000, Arnd Bergmann wrote:
> On Monday 05 November 2012, Rob Herring wrote:
> > But this should work:
> >
> > if (!handle_arch_irq)
> > handle_arch_irq = fpga_handle_irq;
> >
> > As long as the primary controller is always initialized first, this will
> > work. This is guaranteed by DT of_irq_init, and you will probably have
> > other problems if that wasn't the case for non-DT.
>
> How about adding a top-level function in arch/arm that does the assignment
> and hides the handle_arch_irq variable:
>
> void set_handle_irq(void (*handle_irq)(struct pt_regs *))
> {
> if (WARN_ON(handle_arch_irq))
> return;
>
> handle_arch_irq = handle_irq;
> }
> EXPORT_SYMBOL_GPL(set_handle_irq);
>
> Hmm, maybe putting the top-level handler into a loadable module is a bit
> far-fetched, but one can hope ;-)
Definitely no point in exporting this (never export a symbol unless you
really want to use it from a module) - if you don't already have something
in handle_arch_irq, you're not going to get anywhere near the module
loader.
On Monday 05 November 2012, Russell King - ARM Linux wrote:
> On Mon, Nov 05, 2012 at 10:42:26PM +0000, Arnd Bergmann wrote:
> > On Monday 05 November 2012, Rob Herring wrote:
> > > But this should work:
> > >
> > > if (!handle_arch_irq)
> > > handle_arch_irq = fpga_handle_irq;
> > >
> > > As long as the primary controller is always initialized first, this will
> > > work. This is guaranteed by DT of_irq_init, and you will probably have
> > > other problems if that wasn't the case for non-DT.
> >
> > How about adding a top-level function in arch/arm that does the assignment
> > and hides the handle_arch_irq variable:
> >
> > void set_handle_irq(void (*handle_irq)(struct pt_regs *))
> > {
> > if (WARN_ON(handle_arch_irq))
> > return;
> >
> > handle_arch_irq = handle_irq;
> > }
> > EXPORT_SYMBOL_GPL(set_handle_irq);
> >
> > Hmm, maybe putting the top-level handler into a loadable module is a bit
> > far-fetched, but one can hope ;-)
>
> Definitely no point in exporting this (never export a symbol unless you
> really want to use it from a module) - if you don't already have something
> in handle_arch_irq, you're not going to get anywhere near the module
> loader.
Yes, I agree. While I think we might theoretically get to the point where
even the main IRQ handler can be in a module loaded from initramfs, we
are not anywhere close to that now, and it would be better to just export
the symbol once it actually gets used that way.
The one case where I think we might use it earlier is when we have an irq
chip driver that can be either a primary or a secondary chip. It has
to call set_handle_irq() in the former case (as a built-in driver), but
could be a module in the latter case. If this actually happens, we might
want to export the symbol rather than adding an "#ifdef MODULE" in the
driver.
Arnd