Re: [PATCH 01/15] ARM minor irq handler cleanups

Previous thread: none

Next thread: New location of Namesys software by Edward Shishkin on Friday, April 18, 2008 - 4:38 pm. (4 messages)
From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

Avoid confusion by /not/ passing an unused pointer to
arm_rtc_interrupt()

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/arm/mach-integrator/time.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-integrator/time.c b/arch/arm/mach-integrator/time.c
index 5278f58..5235f64 100644
--- a/arch/arm/mach-integrator/time.c
+++ b/arch/arm/mach-integrator/time.c
@@ -125,7 +125,7 @@ static int rtc_probe(struct amba_device *dev, void *id)
 	xtime.tv_sec = __raw_readl(rtc_base + RTC_DR);
 
 	ret = request_irq(dev->irq[0], arm_rtc_interrupt, IRQF_DISABLED,
-			  "rtc-pl030", dev);
+			  "rtc-pl030", NULL);
 	if (ret)
 		goto map_out;
 
-- 
1.5.4.1

--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

- mark timer_interrupt() static

- sparc_floppy_request_irq() prototype should use irq_handler_t

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/sparc/kernel/time.c   |    2 +-
 include/asm-sparc/floppy.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/time.c b/arch/sparc/kernel/time.c
index cfaf22c..53caacb 100644
--- a/arch/sparc/kernel/time.c
+++ b/arch/sparc/kernel/time.c
@@ -105,7 +105,7 @@ __volatile__ unsigned int *master_l10_limit;
 
 #define TICK_SIZE (tick_nsec / 1000)
 
-irqreturn_t timer_interrupt(int irq, void *dev_id)
+static irqreturn_t timer_interrupt(int dummy, void *dev_id)
 {
 	/* last time the cmos clock got updated */
 	static long last_rtc_update;
diff --git a/include/asm-sparc/floppy.h b/include/asm-sparc/floppy.h
index dbe7a58..d3978e0 100644
--- a/include/asm-sparc/floppy.h
+++ b/include/asm-sparc/floppy.h
@@ -280,7 +280,7 @@ static inline void sun_fd_enable_dma(void)
 
 /* Our low-level entry point in arch/sparc/kernel/entry.S */
 extern int sparc_floppy_request_irq(int irq, unsigned long flags,
-				    irqreturn_t (*irq_handler)(int irq, void *));
+				    irq_handler_t irq_handler);
 
 static int sun_fd_request_irq(void)
 {
-- 
1.5.4.1

--

From: David Miller
Date: Friday, April 18, 2008 - 4:33 pm

From: Jeff Garzik <jeff@garzik.org>

Acked-by: David S. Miller <davem@davemloft.net>
--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

- time_sched_init() should use standard irq_handler_t

- sys_timer0_int_handler() should not take 'regs' third argument

- remove pointless cast from void*

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/blackfin/kernel/time.c        |    5 ++---
 arch/blackfin/oprofile/timer_int.c |    5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/blackfin/kernel/time.c b/arch/blackfin/kernel/time.c
index 9bdc8f9..715b394 100644
--- a/arch/blackfin/kernel/time.c
+++ b/arch/blackfin/kernel/time.c
@@ -39,8 +39,7 @@
 /* This is an NTP setting */
 #define	TICK_SIZE (tick_nsec / 1000)
 
-static void time_sched_init(irqreturn_t(*timer_routine)
-			(int, void *));
+static void time_sched_init(irq_handler_t timer_routine);
 static unsigned long gettimeoffset(void);
 
 static struct irqaction bfin_timer_irq = {
@@ -64,7 +63,7 @@ static struct irqaction bfin_timer_irq = {
 #define TIME_SCALE 1
 
 static void
-time_sched_init(irqreturn_t(*timer_routine) (int, void *))
+time_sched_init(irq_handler_t timer_routine)
 {
 	u32 tcount;
 
diff --git a/arch/blackfin/oprofile/timer_int.c b/arch/blackfin/oprofile/timer_int.c
index 6c6f860..e2c825a 100644
--- a/arch/blackfin/oprofile/timer_int.c
+++ b/arch/blackfin/oprofile/timer_int.c
@@ -40,10 +40,9 @@ static void disable_sys_timer0()
 {
 }
 
-static irqreturn_t sys_timer0_int_handler(int irq, void *dev_id,
-					  struct pt_regs *regs)
+static irqreturn_t sys_timer0_int_handler(int irq, void *dev_id)
 {
-	oprofile_add_sample(regs, 0);
+	oprofile_add_sample(get_irq_regs(), 0);
 	return IRQ_HANDLED;
 }
 
-- 
1.5.4.1

--

From: Bryan Wu
Date: Monday, April 21, 2008 - 8:27 pm

Sorry for the delay, it is pretty good for Blackfin.

Acked-by: Bryan Wu <cooloney@kernel.org>

-Bryan
--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

- whitespace cleanups

- remove pointless prototype (uses always follow func implementation)

- 'irq' argument is often used purely as a local variable.  rename
  argument to 'dummy' and define 'irq' as local to make this plain.

- remove pointless casts from void*

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/ppc/8xx_io/fec.c        |    3 +--
 arch/ppc/platforms/sbc82xx.c |    4 +++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/ppc/8xx_io/fec.c b/arch/ppc/8xx_io/fec.c
index 11b0aa6..d7b7ba9 100644
--- a/arch/ppc/8xx_io/fec.c
+++ b/arch/ppc/8xx_io/fec.c
@@ -199,7 +199,6 @@ static int fec_enet_start_xmit(struct sk_buff *skb, struct net_device *dev);
 #ifdef	CONFIG_USE_MDIO
 static void fec_enet_mii(struct net_device *dev);
 #endif	/* CONFIG_USE_MDIO */
-static irqreturn_t fec_enet_interrupt(int irq, void * dev_id);
 #ifdef CONFIG_FEC_PACKETHOOK
 static void  fec_enet_tx(struct net_device *dev, __u32 regval);
 static void  fec_enet_rx(struct net_device *dev, __u32 regval);
@@ -472,7 +471,7 @@ fec_timeout(struct net_device *dev)
  * This is called from the MPC core interrupt.
  */
 static	irqreturn_t
-fec_enet_interrupt(int irq, void * dev_id)
+fec_enet_interrupt(int irq, void *dev_id)
 {
 	struct	net_device *dev = dev_id;
 	volatile fec_t	*fecp;
diff --git a/arch/ppc/platforms/sbc82xx.c b/arch/ppc/platforms/sbc82xx.c
index cc0935c..0df6aac 100644
--- a/arch/ppc/platforms/sbc82xx.c
+++ b/arch/ppc/platforms/sbc82xx.c
@@ -121,8 +121,10 @@ struct hw_interrupt_type sbc82xx_i8259_ic = {
 	.end = sbc82xx_i8259_end_irq,
 };
 
-static irqreturn_t sbc82xx_i8259_demux(int irq, void *dev_id)
+static irqreturn_t sbc82xx_i8259_demux(int dummy, void *dev_id)
 {
+	int irq;
+
 	spin_lock(&sbc82xx_i8259_lock);
 
 	sbc82xx_i8259_map[0] = 0x0c;	/* OCW3: Read IR ...
From: Kumar Gala
Date: Saturday, April 19, 2008 - 7:57 am

arch/ppc is pretty much left for dead at this point.  I'm guessing we  
will end up removing it 2.6.27 if we follow through with our plans of  
killing it this summer.

Acked-by: Kumar Gala <galak@kernel.crashing.org>

- k
--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

- remove pointless casts from void*

- remove needless references to 'irq' function argument, when that
  information is already stored somewhere in a driver-private struct.

- where the 'irq' function argument is known never to be used, rename
  it to 'dummy' to make this more obvious

- remove always-false tests for dev_id==NULL

- remove always-true tests for 'irq == host_struct->irq'

- replace per-irq lookup functions and tables with a direct reference
  to data object obtained via 'dev_id' function argument, passed from
  request_irq()

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

---
 drivers/scsi/aha152x.c |    7 +------
 drivers/scsi/eata.c    |   11 ++++-------
 drivers/scsi/u14-34f.c |    9 ++++-----
 3 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index 6ccdc96..a09b2d3 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -1432,15 +1432,10 @@ static void run(struct work_struct *work)
  */
 static irqreturn_t intr(int irqno, void *dev_id)
 {
-	struct Scsi_Host *shpnt = (struct Scsi_Host *)dev_id;
+	struct Scsi_Host *shpnt = dev_id;
 	unsigned long flags;
 	unsigned char rev, dmacntrl0;
 
-	if (!shpnt) {
-		printk(KERN_ERR "aha152x: catched interrupt %d for unknown controller.\n", irqno);
-		return IRQ_NONE;
-	}
-
 	/*
 	 * Read a couple of registers that are known to not be all 1's. If
 	 * we read all 1's (-1), that means that either:
diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c
index 8be3d76..a73a6bb 100644
--- a/drivers/scsi/eata.c
+++ b/drivers/scsi/eata.c
@@ -2286,17 +2286,14 @@ static void flush_dev(struct scsi_device *dev, unsigned long cursec,
 	}
 }
 
-static irqreturn_t ihdlr(int irq, struct Scsi_Host *shost)
+static irqreturn_t ihdlr(struct Scsi_Host *shost)
 {
 	struct scsi_cmnd *SCpnt;
 	unsigned int i, k, c, ...
From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

- remove always-false tests

- don't overload 'irq' argument, pass data properly via dev_id

- remove pointless casts from void*

- make polled uses of irq handling (where code calls irq handler
  directly) more plain, by splitting polled and non-polled irq handling
  work.  This quite often results in a more-efficient irq handler.

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/char/cyclades.c           |   17 +----------------
 drivers/char/mwave/tp3780i.c      |   14 ++++++++++----
 drivers/char/pcmcia/synclink_cs.c |   10 ++++------
 drivers/char/rio/rio_linux.c      |   23 +++++++++++++----------
 drivers/char/specialix.c          |   10 ++++------
 drivers/char/stallion.c           |    2 +-
 drivers/char/sx.c                 |   22 ++++++++++++++--------
 drivers/char/synclink.c           |   21 +++++++++------------
 drivers/char/synclink_gt.c        |   13 ++++---------
 drivers/char/synclinkmp.c         |   22 +++++++++-------------
 drivers/char/tpm/tpm_tis.c        |    2 +-
 11 files changed, 70 insertions(+), 86 deletions(-)

diff --git a/drivers/char/cyclades.c b/drivers/char/cyclades.c
index e4f579c..823e169 100644
--- a/drivers/char/cyclades.c
+++ b/drivers/char/cyclades.c
@@ -1318,13 +1318,6 @@ static irqreturn_t cyy_interrupt(int irq, void *dev_id)
 	unsigned int chip, too_many, had_work;
 	int index;
 
-	if (unlikely(cinfo == NULL)) {
-#ifdef CY_DEBUG_INTERRUPTS
-		printk(KERN_DEBUG "cyy_interrupt: spurious interrupt %d\n",irq);
-#endif
-		return IRQ_NONE;	/* spurious interrupt */
-	}
-
 	card_base_addr = cinfo->base_addr;
 	index = cinfo->bus_index;
 
@@ -1730,17 +1723,9 @@ static irqreturn_t cyz_interrupt(int irq, void *dev_id)
 {
 	struct cyclades_card *cinfo = dev_id;
 
-	if (unlikely(cinfo == NULL)) {
-#ifdef ...
From: Rogier Wolff
Date: Friday, April 18, 2008 - 11:00 pm

The changes look sane, (for the drivers that I'm involved with). 

However, you're now adding a second parameter to my interrupt
routines: "polled" to replace the "never used" irq parameter. (which
/was/ used to determine wether we were called polled or not... 

On the other hand, most hardware should work if you remove the if
(!polled).


You added a "XXX Using free_irq in the interrupt is not wise!". When I
wrote that code, I didn't know about this. These lines triggered when
the level-triggered PCI interrupt stuck "active" this would mean that
NO userspace code would get executed anymore: Hard lock up. Difficult
to debug. This happend a few times during development when the code
behind the "if (!polled)": "tell the hardware we've seen the
interrupt" didn't work. On the other hand, some failures in the field
have triggered this. So I think it's wise to keep it in. Disabling the
interrupt on the card is not an option, because that's exactly what
this is supposed to catch: We're unable to make the card stop
interrupting the CPU.

Note that it also doesn't work (i.e. hard lock of the machine) if some
other driver is using the same interupt.

	Roger. 

-- 
** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 **
**    Delftechpark 26 2628 XH  Delft, The Netherlands. KVK: 27239233    **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ
--

From: Benjamin Herrenschmidt
Date: Tuesday, April 22, 2008 - 1:05 am

You should let the kernel generic code deal with the runaway interrupt,
it should be capable of doing so nowadays pretty reliably.

free_irq() is definitely not going to be happy when it start messing
with /proc from an interrupt... It will at least give you a WARN_ON.

Ben.


--

From: Rogier Wolff
Date: Tuesday, April 22, 2008 - 3:13 am

The situation is NOT normal operation. It is an emergency measure in
an attempt to prevent a full hang. It is great that other parts of the
kernel also "shout" that something is wrong.

Consider it similar to a "kernel null pointer dereference". Once that
happens, all bets are off. In practise you've probably seen one, and
you were able to continue to work. It is advisable to save everything
you can, and reboot. This is similar. 

The "generic code for runaway interrupts" didn't exist when this was
written. If it exists, and works for the case that this was written
for, then all is fine, and we can remove my code. As you can see, I
copied over the code from one driver to the next after I got bitten
again with the second driver. So having something generic is of course
preferable. :-)

	Roger. 

-- 
** R.E.Wolff@BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 **
**    Delftechpark 26 2628 XH  Delft, The Netherlands. KVK: 27239233    **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ
--

From: Benjamin Herrenschmidt
Date: Tuesday, April 22, 2008 - 3:46 am

But free_irq at interrupt time has great chances of crashing the machine
or currupting data structures in subtle ways. You should -not- do that.

Either let the kernel handle the runaway interrupt, or eventually if you

It would be if you used something like disable_irq_nosync. Not free_irq.

Well, if you know how to trigger the bug, it would be useful to verify
that the kernel generic code properly detects and masks the runaway
interrupt. If that works, then remove your code completely. If not, it
would be useful to figure out why :-) But either way, just replace it
with disable_irq_nosync.

Cheers,
Ben.


--

From: Alan Cox
Date: Saturday, April 19, 2008 - 9:16 am

If you are debugging IRQ funnies you really really want to know which
IRQ or which port.

Ack the rest on the base the __foo_interrupt uglies will go away with the
final change over. A follow up patch to tweak cyclades would be useful and also
to know from Andrew if these are going in so they don't clash with the coding
style cleanups also queued.

--

From: Jeff Garzik
Date: Sunday, April 20, 2008 - 6:38 pm

In the stuff I pushed just now, I took the attitude "if there was 
remotely a peep from anyone, do not send it"

So the stuff ya'll mentioned shouldn't be in the push at all.

In particular, I wanted to note the __foo_interrupt stuff will not go 
away when I remove the 'irq' arg from all handlers...  we still need to 
indicate two separate callsites (local poll or system irq) to achieve 
the same behavior as exists today.

	Jeff


--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

- where the 'irq' function argument is known never to be used, rename
  it to 'dummy' to make this more obvious

- replace per-irq lookup functions and tables with a direct reference
  to data object obtained via 'dev_id' function argument, passed from
  request_irq()

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/scsi/aha1542.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/aha1542.c b/drivers/scsi/aha1542.c
index 5a1471c..1375915 100644
--- a/drivers/scsi/aha1542.c
+++ b/drivers/scsi/aha1542.c
@@ -153,8 +153,6 @@ struct aha1542_hostdata {
 
 #define HOSTDATA(host) ((struct aha1542_hostdata *) &host->hostdata)
 
-static struct Scsi_Host *aha_host[7];	/* One for each IRQ level (9-15) */
-
 static DEFINE_SPINLOCK(aha1542_lock);
 
 
@@ -163,8 +161,7 @@ static DEFINE_SPINLOCK(aha1542_lock);
 
 static void setup_mailboxes(int base_io, struct Scsi_Host *shpnt);
 static int aha1542_restart(struct Scsi_Host *shost);
-static void aha1542_intr_handle(struct Scsi_Host *shost, void *dev_id);
-static irqreturn_t do_aha1542_intr_handle(int irq, void *dev_id);
+static void aha1542_intr_handle(struct Scsi_Host *shost);
 
 #define aha1542_intr_reset(base)  outb(IRST, CONTROL(base))
 
@@ -404,23 +401,19 @@ fail:
 }
 
 /* A quick wrapper for do_aha1542_intr_handle to grab the spin lock */
-static irqreturn_t do_aha1542_intr_handle(int irq, void *dev_id)
+static irqreturn_t do_aha1542_intr_handle(int dummy, void *dev_id)
 {
 	unsigned long flags;
-	struct Scsi_Host *shost;
-
-	shost = aha_host[irq - 9];
-	if (!shost)
-		panic("Splunge!");
+	struct Scsi_Host *shost = dev_id;
 
 	spin_lock_irqsave(shost->host_lock, flags);
-	aha1542_intr_handle(shost, dev_id);
+	aha1542_intr_handle(shost);
 ...
From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

- remove pointless casts from void*

- remove braces around singleton C statements

- remove unused 'intno' argument from rs_interrupt_elsa()

- cs->irq_func() should be defined using standard irq_handler_t

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/isdn/hisax/elsa.c     |    4 ++--
 drivers/isdn/hisax/elsa_ser.c |    4 ++--
 drivers/isdn/hisax/hisax.h    |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/isdn/hisax/elsa.c b/drivers/isdn/hisax/elsa.c
index d272d8c..2c3691f 100644
--- a/drivers/isdn/hisax/elsa.c
+++ b/drivers/isdn/hisax/elsa.c
@@ -299,7 +299,7 @@ elsa_interrupt(int intno, void *dev_id)
 		val = serial_inp(cs, UART_IIR);
 		if (!(val & UART_IIR_NO_INT)) {
 			debugl1(cs,"IIR %02x", val);
-			rs_interrupt_elsa(intno, cs);
+			rs_interrupt_elsa(cs);
 		}
 	}
 #endif
@@ -379,7 +379,7 @@ elsa_interrupt_ipac(int intno, void *dev_id)
 		val = serial_inp(cs, UART_IIR);
 		if (!(val & UART_IIR_NO_INT)) {
 			debugl1(cs,"IIR %02x", val);
-			rs_interrupt_elsa(intno, cs);
+			rs_interrupt_elsa(cs);
 		}
 	}
 #endif
diff --git a/drivers/isdn/hisax/elsa_ser.c b/drivers/isdn/hisax/elsa_ser.c
index 1642dca..f181db4 100644
--- a/drivers/isdn/hisax/elsa_ser.c
+++ b/drivers/isdn/hisax/elsa_ser.c
@@ -384,13 +384,13 @@ static inline void transmit_chars(struct IsdnCardState *cs, int *intr_done)
 }
 
 
-static void rs_interrupt_elsa(int irq, struct IsdnCardState *cs)
+static void rs_interrupt_elsa(struct IsdnCardState *cs)
 {
 	int status, iir, msr;
 	int pass_counter = 0;
 	
 #ifdef SERIAL_DEBUG_INTR
-	printk("rs_interrupt_single(%d)...", irq);
+	printk(KERN_DEBUG "rs_interrupt_single(%d)...", cs->irq);
 #endif
 
 	do {
diff --git a/drivers/isdn/hisax/hisax.h b/drivers/isdn/hisax/hisax.h
index 34733c9..e8d429f 100644
--- ...
From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/avr32/kernel/time.c             |    4 ++--
 arch/avr32/mach-at32ap/time-tc.c     |    2 +-
 include/asm-avr32/arch-at32ap/time.h |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/avr32/kernel/time.c b/arch/avr32/kernel/time.c
index 36a46c3..d6bb69c 100644
--- a/arch/avr32/kernel/time.c
+++ b/arch/avr32/kernel/time.c
@@ -150,7 +150,7 @@ int __weak avr32_hpt_start(void)
  *
  * In UP mode, it is invoked from the (global) timer_interrupt.
  */
-void local_timer_interrupt(int irq, void *dev_id)
+void local_timer_interrupt(void *dev_id)
 {
 	if (current->pid)
 		profile_tick(CPU_PROFILING);
@@ -175,7 +175,7 @@ irqreturn_t __weak timer_interrupt(int irq, void *dev_id)
 	 *
 	 * SMP is not supported yet.
 	 */
-	local_timer_interrupt(irq, dev_id);
+	local_timer_interrupt(dev_id);
 
 	return IRQ_HANDLED;
 }
diff --git a/arch/avr32/mach-at32ap/time-tc.c b/arch/avr32/mach-at32ap/time-tc.c
index 1026586..53e8e6e 100644
--- a/arch/avr32/mach-at32ap/time-tc.c
+++ b/arch/avr32/mach-at32ap/time-tc.c
@@ -209,7 +209,7 @@ irqreturn_t timer_interrupt(int irq, void *dev_id)
 		 *
 		 * SMP is not supported yet.
 		 */
-		local_timer_interrupt(irq, dev_id);
+		local_timer_interrupt(dev_id);
 
 		return IRQ_HANDLED;
 	}
diff --git a/include/asm-avr32/arch-at32ap/time.h b/include/asm-avr32/arch-at32ap/time.h
index cc8a434..d23b5b0 100644
--- a/include/asm-avr32/arch-at32ap/time.h
+++ b/include/asm-avr32/arch-at32ap/time.h
@@ -13,7 +13,7 @@
 
 extern struct irqaction timer_irqaction;
 extern struct platform_device at32_systc0_device;
-extern void local_timer_interrupt(int irq, void *dev_id);
+extern void local_timer_interrupt(void *dev_id);
 
 #define TIMER_BCR					0x000000c0
 #define TIMER_BCR_SYNC						 0
-- ...
From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:22 pm

- remove unused 'irq' argument from pfm_do_interrupt_handler()

- remove pointless cast to void*

- add KERN_xxx prefix to printk()

- remove braces around singleton C statement

- in tioce_provider.c, start tioce_dma_consistent() and
  tioce_error_intr_handler() function declarations in column 0

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/ia64/kernel/perfmon.c        |    4 ++--
 arch/ia64/sn/kernel/huberror.c    |    4 ++--
 arch/ia64/sn/pci/tioce_provider.c |    6 ++++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index d1d24f4..c8e4037 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -5511,7 +5511,7 @@ stop_monitoring:
 }
 
 static int
-pfm_do_interrupt_handler(int irq, void *arg, struct pt_regs *regs)
+pfm_do_interrupt_handler(void *arg, struct pt_regs *regs)
 {
 	struct task_struct *task;
 	pfm_context_t *ctx;
@@ -5591,7 +5591,7 @@ pfm_interrupt_handler(int irq, void *arg)
 
 		start_cycles = ia64_get_itc();
 
-		ret = pfm_do_interrupt_handler(irq, arg, regs);
+		ret = pfm_do_interrupt_handler(arg, regs);
 
 		total_cycles = ia64_get_itc();
 
diff --git a/arch/ia64/sn/kernel/huberror.c b/arch/ia64/sn/kernel/huberror.c
index 0101c79..08b0d9b 100644
--- a/arch/ia64/sn/kernel/huberror.c
+++ b/arch/ia64/sn/kernel/huberror.c
@@ -187,8 +187,8 @@ void hub_error_init(struct hubdev_info *hubdev_info)
 {
 
 	if (request_irq(SGI_II_ERROR, hub_eint_handler, IRQF_SHARED,
-			"SN_hub_error", (void *)hubdev_info)) {
-		printk("hub_error_init: Failed to request_irq for 0x%p\n",
+			"SN_hub_error", hubdev_info)) {
+		printk(KERN_ERR "hub_error_init: Failed to request_irq for 0x%p\n",
 		    hubdev_info);
 		return;
 	}
diff --git a/arch/ia64/sn/pci/tioce_provider.c ...
From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:23 pm

- pass rtc_int_flag to rtc_interrupt() via the standard method, dev_id

- where irq handler has been analyzed and shown never to use its
  'irq' argument, rename it to 'dummy'

- remove pointless casts from void*

- remove uses of 'irq' function arg where it duplicates information
  already stored in driver-private struct

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/x86/kernel/hpet.c       |    4 ++--
 drivers/char/rtc.c           |    7 ++++---
 drivers/rtc/rtc-at32ap700x.c |    2 +-
 drivers/rtc/rtc-ds1374.c     |    4 ++--
 drivers/rtc/rtc-m48t59.c     |    2 +-
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index 36652ea..eddbf39 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -675,7 +675,7 @@ static void hpet_rtc_timer_reinit(void)
 	}
 }
 
-irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id)
+irqreturn_t hpet_rtc_interrupt(int dummy, void *dev_id)
 {
 	struct rtc_time curr_time;
 	unsigned long rtc_int_flag = 0;
@@ -707,7 +707,7 @@ irqreturn_t hpet_rtc_interrupt(int irq, void *dev_id)
 	if (rtc_int_flag) {
 		rtc_int_flag |= (RTC_IRQF | (RTC_NUM_INTS << 8));
 		if (irq_handler)
-			irq_handler(rtc_int_flag, dev_id);
+			irq_handler(-1, (void *) rtc_int_flag);
 	}
 	return IRQ_HANDLED;
 }
diff --git a/drivers/char/rtc.c b/drivers/char/rtc.c
index 5c3142b..6e3de5b 100644
--- a/drivers/char/rtc.c
+++ b/drivers/char/rtc.c
@@ -235,7 +235,7 @@ static inline unsigned char rtc_is_updating(void)
  *	(See ./arch/XXXX/kernel/time.c for the set_rtc_mmss() function.)
  */
 
-irqreturn_t rtc_interrupt(int irq, void *dev_id)
+irqreturn_t rtc_interrupt(int dummy, void *dev_id)
 {
 	/*
 	 *	Can be an alarm interrupt, update complete interrupt,
@@ -248,12 +248,13 @@ irqreturn_t ...
From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:23 pm

- remove always-true test

- neaten request_irq() indentation

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/mips/pmc-sierra/msp71xx/msp_hwbutton.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/arch/mips/pmc-sierra/msp71xx/msp_hwbutton.c b/arch/mips/pmc-sierra/msp71xx/msp_hwbutton.c
index ab96a2d..11769b5 100644
--- a/arch/mips/pmc-sierra/msp71xx/msp_hwbutton.c
+++ b/arch/mips/pmc-sierra/msp71xx/msp_hwbutton.c
@@ -126,9 +126,6 @@ static irqreturn_t hwbutton_handler(int irq, void *data)
 	struct hwbutton_interrupt *hirq = data;
 	unsigned long cic_ext = *CIC_EXT_CFG_REG;
 
-	if (irq != hirq->irq)
-		return IRQ_NONE;
-
 	if (CIC_EXT_IS_ACTIVE_HI(cic_ext, hirq->eirq)) {
 		/* Interrupt: pin is now HI */
 		CIC_EXT_SET_ACTIVE_LO(cic_ext, hirq->eirq);
@@ -164,7 +161,7 @@ static int msp_hwbutton_register(struct hwbutton_interrupt *hirq)
 	*CIC_EXT_CFG_REG = cic_ext;
 
 	return request_irq(hirq->irq, hwbutton_handler, IRQF_DISABLED,
-				hirq->name, (void *)hirq);
+			   hirq->name, hirq);
 }
 
 static int __init msp_hwbutton_setup(void)
-- 
1.5.4.1

--

From: Ralf Baechle
Date: Monday, April 28, 2008 - 1:53 pm

Applied, thanks Jeff.

  Ralf
--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:23 pm

Prefer 'void *dev_id' argument for passing data from request_irq() to
each per-irq handler invocation.

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 arch/x86/kernel/vm86_32.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 38f566f..e610c08 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -735,9 +735,9 @@ 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(int dummy, void *dev_id)
 {
-	int irq_bit;
+	int irq_bit, intno = (int)(unsigned long) dev_id;
 	unsigned long flags;
 
 	spin_lock_irqsave(&irqbits_lock, flags);
@@ -818,7 +818,8 @@ static int do_vm86_irq_handling(int subfunction, int irqnumber)
 			if (!((1 << sig) & ALLOWED_SIGS)) return -EPERM;
 			if (invalid_vm86_irq(irq)) return -EPERM;
 			if (vm86_irqs[irq].tsk) return -EPERM;
-			ret = request_irq(irq, &irq_handler, 0, VM86_IRQNAME, NULL);
+			ret = request_irq(irq, irq_handler, 0, VM86_IRQNAME,
+					  (void *)(unsigned long) irq);
 			if (ret) return ret;
 			vm86_irqs[irq].sig = sig;
 			vm86_irqs[irq].tsk = current;
-- 
1.5.4.1

--

From: Ingo Molnar
Date: Monday, April 21, 2008 - 6:52 am

neat ...

	Ingo
--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:23 pm

* prefer passing data to irq handler using 'void *dev_id' argument

* remove references to 'irq' function arg that either duplicate
  a member of our private struct, or are always [true|false].

* add linux/interrupt.h include where needed

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/atm/ambassador.c               |    7 ++++---
 drivers/i2c/chips/tps65010.c           |    4 ++--
 drivers/input/touchscreen/ucb1400_ts.c |   11 ++++-------
 drivers/serial/8250.c                  |    5 +++--
 include/asm-sh/push-switch.h           |    2 +-
 5 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/atm/ambassador.c b/drivers/atm/ambassador.c
index 7b44a59..da41d26 100644
--- a/drivers/atm/ambassador.c
+++ b/drivers/atm/ambassador.c
@@ -862,8 +862,9 @@ static inline void interrupts_off (amb_dev * dev) {
 
 /********** interrupt handling **********/
 
-static irqreturn_t interrupt_handler(int irq, void *dev_id) {
-  amb_dev * dev = dev_id;
+static irqreturn_t interrupt_handler(int irq, void *dev_id)
+{
+  amb_dev *dev = dev_id;
   
   PRINTD (DBG_IRQ|DBG_FLOW, "interrupt_handler: %p", dev_id);
   
@@ -872,7 +873,7 @@ static irqreturn_t interrupt_handler(int irq, void *dev_id) {
   
     // for us or someone else sharing the same interrupt
     if (!interrupt) {
-      PRINTD (DBG_IRQ, "irq not for me: %d", irq);
+      PRINTD (DBG_IRQ, "no irq events pending");
       return IRQ_NONE;
     }
     
diff --git a/drivers/i2c/chips/tps65010.c b/drivers/i2c/chips/tps65010.c
index 4154a91..78b365c 100644
--- a/drivers/i2c/chips/tps65010.c
+++ b/drivers/i2c/chips/tps65010.c
@@ -437,11 +437,11 @@ static void tps65010_work(struct work_struct *work)
 	mutex_unlock(&tps->lock);
 }
 
-static irqreturn_t tps65010_irq(int irq, void *_tps)
+static irqreturn_t ...
From: Jeff Garzik
Date: Friday, April 18, 2008 - 4:23 pm

NOTE: NOT FOR MERGE, NOT FOR UPSTREAM

Not-signed-off-by: ...

This change's main purpose is to prepare for the patchset in
jgarzik/misc-2.6.git#irq-remove, that explores removal of the
never-used 'irq' argument in each interrupt handler.

---
 drivers/input/serio/i8042.c |   23 +++++++++++++----------
 drivers/pcmcia/i82365.c     |   15 ++++++++++-----
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 65a74cf..ee2dfab 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -108,8 +108,6 @@ static unsigned char i8042_aux_irq_registered;
 static unsigned char i8042_suppress_kbd_ack;
 static struct platform_device *i8042_platform_device;
 
-static irqreturn_t i8042_interrupt(int irq, void *dev_id);
-
 /*
  * The i8042_wait_read() and i8042_wait_write functions wait for the i8042 to
  * be ready for reading values from it / writing values to it.
@@ -302,7 +300,7 @@ static void i8042_stop(struct serio *serio)
  * to the upper layers.
  */
 
-static irqreturn_t i8042_interrupt(int irq, void *dev_id)
+static irqreturn_t __i8042_interrupt(bool polled)
 {
 	struct i8042_port *port;
 	unsigned long flags;
@@ -315,7 +313,7 @@ static irqreturn_t i8042_interrupt(int irq, void *dev_id)
 	str = i8042_read_status();
 	if (unlikely(~str & I8042_STR_OBF)) {
 		spin_unlock_irqrestore(&i8042_lock, flags);
-		if (irq) dbg("Interrupt %d, without any data", irq);
+		if (!polled) dbg("Interrupt without any data");
 		ret = 0;
 		goto out;
 	}
@@ -370,8 +368,8 @@ static irqreturn_t i8042_interrupt(int irq, void *dev_id)
 
 	port = &i8042_ports[port_no];
 
-	dbg("%02x <- i8042 (interrupt, %d, %d%s%s)",
-	    data, port_no, irq,
+	dbg("%02x <- i8042 (interrupt, %d, %s%s)",
+	    data, port_no,
 	    dfl & SERIO_PARITY ? ", bad parity" : "",
 	    dfl & SERIO_TIMEOUT ? ", timeout" : "");
 
@@ -389,6 +387,11 @@ static irqreturn_t i8042_interrupt(int irq, void *dev_id)
 	return ...
From: Andrew Morton
Date: Friday, April 18, 2008 - 4:44 pm

On Fri, 18 Apr 2008 19:22:45 -0400 (EDT)

#irq-remove doesn't seem to be included in the #ALL branch which
I'm grabbing?
--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 5:21 pm

I certainly welcome the exposure.......  but it would be a huge pain for 
you IMO because of the constant breakage.

	Jeff


--

From: Andrew Morton
Date: Friday, April 18, 2008 - 6:17 pm

wow.

 1084 files changed, 2363 insertions(+), 1934 deletions(-)

I didn't realise you'd changed all the interrupt handlers too.  Good luck
with that :)

Is it a flag day or do we have a migration plan?  I'd have thought that we
could do a request_irq_new(irqreturn_t (*)(void *d)) and keep things
compatible?



<checks>

Actually, that tree applies reasonably sanely to the full -mm lineup. 
There are rejects of course, but they're easily fixed and a lot are due to
file motion which git will handle anyway,

The bigger problem is newly-added irq handlers which your patch doesn't
know about:

y:/usr/src/25> grep '^+.*request_irq[(]' patches/*.patch | wc -l
74

If we had a migration plan (ie: request_irq_new(), above) then this of
course wouldn't be a problem.

--

From: Jeff Garzik
Date: Sunday, April 20, 2008 - 3:17 pm

Hey, I did, and last time I checked (months ago, to be honest) it boots 

A fair comment...

My goal has been to get the tree to the point where a flag-day patch 
"make the obvious change to each irq handler" /could/ be applied -- 
following the lead of the huge 'pt_regs arg removal' that went in in Oct 
2006.

Since I knew reaching that point would take time -- I started this 
project in Aug/Sep 2006 -- I simply didn't bother with a migration plan 
at the time.  I figured once the tree was prepped, which has taken over 
a year, _then_ I would waste maintainers' time discussing migration.

	Jeff


--

From: Russell King
Date: Sunday, April 20, 2008 - 3:40 pm

Then we'll be stuck with request_irq_new() for the next 10 years or so.

FWIW (and appologies for hijacking the topic), Jeffs discussion about
changing the ARM integrator RTC driver has triggered a number of cleanups
in the ARM tree:

- converting the ARM integrator PL030 RTC driver to a RTC class driver

  ... which triggered:

- removal of the ARM dyntick code and the unused generic changes
  (including the s390 and sh bits which look like they've never been
   functional.)

  ... which triggered:

- attempting to fix a circular include dependency involving linux/irq.h
  and asm-arm/mach/irq.h

  ... which then triggered:

- allowing PXA platform class to build for more than one PXA platform
  at a time, so I don't have to run 20 (!) separate kernel builds to
  check them all for breakage caused by the elimination of the
  circular dependency.

  ... which also allowed me to find several PXA platform build bugs.

Thanks Jeff. ;)

I'm not intending pushing this stuff into mainline for a bit, although
it will appear in my public tree for others to start looking at and for
them to be aware of.

That does mean that, unfortunately, akpm's going to see those changes
and it might cause Andrew some headaches - sorry about that.  You may
wish to avoid pulling the ARM 'devel' branch until the dust settles.

(Obviously the appropriate fixes will be head towards Linus at the
appropriate time.)

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

From: Jeff Garzik
Date: Sunday, April 20, 2008 - 3:49 pm

(un-hijacking topic <grin>)

Another fair comment.  I am secretly hoping for the ease of a flag day 
(a la pt_regs) rather than a decade-long transition, but I readily to 
confess to dodging the entire issue by doing all these cleanups first :)

	Jeff




--

From: Lennert Buytenhek
Date: Friday, April 18, 2008 - 4:29 pm

What do you mean?  I know at least one of two interrupt handlers
in-tree that use their 'irq' arguments.
--

From: Jeff Garzik
Date: Friday, April 18, 2008 - 5:25 pm

They can use new function get_irqfunc_irq(), similar to the existing 
method of getting pt_regs for the tiny number of users who need that 
sort of info, when pt_regs was removed.

But after having gone over, literally, every single interrupt handler in 
the kernel, I can safely say that 99.8% never reference that argument, 
and 0.1% that do already have the same information via another route.

That leaves only a few drivers that need it without modification, and 
even fewer drivers that need it after modification.

	Jeff


--

From: Lennert Buytenhek
Date: Saturday, April 19, 2008 - 9:14 am

Well, you said that the 'irq' argument was never-used, I was merely

Sounds good.

Very-much-liked-by: Lennert Buytenhek <buytenh@wantstofly.org>
--

From: Russell King
Date: Saturday, April 19, 2008 - 1:17 am

I don't see how these two things are connected.  Yes, it's true that
this RTC driver doesn't use 'dev_id' but that's an entirely separate
issue to removing the 'int irq' argument.

As I see it, this change is just unnecessarily adding to your workload
for this patch set.

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

From: Jeff Garzik
Date: Saturday, April 19, 2008 - 1:28 am

The added workload came from confusion created when I reviewed the code :)

Therefore I considered it better to have to less confusing version of 
the code in my tree for future reviews, and ideally have the less 
confusing version of the code in upstream as well.

I can remove the boilerplate paragraph from the patch description, if 
that's your main complaint.

	Jeff



--

Previous thread: none

Next thread: New location of Namesys software by Edward Shishkin on Friday, April 18, 2008 - 4:38 pm. (4 messages)