Skip to contentSkip to conversation

arm: mvebu: move irq controller driver in drivers/irqchip/

PATCH New Thomas Petazzoni 3 participants 5 messages Oct 25, 2012

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

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

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.

---
 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
arch/arm/mach-mvebu/Makefile +1 -1
index 57f996b..7f4e9f4 100644
@@ -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
arch/arm/mach-mvebu/armada-370-xp.c +1 -0
index 49d7915..1c0021d 100644
@@ -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>
arch/arm/mach-mvebu/common.h +0 -3
index 02f89ea..f0eaa21 100644
@@ -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
drivers/irqchip/Makefile +1 -0
index 054321d..af0412c 100644
@@ -1 +1,2 @@
 obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
+obj-$(CONFIG_MACH_ARMADA_370_XP) += irq-armada-370-xp.o
drivers/irqchip/irq-armada-370-xp.c renamed from arch/arm/mach-mvebu/irq-armada-370-xp.c +0 -0
similarity index 100%
rename from arch/arm/mach-mvebu/irq-armada-370-xp.c
rename to drivers/irqchip/irq-armada-370-xp.c
include/linux/irqchip/armada-370-xp.h new file +19 -0
new file mode 100644
index 0000000..78876c2
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <[email protected]>
+ *
+ * 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
Signature · 1 line
1.7.9.5
#1 lore raw

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

Quoted 7 lines · 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.

---

snip

Quoted 15 lines
include/linux/irqchip/armada-370-xp.h new file +9 -0
new file mode 100644
index 0000000..78876c2
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <[email protected]>
+ *
+ * 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.
+ */

include/linux/irqchip/armada-370-xp.h line 19

Quoted 10 lines
include/linux/irqchip/armada-370-xp.h new file +10 -0
+
+#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

#2 lore raw

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

Rob,

Quoted 7 lines · Rob Herring

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.

Quoted 5 lines

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.

Quoted 1 line

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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#3 lore raw

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

Quoted 9 lines · Thomas Petazzoni
Quoted 7 lines · Rob Herring

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.

Quoted 7 lines
Quoted 5 lines

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.

Quoted 5 lines
Quoted 1 line

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

arch/arm/common/gic.c +2 -0
index aa52699..ec7e00b 100644
@@ -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;
#4 lore raw

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

In reply to Thomas Petazzoni
Quoted 25 lines · Thomas Petazzoni

Rob,

Quoted 7 lines · Rob Herring

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.

Quoted 5 lines

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.

Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 2 lines

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

Great!

Rob