[PATCH 2.6.20] sata_nv: don't rely on NV_INT_DEV indication with ADMA

2007-01-23 Thread Robert Hancock
OK, here it is in full signed-off glory. Hopefully we can get this in 
for 2.6.20.


---

Several people reported issues with certain drive commands timing out on 
sata_nv controllers running in ADMA mode. The commands in question were 
non-DMA-mapped commands, usually FLUSH CACHE or FLUSH CACHE EXT.


From experimentation it appears that the NV_INT_DEV indication isn't 
always set when a legitimate command completion interrupt is received on 
a legacy-mode command, at least not on these controllers in ADMA mode. 
When a command is pending on the port, force the flag on always in the 
irq_stat value before calling nv_host_intr so that the drive busy state 
is always checked by ata_host_intr.


This also fixes some questionable code in nv_host_intr which called 
ata_check_status when a command was pending and ata_host_intr returned 
"unhandled". If the device interrupted at just the wrong time this could 
cause interrupts to be lost.


Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>

--- linux-2.6.20-rc5/drivers/ata/sata_nv.c  2007-01-19 19:18:53.0 
-0600
+++ linux-2.6.20-rc5debug/drivers/ata/sata_nv.c 2007-01-22 22:33:43.0 
-0600
@@ -700,7 +700,6 @@ static void nv_adma_check_cpb(struct ata
 static int nv_host_intr(struct ata_port *ap, u8 irq_stat)
 {
struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
-   int handled;
 
/* freeze if hotplugged */
if (unlikely(irq_stat & (NV_INT_ADDED | NV_INT_REMOVED))) {
@@ -719,13 +718,7 @@ static int nv_host_intr(struct ata_port 
}
 
/* handle interrupt */
-   handled = ata_host_intr(ap, qc);
-   if (unlikely(!handled)) {
-   /* spurious, clear it */
-   ata_check_status(ap);
-   }
-
-   return 1;
+   return ata_host_intr(ap, qc);
 }
 
 static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
@@ -752,6 +745,11 @@ static irqreturn_t nv_adma_interrupt(int
if (pp->flags & NV_ADMA_PORT_REGISTER_MODE) {
u8 irq_stat = readb(host->mmio_base + 
NV_INT_STATUS_CK804)
>> (NV_INT_PORT_SHIFT * i);
+   if(ata_tag_valid(ap->active_tag))
+   /** NV_INT_DEV indication seems 
unreliable at times
+   at least in ADMA mode. Force it on 
always when a
+   command is active, to prevent 
losing interrupts. */
+   irq_stat |= NV_INT_DEV;
handled += nv_host_intr(ap, irq_stat);
continue;
}


[PATCH 2.6.20] sata_nv: don't rely on NV_INT_DEV indication with ADMA

2007-01-23 Thread Robert Hancock
OK, here it is in full signed-off glory. Hopefully we can get this in 
for 2.6.20.


---

Several people reported issues with certain drive commands timing out on 
sata_nv controllers running in ADMA mode. The commands in question were 
non-DMA-mapped commands, usually FLUSH CACHE or FLUSH CACHE EXT.


From experimentation it appears that the NV_INT_DEV indication isn't 
always set when a legitimate command completion interrupt is received on 
a legacy-mode command, at least not on these controllers in ADMA mode. 
When a command is pending on the port, force the flag on always in the 
irq_stat value before calling nv_host_intr so that the drive busy state 
is always checked by ata_host_intr.


This also fixes some questionable code in nv_host_intr which called 
ata_check_status when a command was pending and ata_host_intr returned 
unhandled. If the device interrupted at just the wrong time this could 
cause interrupts to be lost.


Signed-off-by: Robert Hancock [EMAIL PROTECTED]

--- linux-2.6.20-rc5/drivers/ata/sata_nv.c  2007-01-19 19:18:53.0 
-0600
+++ linux-2.6.20-rc5debug/drivers/ata/sata_nv.c 2007-01-22 22:33:43.0 
-0600
@@ -700,7 +700,6 @@ static void nv_adma_check_cpb(struct ata
 static int nv_host_intr(struct ata_port *ap, u8 irq_stat)
 {
struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap-active_tag);
-   int handled;
 
/* freeze if hotplugged */
if (unlikely(irq_stat  (NV_INT_ADDED | NV_INT_REMOVED))) {
@@ -719,13 +718,7 @@ static int nv_host_intr(struct ata_port 
}
 
/* handle interrupt */
-   handled = ata_host_intr(ap, qc);
-   if (unlikely(!handled)) {
-   /* spurious, clear it */
-   ata_check_status(ap);
-   }
-
-   return 1;
+   return ata_host_intr(ap, qc);
 }
 
 static irqreturn_t nv_adma_interrupt(int irq, void *dev_instance)
@@ -752,6 +745,11 @@ static irqreturn_t nv_adma_interrupt(int
if (pp-flags  NV_ADMA_PORT_REGISTER_MODE) {
u8 irq_stat = readb(host-mmio_base + 
NV_INT_STATUS_CK804)
 (NV_INT_PORT_SHIFT * i);
+   if(ata_tag_valid(ap-active_tag))
+   /** NV_INT_DEV indication seems 
unreliable at times
+   at least in ADMA mode. Force it on 
always when a
+   command is active, to prevent 
losing interrupts. */
+   irq_stat |= NV_INT_DEV;
handled += nv_host_intr(ap, irq_stat);
continue;
}