public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Robert Hancock <hancockr@shaw.ca>
To: "Björn Steinbrink" <B.Steinbrink@gmx.de>,
	"Robert Hancock" <hancockr@shaw.ca>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"Larry Walton" <lwalton@real.com>,
	s0348365@sms.ed.ac.uk, pomac@vapor.com, chunkeey@web.de,
	"Jeff Garzik" <jeff@garzik.org>
Subject: [PATCH 2.6.20] sata_nv: don't rely on NV_INT_DEV indication with ADMA
Date: Tue, 23 Jan 2007 20:09:02 -0600	[thread overview]
Message-ID: <45B6BFBE.5060709@shaw.ca> (raw)
In-Reply-To: <20070124003923.GA3093@atjola.homenet>

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

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 <hancockr@shaw.ca>


[-- Attachment #2: sata_nv-force-int-dev-in-interrupt.patch --]
[-- Type: text/plain, Size: 1315 bytes --]

--- linux-2.6.20-rc5/drivers/ata/sata_nv.c	2007-01-19 19:18:53.000000000 -0600
+++ linux-2.6.20-rc5debug/drivers/ata/sata_nv.c	2007-01-22 22:33:43.000000000 -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;
 			}

  reply	other threads:[~2007-01-24  2:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <fa.1kBz5luWz8nR0lLqm1VD4hZZYdw@ifi.uio.no>
     [not found] ` <fa.QZxgjxcwtENaZNY24NMTlKBSgIM@ifi.uio.no>
     [not found]   ` <fa.fkPTbUGmKc/1pt0eD6TE4d02n+Q@ifi.uio.no>
     [not found]     ` <fa.6iQt5OtHZ3x5w8eYbLxwULhLTJ0@ifi.uio.no>
     [not found]       ` <fa.1aqo3IxNGJClHcBVZNTagX6bL9o@ifi.uio.no>
     [not found]         ` <fa.rI60BGlFbSyfLyumqmgiOfDqCI4@ifi.uio.no>
2007-01-23 23:18           ` SATA exceptions with 2.6.20-rc5 Robert Hancock
2007-01-24  0:39             ` Björn Steinbrink
2007-01-24  2:09               ` Robert Hancock [this message]
2007-02-03  1:42               ` Björn Steinbrink
2007-02-03  5:48                 ` Robert Hancock
2007-02-04  1:13                   ` Björn Steinbrink
2007-02-09 12:03                     ` Björn Steinbrink
2007-01-24  8:24             ` Ian Kumlien
2007-01-24 14:41               ` Björn Steinbrink

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45B6BFBE.5060709@shaw.ca \
    --to=hancockr@shaw.ca \
    --cc=B.Steinbrink@gmx.de \
    --cc=chunkeey@web.de \
    --cc=jeff@garzik.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lwalton@real.com \
    --cc=pomac@vapor.com \
    --cc=s0348365@sms.ed.ac.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox