[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Eric Miao]

commit edec494b3dbf52a61a18b484dd05cce6a9d90e2f
Author: Eric Miao <eric.y.miao@gmail.com>
Date:   Sat May 22 11:58:01 2010 +0800

    [ARM] allow each machine to specify it's own IRQ handlers at run-time

    Normally different ARM platform has different way to decode the IRQ
    hardware status and demultiplex to the corresponding IRQ handler.
    This is highly optimized by macro irq_handler in entry-armv.S, and
    each machine class defines their own macro to decode the IRQ number.
    However, this prevents multiple machine classes to be built into a
    single kernel.

    By allowing each machine to specify thier own handler, and making
    function pointer 'handle_arch_irq' to point to it at run time, this
    can be solved. And introduce CONFIG_MULTI_IRQ_HANDLER to allow both
    solutions to work.

    Comparing with the highly optimized macro of irq_handler, the new
    function must be written with care not to lose too much performance.

    Signed-off-by: Eric Miao <eric.y.miao@gmail.com>

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index ddf5676..a8f5f78 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -958,6 +958,11 @@ config CPU_HAS_PMU
 	default y
 	bool

+config MULTI_IRQ_HANDLER
+	bool
+	help
+	  Allow each machine to specify it's own IRQ handler at run time.
+
 if !MMU
 source "arch/arm/Kconfig-nommu"
 endif
diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
index 5ee6f85..b611895 100644
--- a/arch/arm/include/asm/mach/arch.h
+++ b/arch/arm/include/asm/mach/arch.h
@@ -42,6 +42,9 @@ struct machine_desc {
 	void			(*init_irq)(void);
 	struct sys_timer	*timer;		/* system tick timer	*/
 	void			(*init_machine)(void);
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+	void			(*handle_irq)(struct pt_regs *);
+#endif
 };

 /*
diff --git a/arch/arm/include/asm/mach/irq.h b/arch/arm/include/asm/mach/irq.h
index ce3eee9..6ecdad9 100644
--- a/arch/arm/include/asm/mach/irq.h
+++ b/arch/arm/include/asm/mach/irq.h
@@ -22,6 +22,10 @@ extern void (*init_arch_irq)(void);
 extern void init_FIQ(void);
 extern int show_fiq_list(struct seq_file *, void *);

+#ifdef CONFIG_MULTI_IRQ_HANDLER
+extern void (*handle_arch_irq)(struct pt_regs *);
+#endif
+
 /*
  * This is for easy migration, but should be changed in the source
  */
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 7ee48e7..ecb8492 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -28,6 +28,7 @@
 /*
  * Interrupt handling.  Preserves r7, r8, r9
  */
+#ifndef CONFIG_MULTI_IRQ_HANDLER
 	.macro	irq_handler
 	get_irqnr_preamble r5, lr
 1:	get_irqnr_and_base r0, r6, r5, lr
@@ -59,6 +60,14 @@
 #endif

 	.endm
+#else
+	.macro	irq_handler
+	ldr	r4, =handle_arch_irq
+	mov	r0, sp
+	mov	lr, pc
+	ldr	pc, [r4]
+	.endm
+#endif	/* CONFIG_MULTI_IRQ_HANDLER */

 #ifdef CONFIG_KPROBES
 	.section	.kprobes.text,"ax",%progbits
@@ -1251,3 +1260,9 @@ cr_alignment:
 	.space	4
 cr_no_alignment:
 	.space	4
+
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+	.globl	handle_arch_irq
+handle_arch_irq:
+	.long	0
+#endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index f1aee0d..61d5f8d 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -732,6 +732,10 @@ void __init setup_arch(char **cmdline_p)
 	system_timer = mdesc->timer;
 	init_machine = mdesc->init_machine;

+#ifdef CONFIG_MULTI_IRQ_HANDLER
+	handle_arch_irq = mdesc->handle_irq;
+#endif
+
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
 	conswitchp = &vga_con;

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Nicolas Pitre]

On Sat, 22 May 2010, Eric Miao wrote:

> commit edec494b3dbf52a61a18b484dd05cce6a9d90e2f
> Author: Eric Miao <eric.y.miao@gmail.com>
> Date:   Sat May 22 11:58:01 2010 +0800
> 
>     [ARM] allow each machine to specify it's own IRQ handlers at run-time
> 
>     Normally different ARM platform has different way to decode the IRQ
>     hardware status and demultiplex to the corresponding IRQ handler.
>     This is highly optimized by macro irq_handler in entry-armv.S, and
>     each machine class defines their own macro to decode the IRQ number.
>     However, this prevents multiple machine classes to be built into a
>     single kernel.
> 
>     By allowing each machine to specify thier own handler, and making
>     function pointer 'handle_arch_irq' to point to it at run time, this
>     can be solved. And introduce CONFIG_MULTI_IRQ_HANDLER to allow both
>     solutions to work.
> 
>     Comparing with the highly optimized macro of irq_handler, the new
>     function must be written with care not to lose too much performance.
> 
>     Signed-off-by: Eric Miao <eric.y.miao@gmail.com>

Looks fine to me.

Might be a good idea to mention in the patch description text that the 
IPI stuff on SMP is expected to move to the provided arch IRQ handler as 
well.  That would probably be a good thing given the current comment 
above that code.


> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index ddf5676..a8f5f78 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -958,6 +958,11 @@ config CPU_HAS_PMU
>  	default y
>  	bool
> 
> +config MULTI_IRQ_HANDLER
> +	bool
> +	help
> +	  Allow each machine to specify it's own IRQ handler at run time.
> +
>  if !MMU
>  source "arch/arm/Kconfig-nommu"
>  endif
> diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
> index 5ee6f85..b611895 100644
> --- a/arch/arm/include/asm/mach/arch.h
> +++ b/arch/arm/include/asm/mach/arch.h
> @@ -42,6 +42,9 @@ struct machine_desc {
>  	void			(*init_irq)(void);
>  	struct sys_timer	*timer;		/* system tick timer	*/
>  	void			(*init_machine)(void);
> +#ifdef CONFIG_MULTI_IRQ_HANDLER
> +	void			(*handle_irq)(struct pt_regs *);
> +#endif
>  };
> 
>  /*
> diff --git a/arch/arm/include/asm/mach/irq.h b/arch/arm/include/asm/mach/irq.h
> index ce3eee9..6ecdad9 100644
> --- a/arch/arm/include/asm/mach/irq.h
> +++ b/arch/arm/include/asm/mach/irq.h
> @@ -22,6 +22,10 @@ extern void (*init_arch_irq)(void);
>  extern void init_FIQ(void);
>  extern int show_fiq_list(struct seq_file *, void *);
> 
> +#ifdef CONFIG_MULTI_IRQ_HANDLER
> +extern void (*handle_arch_irq)(struct pt_regs *);
> +#endif
> +
>  /*
>   * This is for easy migration, but should be changed in the source
>   */
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index 7ee48e7..ecb8492 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -28,6 +28,7 @@
>  /*
>   * Interrupt handling.  Preserves r7, r8, r9
>   */
> +#ifndef CONFIG_MULTI_IRQ_HANDLER
>  	.macro	irq_handler
>  	get_irqnr_preamble r5, lr
>  1:	get_irqnr_and_base r0, r6, r5, lr
> @@ -59,6 +60,14 @@
>  #endif
> 
>  	.endm
> +#else
> +	.macro	irq_handler
> +	ldr	r4, =handle_arch_irq
> +	mov	r0, sp
> +	mov	lr, pc
> +	ldr	pc, [r4]
> +	.endm
> +#endif	/* CONFIG_MULTI_IRQ_HANDLER */
> 
>  #ifdef CONFIG_KPROBES
>  	.section	.kprobes.text,"ax",%progbits
> @@ -1251,3 +1260,9 @@ cr_alignment:
>  	.space	4
>  cr_no_alignment:
>  	.space	4
> +
> +#ifdef CONFIG_MULTI_IRQ_HANDLER
> +	.globl	handle_arch_irq
> +handle_arch_irq:
> +	.long	0
> +#endif
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index f1aee0d..61d5f8d 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -732,6 +732,10 @@ void __init setup_arch(char **cmdline_p)
>  	system_timer = mdesc->timer;
>  	init_machine = mdesc->init_machine;
> 
> +#ifdef CONFIG_MULTI_IRQ_HANDLER
> +	handle_arch_irq = mdesc->handle_irq;
> +#endif
> +
>  #ifdef CONFIG_VT
>  #if defined(CONFIG_VGA_CONSOLE)
>  	conswitchp = &vga_con;
> 

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Eric Miao]

On Sun, May 23, 2010 at 9:15 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Sat, 22 May 2010, Eric Miao wrote:
>
>> commit edec494b3dbf52a61a18b484dd05cce6a9d90e2f
>> Author: Eric Miao <eric.y.miao@gmail.com>
>> Date: ? Sat May 22 11:58:01 2010 +0800
>>
>> ? ? [ARM] allow each machine to specify it's own IRQ handlers at run-time
>>
>> ? ? Normally different ARM platform has different way to decode the IRQ
>> ? ? hardware status and demultiplex to the corresponding IRQ handler.
>> ? ? This is highly optimized by macro irq_handler in entry-armv.S, and
>> ? ? each machine class defines their own macro to decode the IRQ number.
>> ? ? However, this prevents multiple machine classes to be built into a
>> ? ? single kernel.
>>
>> ? ? By allowing each machine to specify thier own handler, and making
>> ? ? function pointer 'handle_arch_irq' to point to it at run time, this
>> ? ? can be solved. And introduce CONFIG_MULTI_IRQ_HANDLER to allow both
>> ? ? solutions to work.
>>
>> ? ? Comparing with the highly optimized macro of irq_handler, the new
>> ? ? function must be written with care not to lose too much performance.
>>
>> ? ? Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
>
> Looks fine to me.
>

I took that as an Ack-by.

> Might be a good idea to mention in the patch description text that the
> IPI stuff on SMP is expected to move to the provided arch IRQ handler as
> well. ?That would probably be a good thing given the current comment
> above that code.
>

Patch updated as below, and will send to Russell's patch system soon.

commit 01d49a963aee4b598de17f2542f45bd5a54191b3
Author: Eric Miao <eric.miao@canonical.com>
Date:   Sat May 22 11:58:01 2010 +0800

    [ARM] allow each machine to specify it's own IRQ handlers at run-time

    Normally different ARM platform has different way to decode the IRQ
    hardware status and demultiplex to the corresponding IRQ handler.
    This is highly optimized by macro irq_handler in entry-armv.S, and
    each machine class defines their own macro to decode the IRQ number.
    However, this prevents multiple machine classes to be built into a
    single kernel.

    By allowing each machine to specify thier own handler, and making
    function pointer 'handle_arch_irq' to point to it at run time, this
    can be solved. And introduce CONFIG_MULTI_IRQ_HANDLER to allow both
    solutions to work.

    Comparing with the highly optimized macro of irq_handler, the new
    function must be written with care not to lose too much performance.
    And the IPI stuff on SMP is expected to move to the provided arch
    IRQ handler as well.

    Signed-off-by: Eric Miao <eric.miao@canonical.com>
    Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 4ab7523..2dab426 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -959,6 +959,11 @@ config CPU_HAS_PMU
 	default y
 	bool

+config MULTI_IRQ_HANDLER
+	bool
+	help
+	  Allow each machine to specify it's own IRQ handler at run time.
+
 if !MMU
 source "arch/arm/Kconfig-nommu"
 endif
diff --git a/arch/arm/include/asm/mach/arch.h b/arch/arm/include/asm/mach/arch.h
index dd15c0f..73afd4d 100644
--- a/arch/arm/include/asm/mach/arch.h
+++ b/arch/arm/include/asm/mach/arch.h
@@ -43,6 +43,9 @@ struct machine_desc {
 	void			(*init_irq)(void);
 	struct sys_timer	*timer;		/* system tick timer	*/
 	void			(*init_machine)(void);
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+	void			(*handle_irq)(struct pt_regs *);
+#endif
 };

 /*
diff --git a/arch/arm/include/asm/mach/irq.h b/arch/arm/include/asm/mach/irq.h
index ce3eee9..6ecdad9 100644
--- a/arch/arm/include/asm/mach/irq.h
+++ b/arch/arm/include/asm/mach/irq.h
@@ -22,6 +22,10 @@ extern void (*init_arch_irq)(void);
 extern void init_FIQ(void);
 extern int show_fiq_list(struct seq_file *, void *);

+#ifdef CONFIG_MULTI_IRQ_HANDLER
+extern void (*handle_arch_irq)(struct pt_regs *);
+#endif
+
 /*
  * This is for easy migration, but should be changed in the source
  */
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 7ee48e7..ecb8492 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -28,6 +28,7 @@
 /*
  * Interrupt handling.  Preserves r7, r8, r9
  */
+#ifndef CONFIG_MULTI_IRQ_HANDLER
 	.macro	irq_handler
 	get_irqnr_preamble r5, lr
 1:	get_irqnr_and_base r0, r6, r5, lr
@@ -59,6 +60,14 @@
 #endif

 	.endm
+#else
+	.macro	irq_handler
+	ldr	r4, =handle_arch_irq
+	mov	r0, sp
+	mov	lr, pc
+	ldr	pc, [r4]
+	.endm
+#endif	/* CONFIG_MULTI_IRQ_HANDLER */

 #ifdef CONFIG_KPROBES
 	.section	.kprobes.text,"ax",%progbits
@@ -1251,3 +1260,9 @@ cr_alignment:
 	.space	4
 cr_no_alignment:
 	.space	4
+
+#ifdef CONFIG_MULTI_IRQ_HANDLER
+	.globl	handle_arch_irq
+handle_arch_irq:
+	.long	0
+#endif
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index a70c8b7..bcccee6 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -736,6 +736,10 @@ void __init setup_arch(char **cmdline_p)
 	system_timer = mdesc->timer;
 	init_machine = mdesc->init_machine;

+#ifdef CONFIG_MULTI_IRQ_HANDLER
+	handle_arch_irq = mdesc->handle_irq;
+#endif
+
 #ifdef CONFIG_VT
 #if defined(CONFIG_VGA_CONSOLE)
 	conswitchp = &vga_con;

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Uwe Kleine-König]

Hello Eric,

On Sat, May 22, 2010 at 12:23:29PM +0800, Eric Miao wrote:
> commit edec494b3dbf52a61a18b484dd05cce6a9d90e2f
> Author: Eric Miao <eric.y.miao@gmail.com>
> Date:   Sat May 22 11:58:01 2010 +0800
> 
>     [ARM] allow each machine to specify it's own IRQ handlers at run-time
> 
>     Normally different ARM platform has different way to decode the IRQ
>     hardware status and demultiplex to the corresponding IRQ handler.
>     This is highly optimized by macro irq_handler in entry-armv.S, and
>     each machine class defines their own macro to decode the IRQ number.
>     However, this prevents multiple machine classes to be built into a
>     single kernel.
> 
>     By allowing each machine to specify thier own handler, and making
>     function pointer 'handle_arch_irq' to point to it at run time, this
>     can be solved. And introduce CONFIG_MULTI_IRQ_HANDLER to allow both
>     solutions to work.
> 
>     Comparing with the highly optimized macro of irq_handler, the new
>     function must be written with care not to lose too much performance.
> 
>     Signed-off-by: Eric Miao <eric.y.miao@gmail.com>
I didn't thought it would be that easy.  Nice.
Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

Best regards
Uwe

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Russell King - ARM Linux]

On Mon, Jul 12, 2010 at 10:11:06AM +0200, Uwe Kleine-K?nig wrote:
> I didn't thought it would be that easy.  Nice.
> Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

It isn't - if you enable this, your platform will break because
there's no implementations of it.  That makes this patch rather
hard to ack on its own.

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Eric Miao]

2010/7/12 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Mon, Jul 12, 2010 at 10:11:06AM +0200, Uwe Kleine-K?nig wrote:
>> I didn't thought it would be that easy. ?Nice.
>> Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>
> It isn't - if you enable this, your platform will break because
> there's no implementations of it. ?That makes this patch rather
> hard to ack on its own.
>

The idea is for platforms supporting its own IRQ handler to have an
implementation before it's selected in Kconfig, if that makes sense
for this to be standalone?

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Nicolas Pitre]

On Mon, 12 Jul 2010, Russell King - ARM Linux wrote:

> On Mon, Jul 12, 2010 at 10:11:06AM +0200, Uwe Kleine-K?nig wrote:
> > I didn't thought it would be that easy.  Nice.
> > Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> 
> It isn't - if you enable this, your platform will break because
> there's no implementations of it.  That makes this patch rather
> hard to ack on its own.

I think Eric posted additional patches to enable this on PXA in his 
series.


Nicolas

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Uwe Kleine-König]

On Mon, Jul 12, 2010 at 09:12:45AM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 12, 2010 at 10:11:06AM +0200, Uwe Kleine-K?nig wrote:
> > I didn't thought it would be that easy.  Nice.
> > Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> 
> It isn't - if you enable this, your platform will break because
> there's no implementations of it.  That makes this patch rather
> hard to ack on its own.
Well, I didn't test the patch but still noticed that additional code is
needed to make use of it.  And what I saw looked reasonable.  As the
Kconfig symbol isn't selected (until now) it doesn't seem to break
anything.

Does I need more to give my Ack?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[RFC PATCH 1/2] allow each machine to specify it's own IRQ handlers at run-time

[Nicolas Pitre]

On Mon, 12 Jul 2010, Uwe Kleine-K?nig wrote:

> On Mon, Jul 12, 2010 at 09:12:45AM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jul 12, 2010 at 10:11:06AM +0200, Uwe Kleine-K?nig wrote:
> > > I didn't thought it would be that easy.  Nice.
> > > Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > 
> > It isn't - if you enable this, your platform will break because
> > there's no implementations of it.  That makes this patch rather
> > hard to ack on its own.
> Well, I didn't test the patch but still noticed that additional code is
> needed to make use of it.  And what I saw looked reasonable.  As the
> Kconfig symbol isn't selected (until now) it doesn't seem to break
> anything.
> 
> Does I need more to give my Ack?

FYI, I'm NAKing the patch for the moment.  Please see the comment I'm 
about to post.


Nicolas