All of lore.kernel.org
 help / color / mirror / Atom feed
* 2.6.11-rc4 libata-core (irq 30: nobody cared!)
@ 2005-02-24  1:58 Brian Kuschak
  2005-02-24  2:01 ` Brian Kuschak
  2005-02-24  2:34 ` [PATCH] " Jeff Garzik
  0 siblings, 2 replies; 14+ messages in thread
From: Brian Kuschak @ 2005-02-24  1:58 UTC (permalink / raw)
  To: linux-kernel; +Cc:  "Rogério" Brito, jgarzik

I see this problem with the sata_sil.c driver and
SII3112 card.  Others have reported seeing a similar
problem:  http://lkml.org/lkml/2005/2/6/41

There seems to be a pending interrupt from the drive,
but the code has already set the NIEN bit, so the
ATA_IRQ_TRAP macro doesn't help (the ata_interrupt
handler never calls ata_host_intr in this case).

I've implemented a quick workaround hack, but others
should investigate a better fix (maybe acking pending
interrupts before setting NIEN bit in ata_tf_load??)

Regards,
Brian

--- libata-core.c.orig  2005-02-23 17:41:03.831836464
-0800
+++ libata-core.c       2005-02-23 17:31:07.930427248
-0800
@@ -3158,6 +3158,11 @@
                        if (qc && (!(qc->tf.ctl &
ATA_NIEN))) {
                                handled |=
ata_host_intr(ap, qc);
                        }
+                       else {
+                               /* bk - just ack
spurious interrupt here - temp workaround */
+                               ata_irq_ack(ap, 0);
+                               printk(KERN_WARNING
"ata%d: irq trap\n", ap->id);
+                       }
                }
        }


Linux version 2.6.11-rc4 (root@localhost.localdomain)
(gcc version 3.3.2) #27 Wed Feb 23 17:49:05 PST 2005
Built 1 zonelists
Kernel command line: root=/dev/ram rw ramdisk=36000
console=ttyS0
PID hash table entries: 1024 (order: 10, 16384 bytes)
Console: colour dummy device 80x25
Dentry cache hash table entries: 32768 (order: 5,
131072 bytes)
Inode-cache hash table entries: 16384 (order: 4, 65536
bytes)
Memory: 120832k available (2136k kernel code, 916k
data, 108k init, 0k highmem)
Mount-cache hash table entries: 512 (order: 0, 4096
bytes)
checking if image is initramfs...it isn't (no cpio
magic); looks like an initrd
Freeing initrd memory: 5709k freed
NET: Registered protocol family 16
PCI: Probing PCI hardware
SCSI subsystem initialized
Installing knfsd (copyright (C) 1996
okir@monad.swb.de).
Initializing Cryptographic API
Serial: 8250/16550 driver $Revision: 1.90 $ 6 ports,
IRQ sharing disabled
ttyS0 at MMIO 0x0 (irq = 0) is a 16550A
ttyS1 at MMIO 0x0 (irq = 1) is a 16550A
io scheduler noop registered
io scheduler anticipatory registered
io scheduler deadline registered
io scheduler cfq registered
RAMDISK driver initialized: 16 RAM disks of 36000K
size 1024 blocksize
loop: loaded (max 8 devices)
mal0: Initialized, 1 tx channels, 1 rx channels
emac: IBM EMAC Ethernet driver, version 2.0
Maintained by Benjamin Herrenschmidt
<benh@kernel.crashing.org>
eth0: IBM emac, MAC 08:00:3e:26:15:59
eth0: Found Generic MII PHY (0x06)
Uniform Multi-Platform E-IDE driver Revision:
7.00alpha2
ide: Assuming 33MHz system bus speed for PIO modes;
override with idebus=xx
ata1: SATA max UDMA/100 cmd 0xC9002E80 ctl 0xC9002E8A
bmdma 0xC9002E00 irq 30
ata2: SATA max UDMA/100 cmd 0xC9002EC0 ctl 0xC9002ECA
bmdma 0xC9002E08 irq 30
irq 30: nobody cared!
Call trace:
 [c0005630] dump_stack+0x18/0x28
 [c003ae0c] __report_bad_irq+0x34/0xac
 [c003af38] note_interrupt+0x98/0xd4
 [c003a92c] __do_IRQ+0x15c/0x160
 [c0003e54] do_IRQ+0x50/0x98
 [c0002f64] ret_from_except+0x0/0x18
 [c0003ed4] default_idle+0x38/0x5c
 [c0003f20] cpu_idle+0x28/0x38
 [c00023a4] rest_init+0x24/0x34
 [c02dc614] start_kernel+0x170/0x1a8
 [c00022a4] start_here+0x44/0xb0
handlers:
[<c015fc28>] (ata_interrupt+0x0/0x27c)
Disabling IRQ #30
ata1: dev 0 ATA, max UDMA7, 234493056 sectors: lba48
eth0: Link is Up
eth0: Speed: 100, Full duplex.





		
__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - 250MB free storage. Do more. Manage less. 
http://info.mail.yahoo.com/mail_250

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 2.6.11-rc4 libata-core (irq 30: nobody cared!)
  2005-02-24  1:58 2.6.11-rc4 libata-core (irq 30: nobody cared!) Brian Kuschak
@ 2005-02-24  2:01 ` Brian Kuschak
  2005-02-24  2:34 ` [PATCH] " Jeff Garzik
  1 sibling, 0 replies; 14+ messages in thread
From: Brian Kuschak @ 2005-02-24  2:01 UTC (permalink / raw)
  To: Brian Kuschak, linux-kernel; +Cc:  "Rogério" Brito, jgarzik

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

Retry... that patch got screwed up in the last
email...
-Brian



		
__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - Helps protect you from nasty viruses. 
http://promotions.yahoo.com/new_mail

[-- Attachment #2: patch1.txt --]
[-- Type: text/plain, Size: 403 bytes --]

--- libata-core.c.orig	2005-02-23 17:41:03.831836464 -0800
+++ libata-core.c	2005-02-23 17:54:51.287044152 -0800
@@ -3158,6 +3158,11 @@
 			if (qc && (!(qc->tf.ctl & ATA_NIEN))) {
 				handled |= ata_host_intr(ap, qc);
 			}
+			else {
+				/* bk - just ack spurious interrupt here - temp workaround */
+				ata_irq_ack(ap, 0); 		
+				printk(KERN_WARNING "ata%d: irq trap\n", ap->id);
+			}
 		}
 	}
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] Re: 2.6.11-rc4 libata-core (irq 30: nobody cared!)
  2005-02-24  1:58 2.6.11-rc4 libata-core (irq 30: nobody cared!) Brian Kuschak
  2005-02-24  2:01 ` Brian Kuschak
@ 2005-02-24  2:34 ` Jeff Garzik
  2005-02-24  2:36   ` Jeff Garzik
                     ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Jeff Garzik @ 2005-02-24  2:34 UTC (permalink / raw)
  To: Brian Kuschak; +Cc: linux-kernel, "\"Rogério\" Brito"

[-- Attachment #1: Type: text/plain, Size: 46 bytes --]

Does this patch do anything useful?

	Jeff




[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2874 bytes --]

===== drivers/scsi/sata_sil.c 1.44 vs edited =====
--- 1.44/drivers/scsi/sata_sil.c	2005-02-17 19:43:51 -05:00
+++ edited/drivers/scsi/sata_sil.c	2005-02-23 21:27:18 -05:00
@@ -65,6 +65,7 @@
 static u32 sil_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void sil_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static void sil_post_set_mode (struct ata_port *ap);
+static void sil_tf_load(struct ata_port *ap, struct ata_taskfile *tf);
 
 static struct pci_device_id sil_pci_tbl[] = {
 	{ 0x1095, 0x3112, PCI_ANY_ID, PCI_ANY_ID, 0, 0, sil_3112 },
@@ -130,7 +131,7 @@
 static struct ata_port_operations sil_ops = {
 	.port_disable		= ata_port_disable,
 	.dev_config		= sil_dev_config,
-	.tf_load		= ata_tf_load,
+	.tf_load		= sil_tf_load,
 	.tf_read		= ata_tf_read,
 	.check_status		= ata_check_status,
 	.exec_command		= ata_exec_command,
@@ -197,6 +198,69 @@
 MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, sil_pci_tbl);
 MODULE_VERSION(DRV_VERSION);
+
+static void sil_irq_enable(struct ata_port *ap, int disable)
+{
+	void __iomem *mmio = ap->host_set->mmio_base;
+	u32 tmp, new;
+	u32 bit = 1 << (22 + ap->port_no);
+
+	tmp = readl(mmio + SIL_SYSCFG);
+	if (disable)
+		new = tmp | bit;
+	else
+		new = tmp & ~bit;
+	if (new != tmp)
+		writel(new, mmio + SIL_SYSCFG);
+}
+
+static void sil_tf_load(struct ata_port *ap, struct ata_taskfile *tf)
+{
+	struct ata_ioports *ioaddr = &ap->ioaddr;
+	unsigned int is_addr = tf->flags & ATA_TFLAG_ISADDR;
+
+	if (tf->ctl != ap->last_ctl) {
+		sil_irq_enable(ap, tf->ctl & ATA_NIEN);
+		writeb(tf->ctl, (void __iomem *) ap->ioaddr.ctl_addr);
+		ap->last_ctl = tf->ctl;
+		ata_wait_idle(ap);
+	}
+
+	if (is_addr && (tf->flags & ATA_TFLAG_LBA48)) {
+		writeb(tf->hob_feature, (void __iomem *) ioaddr->feature_addr);
+		writeb(tf->hob_nsect, (void __iomem *) ioaddr->nsect_addr);
+		writeb(tf->hob_lbal, (void __iomem *) ioaddr->lbal_addr);
+		writeb(tf->hob_lbam, (void __iomem *) ioaddr->lbam_addr);
+		writeb(tf->hob_lbah, (void __iomem *) ioaddr->lbah_addr);
+		VPRINTK("hob: feat 0x%X nsect 0x%X, lba 0x%X 0x%X 0x%X\n",
+			tf->hob_feature,
+			tf->hob_nsect,
+			tf->hob_lbal,
+			tf->hob_lbam,
+			tf->hob_lbah);
+	}
+
+	if (is_addr) {
+		writeb(tf->feature, (void __iomem *) ioaddr->feature_addr);
+		writeb(tf->nsect, (void __iomem *) ioaddr->nsect_addr);
+		writeb(tf->lbal, (void __iomem *) ioaddr->lbal_addr);
+		writeb(tf->lbam, (void __iomem *) ioaddr->lbam_addr);
+		writeb(tf->lbah, (void __iomem *) ioaddr->lbah_addr);
+		VPRINTK("feat 0x%X nsect 0x%X lba 0x%X 0x%X 0x%X\n",
+			tf->feature,
+			tf->nsect,
+			tf->lbal,
+			tf->lbam,
+			tf->lbah);
+	}
+
+	if (tf->flags & ATA_TFLAG_DEVICE) {
+		writeb(tf->device, (void __iomem *) ioaddr->device_addr);
+		VPRINTK("device 0x%X\n", tf->device);
+	}
+
+	ata_wait_idle(ap);
+}
 
 static void sil_post_set_mode (struct ata_port *ap)
 {

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Re: 2.6.11-rc4 libata-core (irq 30: nobody cared!)
  2005-02-24  2:34 ` [PATCH] " Jeff Garzik
@ 2005-02-24  2:36   ` Jeff Garzik
  2005-02-24  2:58   ` Brian Kuschak
  2005-02-26 19:32   ` Rogério Brito
  2 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2005-02-24  2:36 UTC (permalink / raw)
  To: Brian Kuschak
  Cc: linux-kernel, "\"Rogério\" Brito",
	linux-ide@vger.kernel.org

BTW, please CC your replies to linux-ide@vger.kernel.org as well.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Re: 2.6.11-rc4 libata-core (irq 30: nobody cared!)
  2005-02-24  2:34 ` [PATCH] " Jeff Garzik
  2005-02-24  2:36   ` Jeff Garzik
@ 2005-02-24  2:58   ` Brian Kuschak
  2005-02-26 19:32   ` Rogério Brito
  2 siblings, 0 replies; 14+ messages in thread
From: Brian Kuschak @ 2005-02-24  2:58 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-kernel,  "Rogério" Brito, linux-ide


> Does this patch do anything useful?
> 
> 	Jeff
> 

Not really.  It doesn't print the nobody cared
message, but still hangs at boot.  I'd give you a
backtrace but my MAGIC_SYSRQ doesn't seem to be
working right now.

-Brian


Linux version 2.6.11-rc4 (root@localhost.localdomain)
(gcc version 3.3.2) #28 Wed Feb 23 18:52:22 PST 2005
Built 1 zonelists
Kernel command line: root=/dev/ram rw ramdisk=36000
console=ttyS0
PID hash table entries: 1024 (order: 10, 16384 bytes)
Console: colour dummy device 80x25
Dentry cache hash table entries: 32768 (order: 5,
131072 bytes)
Inode-cache hash table entries: 16384 (order: 4, 65536
bytes)
Memory: 120832k available (2136k kernel code, 916k
data, 108k init, 0k highmem)
Mount-cache hash table entries: 512 (order: 0, 4096
bytes)
checking if image is initramfs...it isn't (no cpio
magic); looks like an initrd
Freeing initrd memory: 5709k freed
NET: Registered protocol family 16
PCI: Probing PCI hardware
SCSI subsystem initialized
Installing knfsd (copyright (C) 1996
okir@monad.swb.de).
Initializing Cryptographic API
Serial: 8250/16550 driver $Revision: 1.90 $ 6 ports,
IRQ sharing disabled
ttyS0 at MMIO 0x0 (irq = 0) is a 16550A
ttyS1 at MMIO 0x0 (irq = 1) is a 16550A
io scheduler noop registered
io scheduler anticipatory registered
io scheduler deadline registered
io scheduler cfq registered
RAMDISK driver initialized: 16 RAM disks of 36000K
size 1024 blocksize
loop: loaded (max 8 devices)
mal0: Initialized, 1 tx channels, 1 rx channels
emac: IBM EMAC Ethernet driver, version 2.0
Maintained by Benjamin Herrenschmidt
<benh@kernel.crashing.org>
eth0: IBM emac, MAC 08:00:3e:26:15:59
eth0: Found Generic MII PHY (0x06)
Uniform Multi-Platform E-IDE driver Revision:
7.00alpha2
ide: Assuming 33MHz system bus speed for PIO modes;
override with idebus=xx
ata1: SATA max UDMA/100 cmd 0xC9002E80 ctl 0xC9002E8A
bmdma 0xC9002E00 irq 30
ata2: SATA max UDMA/100 cmd 0xC9002EC0 ctl 0xC9002ECA
bmdma 0xC9002E08 irq 30
ata1: dev 0 ATA, max UDMA7, 234493056 sectors: lba48
eth0: Link is Up
eth0: Speed: 100, Full duplex.



		
__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - Helps protect you from nasty viruses. 
http://promotions.yahoo.com/new_mail

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Re: 2.6.11-rc4 libata-core (irq 30: nobody cared!)
  2005-02-24  2:34 ` [PATCH] " Jeff Garzik
  2005-02-24  2:36   ` Jeff Garzik
  2005-02-24  2:58   ` Brian Kuschak
@ 2005-02-26 19:32   ` Rogério Brito
  2005-02-26 20:19     ` Jeff Garzik
  2 siblings, 1 reply; 14+ messages in thread
From: Rogério Brito @ 2005-02-26 19:32 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Brian Kuschak, linux-kernel, linux-ide

On Feb 23 2005, Jeff Garzik wrote:
> Does this patch do anything useful?
> 	Jeff
(...)

The patch you posted seems to only affect people using SATA, right?

BTW, just for clarity I'm seeing the message in a PATA environment (on
i386) and Brian is seeing his problem with a SATA device on ppc.


Thanks, Rogério Brito.

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
  Rogério Brito - rbrito@ime.usp.br - http://www.ime.usp.br/~rbrito
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Re: 2.6.11-rc4 libata-core (irq 30: nobody cared!)
  2005-02-26 19:32   ` Rogério Brito
@ 2005-02-26 20:19     ` Jeff Garzik
  2005-02-26 23:18       ` Rogério Brito
  2005-02-28 16:10       ` Mark Lord
  0 siblings, 2 replies; 14+ messages in thread
From: Jeff Garzik @ 2005-02-26 20:19 UTC (permalink / raw)
  To: Rogério Brito; +Cc: Brian Kuschak, linux-kernel, linux-ide

Rogério Brito wrote:
> On Feb 23 2005, Jeff Garzik wrote:
> 
>>Does this patch do anything useful?
>>	Jeff
> 
> (...)
> 
> The patch you posted seems to only affect people using SATA, right?
> 
> BTW, just for clarity I'm seeing the message in a PATA environment (on
> i386) and Brian is seeing his problem with a SATA device on ppc.

"irq XX: nobody cared" is a screaming interrupt situation, which could 
have 1001 causes.

Normally it's something that "pci=biosirq" or "acpi=off" will fix, but 
on occasion the driver itself is what needs fixing.

	Jeff




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Re: 2.6.11-rc4 libata-core (irq 30: nobody cared!)
  2005-02-26 20:19     ` Jeff Garzik
@ 2005-02-26 23:18       ` Rogério Brito
  2005-02-28 16:10       ` Mark Lord
  1 sibling, 0 replies; 14+ messages in thread
From: Rogério Brito @ 2005-02-26 23:18 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Brian Kuschak, linux-kernel, linux-ide

First of all, thank you very much for your reply, Jeff.

On Feb 26 2005, Jeff Garzik wrote:
> "irq XX: nobody cared" is a screaming interrupt situation, which could
> have 1001 causes.

Ok, I didn't know that.

> Normally it's something that "pci=biosirq" or "acpi=off" will fix, but 
> on occasion the driver itself is what needs fixing.

Well, I already tried both of those options (and some others too) and
nothing seems to make my kernel quiet regarding my Promise controller (just
as a reminder, it is a PDC20265, embedded in my Asus A7V motherboard).

If you want me to test any patches, feel free to contact me.


Thanks, Rogério Brito.

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
  Rogério Brito - rbrito@ime.usp.br - http://www.ime.usp.br/~rbrito
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Re: 2.6.11-rc4 libata-core (irq 30: nobody cared!)
  2005-02-26 20:19     ` Jeff Garzik
  2005-02-26 23:18       ` Rogério Brito
@ 2005-02-28 16:10       ` Mark Lord
  2005-03-02  7:18         ` [PATCH 0/3] Fix 3ware irq handling Jeff Garzik
  1 sibling, 1 reply; 14+ messages in thread
From: Mark Lord @ 2005-02-28 16:10 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linux-ide

 >"irq XX: nobody cared" is a screaming interrupt situation, which could have 1001 causes.

Speaking of which.  The 3ware drivers appear to have a bug
in their handling of this.  They *always* report "handled=1"
from their interrupt routines, regardless of whether it was
their hardware or not that was signalling for attention.

Cheers

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 0/3] Fix 3ware irq handling
  2005-02-28 16:10       ` Mark Lord
@ 2005-03-02  7:18         ` Jeff Garzik
  2005-03-02  7:20           ` [PATCH 1/3] Fix 3ware irq handling: correct IRQ_HANDLED Jeff Garzik
                             ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Jeff Garzik @ 2005-03-02  7:18 UTC (permalink / raw)
  To: SCSI Mailing List, James Bottomley, linuxraid; +Cc: Mark Lord

Mark Lord wrote:
>  >"irq XX: nobody cared" is a screaming interrupt situation, which could 
> have 1001 causes.
> 
> Speaking of which.  The 3ware drivers appear to have a bug
> in their handling of this.  They *always* report "handled=1"
> from their interrupt routines, regardless of whether it was
> their hardware or not that was signalling for attention.

Thanks for pointing this out, Mark.

What follows is a patch series to clean this up:

patch 1: only report IRQ_HANDLED when needed
patch 2: remove bogus "my irq == my irq" test
patch 3: fix indentation, after patch #2's change


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] Fix 3ware irq handling: correct IRQ_HANDLED
  2005-03-02  7:18         ` [PATCH 0/3] Fix 3ware irq handling Jeff Garzik
@ 2005-03-02  7:20           ` Jeff Garzik
  2005-03-02  7:21           ` [PATCH 2/3] Fix 3ware irq handling: remove bogus test Jeff Garzik
  2005-03-02  7:22           ` [PATCH 3/3] Fix 3ware irq handling: update irq handler indentation Jeff Garzik
  2 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2005-03-02  7:20 UTC (permalink / raw)
  To: SCSI Mailing List, James Bottomley, linuxraid; +Cc: Mark Lord

[-- Attachment #1: Type: text/plain, Size: 166 bytes --]

Only indicate irq-handled if that is really true

Driver currently returns IRQ_HANDLED for all interrupts.  Fix this.

Signed-off-by: Jeff Garzik <jgarzik@pobox.com>

[-- Attachment #2: patch.1 --]
[-- Type: text/plain, Size: 1294 bytes --]

diff -Nru a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
--- a/drivers/scsi/3w-9xxx.c	2005-03-02 02:14:42 -05:00
+++ b/drivers/scsi/3w-9xxx.c	2005-03-02 02:14:42 -05:00
@@ -1176,14 +1176,14 @@
 	/* See if the interrupt matches this instance */
 	if (tw_dev->tw_pci_dev->irq == (unsigned int)irq) {
 
-		handled = 1;
-
 		/* Read the registers */
 		status_reg_value = readl(TW_STATUS_REG_ADDR(tw_dev));
 
 		/* Check if this is our interrupt, otherwise bail */
 		if (!(status_reg_value & TW_STATUS_VALID_INTERRUPT))
 			goto twa_interrupt_bail;
+
+		handled = 1;
 
 		/* Check controller for errors */
 		if (twa_check_bits(status_reg_value)) {
diff -Nru a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
--- a/drivers/scsi/3w-xxxx.c	2005-03-02 02:14:42 -05:00
+++ b/drivers/scsi/3w-xxxx.c	2005-03-02 02:14:42 -05:00
@@ -2091,14 +2091,14 @@
 	/* See if the interrupt matches this instance */
 	if (tw_dev->tw_pci_dev->irq == (unsigned int)irq) {
 
-		handled = 1;
-
 		/* Read the registers */
 		status_reg_value = inl(TW_STATUS_REG_ADDR(tw_dev));
 
 		/* Check if this is our interrupt, otherwise bail */
 		if (!(status_reg_value & TW_STATUS_VALID_INTERRUPT))
 			goto tw_interrupt_bail;
+
+		handled = 1;
 
 		/* Check controller for errors */
 		if (tw_check_bits(status_reg_value)) {

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] Fix 3ware irq handling: remove bogus test
  2005-03-02  7:18         ` [PATCH 0/3] Fix 3ware irq handling Jeff Garzik
  2005-03-02  7:20           ` [PATCH 1/3] Fix 3ware irq handling: correct IRQ_HANDLED Jeff Garzik
@ 2005-03-02  7:21           ` Jeff Garzik
  2005-03-02  7:22           ` [PATCH 3/3] Fix 3ware irq handling: update irq handler indentation Jeff Garzik
  2 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2005-03-02  7:21 UTC (permalink / raw)
  To: SCSI Mailing List, James Bottomley, linuxraid; +Cc: Mark Lord

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]

Remove bogus irq test.

Driver tested irq handler's "irq" argument against the PCI device's
pci_dev->irq value, a test which would always succeed.

Change this to "if (1)" to avoid re-indenting the [huge]
interrupt handling code.

Signed-off-by: Jeff Garzik <jgarzik@pobox.com>


[-- Attachment #2: patch.2 --]
[-- Type: text/plain, Size: 950 bytes --]

diff -Nru a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
--- a/drivers/scsi/3w-9xxx.c	2005-03-02 02:14:49 -05:00
+++ b/drivers/scsi/3w-9xxx.c	2005-03-02 02:14:49 -05:00
@@ -1173,8 +1173,7 @@
 	/* Get the per adapter lock */
 	spin_lock(tw_dev->host->host_lock);
 
-	/* See if the interrupt matches this instance */
-	if (tw_dev->tw_pci_dev->irq == (unsigned int)irq) {
+	if (1) {
 
 		/* Read the registers */
 		status_reg_value = readl(TW_STATUS_REG_ADDR(tw_dev));
diff -Nru a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
--- a/drivers/scsi/3w-xxxx.c	2005-03-02 02:14:49 -05:00
+++ b/drivers/scsi/3w-xxxx.c	2005-03-02 02:14:49 -05:00
@@ -2088,8 +2088,7 @@
 	/* Get the host lock for io completions */
 	spin_lock(tw_dev->host->host_lock);
 
-	/* See if the interrupt matches this instance */
-	if (tw_dev->tw_pci_dev->irq == (unsigned int)irq) {
+	if (1) {
 
 		/* Read the registers */
 		status_reg_value = inl(TW_STATUS_REG_ADDR(tw_dev));

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] Fix 3ware irq handling: update irq handler indentation
  2005-03-02  7:18         ` [PATCH 0/3] Fix 3ware irq handling Jeff Garzik
  2005-03-02  7:20           ` [PATCH 1/3] Fix 3ware irq handling: correct IRQ_HANDLED Jeff Garzik
  2005-03-02  7:21           ` [PATCH 2/3] Fix 3ware irq handling: remove bogus test Jeff Garzik
@ 2005-03-02  7:22           ` Jeff Garzik
  2005-03-02  7:31             ` Jeff Garzik
  2 siblings, 1 reply; 14+ messages in thread
From: Jeff Garzik @ 2005-03-02  7:22 UTC (permalink / raw)
  To: SCSI Mailing List, James Bottomley, linuxraid; +Cc: Mark Lord

[-- Attachment #1: Type: text/plain, Size: 360 bytes --]

Reduce irq handler indentation level.

The previous patch replaced an always-true test with "if (1)",
a function change.

This patch simply eliminates the "if (1)", and reduces the indentation
level of the entire [rather large] interrupt handler code.

No code changes other than eliminating the "if (1)" test.

Signed-off-by: Jeff Garzik <jgarzik@pobox.com>


[-- Attachment #2: patch.3 --]
[-- Type: text/plain, Size: 22180 bytes --]

diff -Nru a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
--- a/drivers/scsi/3w-9xxx.c	2005-03-02 02:14:58 -05:00
+++ b/drivers/scsi/3w-9xxx.c	2005-03-02 02:14:58 -05:00
@@ -1173,138 +1173,136 @@
 	/* Get the per adapter lock */
 	spin_lock(tw_dev->host->host_lock);
 
-	if (1) {
+	/* Read the registers */
+	status_reg_value = readl(TW_STATUS_REG_ADDR(tw_dev));
 
-		/* Read the registers */
-		status_reg_value = readl(TW_STATUS_REG_ADDR(tw_dev));
-
-		/* Check if this is our interrupt, otherwise bail */
-		if (!(status_reg_value & TW_STATUS_VALID_INTERRUPT))
+	/* Check if this is our interrupt, otherwise bail */
+	if (!(status_reg_value & TW_STATUS_VALID_INTERRUPT))
+		goto twa_interrupt_bail;
+
+	handled = 1;
+
+	/* Check controller for errors */
+	if (twa_check_bits(status_reg_value)) {
+		if (twa_decode_bits(tw_dev, status_reg_value)) {
+			TW_CLEAR_ALL_INTERRUPTS(tw_dev);
 			goto twa_interrupt_bail;
-
-		handled = 1;
-
-		/* Check controller for errors */
-		if (twa_check_bits(status_reg_value)) {
-			if (twa_decode_bits(tw_dev, status_reg_value)) {
-				TW_CLEAR_ALL_INTERRUPTS(tw_dev);
-				goto twa_interrupt_bail;
-			}
 		}
+	}
 
-		/* Handle host interrupt */
-		if (status_reg_value & TW_STATUS_HOST_INTERRUPT)
-			TW_CLEAR_HOST_INTERRUPT(tw_dev);
-
-		/* Handle attention interrupt */
-		if (status_reg_value & TW_STATUS_ATTENTION_INTERRUPT) {
-			TW_CLEAR_ATTENTION_INTERRUPT(tw_dev);
-			if (!(test_and_set_bit(TW_IN_ATTENTION_LOOP, &tw_dev->flags))) {
-				twa_get_request_id(tw_dev, &request_id);
-
-				error = twa_aen_read_queue(tw_dev, request_id);
-				if (error) {
-					tw_dev->state[request_id] = TW_S_COMPLETED;
-					twa_free_request_id(tw_dev, request_id);
-					clear_bit(TW_IN_ATTENTION_LOOP, &tw_dev->flags);
-				}
+	/* Handle host interrupt */
+	if (status_reg_value & TW_STATUS_HOST_INTERRUPT)
+		TW_CLEAR_HOST_INTERRUPT(tw_dev);
+
+	/* Handle attention interrupt */
+	if (status_reg_value & TW_STATUS_ATTENTION_INTERRUPT) {
+		TW_CLEAR_ATTENTION_INTERRUPT(tw_dev);
+		if (!(test_and_set_bit(TW_IN_ATTENTION_LOOP, &tw_dev->flags))) {
+			twa_get_request_id(tw_dev, &request_id);
+
+			error = twa_aen_read_queue(tw_dev, request_id);
+			if (error) {
+				tw_dev->state[request_id] = TW_S_COMPLETED;
+				twa_free_request_id(tw_dev, request_id);
+				clear_bit(TW_IN_ATTENTION_LOOP, &tw_dev->flags);
 			}
 		}
+	}
 
-		/* Handle command interrupt */
-		if (status_reg_value & TW_STATUS_COMMAND_INTERRUPT) {
-			TW_MASK_COMMAND_INTERRUPT(tw_dev);
-			/* Drain as many pending commands as we can */
-			while (tw_dev->pending_request_count > 0) {
-				request_id = tw_dev->pending_queue[tw_dev->pending_head];
-				if (tw_dev->state[request_id] != TW_S_PENDING) {
-					TW_PRINTK(tw_dev->host, TW_DRIVER, 0x19, "Found request id that wasn't pending");
-					TW_CLEAR_ALL_INTERRUPTS(tw_dev);
-					goto twa_interrupt_bail;
-				}
-				if (twa_post_command_packet(tw_dev, request_id, 1)==0) {
-					tw_dev->pending_head = (tw_dev->pending_head + 1) % TW_Q_LENGTH;
-					tw_dev->pending_request_count--;
-				} else {
-					/* If we get here, we will continue re-posting on the next command interrupt */
-					break;
-				}
+	/* Handle command interrupt */
+	if (status_reg_value & TW_STATUS_COMMAND_INTERRUPT) {
+		TW_MASK_COMMAND_INTERRUPT(tw_dev);
+		/* Drain as many pending commands as we can */
+		while (tw_dev->pending_request_count > 0) {
+			request_id = tw_dev->pending_queue[tw_dev->pending_head];
+			if (tw_dev->state[request_id] != TW_S_PENDING) {
+				TW_PRINTK(tw_dev->host, TW_DRIVER, 0x19, "Found request id that wasn't pending");
+				TW_CLEAR_ALL_INTERRUPTS(tw_dev);
+				goto twa_interrupt_bail;
+			}
+			if (twa_post_command_packet(tw_dev, request_id, 1)==0) {
+				tw_dev->pending_head = (tw_dev->pending_head + 1) % TW_Q_LENGTH;
+				tw_dev->pending_request_count--;
+			} else {
+				/* If we get here, we will continue re-posting on the next command interrupt */
+				break;
 			}
 		}
+	}
 
-		/* Handle response interrupt */
-		if (status_reg_value & TW_STATUS_RESPONSE_INTERRUPT) {
+	/* Handle response interrupt */
+	if (status_reg_value & TW_STATUS_RESPONSE_INTERRUPT) {
 
-			/* Drain the response queue from the board */
-			while ((status_reg_value & TW_STATUS_RESPONSE_QUEUE_EMPTY) == 0) {
-				/* Complete the response */
-				response_que.value = readl(TW_RESPONSE_QUEUE_REG_ADDR(tw_dev));
-				request_id = TW_RESID_OUT(response_que.response_id);
-				full_command_packet = tw_dev->command_packet_virt[request_id];
-				error = 0;
-				command_packet = &full_command_packet->command.oldcommand;
-				/* Check for command packet errors */
-				if (full_command_packet->command.newcommand.status != 0) {
-					if (tw_dev->srb[request_id] != 0) {
-						error = twa_fill_sense(tw_dev, request_id, 1, 1);
-					} else {
-						/* Skip ioctl error prints */
-						if (request_id != tw_dev->chrdev_request_id) {
-							error = twa_fill_sense(tw_dev, request_id, 0, 1);
-						}
+		/* Drain the response queue from the board */
+		while ((status_reg_value & TW_STATUS_RESPONSE_QUEUE_EMPTY) == 0) {
+			/* Complete the response */
+			response_que.value = readl(TW_RESPONSE_QUEUE_REG_ADDR(tw_dev));
+			request_id = TW_RESID_OUT(response_que.response_id);
+			full_command_packet = tw_dev->command_packet_virt[request_id];
+			error = 0;
+			command_packet = &full_command_packet->command.oldcommand;
+			/* Check for command packet errors */
+			if (full_command_packet->command.newcommand.status != 0) {
+				if (tw_dev->srb[request_id] != 0) {
+					error = twa_fill_sense(tw_dev, request_id, 1, 1);
+				} else {
+					/* Skip ioctl error prints */
+					if (request_id != tw_dev->chrdev_request_id) {
+						error = twa_fill_sense(tw_dev, request_id, 0, 1);
 					}
 				}
+			}
 
-				/* Check for correct state */
-				if (tw_dev->state[request_id] != TW_S_POSTED) {
-					if (tw_dev->srb[request_id] != 0) {
-						TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1a, "Received a request id that wasn't posted");
-					        TW_CLEAR_ALL_INTERRUPTS(tw_dev);
-						goto twa_interrupt_bail;
-					}
+			/* Check for correct state */
+			if (tw_dev->state[request_id] != TW_S_POSTED) {
+				if (tw_dev->srb[request_id] != 0) {
+					TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1a, "Received a request id that wasn't posted");
+				        TW_CLEAR_ALL_INTERRUPTS(tw_dev);
+					goto twa_interrupt_bail;
 				}
+			}
 
-				/* Check for internal command completion */
-				if (tw_dev->srb[request_id] == 0) {
-					if (request_id != tw_dev->chrdev_request_id) {
-						if (twa_aen_complete(tw_dev, request_id))
-							TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1b, "Error completing AEN during attention interrupt");
-					} else {
-						tw_dev->chrdev_request_id = TW_IOCTL_CHRDEV_FREE;
-						wake_up(&tw_dev->ioctl_wqueue);
-					}
+			/* Check for internal command completion */
+			if (tw_dev->srb[request_id] == 0) {
+				if (request_id != tw_dev->chrdev_request_id) {
+					if (twa_aen_complete(tw_dev, request_id))
+						TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1b, "Error completing AEN during attention interrupt");
 				} else {
-					twa_scsiop_execute_scsi_complete(tw_dev, request_id);
-					/* If no error command was a success */
-					if (error == 0) {
-						tw_dev->srb[request_id]->result = (DID_OK << 16);
-					}
-
-					/* If error, command failed */
-					if (error == 1) {
-						/* Ask for a host reset */
-						tw_dev->srb[request_id]->result = (DID_OK << 16) | (CHECK_CONDITION << 1);
-					}
-
-					/* Now complete the io */
-					tw_dev->state[request_id] = TW_S_COMPLETED;
-					twa_free_request_id(tw_dev, request_id);
-					tw_dev->posted_request_count--;
-					tw_dev->srb[request_id]->scsi_done(tw_dev->srb[request_id]);
-					twa_unmap_scsi_data(tw_dev, request_id);
+					tw_dev->chrdev_request_id = TW_IOCTL_CHRDEV_FREE;
+					wake_up(&tw_dev->ioctl_wqueue);
 				}
+			} else {
+				twa_scsiop_execute_scsi_complete(tw_dev, request_id);
+				/* If no error command was a success */
+				if (error == 0) {
+					tw_dev->srb[request_id]->result = (DID_OK << 16);
+				}
+
+				/* If error, command failed */
+				if (error == 1) {
+					/* Ask for a host reset */
+					tw_dev->srb[request_id]->result = (DID_OK << 16) | (CHECK_CONDITION << 1);
+				}
+
+				/* Now complete the io */
+				tw_dev->state[request_id] = TW_S_COMPLETED;
+				twa_free_request_id(tw_dev, request_id);
+				tw_dev->posted_request_count--;
+				tw_dev->srb[request_id]->scsi_done(tw_dev->srb[request_id]);
+				twa_unmap_scsi_data(tw_dev, request_id);
+			}
 
-				/* Check for valid status after each drain */
-				status_reg_value = readl(TW_STATUS_REG_ADDR(tw_dev));
-				if (twa_check_bits(status_reg_value)) {
-					if (twa_decode_bits(tw_dev, status_reg_value)) {
-						TW_CLEAR_ALL_INTERRUPTS(tw_dev);
-						goto twa_interrupt_bail;
-					}
+			/* Check for valid status after each drain */
+			status_reg_value = readl(TW_STATUS_REG_ADDR(tw_dev));
+			if (twa_check_bits(status_reg_value)) {
+				if (twa_decode_bits(tw_dev, status_reg_value)) {
+					TW_CLEAR_ALL_INTERRUPTS(tw_dev);
+					goto twa_interrupt_bail;
 				}
 			}
 		}
 	}
+
 twa_interrupt_bail:
 	spin_unlock(tw_dev->host->host_lock);
 	return IRQ_RETVAL(handled);
diff -Nru a/drivers/scsi/3w-xxxx.c b/drivers/scsi/3w-xxxx.c
--- a/drivers/scsi/3w-xxxx.c	2005-03-02 02:14:58 -05:00
+++ b/drivers/scsi/3w-xxxx.c	2005-03-02 02:14:58 -05:00
@@ -2088,182 +2088,180 @@
 	/* Get the host lock for io completions */
 	spin_lock(tw_dev->host->host_lock);
 
-	if (1) {
+	/* Read the registers */
+	status_reg_value = inl(TW_STATUS_REG_ADDR(tw_dev));
 
-		/* Read the registers */
-		status_reg_value = inl(TW_STATUS_REG_ADDR(tw_dev));
-
-		/* Check if this is our interrupt, otherwise bail */
-		if (!(status_reg_value & TW_STATUS_VALID_INTERRUPT))
+	/* Check if this is our interrupt, otherwise bail */
+	if (!(status_reg_value & TW_STATUS_VALID_INTERRUPT))
+		goto tw_interrupt_bail;
+
+	handled = 1;
+
+	/* Check controller for errors */
+	if (tw_check_bits(status_reg_value)) {
+		dprintk(KERN_WARNING "3w-xxxx: tw_interrupt(): Unexpected bits.\n");
+		if (tw_decode_bits(tw_dev, status_reg_value, 1)) {
+			TW_CLEAR_ALL_INTERRUPTS(tw_dev);
 			goto tw_interrupt_bail;
-
-		handled = 1;
-
-		/* Check controller for errors */
-		if (tw_check_bits(status_reg_value)) {
-			dprintk(KERN_WARNING "3w-xxxx: tw_interrupt(): Unexpected bits.\n");
-			if (tw_decode_bits(tw_dev, status_reg_value, 1)) {
-				TW_CLEAR_ALL_INTERRUPTS(tw_dev);
-				goto tw_interrupt_bail;
-			}
 		}
+	}
 
-		/* Handle host interrupt */
-		if (status_reg_value & TW_STATUS_HOST_INTERRUPT) {
-			dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): Received host interrupt.\n");
-			TW_CLEAR_HOST_INTERRUPT(tw_dev);
-		}
+	/* Handle host interrupt */
+	if (status_reg_value & TW_STATUS_HOST_INTERRUPT) {
+		dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): Received host interrupt.\n");
+		TW_CLEAR_HOST_INTERRUPT(tw_dev);
+	}
 
-		/* Handle attention interrupt */
-		if (status_reg_value & TW_STATUS_ATTENTION_INTERRUPT) {
-			dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): Received attention interrupt.\n");
-			TW_CLEAR_ATTENTION_INTERRUPT(tw_dev);
-			tw_state_request_start(tw_dev, &request_id);
-			error = tw_aen_read_queue(tw_dev, request_id);
-			if (error) {
-				printk(KERN_WARNING "3w-xxxx: scsi%d: Error reading aen queue.\n", tw_dev->host->host_no);
-				tw_dev->state[request_id] = TW_S_COMPLETED;
-				tw_state_request_finish(tw_dev, request_id);
-			}
+	/* Handle attention interrupt */
+	if (status_reg_value & TW_STATUS_ATTENTION_INTERRUPT) {
+		dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): Received attention interrupt.\n");
+		TW_CLEAR_ATTENTION_INTERRUPT(tw_dev);
+		tw_state_request_start(tw_dev, &request_id);
+		error = tw_aen_read_queue(tw_dev, request_id);
+		if (error) {
+			printk(KERN_WARNING "3w-xxxx: scsi%d: Error reading aen queue.\n", tw_dev->host->host_no);
+			tw_dev->state[request_id] = TW_S_COMPLETED;
+			tw_state_request_finish(tw_dev, request_id);
 		}
+	}
 
-		/* Handle command interrupt */
-		if (status_reg_value & TW_STATUS_COMMAND_INTERRUPT) {
-			/* Drain as many pending commands as we can */
-			while (tw_dev->pending_request_count > 0) {
-				request_id = tw_dev->pending_queue[tw_dev->pending_head];
-				if (tw_dev->state[request_id] != TW_S_PENDING) {
-					printk(KERN_WARNING "3w-xxxx: scsi%d: Found request id that wasn't pending.\n", tw_dev->host->host_no);
-					break;
-				}
-				if (tw_post_command_packet(tw_dev, request_id)==0) {
-					if (tw_dev->pending_head == TW_Q_LENGTH-1) {
-						tw_dev->pending_head = TW_Q_START;
-					} else {
-						tw_dev->pending_head = tw_dev->pending_head + 1;
-					}
-					tw_dev->pending_request_count--;
+	/* Handle command interrupt */
+	if (status_reg_value & TW_STATUS_COMMAND_INTERRUPT) {
+		/* Drain as many pending commands as we can */
+		while (tw_dev->pending_request_count > 0) {
+			request_id = tw_dev->pending_queue[tw_dev->pending_head];
+			if (tw_dev->state[request_id] != TW_S_PENDING) {
+				printk(KERN_WARNING "3w-xxxx: scsi%d: Found request id that wasn't pending.\n", tw_dev->host->host_no);
+				break;
+			}
+			if (tw_post_command_packet(tw_dev, request_id)==0) {
+				if (tw_dev->pending_head == TW_Q_LENGTH-1) {
+					tw_dev->pending_head = TW_Q_START;
 				} else {
-					/* If we get here, we will continue re-posting on the next command interrupt */
-					break;
+					tw_dev->pending_head = tw_dev->pending_head + 1;
 				}
+				tw_dev->pending_request_count--;
+			} else {
+				/* If we get here, we will continue re-posting on the next command interrupt */
+				break;
 			}
-			/* If there are no more pending requests, we mask command interrupt */
-			if (tw_dev->pending_request_count == 0) 
-				TW_MASK_COMMAND_INTERRUPT(tw_dev);
 		}
+		/* If there are no more pending requests, we mask command interrupt */
+		if (tw_dev->pending_request_count == 0) 
+			TW_MASK_COMMAND_INTERRUPT(tw_dev);
+	}
 
-		/* Handle response interrupt */
-		if (status_reg_value & TW_STATUS_RESPONSE_INTERRUPT) {
-			/* Drain the response queue from the board */
-			while ((status_reg_value & TW_STATUS_RESPONSE_QUEUE_EMPTY) == 0) {
-				/* Read response queue register */
-				response_que.value = inl(TW_RESPONSE_QUEUE_REG_ADDR(tw_dev));
-				request_id = TW_RESID_OUT(response_que.response_id);
-				command_packet = (TW_Command *)tw_dev->command_packet_virtual_address[request_id];
-				error = 0;
-
-				/* Check for bad response */
-				if (command_packet->status != 0) {
-					/* If internal command, don't error, don't fill sense */
-					if (tw_dev->srb[request_id] == NULL) {
-						tw_decode_sense(tw_dev, request_id, 0);
-					} else {
-						error = tw_decode_sense(tw_dev, request_id, 1);
-					}
+	/* Handle response interrupt */
+	if (status_reg_value & TW_STATUS_RESPONSE_INTERRUPT) {
+		/* Drain the response queue from the board */
+		while ((status_reg_value & TW_STATUS_RESPONSE_QUEUE_EMPTY) == 0) {
+			/* Read response queue register */
+			response_que.value = inl(TW_RESPONSE_QUEUE_REG_ADDR(tw_dev));
+			request_id = TW_RESID_OUT(response_que.response_id);
+			command_packet = (TW_Command *)tw_dev->command_packet_virtual_address[request_id];
+			error = 0;
+
+			/* Check for bad response */
+			if (command_packet->status != 0) {
+				/* If internal command, don't error, don't fill sense */
+				if (tw_dev->srb[request_id] == NULL) {
+					tw_decode_sense(tw_dev, request_id, 0);
+				} else {
+					error = tw_decode_sense(tw_dev, request_id, 1);
 				}
+			}
 
-				/* Check for correct state */
-				if (tw_dev->state[request_id] != TW_S_POSTED) {
-					if (tw_dev->srb[request_id] != NULL) {
-						printk(KERN_WARNING "3w-xxxx: scsi%d: Received a request id that wasn't posted.\n", tw_dev->host->host_no);
-						error = 1;
-					}
+			/* Check for correct state */
+			if (tw_dev->state[request_id] != TW_S_POSTED) {
+				if (tw_dev->srb[request_id] != NULL) {
+					printk(KERN_WARNING "3w-xxxx: scsi%d: Received a request id that wasn't posted.\n", tw_dev->host->host_no);
+					error = 1;
 				}
+			}
 
-				dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): Response queue request id: %d.\n", request_id);
+			dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): Response queue request id: %d.\n", request_id);
 
-				/* Check for internal command completion */
-				if (tw_dev->srb[request_id] == NULL) {
-					dprintk(KERN_WARNING "3w-xxxx: tw_interrupt(): Found internally posted command.\n");
-					/* Check for chrdev ioctl completion */
-					if (request_id != tw_dev->chrdev_request_id) {
-						retval = tw_aen_complete(tw_dev, request_id);
-						if (retval) {
-							printk(KERN_WARNING "3w-xxxx: scsi%d: Error completing aen.\n", tw_dev->host->host_no);
-						}
-					} else {
-						tw_dev->chrdev_request_id = TW_IOCTL_CHRDEV_FREE;
-						wake_up(&tw_dev->ioctl_wqueue);
+			/* Check for internal command completion */
+			if (tw_dev->srb[request_id] == NULL) {
+				dprintk(KERN_WARNING "3w-xxxx: tw_interrupt(): Found internally posted command.\n");
+				/* Check for chrdev ioctl completion */
+				if (request_id != tw_dev->chrdev_request_id) {
+					retval = tw_aen_complete(tw_dev, request_id);
+					if (retval) {
+						printk(KERN_WARNING "3w-xxxx: scsi%d: Error completing aen.\n", tw_dev->host->host_no);
 					}
 				} else {
-				switch (tw_dev->srb[request_id]->cmnd[0]) {
-					case READ_10:
-					case READ_6:
-						dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught READ_10/READ_6\n");
-						break;
-					case WRITE_10:
-					case WRITE_6:
-						dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught WRITE_10/WRITE_6\n");
-						break;
-					case TEST_UNIT_READY:
-						dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught TEST_UNIT_READY\n");
-						error = tw_scsiop_test_unit_ready_complete(tw_dev, request_id);
-						break;
-					case INQUIRY:
-						dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught INQUIRY\n");
-						error = tw_scsiop_inquiry_complete(tw_dev, request_id);
-						break;
-					case READ_CAPACITY:
-						dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught READ_CAPACITY\n");
-						error = tw_scsiop_read_capacity_complete(tw_dev, request_id);
-						break;
-					case MODE_SENSE:
-						dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught MODE_SENSE\n");
-						error = tw_scsiop_mode_sense_complete(tw_dev, request_id);
-						break;
-					case SYNCHRONIZE_CACHE:
-						dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught SYNCHRONIZE_CACHE\n");
-						break;
-					default:
-						printk(KERN_WARNING "3w-xxxx: case slip in tw_interrupt()\n");
-						error = 1;
-					}
+					tw_dev->chrdev_request_id = TW_IOCTL_CHRDEV_FREE;
+					wake_up(&tw_dev->ioctl_wqueue);
+				}
+			} else {
+			switch (tw_dev->srb[request_id]->cmnd[0]) {
+				case READ_10:
+				case READ_6:
+					dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught READ_10/READ_6\n");
+					break;
+				case WRITE_10:
+				case WRITE_6:
+					dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught WRITE_10/WRITE_6\n");
+					break;
+				case TEST_UNIT_READY:
+					dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught TEST_UNIT_READY\n");
+					error = tw_scsiop_test_unit_ready_complete(tw_dev, request_id);
+					break;
+				case INQUIRY:
+					dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught INQUIRY\n");
+					error = tw_scsiop_inquiry_complete(tw_dev, request_id);
+					break;
+				case READ_CAPACITY:
+					dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught READ_CAPACITY\n");
+					error = tw_scsiop_read_capacity_complete(tw_dev, request_id);
+					break;
+				case MODE_SENSE:
+					dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught MODE_SENSE\n");
+					error = tw_scsiop_mode_sense_complete(tw_dev, request_id);
+					break;
+				case SYNCHRONIZE_CACHE:
+					dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt(): caught SYNCHRONIZE_CACHE\n");
+					break;
+				default:
+					printk(KERN_WARNING "3w-xxxx: case slip in tw_interrupt()\n");
+					error = 1;
+				}
 
-					/* If no error command was a success */
-					if (error == 0) {
-						tw_dev->srb[request_id]->result = (DID_OK << 16);
-					}
+				/* If no error command was a success */
+				if (error == 0) {
+					tw_dev->srb[request_id]->result = (DID_OK << 16);
+				}
 
-					/* If error, command failed */
-					if (error == 1) {
-						/* Ask for a host reset */
-						tw_dev->srb[request_id]->result = (DID_OK << 16) | (CHECK_CONDITION << 1);
-					}
+				/* If error, command failed */
+				if (error == 1) {
+					/* Ask for a host reset */
+					tw_dev->srb[request_id]->result = (DID_OK << 16) | (CHECK_CONDITION << 1);
+				}
 
-					/* Now complete the io */
-					if ((error != TW_ISR_DONT_COMPLETE)) {
-						tw_dev->state[request_id] = TW_S_COMPLETED;
-						tw_state_request_finish(tw_dev, request_id);
-						tw_dev->posted_request_count--;
-						tw_dev->srb[request_id]->scsi_done(tw_dev->srb[request_id]);
+				/* Now complete the io */
+				if ((error != TW_ISR_DONT_COMPLETE)) {
+					tw_dev->state[request_id] = TW_S_COMPLETED;
+					tw_state_request_finish(tw_dev, request_id);
+					tw_dev->posted_request_count--;
+					tw_dev->srb[request_id]->scsi_done(tw_dev->srb[request_id]);
 
-						tw_unmap_scsi_data(tw_dev->tw_pci_dev, tw_dev->srb[request_id]);
-					}
+					tw_unmap_scsi_data(tw_dev->tw_pci_dev, tw_dev->srb[request_id]);
 				}
-				
-				/* Check for valid status after each drain */
-				status_reg_value = inl(TW_STATUS_REG_ADDR(tw_dev));
-				if (tw_check_bits(status_reg_value)) {
-					dprintk(KERN_WARNING "3w-xxxx: tw_interrupt(): Unexpected bits.\n");
-					if (tw_decode_bits(tw_dev, status_reg_value, 1)) {
-						TW_CLEAR_ALL_INTERRUPTS(tw_dev);
-						goto tw_interrupt_bail;
-					}
+			}
+			
+			/* Check for valid status after each drain */
+			status_reg_value = inl(TW_STATUS_REG_ADDR(tw_dev));
+			if (tw_check_bits(status_reg_value)) {
+				dprintk(KERN_WARNING "3w-xxxx: tw_interrupt(): Unexpected bits.\n");
+				if (tw_decode_bits(tw_dev, status_reg_value, 1)) {
+					TW_CLEAR_ALL_INTERRUPTS(tw_dev);
+					goto tw_interrupt_bail;
 				}
 			}
 		}
 	}
+
 tw_interrupt_bail:
 	spin_unlock(tw_dev->host->host_lock);
 	return IRQ_RETVAL(handled);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] Fix 3ware irq handling: update irq handler indentation
  2005-03-02  7:22           ` [PATCH 3/3] Fix 3ware irq handling: update irq handler indentation Jeff Garzik
@ 2005-03-02  7:31             ` Jeff Garzik
  0 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2005-03-02  7:31 UTC (permalink / raw)
  To: SCSI Mailing List, James Bottomley, linuxraid; +Cc: Mark Lord

Jeff Garzik wrote:
> The previous patch replaced an always-true test with "if (1)",
> a function change.

er, s/function/functional/


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2005-03-02  7:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-24  1:58 2.6.11-rc4 libata-core (irq 30: nobody cared!) Brian Kuschak
2005-02-24  2:01 ` Brian Kuschak
2005-02-24  2:34 ` [PATCH] " Jeff Garzik
2005-02-24  2:36   ` Jeff Garzik
2005-02-24  2:58   ` Brian Kuschak
2005-02-26 19:32   ` Rogério Brito
2005-02-26 20:19     ` Jeff Garzik
2005-02-26 23:18       ` Rogério Brito
2005-02-28 16:10       ` Mark Lord
2005-03-02  7:18         ` [PATCH 0/3] Fix 3ware irq handling Jeff Garzik
2005-03-02  7:20           ` [PATCH 1/3] Fix 3ware irq handling: correct IRQ_HANDLED Jeff Garzik
2005-03-02  7:21           ` [PATCH 2/3] Fix 3ware irq handling: remove bogus test Jeff Garzik
2005-03-02  7:22           ` [PATCH 3/3] Fix 3ware irq handling: update irq handler indentation Jeff Garzik
2005-03-02  7:31             ` Jeff Garzik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.