[PATCH] arm: mvebu: move irq controller driver in drivers/irqchip/

[PATCH] arm: mvebu: move irq controller driver in drivers/irqchip/

[Thomas Petazzoni]

The new support for the ARM BCM2835 SoC has introduced the
drivers/irqchip/ directory for IRQ controller drivers. So let's
conform to this good new approach, and move the IRQ controller driver
for Marvell Armada 370/XP in this directory.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/Makefile                       |    2 +-
 arch/arm/mach-mvebu/armada-370-xp.c                |    1 +
 arch/arm/mach-mvebu/common.h                       |    3 ---
 drivers/irqchip/Makefile                           |    1 +
 .../irqchip}/irq-armada-370-xp.c                   |    0
 include/linux/irqchip/armada-370-xp.h              |   19 +++++++++++++++++++
 6 files changed, 22 insertions(+), 4 deletions(-)
 rename {arch/arm/mach-mvebu => drivers/irqchip}/irq-armada-370-xp.c (100%)
 create mode 100644 include/linux/irqchip/armada-370-xp.h

diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile
index 57f996b..7f4e9f4 100644
--- a/arch/arm/mach-mvebu/Makefile
+++ b/arch/arm/mach-mvebu/Makefile
@@ -2,4 +2,4 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \
 	-I$(srctree)/arch/arm/plat-orion/include
 
 obj-y += system-controller.o
-obj-$(CONFIG_MACH_ARMADA_370_XP) += armada-370-xp.o irq-armada-370-xp.o addr-map.o
+obj-$(CONFIG_MACH_ARMADA_370_XP) += armada-370-xp.o addr-map.o
diff --git a/arch/arm/mach-mvebu/armada-370-xp.c b/arch/arm/mach-mvebu/armada-370-xp.c
index 49d7915..1c0021d 100644
--- a/arch/arm/mach-mvebu/armada-370-xp.c
+++ b/arch/arm/mach-mvebu/armada-370-xp.c
@@ -17,6 +17,7 @@
 #include <linux/of_platform.h>
 #include <linux/io.h>
 #include <linux/time-armada-370-xp.h>
+#include <linux/irqchip/armada-370-xp.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 #include <asm/mach/time.h>
diff --git a/arch/arm/mach-mvebu/common.h b/arch/arm/mach-mvebu/common.h
index 02f89ea..f0eaa21 100644
--- a/arch/arm/mach-mvebu/common.h
+++ b/arch/arm/mach-mvebu/common.h
@@ -17,7 +17,4 @@
 
 void mvebu_restart(char mode, const char *cmd);
 
-void armada_370_xp_init_irq(void);
-void armada_370_xp_handle_irq(struct pt_regs *regs);
-
 #endif
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 054321d..af0412c 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
+obj-$(CONFIG_MACH_ARMADA_370_XP) += irq-armada-370-xp.o
diff --git a/arch/arm/mach-mvebu/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
similarity index 100%
rename from arch/arm/mach-mvebu/irq-armada-370-xp.c
rename to drivers/irqchip/irq-armada-370-xp.c
diff --git a/include/linux/irqchip/armada-370-xp.h b/include/linux/irqchip/armada-370-xp.h
new file mode 100644
index 0000000..78876c2
--- /dev/null
+++ b/include/linux/irqchip/armada-370-xp.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef __LINUX_IRQCHIP_ARMADA_370_XP_H_
+#define __LINUX_IRQCHIP_ARMADA_370_XP_H_
+
+#include <asm/exception.h>
+
+void armada_370_xp_init_irq(void);
+void armada_370_xp_handle_irq(struct pt_regs *regs);
+
+#endif
-- 
1.7.9.5

[PATCH] arm: mvebu: move irq controller driver in drivers/irqchip/

[Rob Herring]

On 10/25/2012 07:35 AM, Thomas Petazzoni wrote:
> The new support for the ARM BCM2835 SoC has introduced the
> drivers/irqchip/ directory for IRQ controller drivers. So let's
> conform to this good new approach, and move the IRQ controller driver
> for Marvell Armada 370/XP in this directory.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

snip

> diff --git a/include/linux/irqchip/armada-370-xp.h b/include/linux/irqchip/armada-370-xp.h
> new file mode 100644
> index 0000000..78876c2
> --- /dev/null
> +++ b/include/linux/irqchip/armada-370-xp.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2012 Marvell
> + *
> + * Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#ifndef __LINUX_IRQCHIP_ARMADA_370_XP_H_
> +#define __LINUX_IRQCHIP_ARMADA_370_XP_H_
> +
> +#include <asm/exception.h>
> +
> +void armada_370_xp_init_irq(void);
> +void armada_370_xp_handle_irq(struct pt_regs *regs);
> +
> +#endif

I don't mean to pick on this specific patch, but this is a common
problem of moving various low-level pieces like irqchips, cpuidle,
timers, etc. to drivers/*. If we, just moving the code as is over, we
still need some hooks between arch/arm and drivers. I think if we keep
adding ARM SOC specific headers to include/linux, that will be the next
thing we get yelled at for and will have to clean-up.

For irqchips, the way I see this working is we would have a single call
to of_irq_init with a match list of all irqchips in drivers/irqchips.
This contains the init function within drivers/irqchips. Then all the
machines can just call a generic irqchip_init.

The handle_irq ptr would also need to be plugged in at runtime.

Rob

[PATCH] arm: mvebu: move irq controller driver in drivers/irqchip/

[Thomas Petazzoni]

Rob,

On Thu, 25 Oct 2012 08:48:54 -0500, Rob Herring wrote:

> I don't mean to pick on this specific patch, but this is a common
> problem of moving various low-level pieces like irqchips, cpuidle,
> timers, etc. to drivers/*. If we, just moving the code as is over, we
> still need some hooks between arch/arm and drivers. I think if we keep
> adding ARM SOC specific headers to include/linux, that will be the
> next thing we get yelled at for and will have to clean-up.
> 
> For irqchips, the way I see this working is we would have a single
> call to of_irq_init with a match list of all irqchips in
> drivers/irqchips. This contains the init function within
> drivers/irqchips. Then all the machines can just call a generic
> irqchip_init.

Sounds doable indeed.

> The handle_irq ptr would also need to be plugged in at runtime.

However, do you have a more specific idea here? In setup_arch(), the
value of mdesc->handle_irq gets picked up way before the ->init_irq
machine hook is being called.

An option would be to fill this mdesc->handle_irq() field during the
mdesc->init_early() callback (would require moving things around a bit
in setup_arch(), but sounds reasonable). However, at the time of
mdesc->init_early(), I guess we can't call of_irq_init(). So we would
have a two step process: fill in mdesc->handle_irq during
mdesc->init_early(), and call irqchip_init() (which will do the
of_irq_init()) in mdesc->init_irq().

If that's fine with you, I can cook a patch for BCM2835 and MVEBU that
implements this idea.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH] arm: mvebu: move irq controller driver in drivers/irqchip/

[Arnd Bergmann]

On Thursday 25 October 2012, Thomas Petazzoni wrote:
> On Thu, 25 Oct 2012 08:48:54 -0500, Rob Herring wrote:
> 
> > I don't mean to pick on this specific patch, but this is a common
> > problem of moving various low-level pieces like irqchips, cpuidle,
> > timers, etc. to drivers/*. If we, just moving the code as is over, we
> > still need some hooks between arch/arm and drivers. I think if we keep
> > adding ARM SOC specific headers to include/linux, that will be the
> > next thing we get yelled at for and will have to clean-up.
> > 
> > For irqchips, the way I see this working is we would have a single
> > call to of_irq_init with a match list of all irqchips in
> > drivers/irqchips. This contains the init function within
> > drivers/irqchips. Then all the machines can just call a generic
> > irqchip_init.
> 
> Sounds doable indeed.

Agreed. I had the same idea some time ago, but didn't want to be the
one who starts that discussion.

> > The handle_irq ptr would also need to be plugged in at runtime.
> 
> However, do you have a more specific idea here? In setup_arch(), the
> value of mdesc->handle_irq gets picked up way before the ->init_irq
> machine hook is being called.

The variouos *_of_init functions to initialize each IRQ controller already
get a pointer to the irq parent.

Maybe we can just change those functions so that any irqchip that can
be a top-level chip and has a NULL parent can also assign the
handle_arch_irq pointer like in the snippet below.

	Arnd

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index aa52699..ec7e00b 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -777,6 +777,8 @@ int __init gic_of_init(struct device_node *node, struct device_node *parent)
 	if (parent) {
 		irq = irq_of_parse_and_map(node, 0);
 		gic_cascade_irq(gic_cnt, irq);
+	} else {
+		handle_arch_irq = gic_handle_irq;
 	}
 	gic_cnt++;
 	return 0;

[PATCH] arm: mvebu: move irq controller driver in drivers/irqchip/

[Rob Herring]

On 10/25/2012 09:06 AM, Thomas Petazzoni wrote:
> Rob,
> 
> On Thu, 25 Oct 2012 08:48:54 -0500, Rob Herring wrote:
> 
>> I don't mean to pick on this specific patch, but this is a common
>> problem of moving various low-level pieces like irqchips, cpuidle,
>> timers, etc. to drivers/*. If we, just moving the code as is over, we
>> still need some hooks between arch/arm and drivers. I think if we keep
>> adding ARM SOC specific headers to include/linux, that will be the
>> next thing we get yelled at for and will have to clean-up.
>>
>> For irqchips, the way I see this working is we would have a single
>> call to of_irq_init with a match list of all irqchips in
>> drivers/irqchips. This contains the init function within
>> drivers/irqchips. Then all the machines can just call a generic
>> irqchip_init.
> 
> Sounds doable indeed.
> 
>> The handle_irq ptr would also need to be plugged in at runtime.
> 
> However, do you have a more specific idea here? In setup_arch(), the
> value of mdesc->handle_irq gets picked up way before the ->init_irq
> machine hook is being called.
> 
> An option would be to fill this mdesc->handle_irq() field during the
> mdesc->init_early() callback (would require moving things around a bit
> in setup_arch(), but sounds reasonable). However, at the time of
> mdesc->init_early(), I guess we can't call of_irq_init(). So we would
> have a two step process: fill in mdesc->handle_irq during
> mdesc->init_early(), and call irqchip_init() (which will do the
> of_irq_init()) in mdesc->init_irq().

There's not really any reason I can see that it needs to be setup that
early. You can't really service interrupts before .init_irq completes
anyway. I would export handle_arch_irq and allow the irqchip driver to
set it up.

> If that's fine with you, I can cook a patch for BCM2835 and MVEBU that
> implements this idea.

Great!

Rob