Re: [PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
On Monday 05 November 2012, Russell King - ARM Linux wrote: > On Mon, Nov 05, 2012 at 10:42:26PM +, 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
On Mon, Nov 05, 2012 at 10:42:26PM +, 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip
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 . Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Moved include to - 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 #include #include +#include #include #include #include @@ -56,8 +57,6 @@ #include #include -#include - #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 #include #include +#include #include #include #include @@ -46,7 +47,6 @@ #include #include -#include #include #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 #include #include +#include #include #include #include @@ -51,7 +52,6 @@ #include #include -#include #include #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..000 --- a/arch/arm/plat-versatile/fpga-irq.c +++ /dev/null @@ -1,204 +0,0 @@ -/* - * Support for Versatile FPGA-based IRQ controllers - */ -#include -#include -#include -#include -#include -#include -#include - -#include -#include -#include - -#define IRQ_STATUS 0x00 -#define IRQ_RAW_STATUS 0x04 -#define IRQ_ENABLE_SET 0x08 -#define IRQ_ENABLE_CLEAR 0x0c -#define INT_SOFT_SET