login
Search
WARNING NOT FOR MERGE WARNING NOT FOR MERGE WARNING NOT FOR MERGE This posting is just to demonstrate something that I have been keeping alive in the background. I have no urge to push it upstream anytime soon. The overwhelming majority of drivers do not ever bother with the 'irq' argument that is passed to each driver's irq handler. Of the minority of drivers that do use the arg, the majority of those have the irq number stored in their private-info structure somewhere. There are a tiny few -- a couple Mac drivers -- which do weird things with that argument, but that's it. For the large sweeps through the tree, these patches are grouped into "trivial" changes -- simply removing the unused irq arg -- or all other changes. [IRQ ARG REMOVAL] core interrupt delivery infrastructure updates [IRQ ARG REMOVAL] various non-trivial arch updates [IRQ ARG REMOVAL] trivial arch updates [IRQ ARG REMOVAL] non-trivial driver updates [IRQ ARG REMOVAL] trivial net driver updates [IRQ ARG REMOVAL] trivial sound driver updates [IRQ ARG REMOVAL] trivial scsi driver updates [IRQ ARG REMOVAL] trivial driver updates [IRQ ARG REMOVAL] x86-64 build fixes, cleanups WARNING NOT FOR MERGE WARNING NOT FOR MERGE WARNING NOT FOR MERGE -
commit 008b5fcf3c1d8456005de26ddd4256b1369225e8
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 00:45:51 2007 -0400
[IRQ ARG REMOVAL] core interrupt delivery infrastructure updates
include/asm-generic/irq_regs.h | 25 +++++++++++++++++++++++++
include/linux/interrupt.h | 4 ++--
kernel/irq/handle.c | 5 +++--
kernel/irq/manage.c | 4 ++--
kernel/irq/spurious.c | 3 ++-
lib/irq_regs.c | 5 +++++
6 files changed, 39 insertions(+), 7 deletions(-)
008b5fcf3c1d8456005de26ddd4256b1369225e8
diff --git a/include/asm-generic/irq_regs.h b/include/asm-generic/irq_regs.h
index 5ae1d07..1d99ef4 100644
--- a/include/asm-generic/irq_regs.h
+++ b/include/asm-generic/irq_regs.h
@@ -34,4 +34,29 @@ static inline struct pt_regs *set_irq_regs(struct pt_regs *new_regs)
return old_regs;
}
+DECLARE_PER_CPU(unsigned int, __irqfunc_irqs);
+
+static inline unsigned int get_irqfunc_irq(void)
+{
+ return __get_cpu_var(__irqfunc_irqs);
+}
+
+#if 0
+static inline unsigned int set_irqfunc_irq(unsigned int new_irq)
+{
+ unsigned int old_irq, *pirq = &__get_cpu_var(__irqfunc_irqs);
+
+ old_irq = *pirq;
+ *pirq = new_irq;
+ return old_irq;
+}
+#else
+static inline void set_irqfunc_irq(unsigned int new_irq)
+{
+ int *pirq = &__get_cpu_var(__irqfunc_irqs);
+
+ *pirq = new_irq;
+}
+#endif
+
#endif /* _ASM_GENERIC_IRQ_REGS_H */
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 2306920..98720ea 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -55,7 +55,7 @@
#define IRQF_NOBALANCING 0x00000800
#define IRQF_IRQPOLL 0x00001000
-typedef irqreturn_t (*irq_handler_t)(int, void *);
+typedef irqreturn_t (*irq_handler_t)(void *);
struct irqaction {
irq_handler_t handler;
@@ -68,7 +68,7 @@ struct irqaction {
struct proc_dir_entry *dir;
};
-extern irqreturn_t no_action(int cpl, void *dev_id);
+extern irqreturn_t no_action(void *dev_id);
...The above two hunks need to call set_irqfunc_irq in your model. Eric -
Duh. Thanks for pointing out an obvious mistake :) Fixed and checked into 'irq-remove' branch of git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/misc-2.6.git -
Please look at handle_IRQ_event. Local irqs are enabled so irq recursion can happen. So not handling old_irq is a big nasty bug. Eric -
Do you think set_irqfunc_irq() should be called at all the callsites of set_irq_regs(), or one the fix you mention is applied, do you think current model is sufficient? Jeff -
Good question. At first glance I think the call sites are ok, that is where we have the information now. Non-genirq architectures needs work of course. However given the weird poll case etc that either we need to take this slow and delay this change until all of the drivers are fixed up, to not need an irq parameter (as you suggested). Or that we need to allow both forms of irq handler to coexist temporarily. Eric -
After diving in, in the past couple of hours, I'm pretty confident we
simply do not need {get,set}_irqfunc_irq()
Jeff
-
Sounds good. That was my impression when I was looking at this kind of stuff. Just so long as this doesn't slow us down so much we don't actually drop the ball on this. Eric -
Hey I'm the one who has kept the ball rolling since the day pt_regs were removed... :) Jeff -
'irq' argument is gone from the entire tree, save for drivers/char/tpm/tpm_tis.c drivers/scsi/sym53c416.c drivers/scsi/NCR53C9x.c drivers/scsi/NCR5380.c drivers/net/hamradio/scc.c drivers/ide/ide-io.c So I'd say the task is within reach :) All the irq handler cleanups have been checked into branch 'irq-cleanups', and 'irq-remove' branch is rebased on top of that. Jeff -
commit 8d45690dd90b18daaa21b981ab20caf393220bf0
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 00:46:23 2007 -0400
[IRQ ARG REMOVAL] various non-trivial arch updates
arch/x86/kernel/vm86_32.c | 3 ++-
include/asm-x86/irq_regs_32.h | 25 +++++++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
8d45690dd90b18daaa21b981ab20caf393220bf0
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 157e4be..18aae9e 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -739,10 +739,11 @@ static int irqbits;
| (1 << SIGUSR1) | (1 << SIGUSR2) | (1 << SIGIO) | (1 << SIGURG) \
| (1 << SIGUNUSED) )
-static irqreturn_t irq_handler(int intno, void *dev_id)
+static irqreturn_t irq_handler(void *dev_id)
{
int irq_bit;
unsigned long flags;
+ unsigned int intno = get_irqfunc_irq();
spin_lock_irqsave(&irqbits_lock, flags);
irq_bit = 1 << intno;
diff --git a/include/asm-x86/irq_regs_32.h b/include/asm-x86/irq_regs_32.h
index 3368b20..68a531d 100644
--- a/include/asm-x86/irq_regs_32.h
+++ b/include/asm-x86/irq_regs_32.h
@@ -26,4 +26,29 @@ static inline struct pt_regs *set_irq_regs(struct pt_regs *new_regs)
return old_regs;
}
+DECLARE_PER_CPU(unsigned int, __irqfunc_irqs);
+
+static inline unsigned int get_irqfunc_irq(void)
+{
+ return __get_cpu_var(__irqfunc_irqs);
+}
+
+#if 0
+static inline unsigned int set_irqfunc_irq(unsigned int new_irq)
+{
+ unsigned int old_irq, *pirq = &__get_cpu_var(__irqfunc_irqs);
+
+ old_irq = *pirq;
+ *pirq = new_irq;
+ return old_irq;
+}
+#else
+static inline void set_irqfunc_irq(unsigned int new_irq)
+{
+ int *pirq = &__get_cpu_var(__irqfunc_irqs);
+
+ *pirq = new_irq;
+}
+#endif
+
#endif /* _ASM_I386_IRQ_REGS_H */
-
x86_write_percpu(__irqfunc_irqs, new_irq) would be slightly more
efficient here. Any why the pointer anyway?
J
-
Why the pointer? Honestly, I cannot recall. Its most likely due to my ignorance of the per-cpu API, which always seemed more complicated than I wished :) This code was carried from the original days when pt_regs was removed from the irq handler arguments, so that's probably why x86_write_percpu was not employed. I'll make note to fix that up... Jeff -
Answering the latter question -- that's the way include/asm-generic/irq_regs.h was written, and I simply followed their lead. One attribute of this method is to avoid preemption, which is probably necessary in the bowels of irq handling. Jeff -
In this case we can easily pass the irqno into request_irq, allowing us to do "unsigned int intno = (unsigned int)dev_id;". I suspect this is the case for the majority of the non-trivial users as well. Eric -
Not that easy, alas :) Save for weirdos like the mac drivers I highlighted, it seems like most drivers in the non-trivial already pass a useful pointer to request_irq(). But as I mentioned, most of the "non-trivial" uses are actually trivial -- just not as simple as removing the 'int irq' argument. Most of the time the irq number is used in non-critical ways like printk's. A few times its used to index into a structure (something dev_id could replace). Jeff -
I was talking about vm86_32.c in particular. Where we allow user space Yes. Eric -
commit bbf90280966a9661e1a5357f9ee8a94450132d71
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 00:46:37 2007 -0400
[IRQ ARG REMOVAL] trivial arch updates
arch/frv/kernel/dma.c | 2 +-
arch/frv/kernel/irq-mb93091.c | 2 +-
arch/frv/kernel/irq-mb93093.c | 2 +-
arch/frv/kernel/irq-mb93493.c | 2 +-
arch/frv/kernel/time.c | 4 ++--
arch/ia64/kernel/irq_ia64.c | 4 ++--
arch/ia64/kernel/machvec.c | 2 +-
arch/ia64/kernel/mca.c | 2 +-
arch/ia64/kernel/time.c | 2 +-
arch/ia64/sn/pci/tioca_provider.c | 2 +-
arch/ia64/sn/pci/tioce_provider.c | 2 +-
arch/mips/kernel/time.c | 2 +-
arch/mips/sgi-ip22/ip22-reset.c | 2 +-
arch/powerpc/platforms/cell/interrupt.c | 2 +-
arch/powerpc/platforms/powermac/low_i2c.c | 2 +-
arch/powerpc/platforms/powermac/pfunc_base.c | 2 +-
arch/powerpc/platforms/pseries/ras.c | 8 ++++----
arch/powerpc/platforms/pseries/xics.c | 4 ++--
arch/powerpc/sysdev/mpic.c | 2 +-
arch/sparc/kernel/irq.c | 2 +-
arch/x86/kernel/i8259_32.c | 2 +-
arch/x86/kernel/time_32.c | 2 +-
arch/x86/kernel/vmiclock_32.c | 2 +-
arch/x86/mach-visws/visws_apic.c | 2 +-
arch/x86/mach-voyager/voyager_basic.c | 2 +-
arch/x86/xen/smp.c | 6 +++---
arch/x86/xen/time.c | 2 +-
include/asm-m68k/floppy.h | 6 +++---
include/asm-m68k/sun3xflop.h | 6 +++---
include/asm-parisc/floppy.h | 6 +++---
include/asm-ppc/floppy.h | 7 +++----
include/asm-sh/floppy.h | 6 +++---
...commit 654f4a242cac0148ffe98ce288c9116e65b08e44
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 00:47:17 2007 -0400
[IRQ ARG REMOVAL] non-trivial driver updates
drivers/atm/ambassador.c | 5 +++--
drivers/bluetooth/btuart_cs.c | 2 +-
drivers/bluetooth/dtl1_cs.c | 2 +-
drivers/char/cyclades.c | 21 +++------------------
drivers/char/ip2/ip2main.c | 10 +++++-----
drivers/char/mwave/tp3780i.c | 10 ++++++----
drivers/char/pcmcia/synclink_cs.c | 8 ++++----
drivers/char/rio/rio_linux.c | 14 ++++++++------
drivers/char/riscom8.c | 4 ++--
drivers/char/specialix.c | 7 ++++---
drivers/char/sx.c | 14 +++++++-------
drivers/char/synclink.c | 4 ++--
drivers/char/synclink_gt.c | 9 ++++-----
drivers/char/synclinkmp.c | 7 +++----
drivers/char/tpm/tpm_tis.c | 6 +++---
drivers/ide/ide-io.c | 8 ++++----
drivers/input/serio/i8042.c | 9 +++++----
drivers/isdn/act2000/act2000_isa.c | 3 ++-
drivers/isdn/hisax/amd7930_fn.c | 2 +-
drivers/isdn/hisax/hisax.h | 2 +-
drivers/isdn/hisax/icc.c | 2 +-
drivers/isdn/hisax/isac.c | 2 +-
drivers/isdn/hisax/w6692.c | 4 ++--
drivers/isdn/sc/interrupt.c | 3 ++-
drivers/macintosh/via-macii.c | 6 +++---
drivers/macintosh/via-maciisi.c | 9 ++++-----
drivers/macintosh/via-pmu68k.c | 10 +++++-----
drivers/media/video/planb.c | 7 ++++---
drivers/net/eexpress.c | 5 +++--
drivers/net/forcedeth.c | 26 +++++++++++++-------------
drivers/net/hamradio/scc.c | 6 +++---
drivers/net/irda/au1k_ir.c | 8 +++++---
drivers/net/irda/smsc-ircc2.c | 6 +++---
drivers/net/irda/via-ircc.c | 6 +++---
...Ok. You have some random cleanups as buried in this patch as well as the necessary changes to remove the irq argument Ok. This is to detect to see if we are being called from the poll Bug because now we don't know if we are being called from the poll Bug because we can't detect if we are being called from the poll Hmm. The corresponding change to prototype is missing and the change to the parport code is missing. Eric -
thanks for the comments! I'll work through these. There are definitely plenty of open issues I haven't yet tackled in the driver area, as you are seeing. As you noted, a lot of these are with drivers doing weird things like calling their own interrupt function with (-1, NULL) or (0, NULL), which triggers some magic "I'm polling" those two parport changes were bogus, and actually got fixed up in patch #9 -
commit fb66571c6fde956fb8bddacf11d64101f8df8bf8
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 00:48:35 2007 -0400
[IRQ ARG REMOVAL] trivial scsi driver updates
drivers/scsi/3w-9xxx.c | 2 +-
drivers/scsi/3w-xxxx.c | 2 +-
drivers/scsi/53c700.c | 2 +-
drivers/scsi/53c700.h | 2 +-
drivers/scsi/BusLogic.c | 2 +-
drivers/scsi/BusLogic.h | 2 +-
drivers/scsi/NCR5380.h | 2 +-
drivers/scsi/NCR53C9x.h | 2 +-
drivers/scsi/NCR53c406a.c | 4 ++--
drivers/scsi/NCR_D700.c | 8 ++++----
drivers/scsi/NCR_Q720.c | 5 ++---
drivers/scsi/a100u2w.c | 2 +-
drivers/scsi/a2091.c | 2 +-
drivers/scsi/a3000.c | 2 +-
drivers/scsi/aacraid/rx.c | 4 ++--
drivers/scsi/aacraid/sa.c | 2 +-
drivers/scsi/advansys.c | 2 +-
drivers/scsi/aha1740.c | 2 +-
drivers/scsi/aic7xxx/aic79xx_osm.c | 2 +-
drivers/scsi/aic7xxx/aic79xx_osm.h | 2 +-
drivers/scsi/aic7xxx/aic7xxx_osm.c | 2 +-
drivers/scsi/aic7xxx/aic7xxx_osm.h | 2 +-
drivers/scsi/aic7xxx_old.c | 4 ++--
drivers/scsi/aic94xx/aic94xx_hwi.c | 2 +-
drivers/scsi/aic94xx/aic94xx_hwi.h | 2 +-
drivers/scsi/arcmsr/arcmsr_hba.c | 2 +-
drivers/scsi/arm/acornscsi.c | 7 +++----
drivers/scsi/arm/cumana_2.c | 5 ++---
drivers/scsi/arm/eesox.c | 5 ++---
drivers/scsi/arm/powertec.c | 5 ++---
drivers/scsi/atari_NCR5380.c | 4 ++--
drivers/scsi/atari_dma_emul.c | 4 ++--
drivers/scsi/atari_scsi.c | 10 +++++-----
drivers/scsi/atp870u.c | 2 +-
drivers/scsi/dc395x.c | 2 +-
drivers/scsi/dec_esp.c | 12 ++++++------
...ACK with comment ... This API changed in 2.4.23 switching to irqreturn_t, and 2.6.19 dropping the struct_pt_regs argument, this is yet another API change in the same function. The last one came with no clues to differentiate except kernel version (for those of us that are required to produce updated back-ported driver modules once this patch becomes a legacy). I am *praying* that this API change is clean across 2.6.24 and add my voice to all to ACK this clearly! -
On Fri, 19 Oct 2007 09:00:23 -0400 That was a goofup. I proposed that we should add a #define TWO_ARG_IRQ_HANDLERS (or whatever) and I think I actually wrote the patch, but it got lost. I agree it would be a kind thing to do in this case. Not that I ever want to see these patches again ;) -
Yep, I was thinking that including #define IRQ_HANDLER_V3 would be a good idea. Jeff -
On Fri, 26 Oct 2007 17:47:58 -0400 it sets a certain precedent though.... we don't do this for the 500 other API changes we do each release (see stable-api-nonsense)... so this one is mostly arbitrary picked out -- If you want to reach me at my work email, use arjan@linux.intel.com For development, discussion and tips for power savings, visit http://www.lesswatts.org -
We do for include/linux/netdevice.h, see HAVE_xxx -- and we should do it because the last irq handler change was a pain for backports, and this makes life easier for the backporters. irq handling is probably far more global than any other kernel API except kmalloc() Jeff -
On Fri, 26 Oct 2007 20:12:40 -0400 to be honest, in a perfect world we turn this around, and have the older kernels that people want to backport to have the LEGACY_IRQ_HANDLER defines... not accumulating going forward.... (in fact, with what you're proposing you'll always get #ifndef's..) the other serious question is.. how is IRQ_HANDLER_V3 different from a #ifdef VERSION >= 2.6.24 ..... it's not really ;) -- If you want to reach me at my work email, use arjan@linux.intel.com For development, discussion and tips for power savings, visit http://www.lesswatts.org -
Note my mention of backport -- kernel version isn't relevant when the various enterprise distros have random featuresets under random kernel versions. Jeff -
On Fri, 26 Oct 2007 20:37:47 -0400 yeah and THEY can put the defines in (RH used to do this fwiw as a generic "this is a RH kernel" define).... but afaik no distro vendor backports such an api change nowadays... and hasn't in 2.6 ever -- If you want to reach me at my work email, use arjan@linux.intel.com For development, discussion and tips for power savings, visit http://www.lesswatts.org -
People backport drivers all the time that must support a wide range of kernels. Jeff -
I thought the argument was not that drivers are back ported but that internal kernel APIs changes aren't backported. So that testing the kernel version actually has a chance as a reasonable test for features. Eric -
On Sat, 27 Oct 2007 01:46:15 -0600 or at least for this level of API change For this specific change, I don't see ANY distro just backporting this ... so a version test is just fine for this change imo. -- If you want to reach me at my work email, use arjan@linux.intel.com For development, discussion and tips for power savings, visit http://www.lesswatts.org -
commit 6348eaa5c5320cc3c4fac5ca1d41b587388e74d9
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 00:48:10 2007 -0400
[IRQ ARG REMOVAL] trivial sound driver updates
sound/aoa/core/snd-aoa-gpio-feature.c | 2 +-
sound/aoa/soundbus/i2sbus/i2sbus-core.c | 2 +-
sound/aoa/soundbus/i2sbus/i2sbus-pcm.c | 4 ++--
sound/aoa/soundbus/i2sbus/i2sbus.h | 4 ++--
sound/arm/aaci.c | 2 +-
sound/arm/pxa2xx-ac97.c | 2 +-
sound/drivers/mpu401/mpu401_uart.c | 4 ++--
sound/drivers/mtpav.c | 2 +-
sound/drivers/serial-u16550.c | 2 +-
sound/drivers/vx/vx_core.c | 2 +-
sound/isa/ad1816a/ad1816a_lib.c | 2 +-
sound/isa/ad1848/ad1848_lib.c | 2 +-
sound/isa/cs423x/cs4231_lib.c | 2 +-
sound/isa/es1688/es1688_lib.c | 2 +-
sound/isa/es18xx.c | 4 ++--
sound/isa/gus/gus_irq.c | 2 +-
sound/isa/gus/gusmax.c | 6 +++---
sound/isa/gus/interwave.c | 6 +++---
sound/isa/opl3sa2.c | 6 +++---
sound/isa/opti9xx/opti92x-ad1848.c | 2 +-
sound/isa/sb/es968.c | 2 +-
sound/isa/sb/sb16_main.c | 4 ++--
sound/isa/sb/sb8.c | 2 +-
sound/isa/sgalaxy.c | 2 +-
sound/isa/wavefront/wavefront.c | 2 +-
sound/mips/au1x00.c | 2 +-
sound/oss/ad1848.c | 4 ++--
sound/oss/btaudio.c | 2 +-
sound/oss/dmasound/dmasound_atari.c | 4 ++--
sound/oss/dmasound/dmasound_paula.c | 4 ++--
sound/oss/dmasound/dmasound_q40.c | 8 ++++----
sound/oss/hal2.c | 2 +-
sound/oss/i810_audio.c | 2 +-
sound/oss/mpu401.c | 2 +-
sound/oss/mpu401.h | 2 +-
...commit 93e93ce573545b3702477088cba8650b565fd60e
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 00:47:56 2007 -0400
[IRQ ARG REMOVAL] trivial net driver updates
drivers/net/3c501.c | 3 +--
drivers/net/3c501.h | 2 +-
drivers/net/3c505.c | 2 +-
drivers/net/3c507.c | 4 ++--
drivers/net/3c509.c | 6 +++---
drivers/net/3c515.c | 4 ++--
drivers/net/3c523.c | 4 ++--
drivers/net/3c527.c | 4 ++--
drivers/net/3c59x.c | 14 +++++++-------
drivers/net/7990.c | 4 ++--
drivers/net/8139cp.c | 4 ++--
drivers/net/8139too.c | 6 +++---
drivers/net/82596.c | 6 +++---
drivers/net/8390.c | 4 ++--
drivers/net/8390.h | 2 +-
drivers/net/a2065.c | 2 +-
drivers/net/acenic.c | 2 +-
drivers/net/acenic.h | 2 +-
drivers/net/amd8111e.c | 4 ++--
drivers/net/apne.c | 4 ++--
drivers/net/appletalk/cops.c | 4 ++--
drivers/net/appletalk/ltpc.c | 2 +-
drivers/net/arcnet/arcnet.c | 2 +-
drivers/net/ariadne.c | 4 ++--
drivers/net/arm/am79c961a.c | 6 +++---
drivers/net/arm/at91_ether.c | 4 ++--
drivers/net/arm/ep93xx_eth.c | 2 +-
drivers/net/arm/ether1.c | 4 ++--
drivers/net/arm/ether3.c | 4 ++--
drivers/net/at1700.c | 4 ++--
drivers/net/atarilance.c | 4 ++--
drivers/net/atl1/atl1_main.c | 4 ++--
...commit 497cd0c681f0d7144a240af45f5d5eaf5aea75ae
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 01:27:46 2007 -0400
[IRQ ARG REMOVAL] x86-64 build fixes, cleanups
arch/x86/kernel/time_64.c | 2 +-
drivers/atm/horizon.c | 2 +-
drivers/input/touchscreen/ucb1400_ts.c | 2 +-
drivers/mtd/onenand/onenand_base.c | 2 +-
drivers/net/cxgb3/adapter.h | 2 +-
drivers/net/netxen/netxen_nic_main.c | 2 +-
drivers/parport/parport_mfc3.c | 2 +-
drivers/scsi/gdth.c | 27 +++++++++++++--------------
sound/drivers/mts64.c | 2 --
sound/drivers/portman2x4.c | 2 --
10 files changed, 20 insertions(+), 25 deletions(-)
497cd0c681f0d7144a240af45f5d5eaf5aea75ae
diff --git a/arch/x86/kernel/time_64.c b/arch/x86/kernel/time_64.c
index c821edc..dd16458 100644
--- a/arch/x86/kernel/time_64.c
+++ b/arch/x86/kernel/time_64.c
@@ -148,7 +148,7 @@ int update_persistent_clock(struct timespec now)
return set_rtc_mmss(now.tv_sec);
}
-static irqreturn_t timer_event_interrupt(int irq, void *dev_id)
+static irqreturn_t timer_event_interrupt(void *dev_id)
{
add_pda(irq0_irqs, 1);
diff --git a/drivers/atm/horizon.c b/drivers/atm/horizon.c
index 9b2cf25..ddd1064 100644
--- a/drivers/atm/horizon.c
+++ b/drivers/atm/horizon.c
@@ -1382,7 +1382,7 @@ static inline void rx_data_av_handler (hrz_dev * dev) {
/********** interrupt handler **********/
-static irqreturn_t interrupt_handler(int irq, void *dev_id)
+static irqreturn_t interrupt_handler(void *dev_id)
{
hrz_dev *dev = dev_id;
u32 int_source;
diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c
index 68dac18..1a15e7f 100644
--- a/drivers/input/touchscreen/ucb1400_ts.c
+++ b/drivers/input/touchscreen/ucb1400_ts.c
@@ -356,7 +356,7 @@ static int ucb1400_ts_thread(void *_ucb)
* interrupt by scheduling a RT kernel thread to run in process
...commit 52afddf59be0049d4118b21bdb1ef6bd1c5a9165
Author: Jeff Garzik <jeff@garzik.org>
Date: Fri Oct 19 00:48:47 2007 -0400
[IRQ ARG REMOVAL] trivial driver updates
drivers/acpi/osl.c | 2 +-
drivers/ata/ahci.c | 2 +-
drivers/ata/libata-core.c | 2 +-
drivers/ata/pdc_adma.c | 2 +-
drivers/ata/sata_inic162x.c | 2 +-
drivers/ata/sata_mv.c | 2 +-
drivers/ata/sata_nv.c | 20 ++++++++++----------
drivers/ata/sata_promise.c | 2 +-
drivers/ata/sata_qstor.c | 2 +-
drivers/ata/sata_sil.c | 2 +-
drivers/ata/sata_sil24.c | 2 +-
drivers/ata/sata_sx4.c | 2 +-
drivers/ata/sata_vsc.c | 2 +-
drivers/atm/eni.c | 2 +-
drivers/atm/firestream.c | 2 +-
drivers/atm/fore200e.c | 2 +-
drivers/atm/he.c | 4 ++--
drivers/atm/idt77252.c | 2 +-
drivers/atm/iphase.c | 2 +-
drivers/atm/lanai.c | 2 +-
drivers/atm/nicstar.c | 4 ++--
drivers/atm/zatm.c | 2 +-
drivers/block/DAC960.c | 21 +++++++--------------
drivers/block/DAC960.h | 12 ++++++------
drivers/block/amiflop.c | 4 ++--
drivers/block/ataflop.c | 4 ++--
drivers/block/cciss.c | 6 +++---
drivers/block/cpqarray.c | 4 ++--
drivers/block/floppy.c | 4 ++--
drivers/block/lguest_blk.c | 2 +-
drivers/block/ps2esdi.c | 4 ++--
drivers/block/swim3.c | 8 ++++----
drivers/block/sx8.c ...Jeff, thanks for doing this. Full ACK. We should do this right at the edge of -rc1. And let's do this right now in .24 and not drag it out for no good reason. Thanks, tglx -
Yes. keeping this alive is good.
The practical question is how do we make this change without breaking
There are two problems with that suggestion.
- We don't have all of the architectures converted.
- We don't have a solid plan for how to keep drivers that are
using the irq parameter today working.
I don't think the irq argument is something we want to keep
around forever, and I certainly don't see the need to do
the error prone get_irqfunc_irq and set_irqfunc_irq logic.
How about:
irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction *action)
{
irqreturn_t ret, retval = IRQ_NONE;
unsigned int status = 0;
handle_dynamic_tick(action);
if (!(action->flags & IRQF_DISABLED))
local_irq_enable_in_hardirq();
do {
if (action->flags & IRQF_VERBOSE)
ret = action->handler_verbose(irq, action->dev_id);
else
ret = action->handler(action->dev_id);
if (ret == IRQ_HANDLED)
status |= action->flags;
retval |= ret;
action = action->next;
} while (action);
if (status & IRQF_SAMPLE_RANDOM)
add_interrupt_randomness(irq);
local_irq_disable();
return retval;
}
And then:
typedef irqreturn_t (*irq_handler_verbose_t)(int, void *);
typedef irqreturn_t (*irq_handler_t)(void *);
struct irqaction {
union {
irq_handler_verbose_t handler_verbose;
irq_handler_t handler;
};
unsigned long flags;
cpumask_t mask;
const char *name;
void *dev_id;
struct irqaction *next;
int irq;
struct proc_dir_entry *dir;
};
int request_irq(unsigned int irq, irq_handler_t handler,
unsigned long irqflags, const char *devname, void *dev_id)
int request_verbose_irq(unsigned int irq, irq_handler_verbose_t handler,
unsigned long irqflags, const char *devname, void *dev_id)
Then we just need to go through all of the drivers and either change
their interrupt handler prototype or change the flavor of request_irq.
It is a bit of a pain but we should be able to do that without breaking
any drivers. ...This is why I'm taking it slow, and not rushing to get this upstream :) I am finding a ton of bugs in each get_irqfunc_irq() driver, so I would rather patiently sift through them, and push fixes and cleanups upstream. Once that effort is done, everything should be in the 'trivial' pile and not have the logic that you are worried about (and thus there would be no need to add an additional branch to the error handling path). Jeff -
er, s/error/irq/ the perils of a multi-threaded brain... -
the get_irq_regs() approach worked out really well. We should do a get_irq_nr() and be done with it? (Then once the mechanic conversion has been done we could eliminate all uses of get_irq_nr() and remove the small overhead it takes to maintain this per-cpu value in the irq entry layer.) full ACK on the concept from me too. Please go ahead! :) Ingo -
The problem are some drivers today pass in 0 for their irq number to flag that they are calling the interrupt handler in a polling mode (not from interrupt context?) so the same logic doesn't quite apply. Do what you suggest would likely break those drivers. Eric -
How many of them do we have ? This is a wilful abuse of the API, so
its not a big damage if they break.
tglx
-
I would prefer to simply clean up the drivers such that get_irqfunc_irq() and set_irqfunc_irq() are not needed. One of the many reasons why I'm explicitly not pushing it upstream yet :) Jeff -
