Re: [PATCH v2] ARM: plat-versatile: move FPGA irq driver to drivers/irqchip

2012-11-06 Thread Arnd Bergmann
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

2012-11-05 Thread Russell King - ARM Linux
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

2012-11-05 Thread Arnd Bergmann
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

2012-11-05 Thread Rob Herring
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

2012-11-02 Thread Russell King - ARM Linux
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

2012-11-01 Thread Thomas Petazzoni
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

2012-11-01 Thread Linus Walleij
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