[ORLinux] [PATCH] openrisc: irq: use irqchip framework

[Stefan Kristiansson]

In addition to consolidating the or1k-pic initialization with
how other interrupt controllers are initialized, this makes
OpenRISC less tied to its on-cpu interrupt controller.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>
---
arch/openrisc/kernel/irq.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/openrisc/kernel/irq.c b/arch/openrisc/kernel/irq.c
index 8ec77bc..f6f4683 100644
--- a/arch/openrisc/kernel/irq.c
+++ b/arch/openrisc/kernel/irq.c
@@ -19,10 +19,13 @@
#include <linux/of.h>
#include <linux/ftrace.h>
#include <linux/irq.h>
+#include <linux/irqchip.h>
#include <linux/export.h>
#include <linux/irqdomain.h>
#include <linux/irqflags.h>

+#include "../../drivers/irqchip/irqchip.h"
+
/* read interrupt enabled status */
unsigned long arch_local_save_flags(void)
{
@@ -151,24 +154,22 @@ static const struct irq_domain_ops or1k_irq_domain_ops = {
* 1000 CPU. This is the "root" domain as these are the interrupts
* that directly trigger an exception in the CPU.
*/
-static void __init or1k_irq_init(void)
+static int __init
+or1k_irq_init(struct device_node *intc, struct device_node *parent)
{
- struct device_node *intc = NULL;
-
- /* The interrupt controller device node is mandatory */
- intc = of_find_compatible_node(NULL, NULL, "opencores,or1k-pic");
- BUG_ON(!intc);
-
/* Disable all interrupts until explicitly requested */
mtspr(SPR_PICMR, (0UL));

root_domain = irq_domain_add_linear(intc, 32,
&or1k_irq_domain_ops, NULL);
+
+ return 0;
}
+IRQCHIP_DECLARE(or1k_pic, "opencores,or1k-pic", or1k_irq_init);

void __init init_IRQ(void)
{
- or1k_irq_init();
+ irqchip_init();
}

void __irq_entry do_IRQ(struct pt_regs *regs)

[Jonas Bonn]

Hi Stefan,

This looks good. Let's complete the the cleanup of this driver while
we're at it:

i) Move this file to drivers/irqchip/
ii) Put a Depends on CONFIG_ARCH_OPENRISC in the Kconfig
iii) Provide documentation for the binding at
Documentation/devicetree/bindings/interrupt-controller/

Copy final result to Thomas Gleixner who maintains the irqchip bits.

Thanks,
Jonas

Post by Stefan Kristiansson
In addition to consolidating the or1k-pic initialization with
how other interrupt controllers are initialized, this makes
OpenRISC less tied to its on-cpu interrupt controller.
Signed-off-by: Stefan Kristiansson <stefan.kristiansson at saunalahti.fi>
---
arch/openrisc/kernel/irq.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/openrisc/kernel/irq.c b/arch/openrisc/kernel/irq.c
index 8ec77bc..f6f4683 100644
--- a/arch/openrisc/kernel/irq.c
+++ b/arch/openrisc/kernel/irq.c
@@ -19,10 +19,13 @@
#include <linux/of.h>
#include <linux/ftrace.h>

Do we really need to pull in ftrace.h?

<snip>

This last bit (below) stays in the arch/openrisc directory.

Post by Stefan Kristiansson
void __init init_IRQ(void)
{
- or1k_irq_init();
+ irqchip_init();
}
void __irq_entry do_IRQ(struct pt_regs *regs)

[Stefan Kristiansson]

Post by Jonas Bonn
This looks good. Let's complete the the cleanup of this driver while
i) Move this file to drivers/irqchip/

Sure, that sounds like a good idea.

Post by Jonas Bonn
ii) Put a Depends on CONFIG_ARCH_OPENRISC in the Kconfig

Hmm, do we really need that?
The irqchip driver will picked by 'select'ing it from arch/openrisc/Kconfig
So, it won't really be exposed to anyone not explicitly picking it like that.

Post by Jonas Bonn
iii) Provide documentation for the binding at
Documentation/devicetree/bindings/interrupt-controller/
Copy final result to Thomas Gleixner who maintains the irqchip bits.

Will do.

Post by Jonas Bonn

Post by Stefan Kristiansson
@@ -19,10 +19,13 @@
#include <linux/of.h>
#include <linux/ftrace.h>

Do we really need to pull in ftrace.h?

The '__irq_entry' that is used below is defined in that

Post by Jonas Bonn

Post by Stefan Kristiansson
void __irq_entry do_IRQ(struct pt_regs *regs)

Stefan

[Jonas Bonn]

Post by Stefan Kristiansson

Post by Jonas Bonn
ii) Put a Depends on CONFIG_ARCH_OPENRISC in the Kconfig

Hmm, do we really need that?
The irqchip driver will picked by 'select'ing it from arch/openrisc/Kconfig
So, it won't really be exposed to anyone not explicitly picking it like that.

It will be exposed by allyesconfig. The driver uses mfspr which is
openrisc-only, so the driver really needs to hide behind an
openrisc-arch config option.

/Jonas

[Stefan Kristiansson]

Post by Jonas Bonn

Post by Stefan Kristiansson

Post by Jonas Bonn
ii) Put a Depends on CONFIG_ARCH_OPENRISC in the Kconfig

Hmm, do we really need that?
The irqchip driver will picked by 'select'ing it from arch/openrisc/Kconfig
So, it won't really be exposed to anyone not explicitly picking it like that.

It will be exposed by allyesconfig.

allyesconfig is still ARCH specific, so it will not be exposed by that.

Post by Jonas Bonn
The driver uses mfspr which is
openrisc-only, so the driver really needs to hide behind an
openrisc-arch config option.

Yes, I understand that this needs to be OpenRISC only,
but usually it is frown upon adding things to the Kconfig that are implied
by the default behaviour.
And I believe the default behaviour does imply this, OpenRISC will be the
only ARCH that will pick this driver.

Stefan