linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Lord <liml@rtr.ca>
To: Jeff Garzik <jgarzik@pobox.com>, Tejun Heo <htejun@gmail.com>,
	IDE/ATA development list <linux-ide@vger.kernel.org>
Subject: [PATCH 07/12] sata_mv NCQ and SError fixes for mv_err_intr
Date: Fri, 02 May 2008 02:12:34 -0400	[thread overview]
Message-ID: <481AB0D2.3070103@rtr.ca> (raw)
In-Reply-To: <481AB0A1.3040009@rtr.ca>

Sigh.  Undo some earlier changes to mv_port_intr(),
so that we now read/clear SError again in all cases.

Arrange the top of the function to be as close as possible
to what we need for a later update (in this series) for ERR_DEV handling.

Fix things so that libata-eh can attempt a READ_LOG_EXT_10H
in response to a failed NCQ command, by just doing a local
mv_eh_freeze() rather than ata_port_freeze().

This will now fully handle NCQ errors much of the time,
but more fixes are needed for FBS/PMP, and for certain chip errata.

Signed-off-by: Mark Lord <mlord@pobox.com>

--- old/drivers/ata/sata_mv.c	2008-05-01 18:18:27.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-01 18:39:27.000000000 -0400
@@ -1627,7 +1627,7 @@
  *      LOCKING:
  *      Inherited from caller.
  */
-static void mv_err_intr(struct ata_port *ap, struct ata_queued_cmd *qc)
+static void mv_err_intr(struct ata_port *ap)
 {
 	void __iomem *port_mmio = mv_ap_base(ap);
 	u32 edma_err_cause, eh_freeze_mask, serr = 0;
@@ -1635,24 +1635,33 @@
 	struct mv_host_priv *hpriv = ap->host->private_data;
 	unsigned int action = 0, err_mask = 0;
 	struct ata_eh_info *ehi = &ap->link.eh_info;
-
-	ata_ehi_clear_desc(ehi);
+	struct ata_queued_cmd *qc;
+	int abort = 0;
 
 	/*
-	 * Read and clear the err_cause bits.  This won't actually
-	 * clear for some errors (eg. SError), but we will be doing
-	 * a hard reset in those cases regardless, which *will* clear it.
+	 * Read and clear the SError and err_cause bits.
 	 */
+	sata_scr_read(&ap->link, SCR_ERROR, &serr);
+	sata_scr_write_flush(&ap->link, SCR_ERROR, serr);
+
 	edma_err_cause = readl(port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
 	writelfl(~edma_err_cause, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
 
-	ata_ehi_push_desc(ehi, "edma_err_cause=%08x", edma_err_cause);
+	ata_port_printk(ap, KERN_INFO, "%s: err_cause=%08x pp_flags=0x%x\n",
+			__func__, edma_err_cause, pp->pp_flags);
 
+	qc = mv_get_active_qc(ap);
+	ata_ehi_clear_desc(ehi);
+	ata_ehi_push_desc(ehi, "edma_err_cause=%08x pp_flags=%08x",
+			  edma_err_cause, pp->pp_flags);
 	/*
 	 * All generations share these EDMA error cause bits:
 	 */
-	if (edma_err_cause & EDMA_ERR_DEV)
+	if (edma_err_cause & EDMA_ERR_DEV) {
 		err_mask |= AC_ERR_DEV;
+		action |= ATA_EH_RESET;
+		ata_ehi_push_desc(ehi, "dev error");
+	}
 	if (edma_err_cause & (EDMA_ERR_D_PAR | EDMA_ERR_PRD_PAR |
 			EDMA_ERR_CRQB_PAR | EDMA_ERR_CRPB_PAR |
 			EDMA_ERR_INTRL_PAR)) {
@@ -1684,13 +1693,6 @@
 			ata_ehi_push_desc(ehi, "EDMA self-disable");
 		}
 		if (edma_err_cause & EDMA_ERR_SERR) {
-			/*
-			 * Ensure that we read our own SCR, not a pmp link SCR:
-			 */
-			ap->ops->scr_read(ap, SCR_ERROR, &serr);
-			/*
-			 * Don't clear SError here; leave it for libata-eh:
-			 */
 			ata_ehi_push_desc(ehi, "SError=%08x", serr);
 			err_mask |= AC_ERR_ATA_BUS;
 			action |= ATA_EH_RESET;
@@ -1710,10 +1712,29 @@
 	else
 		ehi->err_mask |= err_mask;
 
-	if (edma_err_cause & eh_freeze_mask)
+	if (err_mask == AC_ERR_DEV) {
+		/*
+		 * Cannot do ata_port_freeze() here,
+		 * because it would kill PIO access,
+		 * which is needed for further diagnosis.
+		 */
+		mv_eh_freeze(ap);
+		abort = 1;
+	} else if (edma_err_cause & eh_freeze_mask) {
+		/*
+		 * Note to self: ata_port_freeze() calls ata_port_abort()
+		 */
 		ata_port_freeze(ap);
-	else
-		ata_port_abort(ap);
+	} else {
+		abort = 1;
+	}
+
+	if (abort) {
+		if (qc)
+			ata_link_abort(qc->dev->link);
+		else
+			ata_port_abort(ap);
+	}
 }
 
 static void mv_process_crpb_response(struct ata_port *ap,
@@ -1740,8 +1761,9 @@
 			}
 		}
 		ata_status = edma_status >> CRPB_FLAG_STATUS_SHIFT;
-		qc->err_mask |= ac_err_mask(ata_status);
-		ata_qc_complete(qc);
+		if (!ac_err_mask(ata_status))
+			ata_qc_complete(qc);
+		/* else: leave it for mv_err_intr() */
 	} else {
 		ata_port_printk(ap, KERN_ERR, "%s: no qc for tag=%d\n",
 				__func__, tag);
@@ -1845,7 +1867,7 @@
 		 * Handle chip-reported errors, or continue on to handle PIO.
 		 */
 		if (unlikely(port_cause & ERR_IRQ)) {
-			mv_err_intr(ap, mv_get_active_qc(ap));
+			mv_err_intr(ap);
 		} else if (hc_irq_cause & (DEV_IRQ << hardport)) {
 			if (!(pp->pp_flags & MV_PP_FLAG_EDMA_EN)) {
 				struct ata_queued_cmd *qc = mv_get_active_qc(ap);

  reply	other threads:[~2008-05-02  6:12 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-02  6:07 [PATCH 00/12] sata_mv: The last big set for 2.6.26 Mark Lord
2008-05-02  6:07 ` [PATCH 01/12] sata_mv more cosmetic changes Mark Lord
2008-05-02  6:08   ` [PATCH 02/12] sata_mv pci features Mark Lord
2008-05-02  6:09     ` [PATCH 03/12] sata_mv wait for empty+idle Mark Lord
2008-05-02  6:10       ` [PATCH 04/12] sata_mv new mv_qc_defer method Mark Lord
2008-05-02  6:10         ` [PATCH 05/12] sata_mv errata workaround for sata25 part 1 Mark Lord
2008-05-02  6:11           ` [PATCH 06/12] sata_mv rearrange mv_config_fbs Mark Lord
2008-05-02  6:12             ` Mark Lord [this message]
2008-05-02  6:13               ` [PATCH 08/12] sata_mv fix mv_host_intr bug for hc_irq_cause Mark Lord
2008-05-02  6:14                 ` [PATCH 09/12] sata_mv new mv_port_intr function Mark Lord
2008-05-02  6:14                   ` [PATCH 10/12] sata_mv libata: export ata_eh_analyze_ncq_error Mark Lord
2008-05-02  6:15                     ` [PATCH 11/12] sata_mv delayed eh handling Mark Lord
2008-05-02  6:16                       ` [PATCH 12/12] sata_mv NCQ-EH for FIS-based switching Mark Lord
2008-05-02 12:44                         ` Mark Lord
2008-05-02 16:52                           ` Grant Grundler
2008-05-02 16:54                             ` Grant Grundler
2008-05-02 17:46                               ` Mark Lord
2008-05-02 17:52                                 ` Alan Cox
2008-05-02 17:56                         ` [PATCH 13/13] sata_mv use hweight16() for bit counting Mark Lord
2008-05-02 18:02                           ` Mark Lord
2008-05-02 18:02                           ` [PATCH 13/13] sata_mv use hweight16() for bit counting (V2) Mark Lord
2008-05-02 18:31                             ` Grant Grundler
2008-05-02 16:42       ` [PATCH 03/12] sata_mv wait for empty+idle Grant Grundler
2008-05-02 17:45         ` Mark Lord
2008-05-02 18:39           ` Grant Grundler
2008-05-02 20:09             ` Mark Lord
2008-05-02 16:06     ` [PATCH 02/12] sata_mv pci features Grant Grundler
2008-05-02 17:41       ` Mark Lord
2008-05-02 18:28         ` Grant Grundler
2008-05-06 14:10   ` [PATCH 01/12] sata_mv more cosmetic changes Jeff Garzik
2008-05-06 17:57     ` Mark Lord
2008-05-06 14:17   ` Jeff Garzik

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=481AB0D2.3070103@rtr.ca \
    --to=liml@rtr.ca \
    --cc=htejun@gmail.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).