All archs: NO_IRQ definition

All archs: NO_IRQ definition

[Benjamin Herrenschmidt]

Hi !

I need to fix a few wrong cases where driver assume that irq = 0 means
no interrupt. This is wrong, some arch have pefectly valid IRQ 0. We
started defining NO_IRQ a while ago for that reason, but it seems lot
of archs didn't catch up. I plan to collect a patch fixing them all but
you guys will have to tell me what value to use for NO_IRQ (0 ? -1 ?).

Currently, only ppc64, arm and mips add it, and I know what to do for
ppc, that leaves me with request for the proper value to use for those
archs:

alpha
arm
arm26
cris
h8300
i386
ia64
m32r
m68k
m68knommu
parisc
sh
sh64
sparc
sparc64
um
v850
x86_64

Thanks ! 

Ben.

Re: All archs: NO_IRQ definition

[Matthew Wilcox]

On Tue, Oct 19, 2004 at 04:57:57PM +1000, Benjamin Herrenschmidt wrote:
> parisc

-1 for parisc, please.

BTW, we could just force everyone to add it themselves by merging this
patch:

Index: drivers/serial/serial_core.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/serial/serial_core.c,v
retrieving revision 1.14.2.1
retrieving revision 1.14
diff -u -p -r1.14.2.1 -r1.14
--- drivers/serial/serial_core.c        19 Oct 2004 00:01:34 -0000      1.14.2.1
+++ drivers/serial/serial_core.c        11 Oct 2004 21:41:17 -0000      1.14
@@ -230,7 +230,8 @@ static void uart_shutdown(struct uart_st
        /*
         * Ensure that the IRQ handler isn't running on another CPU.
         */
-       synchronize_irq(port->irq);
+       if (port->irq != IRQ_NONE)
+               synchronize_irq(port->irq);
 
        /*
         * Free the transmit buffer page.
@@ -1972,7 +1973,11 @@ uart_report_port(struct uart_driver *drv
                printk("MMIO 0x%lx", port->mapbase);
                break;
        }
-       printk(" (irq = %d) is a %s\n", port->irq, uart_type(port));
+       if (port->irq == IRQ_NONE) {
+               printk(" (polled) is a %s\n", uart_type(port));
+       } else {
+               printk(" (irq = %d) is a %s\n", port->irq, uart_type(port));
+       }
 }
 
 static void


-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: All archs: NO_IRQ definition

[Russell King]

On Tue, Oct 19, 2004 at 10:22:38AM +0100, Matthew Wilcox wrote:
> BTW, we could just force everyone to add it themselves by merging this
> patch:

IRQ_NONE?  What's that?  We're talking about NO_IRQ here.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: All archs: NO_IRQ definition

[Matthew Wilcox]

On Tue, Oct 19, 2004 at 10:23:59AM +0100, Russell King wrote:
> On Tue, Oct 19, 2004 at 10:22:38AM +0100, Matthew Wilcox wrote:
> > BTW, we could just force everyone to add it themselves by merging this
> > patch:
> 
> IRQ_NONE?  What's that?  We're talking about NO_IRQ here.

Er, sorry.  Hand-edited patch (it's currently SERIAL_IRQ_NONE in my tree)
and insomnia aren't a great mix.  Let's all assume I meant NO_IRQ.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: All archs: NO_IRQ definition

[David Woodhouse]

On Tue, 2004-10-19 at 10:22 +0100, Matthew Wilcox wrote:
> On Tue, Oct 19, 2004 at 04:57:57PM +1000, Benjamin Herrenschmidt wrote:
> > parisc
> 
> -1 for parisc, please.

And also for every arch where there isn't an overriding reason for it
_not_ to be -1. If either zero or -1 works, use -1. We'll get better
coverage of random driver authors doing 'if (!irq)' that way. 

So your question becomes: anyone whose architecture can't cope with
NO_IRQ being -1, please shout now before we break it.

> BTW, we could just force everyone to add it themselves by merging this
> patch:

Do it.

-- 
dwmw2

Re: All archs: NO_IRQ definition

[Geert Uytterhoeven]

On Tue, 19 Oct 2004, David Woodhouse wrote:
> On Tue, 2004-10-19 at 10:22 +0100, Matthew Wilcox wrote:
> > On Tue, Oct 19, 2004 at 04:57:57PM +1000, Benjamin Herrenschmidt wrote:
> > > parisc
> > 
> > -1 for parisc, please.
> 
> And also for every arch where there isn't an overriding reason for it
> _not_ to be -1. If either zero or -1 works, use -1. We'll get better
> coverage of random driver authors doing 'if (!irq)' that way. 
> 
> So your question becomes: anyone whose architecture can't cope with
> NO_IRQ being -1, please shout now before we break it.
> 
> > BTW, we could just force everyone to add it themselves by merging this
> > patch:
> 
> Do it.

m68k starts counting interrupt numbers from 1 (probably we mimiced the ia32
port too closely in the early nineties :-), so you can use either 0 or -1.

Let's use -1.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: All archs: NO_IRQ definition

[Benjamin Herrenschmidt]

On Tue, 2004-10-19 at 19:27, David Woodhouse wrote:

> And also for every arch where there isn't an overriding reason for it
> _not_ to be -1. If either zero or -1 works, use -1. We'll get better
> coverage of random driver authors doing 'if (!irq)' that way. 
> 
> So your question becomes: anyone whose architecture can't cope with
> NO_IRQ being -1, please shout now before we break it.

There is a problem using -1 that I noticed when converting drivers/ide.

The probe_irq_off() thing (that we never implemented on ppc, but I
suppose we'll inherit of it via mingo patches now) is defined as...
returning 0 when no interrupt is found. Some drivers actually rely
on that.

So what should I do ? Fix probe_irq_off() & friends to return NO_IRQ
and fix as many drivers as I can find ?

Ben.

Re: All archs: NO_IRQ definition

[David Woodhouse]

On Wed, 2004-10-20 at 13:26 +1000, Benjamin Herrenschmidt wrote:
> So what should I do ? Fix probe_irq_off() & friends to return NO_IRQ
> and fix as many drivers as I can find ?

Yes, I think so.

-- 
dwmw2

Re: All archs: NO_IRQ definition

[Russell King]

On Wed, Oct 20, 2004 at 01:26:59PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2004-10-19 at 19:27, David Woodhouse wrote:
> 
> > And also for every arch where there isn't an overriding reason for it
> > _not_ to be -1. If either zero or -1 works, use -1. We'll get better
> > coverage of random driver authors doing 'if (!irq)' that way. 
> > 
> > So your question becomes: anyone whose architecture can't cope with
> > NO_IRQ being -1, please shout now before we break it.
> 
> There is a problem using -1 that I noticed when converting drivers/ide.
> 
> The probe_irq_off() thing (that we never implemented on ppc, but I
> suppose we'll inherit of it via mingo patches now) is defined as...
> returning 0 when no interrupt is found. Some drivers actually rely
> on that.

That's a bit of a problem - negative numbers means that an interrupt
was detected, but that other interrupts also triggered...  However,
I don't think any driver actually makes use of this information.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: All archs: NO_IRQ definition

[Benjamin Herrenschmidt]

On Wed, 2004-10-20 at 18:13, Russell King wrote:

> That's a bit of a problem - negative numbers means that an interrupt
> was detected, but that other interrupts also triggered...  However,
> I don't think any driver actually makes use of this information.

Well, I hate that probe thing ... note that the current bk blows up
on me at boot with yenta_socket trying to do similar probing (hrm...
it should have a perfectly working PCI irq on ppc, and nothing else)
and the new "common" code actually implements that probe stuff... and
ends up calling a NULL function pointer. I'll have to check that out
too.

Anyway, I'll have a patch tonight or tomorrow after reviewing all
drivers using that interface. I'll also fix IDE & 8250 to use NO_IRQ,
I'll leave other drivers alone.

Ben.

Re: All archs: NO_IRQ definition

[Ralf Baechle]

On Tue, Oct 19, 2004 at 04:57:57PM +1000, Benjamin Herrenschmidt wrote:

> I need to fix a few wrong cases where driver assume that irq = 0 means
> no interrupt. This is wrong, some arch have pefectly valid IRQ 0. We
> started defining NO_IRQ a while ago for that reason, but it seems lot
> of archs didn't catch up. I plan to collect a patch fixing them all but
> you guys will have to tell me what value to use for NO_IRQ (0 ? -1 ?).
> 
> Currently, only ppc64, arm and mips add it, and I know what to do for
> ppc, that leaves me with request for the proper value to use for those
> archs:

I guess your grep must have picked up SGI_VINO_IRQ by accident - MIPS
doesn't define NO_IRQ.  0 is used as a normal interrupt number on MIPS,
so -1 is the prefered value for NO_IRQ.

  Ralf