Skip to contentSkip to conversation

Fix for spurious interrupts on e100 resume

PATCH In review Jeremy Fitzhardinge 2 participants 4 messages Sep 9, 2004 – Sep 13, 2004 Network drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix for spurious interrupts on e100 resume

I've been having problems with spurious interrupts being raised when the e100 driver resets the chip during a resume:
irq 11: nobody cared!

 [<c0105937>] dump_stack+0x17/0x20
 [<c0107457>] __report_bad_irq+0x27/0x90
 [<c0107a1b>] note_interrupt+0x6b/0x1f0
 [<c0108132>] do_IRQ+0x272/0x360
 [<c010533c>] common_interrupt+0x18/0x20
 [<c012732b>] do_softirq+0x2b/0x30
 [<c01080d7>] do_IRQ+0x217/0x360
 [<c010533c>] common_interrupt+0x18/0x20
 [<c01f7d4c>] __delay+0xc/0x10
 [<f8b4806b>] e100_hw_reset+0x6b/0x90 [e100]
 [<f8b48d70>] e100_hw_init+0x10/0x1490 [e100]
 [<f8b4cd9f>] e100_up+0x3f/0x2e0 [e100]
 [<f8b4f11f>] e100_resume+0x8f/0xb0 [e100]
 [<c01fca62>] pci_device_resume+0x22/0x30
 [<c023736a>] resume_device+0x1a/0x20
 [<c02373dd>] dpm_resume+0x6d/0x70
 [<c02373fc>] device_resume+0x1c/0x30
 [<c01150d0>] suspend+0x540/0x7c0
 [<c0115e33>] do_ioctl+0x123/0x180
 [<c0185397>] sys_ioctl+0xb7/0x210
 [<c010497d>] sysenter_past_esp+0x52/0x71

handlers:
[<c02782c0>] (yenta_interrupt+0x0/0x30) [<f89c3310>] (radeon_driver_irq_handler+0x0/0xb0 [radeon]) [<f88c9320>] (usb_hcd_irq+0x0/0x60 [usbcore]) Disabling IRQ #11

The interrupt seems to be generated during the [um]sleep after one of the writel's in e100_hw_reset. To fix this, I simply moved the call to e100_disable_irq() to be first in the function, which seems to work well.

J

Patch:

On resume, the e100 chip seems to raise an interrupt during chip reset. Since there's no IRQ handler registered yet, the kernel complains that "nobody cared" about the interrupt. This change moves the call to e100_disable_irq to be first in e100_hw_reset() so there are no spurious interrupts.

 drivers/net/e100.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
drivers/net/e100.c renamed from drivers/net/e100.c~e100-restore-irq +4 -3
diff -puN drivers/net/e100.c~e100-restore-irq drivers/net/e100.c
@@ -587,6 +587,10 @@ static inline void e100_disable_irq(stru
 
 static void e100_hw_reset(struct nic *nic)
 {
+	/* Mask off our interrupt line - it's unmasked after reset 
+	   Do it early to prevent spurious interrupts. */
+	e100_disable_irq(nic);
+
 	/* Put CU and RU into idle with a selective reset to get
 	 * device off of PCI bus */
 	writel(selective_reset, &nic->csr->port);
@@ -605,9 +609,6 @@ static void e100_hw_reset(struct nic *ni
 		writeb(cuc_load_base, &nic->csr->scb.cmd_lo);
 		mdelay(20);
 	}
-
-	/* Mask off our interrupt line - it's unmasked after reset */
-	e100_disable_irq(nic);
 }
 
 static int e100_self_test(struct nic *nic)

_

#1 lore raw
Quoted 11 lines · Jeremy Fitzhardinge

static void e100_hw_reset(struct nic *nic)
{

drivers/net/e100.c renamed from drivers/net/e100.c~e100-restore-irq +4 -0
+	/* Mask off our interrupt line - it's unmasked after reset 
+	   Do it early to prevent spurious interrupts. */
+	e100_disable_irq(nic);
+
	/* Put CU and RU into idle with a selective reset to get
	 * device off of PCI bus */
	writel(selective_reset, &nic->csr->port);

drivers/net/e100.c static void e100_hw_reset(struct nic *ni · line 614

Quoted 9 lines
drivers/net/e100.c renamed from drivers/net/e100.c~e100-restore-irq +0 -3
@@ -605,9 +609,6 @@ static void e100_hw_reset(struct nic *ni
 		writeb(cuc_load_base, &nic->csr->scb.cmd_lo);
 		mdelay(20);
 	}
-
-	/* Mask off our interrupt line - it's unmasked after reset */
-	e100_disable_irq(nic);
 }
 
 static int e100_self_test(struct nic *nic)

You sure this is right? The comment seems to imply that writing the reset command to the registers also clears the interrupt mask. So you might need to have the e100_disable_irq() both before and after the reset.

#2 lore raw
Quoted 3 lines · Nathan Bryant

You sure this is right? The comment seems to imply that writing the reset command to the registers also clears the interrupt mask. So you might need to have the e100_disable_irq() both before and after the reset.

Good point - I'm not sure. I don't really know what would cause the chip to raise an interrupt. Could it do it spontaneously, in which case there's always a window between poking the selective/software_reset and the disable_irq? Or is it that the chip raises an interrupt on reset, which is masked by disabling the interrupt, but then it enables the interrupt after the reset is complete? Or perhaps "it's unmasked after reset" means "after a hardware reset (power cycle)" and not by poking the reset registers.

Well, there was definitely a real bug there before, and it works with a bit of testing. We'll see how it goes over the next few days (and maybe someone who actually understands this hardware will weigh in).

J

#3 lore raw
In reply to Jeremy Fitzhardinge
Quoted 2 lines · Jeremy Fitzhardinge

I've been having problems with spurious interrupts being raised when the e100 driver resets the chip during a resume:

OK, that patch didn't really work terribly well - the interrupt still happens. I've changed it to simply disable the interrupt during e100_up (), which does seem to work properly.

J

On resume, the e100 chip seems to raise an interrupt during chip reset. Since there's no IRQ handler registered yet, the kernel complains that "nobody cared" about the interrupt. This change disables the IRQ during e100_up(), while the hardware is being (re-)initialized.

 drivers/net/e100.c |    6 ++++++
 1 files changed, 6 insertions(+)
drivers/net/e100.c renamed from drivers/net/e100.c~e100-restore-irq +6 -0
diff -puN drivers/net/e100.c~e100-restore-irq drivers/net/e100.c
@@ -1678,6 +1678,9 @@ static int e100_up(struct nic *nic)
 
 	if((err = e100_rx_alloc_list(nic)))
 		return err;
+
+	disable_irq(nic->pdev->irq);
+
 	if((err = e100_alloc_cbs(nic)))
 		goto err_rx_clean_list;
 	if((err = e100_hw_init(nic)))
@@ -1689,6 +1692,7 @@ static int e100_up(struct nic *nic)
 		nic->netdev->name, nic->netdev)))
 		goto err_no_irq;
 	e100_enable_irq(nic);
+	enable_irq(nic->pdev->irq);
 	netif_wake_queue(nic->netdev);
 	return 0;
 
@@ -1698,6 +1702,8 @@ err_clean_cbs:
 	e100_clean_cbs(nic);
 err_rx_clean_list:
 	e100_rx_clean_list(nic);
+
+	enable_irq(nic->pdev->irq);
 	return err;
 }
 

_