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

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

[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 <dilinger-pFFUokh25LWsTnJN9+BGXg@public.gmane.org>
---
 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(-)

diff --git a/arch/x86/include/asm/irq.h b/arch/x86/include/asm/irq.h
index 5458380..8ffa06f 100644
--- a/arch/x86/include/asm/irq.h
+++ b/arch/x86/include/asm/irq.h
@@ -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);
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 91fd0c7..b78bb6f 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -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)
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index 090cbaa..109e013 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -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>
 
-- 
1.5.6.5

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

[Grant Likely]

On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon wrote:
> 
>  - 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 <dilinger-pFFUokh25LWsTnJN9+BGXg@public.gmane.org>

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

g.

> ---
>  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(-)
> 
> diff --git a/arch/x86/include/asm/irq.h b/arch/x86/include/asm/irq.h
> index 5458380..8ffa06f 100644
> --- a/arch/x86/include/asm/irq.h
> +++ b/arch/x86/include/asm/irq.h
> @@ -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);
> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> index 91fd0c7..b78bb6f 100644
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -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)
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index 090cbaa..109e013 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -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>
>  
> -- 
> 1.5.6.5
> 

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

[Ingo Molnar]

* Grant Likely <grant.likely@secretlab.ca> wrote:

> On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon wrote:
> > 
> >  - 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 <dilinger@queued.net>
> 
> 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

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

[Grant Likely]

On Fri, Sep 10, 2010 at 08:14:58PM +0200, Ingo Molnar wrote:
> 
> * Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> 
> > On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon wrote:
> > > 
> > >  - 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 <dilinger-pFFUokh25LWsTnJN9+BGXg@public.gmane.org>
> > 
> > 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.

> 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.

g.

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

[Andres Salomon]

On Fri, 10 Sep 2010 12:21:35 -0600
Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:

> On Fri, Sep 10, 2010 at 08:14:58PM +0200, Ingo Molnar wrote:
> > 
> > * Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > 
> > > On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon wrote:
> > > > 
> > > >  - 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 <dilinger-pFFUokh25LWsTnJN9+BGXg@public.gmane.org>
> > > 
> > > 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.
> 
> > 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.
> 
> 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.

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

[H. Peter Anvin]

On 09/20/2010 07:06 PM, Andres Salomon wrote:
> 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


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

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

[Thomas Gleixner]

On Mon, 20 Sep 2010, Andres Salomon wrote:
> On Fri, 10 Sep 2010 12:21:35 -0600
> Grant Likely <grant.likely@secretlab.ca> wrote:
> 
> > On Fri, Sep 10, 2010 at 08:14:58PM +0200, Ingo Molnar wrote:
> > > 
> > > * Grant Likely <grant.likely@secretlab.ca> wrote:
> > > 
> > > > On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon wrote:
> > > > > 
> > > > >  - 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 <dilinger@queued.net>
> > > > 
> > > > 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.
> > 
> > > 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.
> > 
> > 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

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

[Andres Salomon]

On Tue, 21 Sep 2010 11:45:37 +0200 (CEST)
Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:

> On Mon, 20 Sep 2010, Andres Salomon wrote:
> > On Fri, 10 Sep 2010 12:21:35 -0600
> > Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > 
> > > On Fri, Sep 10, 2010 at 08:14:58PM +0200, Ingo Molnar wrote:
> > > > 
> > > > * Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
> > > > 
> > > > > On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon
> > > > > wrote:
> > > > > > 
> > > > > >  - 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 <dilinger-pFFUokh25LWsTnJN9+BGXg@public.gmane.org>
> > > > > 
> > > > > 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.
> > > 
> > > > 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.
> > > 
> > > 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

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?

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

[Thomas Gleixner]

On Tue, 21 Sep 2010, Andres Salomon wrote:
> On Tue, 21 Sep 2010 11:45:37 +0200 (CEST)
> Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
> 
> > > 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
> 
> 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

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

[Grant Likely]

On Tue, Sep 21, 2010 at 1:27 PM, Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
> On Tue, 21 Sep 2010, Andres Salomon wrote:
>> On Tue, 21 Sep 2010 11:45:37 +0200 (CEST)
>> Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
>>
>> > > 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
>>
>> 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.

g.

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

[Thomas Gleixner]

On Fri, 10 Sep 2010, Grant Likely wrote:

> On Fri, Sep 10, 2010 at 08:14:58PM +0200, Ingo Molnar wrote:
> > 
> > * Grant Likely <grant.likely@secretlab.ca> wrote:
> > 
> > > On Fri, Sep 10, 2010 at 06:01:51AM -0700, Andres Salomon wrote:
> > > > 
> > > >  - 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 <dilinger@queued.net>
> > > 
> > > 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.
> 
> > 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.

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