* [PATCH 4/5] Centralise NO_IRQ definition
@ 2005-11-21 1:14 Matthew Wilcox
2005-11-21 11:12 ` David Howells
0 siblings, 1 reply; 42+ messages in thread
From: Matthew Wilcox @ 2005-11-21 1:14 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: Matthew Wilcox, Ingo Molnar, linux-kernel, Russell King,
Ian Molton, David Howells, Benjamin Herrenschmidt,
Paul Mackerras
Move the definition of NO_IRQ from asm directories to <linux/hardirq.h>.
All existing definitions were already -1.
Signed-off-by: Matthew Wilcox <matthew@wil.cx>
Acked-by: Ingo Molnar <mingo@elte.hu>
---
include/asm-arm/irq.h | 8 --------
include/asm-arm26/irq.h | 8 --------
include/asm-frv/irq.h | 3 ---
include/asm-parisc/irq.h | 2 --
include/asm-powerpc/irq.h | 3 ---
include/linux/hardirq.h | 8 ++++++++
6 files changed, 8 insertions(+), 24 deletions(-)
applies-to: c2fd3b28a74be2ae6274c85c7a9538c7bf1dd949
604c0538936822cbb1c0958f0f835cbbaf723530
diff --git a/include/asm-arm/irq.h b/include/asm-arm/irq.h
index 59975ee..0545e36 100644
--- a/include/asm-arm/irq.h
+++ b/include/asm-arm/irq.h
@@ -11,14 +11,6 @@
#define NR_IRQS 128
#endif
-/*
- * Use this value to indicate lack of interrupt
- * capability
- */
-#ifndef NO_IRQ
-#define NO_IRQ ((unsigned int)(-1))
-#endif
-
struct irqaction;
extern void disable_irq_nosync(unsigned int);
diff --git a/include/asm-arm26/irq.h b/include/asm-arm26/irq.h
index 06bd5a5..7957d4e 100644
--- a/include/asm-arm26/irq.h
+++ b/include/asm-arm26/irq.h
@@ -14,14 +14,6 @@
#endif
-/*
- * Use this value to indicate lack of interrupt
- * capability
- */
-#ifndef NO_IRQ
-#define NO_IRQ ((unsigned int)(-1))
-#endif
-
struct irqaction;
#define disable_irq_nosync(i) disable_irq(i)
diff --git a/include/asm-frv/irq.h b/include/asm-frv/irq.h
index 2c16d8d..fbc5bd7 100644
--- a/include/asm-frv/irq.h
+++ b/include/asm-frv/irq.h
@@ -20,9 +20,6 @@
* drivers
*/
-/* this number is used when no interrupt has been assigned */
-#define NO_IRQ (-1)
-
#define NR_IRQ_LOG2_ACTIONS_PER_GROUP 5
#define NR_IRQ_ACTIONS_PER_GROUP (1 << NR_IRQ_LOG2_ACTIONS_PER_GROUP)
#define NR_IRQ_GROUPS 4
diff --git a/include/asm-parisc/irq.h b/include/asm-parisc/irq.h
index b0a30e2..26ab3be 100644
--- a/include/asm-parisc/irq.h
+++ b/include/asm-parisc/irq.h
@@ -11,8 +11,6 @@
#include <linux/cpumask.h>
#include <asm/types.h>
-#define NO_IRQ (-1)
-
#ifdef CONFIG_GSC
#define GSC_IRQ_BASE 16
#define GSC_IRQ_MAX 63
diff --git a/include/asm-powerpc/irq.h b/include/asm-powerpc/irq.h
index 8eb7e85..1b41206 100644
--- a/include/asm-powerpc/irq.h
+++ b/include/asm-powerpc/irq.h
@@ -15,9 +15,6 @@
#include <asm/types.h>
#include <asm/atomic.h>
-/* this number is used when no interrupt has been assigned */
-#define NO_IRQ (-1)
-
/*
* These constants are used for passing information about interrupt
* signal polarity and level/edge sensing to the low-level PIC chip
diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h
index 71d2b8a..9ee4946 100644
--- a/include/linux/hardirq.h
+++ b/include/linux/hardirq.h
@@ -87,6 +87,14 @@ extern void synchronize_irq(unsigned int
# define synchronize_irq(irq) barrier()
#endif
+/*
+ * This value means "Device has no interrupt". For most pieces of code,
+ * any value above NR_IRQS would do, but -1 is traditional. The PCI
+ * subsystem currently uses 0, but that's a legal IRQ number on some
+ * architectures.
+ */
+#define NO_IRQ ((unsigned int)(-1))
+
#define nmi_enter() irq_enter()
#define nmi_exit() sub_preempt_count(HARDIRQ_OFFSET)
---
0.99.8.GIT
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 1:14 [PATCH 4/5] Centralise NO_IRQ definition Matthew Wilcox
@ 2005-11-21 11:12 ` David Howells
2005-11-21 12:14 ` Matthew Wilcox
0 siblings, 1 reply; 42+ messages in thread
From: David Howells @ 2005-11-21 11:12 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Linus Torvalds, Andrew Morton, Ingo Molnar, linux-kernel,
Russell King, Ian Molton, David Howells, Benjamin Herrenschmidt,
Paul Mackerras
Matthew Wilcox <matthew@wil.cx> wrote:
> +#define NO_IRQ ((unsigned int)(-1))
Should this be wrapped with #ifndef?
David
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 11:12 ` David Howells
@ 2005-11-21 12:14 ` Matthew Wilcox
2005-11-21 18:55 ` Linus Torvalds
0 siblings, 1 reply; 42+ messages in thread
From: Matthew Wilcox @ 2005-11-21 12:14 UTC (permalink / raw)
To: David Howells
Cc: Linus Torvalds, Andrew Morton, Ingo Molnar, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt, Paul Mackerras
On Mon, Nov 21, 2005 at 11:12:36AM +0000, David Howells wrote:
> Matthew Wilcox <matthew@wil.cx> wrote:
>
> > +#define NO_IRQ ((unsigned int)(-1))
>
> Should this be wrapped with #ifndef?
*sigh*. The one piece of feedback I got on the last series was from
Ingo, and he asked that I *not* wrap it with ifndef. So, no.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 12:14 ` Matthew Wilcox
@ 2005-11-21 18:55 ` Linus Torvalds
2005-11-21 19:06 ` Matthew Wilcox
0 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 18:55 UTC (permalink / raw)
To: Matthew Wilcox
Cc: David Howells, Andrew Morton, Ingo Molnar, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt, Paul Mackerras
On Mon, 21 Nov 2005, Matthew Wilcox wrote:
>
> On Mon, Nov 21, 2005 at 11:12:36AM +0000, David Howells wrote:
> > Matthew Wilcox <matthew@wil.cx> wrote:
> >
> > > +#define NO_IRQ ((unsigned int)(-1))
> >
> > Should this be wrapped with #ifndef?
>
> *sigh*. The one piece of feedback I got on the last series was from
> Ingo, and he asked that I *not* wrap it with ifndef. So, no.
Quite frankly, if we change [PCI_]NO_IRQ to -1, there's almost certainly
going to be a lot of drivers breaking.
On x86, 0 has been the lack of IRQ since basically forever, in one form or
another (for devices, that is - there _is_ a timer irq 0, but that's set
up separately).
Which means that I'd _much_ prefer to other architectures to just follow
suit.
So for an architecture where irq0 is a valid physical interrupt for a
device, why not just translate that "real irq 0" into some other thing?
Much less likely to break anything.
In fact, it could be as simple as setting the high bit for any valid
interrupt, and then just masking it out in request_irq() and friends. Do
something like
#define PCI_IRQ_VALID_MASK (1u << 31)
and then if the physical irq is 0, just or in that VALID mask, turning the
interrupt into a non-zero, and then when registering it, just and it away
again..
This is not theory: a _lot_ of real-life PCI devices very much think that
irq 0 means "disabled". Not even just in drivers - in actual _hardware_.
When you write 0 to the irq number for irq routers, they disable that
line. So the "zero as NO_IRQ" is more than just a "several drivers think
that is how it is", it's how a lot of hardware actually works.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 18:55 ` Linus Torvalds
@ 2005-11-21 19:06 ` Matthew Wilcox
2005-11-21 19:27 ` Linus Torvalds
0 siblings, 1 reply; 42+ messages in thread
From: Matthew Wilcox @ 2005-11-21 19:06 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Howells, Andrew Morton, Ingo Molnar, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt, Paul Mackerras
On Mon, Nov 21, 2005 at 10:55:24AM -0800, Linus Torvalds wrote:
> Quite frankly, if we change [PCI_]NO_IRQ to -1, there's almost certainly
> going to be a lot of drivers breaking.
There's only one driver using NO_IRQ today (outside of architectures
which define NO_IRQ to -1, that is). So *this* series of patches should
break nothing. The next series of patches should find all the PCI
drivers assuming dev->irq == 0 means no interrupt, and then we try to
break drivers by getting rid of PCI_NO_IRQ.
> This is not theory: a _lot_ of real-life PCI devices very much think that
> irq 0 means "disabled". Not even just in drivers - in actual _hardware_.
> When you write 0 to the irq number for irq routers, they disable that
> line. So the "zero as NO_IRQ" is more than just a "several drivers think
> that is how it is", it's how a lot of hardware actually works.
That's a common misreading of the PCI spec -- it actually says the
opposite. Interrupt Pin works as you've described. But what we're
concerned with is Interrupt Line, and PCI 2.3 has the following to say
in a footnote:
43 For x86 based PCs, the values in this register correspond to
IRQ numbers (0-15) of the standard dual 8259 configuration. The
value 255 is defined as meaning "unknown" or "no connection" to
the interrupt controller. Values between 15 and 254 are reserved.
It's just that Linux doesn't bother to read Interrupt Line if Interrupt
Pin is 0, and thus leaves the interrupt value at 0 (since the struct was
memset).
Really, this patch brings x86 back into compliance with what the
BIOS does ;-)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 19:06 ` Matthew Wilcox
@ 2005-11-21 19:27 ` Linus Torvalds
2005-11-21 19:43 ` Matthew Wilcox
0 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 19:27 UTC (permalink / raw)
To: Matthew Wilcox
Cc: David Howells, Andrew Morton, Ingo Molnar, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt, Paul Mackerras
On Mon, 21 Nov 2005, Matthew Wilcox wrote:
>
> On Mon, Nov 21, 2005 at 10:55:24AM -0800, Linus Torvalds wrote:
> > Quite frankly, if we change [PCI_]NO_IRQ to -1, there's almost certainly
> > going to be a lot of drivers breaking.
>
> There's only one driver using NO_IRQ today (outside of architectures
> which define NO_IRQ to -1, that is). So *this* series of patches should
> break nothing.
Right. But the point is that most drivers will do something like
if (!dev->irq)
return;
(whatever, made up). And that having NO_IRQ be anything but 0 is thus
fundamentally broken.
Because the fact is, NO_IRQ _is_ zero. And exactly because it's zero, and
that is encoded everywhere, nobody uses it.
> That's a common misreading of the PCI spec -- it actually says the
> opposite.
I'm NOT talking about PCI specs.
I'm talking about real hardware.
Read pretty much _any_ data-sheet for an interrupt router, and you'll see
that the bit pattern 0000 means _disabled_.
In other words, I'm talking about HARD REALITY.
I know, it's a bitch.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 19:27 ` Linus Torvalds
@ 2005-11-21 19:43 ` Matthew Wilcox
2005-11-21 19:59 ` Linus Torvalds
0 siblings, 1 reply; 42+ messages in thread
From: Matthew Wilcox @ 2005-11-21 19:43 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Howells, Andrew Morton, Ingo Molnar, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt, Paul Mackerras
On Mon, Nov 21, 2005 at 11:27:05AM -0800, Linus Torvalds wrote:
> On Mon, 21 Nov 2005, Matthew Wilcox wrote:
> >
> > On Mon, Nov 21, 2005 at 10:55:24AM -0800, Linus Torvalds wrote:
> > > Quite frankly, if we change [PCI_]NO_IRQ to -1, there's almost certainly
> > > going to be a lot of drivers breaking.
> >
> > There's only one driver using NO_IRQ today (outside of architectures
> > which define NO_IRQ to -1, that is). So *this* series of patches should
> > break nothing.
>
> Right. But the point is that most drivers will do something like
>
> if (!dev->irq)
> return;
>
> (whatever, made up). And that having NO_IRQ be anything but 0 is thus
> fundamentally broken.
The idea was to give them something better to use instead of this.
Whether that be if (has_irq(dev)) return; or some other similar
construct, I'm not terribly fussed.
> I'm NOT talking about PCI specs.
>
> I'm talking about real hardware.
>
> Read pretty much _any_ data-sheet for an interrupt router, and you'll see
> that the bit pattern 0000 means _disabled_.
The only relevant thing I found with google was
http://www.microsoft.com/whdc/archive/pciirq.mspx
Where it talks about 0 meaning disabled, it says:
Link Value for INTn#:A value of zero means this interrupt pin is
not connected to any other interrupt pins and is not connected
to any of the Interrupt Router's interrupt pins.
which is a different bit from where it talks about the AT-compatible
IRQ numbers.
Everything else I find seems to be talking about Arcnet hardware (!)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 19:43 ` Matthew Wilcox
@ 2005-11-21 19:59 ` Linus Torvalds
2005-11-21 21:15 ` Ingo Molnar
2005-11-21 21:16 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 19:59 UTC (permalink / raw)
To: Matthew Wilcox
Cc: David Howells, Andrew Morton, Ingo Molnar, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt, Paul Mackerras
On Mon, 21 Nov 2005, Matthew Wilcox wrote:
> >
> > if (!dev->irq)
> > return;
> >
> > (whatever, made up). And that having NO_IRQ be anything but 0 is thus
> > fundamentally broken.
>
> The idea was to give them something better to use instead of this.
Why?
The above is obvious even to a non-programmer. There _is_ no better thing
from a clarity standpoint.
> The only relevant thing I found with google was
> http://www.microsoft.com/whdc/archive/pciirq.mspx
Look at, for example, any cardbus controller. The way to disable
generation of the legacy interrupt? Write an irq value of 0 to the
interrupt control register.
Or check out the irq routers in arch/i386/pci/irq.c. Every _single_ one of
them does the same. Zero means disabled.
The point I'm trying to make is that there already _is_ a special bit
pattern that means "no irq", and it's what
- the kernel has used for the last 14 years
- is the easiest and most logical one to test for
- real hardware uses
so why not just use it?
Anybody who says that
if (!dev->irq)
return;
isn't logical is just on drugs. It's logical _and_ readable. More so than
any alternative, especially as any alternative will always have the
problem that some people will write the above _anyway_.
If you make NO_IRQ be something else than 0, for safety you should also
make the irq thing be a structure so that the test against zero (which is
special, as _defined_ by the C language) won't work.
At which point you might as well just do something like
struct interrupt_descriptor {
unsigned int nr:31;
unsigned int valid:1;
};
and then people can just say
if (!dev->irq.valid)
return;
instead, which is also readable, and where you simply cannot do the old
"if (!dev->irq)" at all.
The fact is, 0 _is_ special. Not just for hardware, but because 0 has a
magical meaning as "false" in the C language.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 19:59 ` Linus Torvalds
@ 2005-11-21 21:15 ` Ingo Molnar
2005-11-21 21:25 ` Paul Mackerras
2005-11-23 1:45 ` Pavel Machek
2005-11-21 21:16 ` Benjamin Herrenschmidt
1 sibling, 2 replies; 42+ messages in thread
From: Ingo Molnar @ 2005-11-21 21:15 UTC (permalink / raw)
To: Linus Torvalds
Cc: Matthew Wilcox, David Howells, Andrew Morton, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt, Paul Mackerras
* Linus Torvalds <torvalds@osdl.org> wrote:
> At which point you might as well just do something like
>
> struct interrupt_descriptor {
> unsigned int nr:31;
> unsigned int valid:1;
> };
>
> and then people can just say
>
> if (!dev->irq.valid)
> return;
>
> instead, which is also readable, and where you simply cannot do the old
> "if (!dev->irq)" at all.
>
> The fact is, 0 _is_ special. Not just for hardware, but because 0 has
> a magical meaning as "false" in the C language.
yeah, i wanted to suggest this originally, but got distracted by the x86
quirk that 'IRQ#0' is often the i8253 timer interrupt.
is there any architecture where irq 0 is a legitimate setting that could
occur in drivers, and which would make NO_IRQ define of 0 non-practical?
If not (which i think is the case) then we should indeed standardize on
0. (in one way or another) It's not like any real driver will ever have
IRQ#0 even on a PC: the timer IRQ is 'known' to be routed to 0, and we
do a platform-specific setup_irq() on it. Not a good reason to abstract
the notion of 'no irq' away into a define.
in fact we dont even have to do the irq.valid thing, !dev->irq is
obviously readable.
Ingo
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 19:59 ` Linus Torvalds
2005-11-21 21:15 ` Ingo Molnar
@ 2005-11-21 21:16 ` Benjamin Herrenschmidt
2005-11-21 21:38 ` Linus Torvalds
1 sibling, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2005-11-21 21:16 UTC (permalink / raw)
To: Linus Torvalds
Cc: Matthew Wilcox, David Howells, Andrew Morton, Ingo Molnar,
linux-kernel, Russell King, Ian Molton, Paul Mackerras
> The fact is, 0 _is_ special. Not just for hardware, but because 0 has a
> magical meaning as "false" in the C language.
I don't agree, irq 0 has been a valid irq on a number of platforms for
ages (including your own G5, at least some of them have the SATA irq at
0 :) and this didn't cause any problem for most drivers. The few ones
that have done broken assumption have been easily fixed using NO_IRQ.
"Translating" it means some ugly translation work all over the place,
which means overhead in the interrupt path (ok, not that much but
still), plus finding some magic number to put 0 on, which makes things
much more complicated for archs that have interrupts sorted in nice
blocks of power of two, etc... a significant burden on arch/PIC code for
no good reason imho.
I hate arbitrary "translations" when they aren't strictly necessary.
It's common to have a constant for a "not valid" number in spaces where
"0" is a valid value, I don't think that "looking simpler" to dump
driver writers is worth it in this case.
Ben.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:15 ` Ingo Molnar
@ 2005-11-21 21:25 ` Paul Mackerras
2005-11-21 21:35 ` Ingo Molnar
` (3 more replies)
2005-11-23 1:45 ` Pavel Machek
1 sibling, 4 replies; 42+ messages in thread
From: Paul Mackerras @ 2005-11-21 21:25 UTC (permalink / raw)
To: Ingo Molnar
Cc: Linus Torvalds, Matthew Wilcox, David Howells, Andrew Morton,
linux-kernel, Russell King, Ian Molton, Benjamin Herrenschmidt
Ingo Molnar writes:
> is there any architecture where irq 0 is a legitimate setting that could
> occur in drivers, and which would make NO_IRQ define of 0 non-practical?
Yes, G5 powermacs have the SATA controller on irq 0. So if we can't
use irq 0, I can't get to my hard disk. :) Other powermacs also use
irq 0 for various things, as do embedded PPC machines.
Paul.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:25 ` Paul Mackerras
@ 2005-11-21 21:35 ` Ingo Molnar
2005-11-21 21:51 ` Linus Torvalds
2005-11-21 21:49 ` Linus Torvalds
` (2 subsequent siblings)
3 siblings, 1 reply; 42+ messages in thread
From: Ingo Molnar @ 2005-11-21 21:35 UTC (permalink / raw)
To: Paul Mackerras
Cc: Linus Torvalds, Matthew Wilcox, David Howells, Andrew Morton,
linux-kernel, Russell King, Ian Molton, Benjamin Herrenschmidt
* Paul Mackerras <paulus@samba.org> wrote:
> Ingo Molnar writes:
>
> > is there any architecture where irq 0 is a legitimate setting that could
> > occur in drivers, and which would make NO_IRQ define of 0 non-practical?
>
> Yes, G5 powermacs have the SATA controller on irq 0. So if we can't
> use irq 0, I can't get to my hard disk. :) Other powermacs also use
> irq 0 for various things, as do embedded PPC machines.
oh well [*]. Then it's gotta be the !dev->irq.valid thing i guess. OTOH
that has some disadvantages too: any normal access to dev->irq.nr will
mean implicit 0xffffffff (or 0xffffffffffffffff) masking generated by
the compiler. Also, unless there's some compiler trick, tons of drivers
will be affected - because dev->irq isnt valid anymore. A quick grep
suggests 5381 lines of code affected, spread out in 917 files. Quite
impractical. So we are back to square one and Matthew's patch(es).
Ingo
[*] is there any weird architecture that hardcodes IRQ -1 to some
device? ;-)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:16 ` Benjamin Herrenschmidt
@ 2005-11-21 21:38 ` Linus Torvalds
2005-11-21 21:53 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 21:38 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Matthew Wilcox, David Howells, Andrew Morton, Ingo Molnar,
linux-kernel, Russell King, Ian Molton, Paul Mackerras
On Tue, 22 Nov 2005, Benjamin Herrenschmidt wrote:
>
> > The fact is, 0 _is_ special. Not just for hardware, but because 0 has a
> > magical meaning as "false" in the C language.
>
> I don't agree, irq 0 has been a valid irq on a number of platforms for
> ages
The point is, it's _not_ a valid irq for 99.9% of all machines and drivers
that have ever been tested.
Also, if you don't agree that 0 is special in the C language, then you're
just strictly _wrong_. It's an undeniable fact that zero _is_ special.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:25 ` Paul Mackerras
2005-11-21 21:35 ` Ingo Molnar
@ 2005-11-21 21:49 ` Linus Torvalds
2005-11-21 22:06 ` Benjamin Herrenschmidt
` (2 more replies)
2005-11-21 21:50 ` Benjamin Herrenschmidt
2005-11-21 22:20 ` Alan Cox
3 siblings, 3 replies; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 21:49 UTC (permalink / raw)
To: Paul Mackerras
Cc: Ingo Molnar, Matthew Wilcox, David Howells, Andrew Morton,
linux-kernel, Russell King, Ian Molton, Benjamin Herrenschmidt
On Tue, 22 Nov 2005, Paul Mackerras wrote:
>
> Yes, G5 powermacs have the SATA controller on irq 0. So if we can't
> use irq 0, I can't get to my hard disk. :) Other powermacs also use
> irq 0 for various things, as do embedded PPC machines.
That doesn't change any of the logic. There already is no "1:1" mapping of
PCI interrupts to what the machine does.
On all PC hardware, having a zero in the PCI irq register basically means
that no irq is enabled. That's a _fact_. It's a fact however much you may
not like it. It's how the hardware comes up, and it's how the BIOS leaves
it. So "0" absolutely does mean "not allocated".
Now, the second part of the story is that when it comes to PCI, it doesn't
matter what Apple, Sun, or pretty much anybody else has done. The reason
PCI has a separate MMIO and IO space is that it comes from a PC
background, and the reason Apple and others use PCI is that through that,
there are thousands of controller cards that are sold for PC's that also
happen to work on non-PC's.
So PC usage really is a defining part of PCI. It's what defines basically
_all_ of the testing, even under Linux.
So let's face those facts:
- we have a 8-bit register (0-255) for firmware telling the kernel what
the pre-allocated interrupt is.
- all of those 256 numbers _may_ in fact be valid on some piece of
hardware.
- only one of those numbers (0) is de-facto the "no irq line set up"
value.
- pretty much all drivers have been tested mainly with 0 being the "no
irq" value.
Those are FACTS. Denying them is a sign of stupidity.
I'd suggest that if some architecture can't live with those facts, it
either:
- define it's own PCI_NO_IRQ value, and face the fact that it will have
to test the drivers and hope they work (and that a lot of them simply
will _not_ work).
This is what we have today. It mostly works. Maybe we shouldn't change
it.
- realize that 0 is special, and use another number for when firmware
tells it 0 _is_ actually a valid irq (maybe 256. Maybe "1u<<31". I
don't care.)
And I won't apply the "turn PCI_NO_IRQ" into -1 generally, because I
consider it to be strictly _worse_ than what we have now.
Comprende?
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:25 ` Paul Mackerras
2005-11-21 21:35 ` Ingo Molnar
2005-11-21 21:49 ` Linus Torvalds
@ 2005-11-21 21:50 ` Benjamin Herrenschmidt
2005-11-21 22:20 ` Alan Cox
3 siblings, 0 replies; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2005-11-21 21:50 UTC (permalink / raw)
To: Paul Mackerras
Cc: Ingo Molnar, Linus Torvalds, Matthew Wilcox, David Howells,
Andrew Morton, linux-kernel, Russell King, Ian Molton
On Tue, 2005-11-22 at 08:25 +1100, Paul Mackerras wrote:
> Ingo Molnar writes:
>
> > is there any architecture where irq 0 is a legitimate setting that could
> > occur in drivers, and which would make NO_IRQ define of 0 non-practical?
>
> Yes, G5 powermacs have the SATA controller on irq 0. So if we can't
> use irq 0, I can't get to my hard disk. :) Other powermacs also use
> irq 0 for various things, as do embedded PPC machines.
And other non-ppc embedded things I've seen in the past... I think it's
quite common outside of the x86 world
Ben.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:35 ` Ingo Molnar
@ 2005-11-21 21:51 ` Linus Torvalds
2005-11-21 22:09 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 21:51 UTC (permalink / raw)
To: Ingo Molnar
Cc: Paul Mackerras, Matthew Wilcox, David Howells, Andrew Morton,
linux-kernel, Russell King, Ian Molton, Benjamin Herrenschmidt
On Mon, 21 Nov 2005, Ingo Molnar wrote:
>
> oh well [*]. Then it's gotta be the !dev->irq.valid thing i guess.
No it's not.
The ppc PCI probing could trivilly just turn a 0 into 256 (or equivalent),
and mask off the low 7 bits when installing the handler. They know the
interrupt is _really_ 0 from other sources (ie they have a different
firmware, with explicit callbacks, and/or hardcoded knowledge).
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:38 ` Linus Torvalds
@ 2005-11-21 21:53 ` Benjamin Herrenschmidt
2005-11-21 22:18 ` Linus Torvalds
0 siblings, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2005-11-21 21:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: Matthew Wilcox, David Howells, Andrew Morton, Ingo Molnar,
linux-kernel, Russell King, Ian Molton, Paul Mackerras
On Mon, 2005-11-21 at 13:38 -0800, Linus Torvalds wrote:
> The point is, it's _not_ a valid irq for 99.9% of all machines and drivers
> that have ever been tested.
It is a valid irq for a wide range of embedded devices.
> Also, if you don't agree that 0 is special in the C language, then you're
> just strictly _wrong_. It's an undeniable fact that zero _is_ special.
And ? I really don't agree that just because 0 "looks kewl", we should
enforce that and add some dodgy remapping all over the place.
It's not like it was difficult to fix the few drivers that make bogus
assumptions and use NO_IRQ instead ...
Ben.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:49 ` Linus Torvalds
@ 2005-11-21 22:06 ` Benjamin Herrenschmidt
2005-11-21 22:28 ` Linus Torvalds
2005-11-21 23:20 ` Paul Mackerras
2005-11-22 2:45 ` Matthew Wilcox
2 siblings, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2005-11-21 22:06 UTC (permalink / raw)
To: Linus Torvalds
Cc: Paul Mackerras, Ingo Molnar, Matthew Wilcox, David Howells,
Andrew Morton, linux-kernel, Russell King, Ian Molton
> On all PC hardware, having a zero in the PCI irq register basically means
> that no irq is enabled. That's a _fact_. It's a fact however much you may
> not like it. It's how the hardware comes up, and it's how the BIOS leaves
> it. So "0" absolutely does mean "not allocated".
It's not the case on various non-x86 architectures, not the case in the
PCI spec neither.
> Now, the second part of the story is that when it comes to PCI, it doesn't
> matter what Apple, Sun, or pretty much anybody else has done. The reason
> PCI has a separate MMIO and IO space is that it comes from a PC
> background, and the reason Apple and others use PCI is that through that,
> there are thousands of controller cards that are sold for PC's that also
> happen to work on non-PC's.
And ?
> So PC usage really is a defining part of PCI. It's what defines basically
> _all_ of the testing, even under Linux.
>
> So let's face those facts:
> - we have a 8-bit register (0-255) for firmware telling the kernel what
> the pre-allocated interrupt is.
That register is mostly useless on a wide range of architectures tho :)
> - all of those 256 numbers _may_ in fact be valid on some piece of
> hardware.
> - only one of those numbers (0) is de-facto the "no irq line set up"
> value.
No, it's not.
> - pretty much all drivers have been tested mainly with 0 being the "no
> irq" value.
>
> Those are FACTS. Denying them is a sign of stupidity.
I must have lost a lot of neurons in the past few years then.
> I'd suggest that if some architecture can't live with those facts, it
> either:
>
> - define it's own PCI_NO_IRQ value, and face the fact that it will have
> to test the drivers and hope they work (and that a lot of them simply
> will _not_ work).
Very few driver bother at all about the irq value. Most don't test it,
they just use it and be done with it, and that's the way it works. Some
drivers, mostly ISA stuff or legacy stuff or drivers that deal with old
weird chipsets actually look at the irq value, and those can be easily
fixed to use NO_IRQ. I really don't understand why you are making such a
fuss about this...
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:51 ` Linus Torvalds
@ 2005-11-21 22:09 ` Benjamin Herrenschmidt
2005-11-21 22:34 ` Linus Torvalds
0 siblings, 1 reply; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2005-11-21 22:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Ingo Molnar, Paul Mackerras, Matthew Wilcox, David Howells,
Andrew Morton, linux-kernel, Russell King, Ian Molton
On Mon, 2005-11-21 at 13:51 -0800, Linus Torvalds wrote:
>
> On Mon, 21 Nov 2005, Ingo Molnar wrote:
> >
> > oh well [*]. Then it's gotta be the !dev->irq.valid thing i guess.
>
> No it's not.
>
> The ppc PCI probing could trivilly just turn a 0 into 256 (or equivalent),
> and mask off the low 7 bits when installing the handler. They know the
> interrupt is _really_ 0 from other sources (ie they have a different
> firmware, with explicit callbacks, and/or hardcoded knowledge).
The ppc irq handling is more complex than that due to the wide range of
different hardware. We parse the irq tree from OF and assign them to
ranges of numbers allocated per controller. Adding some remapping of
some numbers would add complexity and possible bugs.
Ben.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:53 ` Benjamin Herrenschmidt
@ 2005-11-21 22:18 ` Linus Torvalds
2005-11-21 22:20 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 22:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Matthew Wilcox, David Howells, Andrew Morton, Ingo Molnar,
linux-kernel, Russell King, Ian Molton, Paul Mackerras
On Tue, 22 Nov 2005, Benjamin Herrenschmidt wrote:
>
> And ? I really don't agree that just because 0 "looks kewl", we should
> enforce that and add some dodgy remapping all over the place.
That's not at all what I'm saying.
THE "BAD IRQ" MAPPING IS REQUIRED REGARDLESS.
If we make PCI_NO_IRQ be -1, then PC's need to remap 0 to that value. In
pretty much _exactly_ the same places that I suggest that the ppc code
should do it.
And there are several thousand times more PC's than there are other
things.
Got it?
Everybody who argues for PCI_NO_IRQ being -1 is arguing for all the same
things I argue that the ppc port should do, except they _also_ argue that
we should break now-working setups.
Is that so hard to understand? -1 is no different from 0, except it is in
many way sprovably _inferior_. Both need some mapping. But the -1 case
needs more of it, _and_ will result in a inferior end result (because the
nice "!dev->pci" thing suddenly doesn't work).
See? You're arguing for a technically inferior solution.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:25 ` Paul Mackerras
` (2 preceding siblings ...)
2005-11-21 21:50 ` Benjamin Herrenschmidt
@ 2005-11-21 22:20 ` Alan Cox
2005-11-22 11:13 ` David Woodhouse
3 siblings, 1 reply; 42+ messages in thread
From: Alan Cox @ 2005-11-21 22:20 UTC (permalink / raw)
To: Paul Mackerras
Cc: Ingo Molnar, Linus Torvalds, Matthew Wilcox, David Howells,
Andrew Morton, linux-kernel, Russell King, Ian Molton,
Benjamin Herrenschmidt
On Maw, 2005-11-22 at 08:25 +1100, Paul Mackerras wrote:
> Ingo Molnar writes:
>
> > is there any architecture where irq 0 is a legitimate setting that could
> > occur in drivers, and which would make NO_IRQ define of 0 non-practical?
>
> Yes, G5 powermacs have the SATA controller on irq 0. So if we can't
> use irq 0, I can't get to my hard disk. :) Other powermacs also use
> irq 0 for various things, as do embedded PPC machines.
G5 powermacs have the SATA controller on physical IRQ value 0. Linux IRQ
values don't need to exactly map. One of the x86 ports handles 'real IRQ
0' exactly this way. Its a cookie. Sure would benefit from a function
for turning an IRQ into a description as a cleanup.
Alan
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 22:18 ` Linus Torvalds
@ 2005-11-21 22:20 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2005-11-21 22:20 UTC (permalink / raw)
To: Linus Torvalds
Cc: Matthew Wilcox, David Howells, Andrew Morton, Ingo Molnar,
linux-kernel, Russell King, Ian Molton, Paul Mackerras
> If we make PCI_NO_IRQ be -1, then PC's need to remap 0 to that value. In
> pretty much _exactly_ the same places that I suggest that the ppc code
> should do it.
No. Remapping a valid number to something else means remapping at
runtime for all things that manipulate an irq : issuing it,
enable/disable callbacks, etc...
"Fixing" the definition of "no irq" on x86 means only changing the boot
code that sets up irq numbers, whatever it is.
Thus what solution is technically superior can be argued based on the
fact that your solution introduce overhead to code path that are hot
during normal kernel operations.
Ben.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 22:06 ` Benjamin Herrenschmidt
@ 2005-11-21 22:28 ` Linus Torvalds
2005-11-21 22:58 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 22:28 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Paul Mackerras, Ingo Molnar, Matthew Wilcox, David Howells,
Andrew Morton, linux-kernel, Russell King, Ian Molton
On Tue, 22 Nov 2005, Benjamin Herrenschmidt wrote:
>
> > On all PC hardware, having a zero in the PCI irq register basically means
> > that no irq is enabled. That's a _fact_. It's a fact however much you may
> > not like it. It's how the hardware comes up, and it's how the BIOS leaves
> > it. So "0" absolutely does mean "not allocated".
>
> It's not the case on various non-x86 architectures, not the case in the
> PCI spec neither.
So?
The point is, we have to handle the 0 on a PC architecture _anyway_.
The PCI spec does NOT MATTER. The 0 case is a _fact_.
If the PCI spec said that the world was flat, would that make a
difference? No.
The fact is, we need to pick some value for "No irq", and that value needs
translation. I claim that 0 is superior, because
- we know it works on the biggest hardware base
- it's de facto what pretty much all of the current source code _does_
anyway
- it's easier to test for, and more importantly it allows the most
natural source code syntax with fewer mistakes.
- it requires no more translation than any other value, including -1.
These are all just undeniable facts.
In contrast, the PCI spec gives us a 8-bit value that is _known_ to not be
sufficient anyway: (a) it has no "undefined" value at all, although one is
clearly needed (and on PC's, 0 _is_ that value) and (b) it's really too
small to cover the possible interrupt sources anyway (ie big machines
already tend to have more than 256 interrupts).
So you could just make the ppc PCI probing do
dev->irq = PCI_IRQ_BASE + node->intrs[0].line;
and suddenly 0 works equally well for you as it does on a regular PC.
Notice? Magic. Suddenly "0" means "No irq" on ppc too.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 22:09 ` Benjamin Herrenschmidt
@ 2005-11-21 22:34 ` Linus Torvalds
2005-11-21 23:00 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-21 22:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Ingo Molnar, Paul Mackerras, Matthew Wilcox, David Howells,
Andrew Morton, linux-kernel, Russell King, Ian Molton
On Tue, 22 Nov 2005, Benjamin Herrenschmidt wrote:
>
> The ppc irq handling is more complex than that due to the wide range of
> different hardware. We parse the irq tree from OF and assign them to
> ranges of numbers allocated per controller. Adding some remapping of
> some numbers would add complexity and possible bugs.
Let me rephrase that as: "On ppc we already _do_ irq mapping, but we made
a mistake to map to 0, so to cover up that mistake we now want everybody
else to change how they have done things since day 1".
See? You argue that there might be "possible bugs". Yet you totally ignore
the fact that I can absolutely point to code that we _know_ depends on
"zero means no irq". So when you argue for changing away from that, I
GUARANTEE you that it has a higher likelihood of bugs AND that it affects
a hell of a lot more code than the alternative I propose.
Anyway, let's just stop this discussion. Let's just leave PCI_NO_IRQ as 0
on x86, and let PPC has it as -1. Problem solved. That way we are
guaranteed to not introduce any new bugs. It works today, and we can just
ignore this whole patch-series, since centralizing NO_IRQ is clearly the
wrong thing to do.
By centralizing NO_IRQ, you either have to break a lot of existing PC
setups, or you have to potentially break (far fewer) PowerPC setups. So
let's not do it at all.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 22:28 ` Linus Torvalds
@ 2005-11-21 22:58 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2005-11-21 22:58 UTC (permalink / raw)
To: Linus Torvalds
Cc: Paul Mackerras, Ingo Molnar, Matthew Wilcox, David Howells,
Andrew Morton, linux-kernel, Russell King, Ian Molton
> So you could just make the ppc PCI probing do
>
> dev->irq = PCI_IRQ_BASE + node->intrs[0].line;
>
> and suddenly 0 works equally well for you as it does on a regular PC.
>
> Notice? Magic. Suddenly "0" means "No irq" on ppc too.
Not really, we also need to fix the interrupt controller code among
others...
Ben.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 22:34 ` Linus Torvalds
@ 2005-11-21 23:00 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 42+ messages in thread
From: Benjamin Herrenschmidt @ 2005-11-21 23:00 UTC (permalink / raw)
To: Linus Torvalds
Cc: Ingo Molnar, Paul Mackerras, Matthew Wilcox, David Howells,
Andrew Morton, linux-kernel, Russell King, Ian Molton
> By centralizing NO_IRQ, you either have to break a lot of existing PC
> setups, or you have to potentially break (far fewer) PowerPC setups. So
> let's not do it at all.
I have no strong feeling vs. centralized or not centralized NO_IRQ
value. All I want is NO_IRQ to exist on all archs so I can fix the few
drivers that assume that 0 is no irq.
Ben.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:49 ` Linus Torvalds
2005-11-21 22:06 ` Benjamin Herrenschmidt
@ 2005-11-21 23:20 ` Paul Mackerras
2005-11-22 1:26 ` Linus Torvalds
2005-11-22 2:45 ` Matthew Wilcox
2 siblings, 1 reply; 42+ messages in thread
From: Paul Mackerras @ 2005-11-21 23:20 UTC (permalink / raw)
To: Linus Torvalds
Cc: Ingo Molnar, Matthew Wilcox, David Howells, Andrew Morton,
linux-kernel, Russell King, Ian Molton, Benjamin Herrenschmidt
Linus Torvalds writes:
> On all PC hardware, having a zero in the PCI irq register basically means
> that no irq is enabled. That's a _fact_. It's a fact however much you may
> not like it. It's how the hardware comes up, and it's how the BIOS leaves
> it. So "0" absolutely does mean "not allocated".
First, are you talking about the interrupt pin register or the
interrupt line register?
Secondly, I would say that any driver that looks at either of those
registers is broken. Drivers should be looking at dev->irq, which is
set up by platform code, and may be quite different from what is in
the interrupt line register.
So the question of what PCI devices do with the value in the interrupt
line register is pretty much irrelevant. Those few devices that can
control their interrupt routing (such as cardbus bridges) don't use
the interrupt line register for that AFAIK.
I think all we need is for the convention for drivers to be that they
test for whether the device has an interrupt is to test (dev->irq !=
NO_IRQ). Then x86 can have NO_IRQ = 0 and PPC can have NO_IRQ = -1
and I think everyone is happy (well, nearly everyone, anyway :).
Paul.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 23:20 ` Paul Mackerras
@ 2005-11-22 1:26 ` Linus Torvalds
0 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2005-11-22 1:26 UTC (permalink / raw)
To: Paul Mackerras
Cc: Ingo Molnar, Matthew Wilcox, David Howells, Andrew Morton,
linux-kernel, Russell King, Ian Molton, Benjamin Herrenschmidt
On Tue, 22 Nov 2005, Paul Mackerras wrote:
>
> First, are you talking about the interrupt pin register or the
> interrupt line register?
Interrupt line. The interrupt pin is totally separate.
> Secondly, I would say that any driver that looks at either of those
> registers is broken. Drivers should be looking at dev->irq, which is
> set up by platform code, and may be quite different from what is in
> the interrupt line register.
But that's part of the POINT, Paul.
The platform code needs to set up something in dev->irq. And we have
_always_ had "dev->irq == 0" meaning "no irq".
So if PCI irq (the interrupt line register or whatever) 0 means something
for you on PPC, then BY DEFINITION you should not have translated it into
"dev->irq". But PPC did. Tough. Don't blame that mistake on me, or try to
force that mistake on other architectures.
The fact that PPC screwed that up is a PPC problem, and it's a PPC problem
from the very beginning, because the "dev->irq" value doesn't have to
match the PCI irq at all.
On sparc, for example, "dev->irq" used to be some random cookie, if I
remember correctly. But "0" still meant "unallocated".
So face it. PPC screwed up, and if it had just followed what the
"dev->irq" meant on the regular x86 platforms, it wouldn't have needed
that NO_IRQ in the first place.
The whole notion of needing "NO_IRQ" is broken. The way to test for not
having an irq is "!dev->irq". Any architecture that uses NO_IRQ is just a
bug waiting to happen, for any number of drivers. And for no good reason.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:49 ` Linus Torvalds
2005-11-21 22:06 ` Benjamin Herrenschmidt
2005-11-21 23:20 ` Paul Mackerras
@ 2005-11-22 2:45 ` Matthew Wilcox
2 siblings, 0 replies; 42+ messages in thread
From: Matthew Wilcox @ 2005-11-22 2:45 UTC (permalink / raw)
To: Linus Torvalds
Cc: Paul Mackerras, Ingo Molnar, David Howells, Andrew Morton,
linux-kernel, Russell King, Ian Molton, Benjamin Herrenschmidt
On Mon, Nov 21, 2005 at 01:49:45PM -0800, Linus Torvalds wrote:
> On all PC hardware, having a zero in the PCI irq register basically means
> that no irq is enabled. That's a _fact_. It's a fact however much you may
> not like it. It's how the hardware comes up, and it's how the BIOS leaves
> it. So "0" absolutely does mean "not allocated".
Actually, no. Here's my x86 laptop's config space ...
0000:02:06.2 System peripheral: Texas Instruments PCI1620 Firmware Loading Function (rev 01)
Subsystem: Hewlett-Packard Company: Unknown device 08b0
Flags: bus master, medium devsel, latency 64
I/O ports at 4000 [size=64]
Capabilities: [44] Power Management version 2
00: 4c 10 01 82 07 01 10 02 01 00 80 08 08 40 00 00
10: 01 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 3c 10 b0 08
30: 00 00 00 00 44 00 00 00 00 00 00 00 ff 00 07 04
Interrupt Pin is 0x3d and Interrupt Line is 0x3c. As I explained earlier,
Linux simply never reads Interrupt Line if Interrupt Pin is 0.
Anyway, this doesn't matter too much. I don't personally care about
NO_IRQ being 0 or -1; that was something benh tried to make me care
about ;-)
So, how about this for a new patch series:
- Patch 1, same as this series (it just makes sense to bounds-check in
the irq management functions).
- Patch 2, add #ifndef NO_IRQ #define NO_IRQ 0 #endif to linux/hardirq.h
- Patch 3, set dev->irq to NO_IRQ in drivers/pci/probe.c
- Patch 4, remove custom definition of NO_IRQ from pd6729
- Patch 5, use NO_IRQ in serial_core
I'll start work on that now, since that fixes the problems I care about
and doesn't negatively affect people with problems I don't care about ;-)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 22:20 ` Alan Cox
@ 2005-11-22 11:13 ` David Woodhouse
2005-11-22 14:15 ` Alan Cox
0 siblings, 1 reply; 42+ messages in thread
From: David Woodhouse @ 2005-11-22 11:13 UTC (permalink / raw)
To: Alan Cox
Cc: Paul Mackerras, Ingo Molnar, Linus Torvalds, Matthew Wilcox,
David Howells, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Mon, 2005-11-21 at 22:20 +0000, Alan Cox wrote:
> > Yes, G5 powermacs have the SATA controller on irq 0. So if we can't
> > use irq 0, I can't get to my hard disk. :) Other powermacs also use
> > irq 0 for various things, as do embedded PPC machines.
>
> G5 powermacs have the SATA controller on physical IRQ value 0. Linux IRQ
> values don't need to exactly map. One of the x86 ports handles 'real IRQ
> 0' exactly this way. Its a cookie. Sure would benefit from a function
> for turning an IRQ into a description as a cleanup.
Remapping in that way sounds like a half-arsed hack to work around the
problem which Matthew is trying to fix properly by using NO_IRQ == -1.
Yes, there are drivers which are currently broken and assume irq 0 is
'no irq'. They are broken. Let's just fix them and not continue the
brain-damage.
--
dwmw2
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 14:15 ` Alan Cox
@ 2005-11-22 14:04 ` Matthew Wilcox
2005-11-22 17:03 ` Linus Torvalds
2005-11-22 18:37 ` David Howells
2 siblings, 0 replies; 42+ messages in thread
From: Matthew Wilcox @ 2005-11-22 14:04 UTC (permalink / raw)
To: Alan Cox
Cc: David Woodhouse, Paul Mackerras, Ingo Molnar, Linus Torvalds,
David Howells, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Tue, Nov 22, 2005 at 02:15:39PM +0000, Alan Cox wrote:
> On Maw, 2005-11-22 at 11:13 +0000, David Woodhouse wrote:
> > Yes, there are drivers which are currently broken and assume irq 0 is
> > 'no irq'. They are broken. Let's just fix them and not continue the
> > brain-damage.
>
> 0 in the Linux kernel has always meant 'no IRQ' and it makes it natural
> to express in C (and on some cpus more efficient too).
>
> What if my hardware has an IRQ -1 ;)
Then it falls off the bottom of the irq_desc array. Already tried that,
hence patch 1/5.
That was a great one to debug, btw. "The machine hangs when I select
processor type PA8000 but works with processor type PA7200". Why?
synchronize_irq() spins waiting for a bit to become clear ... for some
reason that bit was always set with PA8000 and always clear with PA7200.
http://lists.parisc-linux.org/pipermail/parisc-linux/2005-October/027485.html
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 11:13 ` David Woodhouse
@ 2005-11-22 14:15 ` Alan Cox
2005-11-22 14:04 ` Matthew Wilcox
` (2 more replies)
0 siblings, 3 replies; 42+ messages in thread
From: Alan Cox @ 2005-11-22 14:15 UTC (permalink / raw)
To: David Woodhouse
Cc: Paul Mackerras, Ingo Molnar, Linus Torvalds, Matthew Wilcox,
David Howells, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Maw, 2005-11-22 at 11:13 +0000, David Woodhouse wrote:
> Yes, there are drivers which are currently broken and assume irq 0 is
> 'no irq'. They are broken. Let's just fix them and not continue the
> brain-damage.
0 in the Linux kernel has always meant 'no IRQ' and it makes it natural
to express in C (and on some cpus more efficient too).
What if my hardware has an IRQ -1 ;)
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 14:15 ` Alan Cox
2005-11-22 14:04 ` Matthew Wilcox
@ 2005-11-22 17:03 ` Linus Torvalds
2005-11-22 18:20 ` Matthew Wilcox
2005-11-22 18:37 ` David Howells
2 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-22 17:03 UTC (permalink / raw)
To: Alan Cox
Cc: David Woodhouse, Paul Mackerras, Ingo Molnar, Matthew Wilcox,
David Howells, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Tue, 22 Nov 2005, Alan Cox wrote:
>
> On Maw, 2005-11-22 at 11:13 +0000, David Woodhouse wrote:
> > Yes, there are drivers which are currently broken and assume irq 0 is
> > 'no irq'. They are broken. Let's just fix them and not continue the
> > brain-damage.
>
> 0 in the Linux kernel has always meant 'no IRQ' and it makes it natural
> to express in C (and on some cpus more efficient too).
Ahh, a voice of sanity!
> What if my hardware has an IRQ -1 ;)
And "-1" isn't actually a valid value in the first place.
The struct pci_device definition is:
unsigned int irq;
and anybody who uses -1 is just a total idiot and nincompoop. It's going
to be a major pain in the ass (others use "int irq", others use "unsigned
long irq").
Using (~0u) would be more correct, but still insane.
The fact is, 0 _is_ "no interrupt". Always has been. And anybody who says
that "-1" is "correct" is just totally wrong. Making it -1 would be
guaranteed to generate tons of breakage, and most people won't ever even
notice, because in most cases the irq _is_ actually there, and you never
hit the path. Which just makes the breakage EVEN WORSE.
So David Woodhouse, you're just wrong. But hey, you seem to (sadly) not be
alone.
In short: NO_IRQ _is_ 0. Always has been. It's the only sane value. And
btw, there is no need for that #define at all, exactly because the way you
test for "is this no irq" is by doing "!dev->irq".
Anybody who does anything else is a bug waiting to happen.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 17:03 ` Linus Torvalds
@ 2005-11-22 18:20 ` Matthew Wilcox
0 siblings, 0 replies; 42+ messages in thread
From: Matthew Wilcox @ 2005-11-22 18:20 UTC (permalink / raw)
To: Linus Torvalds
Cc: Alan Cox, David Woodhouse, Paul Mackerras, Ingo Molnar,
David Howells, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Tue, Nov 22, 2005 at 09:03:12AM -0800, Linus Torvalds wrote:
> In short: NO_IRQ _is_ 0. Always has been. It's the only sane value. And
> btw, there is no need for that #define at all, exactly because the way you
> test for "is this no irq" is by doing "!dev->irq".
Could you at least take the first patch that checks that we don't go
outside the bounds of the irq_desc array?
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 14:15 ` Alan Cox
2005-11-22 14:04 ` Matthew Wilcox
2005-11-22 17:03 ` Linus Torvalds
@ 2005-11-22 18:37 ` David Howells
2005-11-22 19:03 ` David Woodhouse
` (2 more replies)
2 siblings, 3 replies; 42+ messages in thread
From: David Howells @ 2005-11-22 18:37 UTC (permalink / raw)
To: Linus Torvalds
Cc: Alan Cox, David Woodhouse, Paul Mackerras, Ingo Molnar,
Matthew Wilcox, David Howells, Andrew Morton, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt
Linus Torvalds <torvalds@osdl.org> wrote:
> The fact is, 0 _is_ "no interrupt". Always has been.
The fact that it always has been doesn't make it correct; and the fact that
the C particularly likes zeros is only optimisable on some hardware. On RISC
processors where you can't test memory directly, it seems to be the case that
any small positive or negative integer is as good as zero; for instance, in
FRV:
ldi @(gr8,#0),gr4 # doesn't set the condition codes
subicc gr4,#-1,gr0,icc0 # cmp to -1, result to immutable gr0
beq icc0,#0,it_was_unset
I suspect a lot of RISC archs will be like this. It's only certain ones like
M68K and x86 where you can test memory directly:
testl (%eax)
je it_was_unset
that it's usefully optimisable, and in those cases it might be possible to do
this sort of thing:
cmpl $-1,(%eax)
je it_was_unset
though the instruction will be longer.
> In short: NO_IRQ _is_ 0. Always has been.
So what? That hasn't stopped you imposing a blanket change before.
> It's the only sane value.
Has anyone ever accused you of being sane? :-)
> Anybody who does anything else is a bug waiting to happen.
My three main concerns are this:
(1) Changing the no-irq value away from zero is going to cause problems in
certain drivers that assume they can do !dev->irq. I'd like my drivers to
work without me having to do anything to them, but there's a lot of
rubbish drivers out there, even allowing for this. I suspect this is a
tiny part of the problem, and easily fixed in the drivers in the kernel.
(2) 0 is a valid IRQ in lots of places, including x86. IIRC it is permissible
for ISAPNP and PNP-BIOS (and presumably ACPI) to indicate something is
attached to IRQ 0 (usually only the timer is there though, but it can be
possible to reconfigure that).
Fortunately, for the FRV arch IRQ 0 is not used - level 0 in the interrupt
mask register permits all interrupts; and for the AM33 arch, whilst there
is an IRQ 0, that's the NMI interrupt and so has to be handled specially
anyway.
The only reason NO_IRQ on FRV is -1 is that I copied the code from
elsewhere. It could easily be changed to 0.
(3) Having to translate a cookie for a specific IRQ means that the IRQ
handling code will be slower and more complex, or is going to avoid the
issue and be naughty and not deal with irq == NO_IRQ properly:
The x86 PIC reports it as IRQ 0 having happened. In which case, by your
argument, you _have_ to translate it: you're not allowed to pass NO_IRQ to
setup_irq(), and you're not allowed to pass it to the interrupt handler -
in this case timer_interrupt(). Doing otherwise is wrong, insane, etc...
It may even be possible to simplify the x86/x86_64 arch code by making IRQ
0 a normal IRQ instead of something special.
The only argument for not doing so is that it's hidden inside the arch
where it can't be seen... apart from by those looking for examples of good
code to copy (the i386 arch is used as a model for a lot of things).
I'd like to see dev->irq as a pointer to a structure. As you say, the number is
a cookie, but it's also very much dependent on the bus, so why shouldn't it be
associated with the bus in the same way I/O ports and memory ranges are? I'd
also like to see it arranged as a tree: with FRV, I can add extra PIC's into
the tree, thus expanding the IRQ space available dynamically.
However, as far as the current issue goes, I've no concerns for FRV or AM33
should NO_IRQ become 0.
Whatever you decide to do *please* document this in Documentation/ somewhere!
Then you have a "standard" at which to point and say "so it is written". At the
moment it's documented in the code, and that is inconsistent, perhaps
reasonably.
David
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 18:37 ` David Howells
@ 2005-11-22 19:03 ` David Woodhouse
2005-11-22 19:21 ` Linus Torvalds
2005-11-22 19:05 ` Linus Torvalds
2005-11-22 19:38 ` David Howells
2 siblings, 1 reply; 42+ messages in thread
From: David Woodhouse @ 2005-11-22 19:03 UTC (permalink / raw)
To: David Howells
Cc: Linus Torvalds, Alan Cox, Paul Mackerras, Ingo Molnar,
Matthew Wilcox, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Tue, 2005-11-22 at 18:37 +0000, David Howells wrote:
>
> (3) Having to translate a cookie for a specific IRQ means that the IRQ
> handling code will be slower and more complex, or is going to avoid the
> issue and be naughty and not deal with irq == NO_IRQ properly:
>
> The x86 PIC reports it as IRQ 0 having happened. In which case, by your
> argument, you _have_ to translate it: you're not allowed to pass NO_IRQ to
> setup_irq(), and you're not allowed to pass it to the interrupt handler -
> in this case timer_interrupt(). Doing otherwise is wrong, insane, etc...
This is true. If we're suddenly going to start pretending that IRQ 0
isn't a valid interrupt merely on the basis that "x86 doesn't use it"¹,
then we can't really go making an exception to allow us to use IRQ 0 on
i386.
--
dwmw2
¹ ...despite the fact that even that isn't true on legacy-free machines.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 18:37 ` David Howells
2005-11-22 19:03 ` David Woodhouse
@ 2005-11-22 19:05 ` Linus Torvalds
2005-11-22 19:38 ` David Howells
2 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2005-11-22 19:05 UTC (permalink / raw)
To: David Howells
Cc: Alan Cox, David Woodhouse, Paul Mackerras, Ingo Molnar,
Matthew Wilcox, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Tue, 22 Nov 2005, David Howells wrote:
>
> > In short: NO_IRQ _is_ 0. Always has been.
>
> So what? That hasn't stopped you imposing a blanket change before.
>
> > It's the only sane value.
>
> Has anyone ever accused you of being sane? :-)
Good arguments, but usually when I'm insane I do blanket changes that I at
least personally agree with.
So my insanity is a "self-consistent" one, although part of it is that I
do tend to change my mind (ie I may be self-consistent at any particular
point in time, but I reserve the right to be inconsistent over the span of
a day, week, or a year).
> > Anybody who does anything else is a bug waiting to happen.
>
> My three main concerns are this:
>
> (1) Changing the no-irq value away from zero is going to cause problems in
> certain drivers that assume they can do !dev->irq. I'd like my drivers to
> work without me having to do anything to them, but there's a lot of
> rubbish drivers out there, even allowing for this. I suspect this is a
> tiny part of the problem, and easily fixed in the drivers in the kernel.
Drivers really are the bulk of the kernel. They are a huge problem spot in
general, because quite often driver writers are pretty limited in their
understanding of the big picture and the rest of the kernel, which
together with the fact that drivers are often hard to write _anyway_ (bad
hw docs if you have any at all, coupled with often the hardware itself
having strange "features") means that drivers often have the lowest
quality of code.
To make matters worse, most drivers will have very limited testing anyway,
because they have a limited audience. There's a few _very_ common drivers,
but there's a lot of drivers that are relevant to only a small percentage
of kernel users.
Finally, in this particular case, the notion of NO_IRQ is usually an issue
only for a small percentage of that small percentage. I bet there are tons
of drivers that don't even bother to check, because they simply don't need
to: they always have an interrupt available, or the machine would be so
broken that it wouldn't ever get used anyway.
This is part of the reason why I'm so adamant that _only_ PC's matter in
this space. For all other architectures, the small percentage of a small
percentage of a small percentage basically means that the driver is
totally irrelevant and has never had any users and absolutely zero
testing.
So when it comes to drivers, other architectures simply _have_ to look
like a PC, because if they don't, they'll be sorry. They'll _never_ get
the kind of testing overhead that PC's get. And even on bog-standard
Linux/x86, we have several times had an issue of a driver being broken for
over a _year_ or more, without anybody noticing, and only then being
fixed (because the users were so few that it took a long time before they
upgraded).
And this is why I absolutely _hate_ to make changes that you can only test
by actually having the hardware. Sometimes we have to do it (the big
interrupt handling changes for SMP), and for all I know we _still_ have
drivers that simply don't even compile on SMP because they depend on the
old global "cli/sti" behaviour.
And in this case, I'm 100% convinced we do not want to change that old PC
behaviour. The fact is, -1 really _is_ technically worse than 0. As
already mentioned "unsigned int" is actually the canonical form of NO_IRQ,
so -1 really ends up being "~0u", and that coupled with the fact that some
people use "int", others use "unsigned long" is just clearly not good.
So my suggested (very _strongly_ suggested) solution is for people to just
consider "irq" to be a cookie, with the magical value 0 meaning "not
there" but no inherent meaning otherwise. That just solves all the
fundamentally hard problems, and has no fundamental problems of its own.
The thing is, we'll have to do that _anyway_ over time. We _know_ that
even PCI interrupts are changing. We're clearly moving towards a world
where an interrupt is _literally_ a cookie from the device, and this very
much includes PCI. What do you think MSI (message signalled interrupts)
are all about?
So thinking about "dev->irq" as a cookie is literally the right thing to
do. It is _not_ an index, and it certainly has nothing to do with things
like the pci interrupt line register or what the actual hardware
connection to the CPU is. It's a cookie. And for historical reasons we
have value 0 being the "no cookie" value.
There are actually some good reasons to think of interrupts as kernel
_pointers_ (and they'd point to the "irq descriptor"), but quite frankly,
right now that's just too big of a change. But if we ever do that change,
the value 0 would _still_ be the "no cookie" value, and you'd _still_ just
use "if (!dev->irq)" to test whether you had an irq or not.
(If somebody wants to try to make "irq" into a pointer, I'd actually be a
lot more supportive of it than of this "NO_IRQ" thing. I suspect it's a
bigger patch than we really want, for not a lot of gain, but at least it
would result in compile warnings for drivers, which is a way to control
the damage. That's what we did for the SMP irq disable/enable changes too)
> I'd like to see dev->irq as a pointer to a structure.
Yes, indeed. We already even probably have the right structure (ie likely
the right thing to do is to just use the current "irq_desc_t *").
If somebody wants to do that, I'll happily accept it. I just suspect it's
a _lot_ of work.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 19:03 ` David Woodhouse
@ 2005-11-22 19:21 ` Linus Torvalds
2005-11-22 23:58 ` David Woodhouse
0 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2005-11-22 19:21 UTC (permalink / raw)
To: David Woodhouse
Cc: David Howells, Alan Cox, Paul Mackerras, Ingo Molnar,
Matthew Wilcox, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
[-- Attachment #1: Type: TEXT/PLAIN, Size: 655 bytes --]
On Tue, 22 Nov 2005, David Woodhouse wrote:
>
> This is true. If we're suddenly going to start pretending that IRQ 0
> isn't a valid interrupt merely on the basis that "x86 doesn't use it"¹,
> then we can't really go making an exception to allow us to use IRQ 0 on
> i386.
Of _course_ "irq0" is a valid irq. On PC's, it's usually the timer
interrupt.
It's the "dev->irq" _cookie_ zero that means it is does not have an irq.
If you have a physical "irq 0" that is bound to a device, it needs a
cookie, and that cookie can't be 0, because that means the device has no
interrupt.
How hard is that to understand? Why do people mix these up?
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 18:37 ` David Howells
2005-11-22 19:03 ` David Woodhouse
2005-11-22 19:05 ` Linus Torvalds
@ 2005-11-22 19:38 ` David Howells
2005-11-22 19:51 ` Linus Torvalds
2 siblings, 1 reply; 42+ messages in thread
From: David Howells @ 2005-11-22 19:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Howells, Alan Cox, David Woodhouse, Paul Mackerras,
Ingo Molnar, Matthew Wilcox, Andrew Morton, linux-kernel,
Russell King, Ian Molton, Benjamin Herrenschmidt
Linus Torvalds <torvalds@osdl.org> wrote:
> bad hw docs if you have any at all
The same could be said of Linux. The docs there could be a *lot* better.
I wonder... might it be worth creating a Wiki or something on kernel.org in
which kernel docs can be maintained?
David
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 19:38 ` David Howells
@ 2005-11-22 19:51 ` Linus Torvalds
0 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2005-11-22 19:51 UTC (permalink / raw)
To: David Howells
Cc: Alan Cox, David Woodhouse, Paul Mackerras, Ingo Molnar,
Matthew Wilcox, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Tue, 22 Nov 2005, David Howells wrote:
>
> The same could be said of Linux. The docs there could be a *lot* better.
>
> I wonder... might it be worth creating a Wiki or something on kernel.org in
> which kernel docs can be maintained?
I'm not sure a wiki will save us, but I don't think we'd have anything to
lose from trying.
Whether kernel.org wants to have a wiki and the related infrastructure is
another matter. Right now I think everything is set up so that the public
machines are pure mirrors of the master machine, and the master machine
doesn't allow any public interaction at all.
So the wiki would have to be a totally separate setup (probably a
dedicated public wiki machine at "wiki.kernel.org" or something).
IOW, I suspect it would be best set up the same way "vger.kernel.org" is,
rather than necessarily any of the current core kernel.org machines. So
maybe somebody can just try to set it up, and ask for the DNS magic to be
done?
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-22 19:21 ` Linus Torvalds
@ 2005-11-22 23:58 ` David Woodhouse
0 siblings, 0 replies; 42+ messages in thread
From: David Woodhouse @ 2005-11-22 23:58 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Howells, Alan Cox, Paul Mackerras, Ingo Molnar,
Matthew Wilcox, Andrew Morton, linux-kernel, Russell King,
Ian Molton, Benjamin Herrenschmidt
On Tue, 2005-11-22 at 11:21 -0800, Linus Torvalds wrote:
> Of _course_ "irq0" is a valid irq. On PC's, it's usually the timer
> interrupt.
>
> It's the "dev->irq" _cookie_ zero that means it is does not have an irq.
>
> If you have a physical "irq 0" that is bound to a device, it needs a
> cookie, and that cookie can't be 0, because that means the device has no
> interrupt.
>
> How hard is that to understand? Why do people mix these up?
Mostly I'd suggest that people mix them up because you've only just made
up this 'cookie' nonsense, and the number we store in dev->irq has
always been just the irq number -- at least on all but SPARC, which was
always 'special'. That's precisely _why_ we have a mix of 'int' and
'long' and other types in which we store it, as you've said. It's never
been considered a 'cookie'.
But turning it into a cookie makes enough sense, I suppose, as long as
we stop confusing it with irq 'numbers' -- and that includes banning
anyone from printing it with '%d'.
I'm inclined to agree with David's suggestion of making it a pointer.
Then it can point to a structure which is part a proper _tree_ of
interrupts which actually resembles reality. And SPARC can stop being a
special (and probably saner) case. And all the mess which platforms have
with assigning interrupt 'number' ranges for various PICs can go away.
And hotplug interrupt controllers (it happens, on CardBus or with
hotplug PCI) can work sanely. But Matthew still can't have his pony.
Yes, you still need a mapping at _setup_ time from stuff like isapnp and
OpenFirmware interrupt numbers to the appropriate 'cookie', but that's
not so hard.
--
dwmw2
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/5] Centralise NO_IRQ definition
2005-11-21 21:15 ` Ingo Molnar
2005-11-21 21:25 ` Paul Mackerras
@ 2005-11-23 1:45 ` Pavel Machek
1 sibling, 0 replies; 42+ messages in thread
From: Pavel Machek @ 2005-11-23 1:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: Linus Torvalds, Matthew Wilcox, David Howells, Andrew Morton,
linux-kernel, Russell King, Ian Molton, Benjamin Herrenschmidt,
Paul Mackerras
Hi!
> > and then people can just say
> >
> > if (!dev->irq.valid)
> > return;
> >
> > instead, which is also readable, and where you simply cannot do the old
> > "if (!dev->irq)" at all.
> >
> > The fact is, 0 _is_ special. Not just for hardware, but because 0 has
> > a magical meaning as "false" in the C language.
>
> yeah, i wanted to suggest this originally, but got distracted by the x86
> quirk that 'IRQ#0' is often the i8253 timer interrupt.
>
> is there any architecture where irq 0 is a legitimate setting that could
> occur in drivers, and which would make NO_IRQ define of 0 non-practical?
> If not (which i think is the case) then we should indeed standardize on
> 0. (in one way or another) It's not like any real driver will ever have
> IRQ#0 even on a PC: the timer IRQ is 'known' to be routed to 0, and we
Well, I still may want for example disk driver (with broken interrupt)
to hook onto irq#0 (timer). Better than no interrupts at all....
Pavel
--
Thanks, Sharp!
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2005-11-23 2:41 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-11-21 1:14 [PATCH 4/5] Centralise NO_IRQ definition Matthew Wilcox
2005-11-21 11:12 ` David Howells
2005-11-21 12:14 ` Matthew Wilcox
2005-11-21 18:55 ` Linus Torvalds
2005-11-21 19:06 ` Matthew Wilcox
2005-11-21 19:27 ` Linus Torvalds
2005-11-21 19:43 ` Matthew Wilcox
2005-11-21 19:59 ` Linus Torvalds
2005-11-21 21:15 ` Ingo Molnar
2005-11-21 21:25 ` Paul Mackerras
2005-11-21 21:35 ` Ingo Molnar
2005-11-21 21:51 ` Linus Torvalds
2005-11-21 22:09 ` Benjamin Herrenschmidt
2005-11-21 22:34 ` Linus Torvalds
2005-11-21 23:00 ` Benjamin Herrenschmidt
2005-11-21 21:49 ` Linus Torvalds
2005-11-21 22:06 ` Benjamin Herrenschmidt
2005-11-21 22:28 ` Linus Torvalds
2005-11-21 22:58 ` Benjamin Herrenschmidt
2005-11-21 23:20 ` Paul Mackerras
2005-11-22 1:26 ` Linus Torvalds
2005-11-22 2:45 ` Matthew Wilcox
2005-11-21 21:50 ` Benjamin Herrenschmidt
2005-11-21 22:20 ` Alan Cox
2005-11-22 11:13 ` David Woodhouse
2005-11-22 14:15 ` Alan Cox
2005-11-22 14:04 ` Matthew Wilcox
2005-11-22 17:03 ` Linus Torvalds
2005-11-22 18:20 ` Matthew Wilcox
2005-11-22 18:37 ` David Howells
2005-11-22 19:03 ` David Woodhouse
2005-11-22 19:21 ` Linus Torvalds
2005-11-22 23:58 ` David Woodhouse
2005-11-22 19:05 ` Linus Torvalds
2005-11-22 19:38 ` David Howells
2005-11-22 19:51 ` Linus Torvalds
2005-11-23 1:45 ` Pavel Machek
2005-11-21 21:16 ` Benjamin Herrenschmidt
2005-11-21 21:38 ` Linus Torvalds
2005-11-21 21:53 ` Benjamin Herrenschmidt
2005-11-21 22:18 ` Linus Torvalds
2005-11-21 22:20 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).