x86: of: define irq functions to allow drivers/of/* to build on x86

PATCH Applied Andres Salomon 5 participants 11 messages Sep 10, 2010 – Sep 21, 2010

Subsystems (2)

x86 Device tree

lore

Share & tools

lore

Andres Salomon

author

2010-09-10 11:09 UTC #0 lore raw

PATCH 2/2 x86: of: define irq functions to allow drivers/of/* to build on x86

- Define a stub irq_create_of_mapping for x86 as a stop-gap solution until drivers/of/irq is further along.
- Define irq_dispose_mapping for x86 to appease of_i2c.c

Signed-off-by Andres Salomon <[email protected]>

---
 arch/x86/include/asm/irq.h |    3 +++
 arch/x86/kernel/irq.c      |    8 ++++++++
 include/linux/of_irq.h     |    1 +
 3 files changed, 12 insertions(+), 0 deletions(-)

arch/x86/include/asm/irq.h +3 -0

index 5458380..8ffa06f 100644
@@ -10,6 +10,9 @@
 #include <asm/apicdef.h>
 #include <asm/irq_vectors.h>
 
+/* Even though we don't support this, supply it to appease OF */
+static inline void irq_dispose_mapping(unsigned int virq) { }
+
 static inline int irq_canonicalize(int irq)
 {
 	return ((irq == 2) ? 9 : irq);

arch/x86/kernel/irq.c +8 -0

index 91fd0c7..b78bb6f 100644
@@ -4,6 +4,7 @@
 #include <linux/cpu.h>
 #include <linux/interrupt.h>
 #include <linux/kernel_stat.h>
+#include <linux/of.h>
 #include <linux/seq_file.h>
 #include <linux/smp.h>
 #include <linux/ftrace.h>

@@ -275,6 +276,13 @@ void smp_x86_platform_ipi(struct pt_regs *regs)
 
 EXPORT_SYMBOL_GPL(vector_used_by_percpu_irq);
 
+unsigned int irq_create_of_mapping(struct device_node *controller,
+		const u32 *intspec, unsigned int intsize)
+{
+	return intspec[0] + 1;
+}
+EXPORT_SYMBOL_GPL(irq_create_of_mapping);
+
 #ifdef CONFIG_HOTPLUG_CPU
 /* A cpu has been removed from cpu_online_mask.  Reset irq affinities. */
 void fixup_irqs(void)

include/linux/of_irq.h +1 -0

index 090cbaa..109e013 100644
@@ -5,6 +5,7 @@
 struct of_irq;
 #include <linux/types.h>
 #include <linux/errno.h>
+#include <linux/irq.h>
 #include <linux/ioport.h>
 #include <linux/of.h>

Signature · 1 line

1.5.6.5

Grant Likely

2010-09-10 17:25 UTC #1 lore raw

Quoted 6 lines · Andres Salomon

- Define a stub irq_create_of_mapping for x86 as a stop-gap solution until drivers/of/irq is further along.
- Define irq_dispose_mapping for x86 to appease of_i2c.c
Signed-off-by Andres Salomon <[email protected]>

Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch.

Quoted 61 lines

---
 arch/x86/include/asm/irq.h |    3 +++
 arch/x86/kernel/irq.c      |    8 ++++++++
 include/linux/of_irq.h     |    1 +
 3 files changed, 12 insertions(+), 0 deletions(-)

arch/x86/include/asm/irq.h +3 -0

index 5458380..8ffa06f 100644
@@ -10,6 +10,9 @@
 #include <asm/apicdef.h>
 #include <asm/irq_vectors.h>
 
+/* Even though we don't support this, supply it to appease OF */
+static inline void irq_dispose_mapping(unsigned int virq) { }
+
 static inline int irq_canonicalize(int irq)
 {
 	return ((irq == 2) ? 9 : irq);

arch/x86/kernel/irq.c +8 -0

index 91fd0c7..b78bb6f 100644
@@ -4,6 +4,7 @@
 #include <linux/cpu.h>
 #include <linux/interrupt.h>
 #include <linux/kernel_stat.h>
+#include <linux/of.h>
 #include <linux/seq_file.h>
 #include <linux/smp.h>
 #include <linux/ftrace.h>

@@ -275,6 +276,13 @@ void smp_x86_platform_ipi(struct pt_regs *regs)
 
 EXPORT_SYMBOL_GPL(vector_used_by_percpu_irq);
 
+unsigned int irq_create_of_mapping(struct device_node *controller,
+		const u32 *intspec, unsigned int intsize)
+{
+	return intspec[0] + 1;
+}
+EXPORT_SYMBOL_GPL(irq_create_of_mapping);
+
 #ifdef CONFIG_HOTPLUG_CPU
 /* A cpu has been removed from cpu_online_mask.  Reset irq affinities. */
 void fixup_irqs(void)

include/linux/of_irq.h +1 -0

index 090cbaa..109e013 100644
@@ -5,6 +5,7 @@
 struct of_irq;
 #include <linux/types.h>
 #include <linux/errno.h>
+#include <linux/irq.h>
 #include <linux/ioport.h>
 #include <linux/of.h>

Signature · 1 line

1.5.6.5

Ingo Molnar

2010-09-10 18:14 UTC #2 lore raw

Quoted 10 lines · Grant Likely

Quoted 6 lines · Andres Salomon
- Define a stub irq_create_of_mapping for x86 as a stop-gap solution until drivers/of/irq is further along.
- Define irq_dispose_mapping for x86 to appease of_i2c.c
Signed-off-by Andres Salomon <[email protected]>
Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch.

The purpose of the patch is not clear to me. What does it do and why? The changelog says it's a stopgap measure - what exactly is the long term plan and how long will it take?

Thanks,

Ingo

Grant Likely

2010-09-10 18:21 UTC #3 lore raw

Quoted 9 lines · Ingo Molnar

Quoted 6 lines · Grant Likely
Quoted 5 lines · Andres Salomon
- Define a stub irq_create_of_mapping for x86 as a stop-gap solution until drivers/of/irq is further along.
- Define irq_dispose_mapping for x86 to appease of_i2c.c

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch.
The purpose of the patch is not clear to me. What does it do and why?

It allows CONFIG_OF to be enabled on x86 without a build failure.

Quoted 2 lines

The changelog says it's a stopgap measure - what exactly is the long term plan and how long will it take?

It is a stop gap because it performs a trivial direct map of an IRQ number in the device tree data structure to a Linux irq number. This works for a single IRQ controller, but falls apart when cascaded controller are described in the device tree. The long term plan is to have the drivers/of/ code handling the mapping intelligently like powerpc currently does.

Andres Salomon

author

2010-09-21 02:04 UTC #4 lore raw

Quoted 23 lines · On Fri, 10 Sep 2010 12:21:35 -0600 Grant Likely <[email protected]>…

Quoted 9 lines · Ingo Molnar
Quoted 6 lines · Grant Likely
Quoted 5 lines · Andres Salomon
- Define a stub irq_create_of_mapping for x86 as a stop-gap
solution until drivers/of/irq is further along.
- Define irq_dispose_mapping for x86 to appease of_i2c.c
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch.
The purpose of the patch is not clear to me. What does it do and why?
It allows CONFIG_OF to be enabled on x86 without a build failure.
Quoted 2 lines
The changelog says it's a stopgap measure - what exactly is the long term plan and how long will it take?

Quoted 9 lines

It is a stop gap because it performs a trivial direct map of an IRQ number in the device tree data structure to a Linux irq number. This works for a single IRQ controller, but falls apart when cascaded controller are described in the device tree. The long term plan is to have the drivers/of/ code handling the mapping intelligently like powerpc currently does.
g.

Any additional comments (ACKs, NACKs, etc) on the patches? If I need to rework it (or them), I can certainly make the patch description longer.

H. Peter Anvin

reviewer

2010-09-21 02:32 UTC #5 lore raw

Quoted 3 lines · Andres Salomon

Any additional comments (ACKs, NACKs, etc) on the patches? If I need to rework it (or them), I can certainly make the patch description longer.

Sorry... fell off my radar. Will try to look at it tomorrow.

-hpa

Signature · 2 lines

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Thomas Gleixner

2010-09-21 09:45 UTC #6 lore raw

In reply to Andres Salomon

Quoted 33 lines · Andres Salomon

Quoted 30 lines · On Fri, 10 Sep 2010 12:21:35 -0600 Grant Likely <[email protected]>…
Quoted 9 lines · Ingo Molnar
Quoted 6 lines · Grant Likely
Quoted 5 lines · Andres Salomon
- Define a stub irq_create_of_mapping for x86 as a stop-gap
solution until drivers/of/irq is further along.
- Define irq_dispose_mapping for x86 to appease of_i2c.c
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch.
The purpose of the patch is not clear to me. What does it do and why?
It allows CONFIG_OF to be enabled on x86 without a build failure.
Quoted 2 lines
The changelog says it's a stopgap measure - what exactly is the long term plan and how long will it take?
It is a stop gap because it performs a trivial direct map of an IRQ number in the device tree data structure to a Linux irq number. This works for a single IRQ controller, but falls apart when cascaded controller are described in the device tree. The long term plan is to have the drivers/of/ code handling the mapping intelligently like powerpc currently does.

Quoted 6 lines

Quoted 2 lines
g.
Any additional comments (ACKs, NACKs, etc) on the patches? If I need to rework it (or them), I can certainly make the patch description longer.

Fine with me, but you could make the "mapping" function an inline as well. While at it, some sensible changelog would be helpful :)

Thanks,

tglx

Andres Salomon

author

2010-09-21 15:50 UTC #7 lore raw

Quoted 42 lines · On Tue, 21 Sep 2010 11:45:37 +0200 (CEST) Thomas Gleixner <[email protected]>…

Quoted 34 lines · Andres Salomon
Quoted 31 lines · On Fri, 10 Sep 2010 12:21:35 -0600 Grant Likely <[email protected]>…
Quoted 10 lines · Ingo Molnar
Quoted 7 lines · Grant Likely
Quoted 5 lines · Andres Salomon
- Define a stub irq_create_of_mapping for x86 as a stop-gap
solution until drivers/of/irq is further along.
- Define irq_dispose_mapping for x86 to appease of_i2c.c
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch.
The purpose of the patch is not clear to me. What does it do and why?
It allows CONFIG_OF to be enabled on x86 without a build failure.
Quoted 2 lines
The changelog says it's a stopgap measure - what exactly is the long term plan and how long will it take?
It is a stop gap because it performs a trivial direct map of an IRQ number in the device tree data structure to a Linux irq number. This works for a single IRQ controller, but falls apart when cascaded controller are described in the device tree. The long term plan is to have the drivers/of/ code handling the mapping intelligently like powerpc currently does.
Quoted 6 lines
Quoted 2 lines
g.
Any additional comments (ACKs, NACKs, etc) on the patches? If I need to rework it (or them), I can certainly make the patch description longer.

Quoted 6 lines

Fine with me, but you could make the "mapping" function an inline as well. While at it, some sensible changelog would be helpful :)
Thanks,
tglx

Hm, inlining it would require some additional x86 #ifdefs in include/linux/of_irq.h (where the mapping function is declared). You'd prefer that?

Thomas Gleixner

2010-09-21 16:27 UTC #8 lore raw

Quoted 10 lines · Andres Salomon

Quoted 7 lines · On Tue, 21 Sep 2010 11:45:37 +0200 (CEST) Thomas Gleixner…
Quoted 3 lines
Any additional comments (ACKs, NACKs, etc) on the patches? If I need to rework it (or them), I can certainly make the patch description longer.
Fine with me, but you could make the "mapping" function an inline as well. While at it, some sensible changelog would be helpful :)

Quoted 7 lines

Quoted 3 lines
Thanks,
tglx
Hm, inlining it would require some additional x86 #ifdefs in include/linux/of_irq.h (where the mapping function is declared). You'd prefer that?

Urgh, no.

tglx

Grant Likely

2010-09-21 19:27 UTC #9 lore raw

Quoted 13 lines · Thomas Gleixner

Quoted 12 lines · Andres Salomon
Quoted 9 lines · On Tue, 21 Sep 2010 11:45:37 +0200 (CEST) Thomas Gleixner…
Quoted 3 lines
Any additional comments (ACKs, NACKs, etc) on the patches? If I need to rework it (or them), I can certainly make the patch description longer.
Fine with me, but you could make the "mapping" function an inline as well. While at it, some sensible changelog would be helpful :)
Thanks,

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
tglx
Hm, inlining it would require some additional x86 #ifdefs in include/linux/of_irq.h (where the mapping function is declared). You'd prefer that?
Urgh, no.

In OF code, I *have* been moving to the following pattern when the OF code has a sane default:

#ifndef <symbol>
static inline <symbol>(...)
{
<default implementation>
}
#define <symbol> <symbol>
#endif

I'd normally be totally okay with this in of_irq.h, except for the fact that this implementation is a temporary solution. Once I've got the irq remapping code generalized for all architecture then the map function could become a static inline in this form.

Fortunately, this code should never be on a hot path.

Thomas Gleixner

2010-09-21 09:43 UTC #10 lore raw

In reply to Grant Likely

Quoted 22 lines · Grant Likely

Quoted 9 lines · Ingo Molnar
Quoted 6 lines · Grant Likely
Quoted 5 lines · Andres Salomon
- Define a stub irq_create_of_mapping for x86 as a stop-gap solution until drivers/of/irq is further along.
- Define irq_dispose_mapping for x86 to appease of_i2c.c
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Andres Salomon <[email protected]>
Applied to my test-devicetree branch. I'll need an ack from the x86 maintainers before I put it into my -next branch.
The purpose of the patch is not clear to me. What does it do and why?
It allows CONFIG_OF to be enabled on x86 without a build failure.
Quoted 2 lines
The changelog says it's a stopgap measure - what exactly is the long term plan and how long will it take?

Quoted 6 lines

It is a stop gap because it performs a trivial direct map of an IRQ number in the device tree data structure to a Linux irq number. This works for a single IRQ controller, but falls apart when cascaded controller are described in the device tree. The long term plan is to have the drivers/of/ code handling the mapping intelligently like powerpc currently does.

Sounds good. We need this for other embedded x86 platforms as well and I was looking into the powerpc code before and wondered how we can make that generic. The question is whether this should be tied into drivers/of/irq or just provided as an OF independent generic facility in kernel/irq.

Thanks,

tglx