Re: [PATCH] Fix irqpoll on IA64 (timer interrupt != 0)

[Bernhard Walle <bwalle@suse.de>]

* Andrew Morton <akpm@linux-foundation.org> [2007-03-22 22:09]:
> On Tue, 20 Mar 2007 16:00:27 +0100
> Bernhard Walle <bwalle@suse.de> wrote:
> 
> > On IA64, the timer interrupt is not (always?) zero as it is on x86 platforms.
> > Also, the timer interrupt is CPU-local. Two things need to be changed to make
> > the irqpoll option make also working on IA64:
> > 
> >   o Call note_interrupt() also on CPU-local interrupts in __do_IRQ().
> >   o Set a variable timer_irq to the value of the timer interrupt
> >     after the timer interrupt has been registered and assigned.
> > 
> > That requires changes in Linux-generic files. The default of timer_irq is 0, so
> > the patch doesn't break i386/x86_64. However, other platforms also may also
> > have a timer interrupt non-equal to zero, so they can also use the new
> > set_timer_interrupt() function.
> 
> Couple of things..
> 
> I think the term 'timer_interrupt' is a bit generic-sounding.  Would it be
> better to call it irqpoll_interrupt?  After all, some architecture might
> want to use, umm, the keyboard interrupt to trigger IRQ polling ;)  

Well, the documentation of irqpoll says that it's called on the timer
interrupt. But maybe also the documentation should be changed. :)

> Also, the code presently passes the magic IRQ number into the generic IRQ
> code.  I wonder if we'd get a more pleasing result if we were to make the
> generic IRQ code call _out_ to the architecture:

I think I have a third solution. There's already IRQF_TIMER, and
that's used and defined in a few architectures already (like Sh), so
why not simply use that here. Maybe we should introduce IRQF_IRQPOLL,
but the concept would be the same. If we don't take care of shared
interrupt handlers (and I think sharing the timer interrupt is a
_very_ bad idea, are there architectures that do this?), the could
would look like this (+ the change in __do_IRQ).

What's your opinion on this approach? Of course, we would have to make
sure that IRQF_TIMER is defined on _every_ architectures, but that
would give us the chance to find out each architecture that also has a
timer interrupt != 0.


Index: mainline-msi-init/arch/ia64/kernel/time.c
===================================================================
--- mainline-msi-init.orig/arch/ia64/kernel/time.c
+++ mainline-msi-init/arch/ia64/kernel/time.c
@@ -235,7 +235,7 @@ ia64_init_itm (void)
 
 static struct irqaction timer_irqaction = {
 	.handler =	timer_interrupt,
-	.flags =	IRQF_DISABLED,
+	.flags =	IRQF_DISABLED | IRQF_TIMER,
 	.name =		"timer"
 };
 
Index: mainline-msi-init/arch/x86_64/kernel/time.c
===================================================================
--- mainline-msi-init.orig/arch/x86_64/kernel/time.c
+++ mainline-msi-init/arch/x86_64/kernel/time.c
@@ -320,7 +320,7 @@ void __init stop_timer_interrupt(void)
 }
 
 static struct irqaction irq0 = {
-	timer_interrupt, IRQF_DISABLED, CPU_MASK_NONE, "timer", NULL, NULL
+	timer_interrupt, IRQF_DISABLED | IRQF_TIMER, CPU_MASK_NONE, "timer", NULL, NULL
 };
 
 void __init time_init(void)
Index: mainline-msi-init/kernel/irq/spurious.c
===================================================================
--- mainline-msi-init.orig/kernel/irq/spurious.c
+++ mainline-msi-init/kernel/irq/spurious.c
@@ -146,7 +146,7 @@ void note_interrupt(unsigned int irq, st
 
 	if (unlikely(irqfixup)) {
 		/* Don't punish working computers */
-		if ((irqfixup == 2 && irq == 0) || action_ret == IRQ_NONE) {
+		if ((irqfixup == 2 && (desc->action->flags & IRQF_TIMER)) || action_ret == IRQ_NONE) {
 			int ok = misrouted_irq(irq);
 			if (action_ret == IRQ_NONE)
 				desc->irqs_unhandled -= ok;