All of lore.kernel.org
 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 7/8] sata_mv leave SError bits untouched in mv_err_intr
Date: Sat, 19 Apr 2008 14:53:55 -0400	[thread overview]
Message-ID: <480A3FC3.9070006@rtr.ca> (raw)
In-Reply-To: <480A3D2E.4040503@rtr.ca>

Rework mv_err_intr() to leave the SError bits as-is,
so that libata-eh has a chance to see/use them.

We originally thought that clearing them here was necessary
before writing back to edma_err_cause (per the Marvell datasheets),
but we will end up reseting the chip regardless in those cases.

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

--- old/drivers/ata/sata_mv.c	2008-04-19 13:47:23.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-04-19 13:53:33.000000000 -0400
@@ -1519,13 +1519,11 @@
 /**
  *      mv_err_intr - Handle error interrupts on the port
  *      @ap: ATA channel to manipulate
- *      @reset_allowed: bool: 0 == don't trigger from reset here
+ *      @qc: affected command (non-NCQ), or NULL
  *
- *      In most cases, just clear the interrupt and move on.  However,
- *      some cases require an eDMA reset, which also performs a COMRESET.
- *      The SERR case requires a clear of pending errors in the SATA
- *      SERROR register.  Finally, if the port disabled DMA,
- *      update our cached copy to match.
+ *      Most cases require a full reset of the chip's state machine,
+ *      which also performs a COMRESET.
+ *      Also, if the port disabled DMA, update our cached copy to match.
  *
  *      LOCKING:
  *      Inherited from caller.
@@ -1536,21 +1534,18 @@
 	u32 edma_err_cause, eh_freeze_mask, serr = 0;
 	struct mv_port_priv *pp = ap->private_data;
 	struct mv_host_priv *hpriv = ap->host->private_data;
-	unsigned int edma_enabled = (pp->pp_flags & MV_PP_FLAG_EDMA_EN);
 	unsigned int action = 0, err_mask = 0;
 	struct ata_eh_info *ehi = &ap->link.eh_info;
 
 	ata_ehi_clear_desc(ehi);
 
-	if (!edma_enabled) {
-		/* just a guess: do we need to do this? should we
-		 * expand this, and do it in all cases?
-		 */
-		sata_scr_read(&ap->link, SCR_ERROR, &serr);
-		sata_scr_write_flush(&ap->link, SCR_ERROR, serr);
-	}
-
+	/*
+	 * 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.
+	 */
 	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);
 
@@ -1590,16 +1585,19 @@
 			ata_ehi_push_desc(ehi, "EDMA self-disable");
 		}
 		if (edma_err_cause & EDMA_ERR_SERR) {
-			sata_scr_read(&ap->link, SCR_ERROR, &serr);
-			sata_scr_write_flush(&ap->link, SCR_ERROR, serr);
-			err_mask = AC_ERR_ATA_BUS;
+			/*
+			 * Ensure that we only 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;
 		}
 	}
 
-	/* Clear EDMA now that SERR cleanup done */
-	writelfl(~edma_err_cause, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
-
 	if (!err_mask) {
 		err_mask = AC_ERR_OTHER;
 		action |= ATA_EH_RESET;

  parent reply	other threads:[~2008-04-19 18:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-19 18:42 [PATCH 0/8] sata_mv interrupt/eh fixes etc Mark Lord
2008-04-19 18:43 ` [PATCH 1/8] sata_mv more cosmetics Mark Lord
2008-04-25  5:15   ` Jeff Garzik
2008-04-25 13:46     ` Mark Lord
2008-04-25 16:40       ` Grant Grundler
2008-04-25 16:57         ` Jeff Garzik
2008-04-25 17:35           ` Mark Lord
2008-04-25 17:39           ` Grant Grundler
2008-04-25 17:31         ` Mark Lord
2008-04-19 18:44 ` [PATCH 2/8] sata_mv mask all interrupt coalescing bits Mark Lord
2008-04-19 18:45 ` [PATCH 3/8] sata_mv simplify freeze/thaw bit-shift calculations Mark Lord
2008-04-19 19:05   ` REPOST: " Mark Lord
2008-04-19 18:46 ` [PATCH 4/8] sata_mv simplify request/response queue handling Mark Lord
2008-04-19 19:06   ` REPOST: " Mark Lord
2008-04-19 18:52 ` [PATCH 5/8] sata_mv tidy host controller interrupt handling Mark Lord
2008-04-19 19:07   ` REPOST: " Mark Lord
2008-04-19 18:53 ` [PATCH 6/8] sata_mv more interrupt handling rework Mark Lord
2008-04-19 18:53 ` Mark Lord [this message]
2008-04-19 19:07   ` REPOST: [PATCH 7/8] sata_mv leave SError bits untouched in mv_err_intr Mark Lord
2008-04-19 18:54 ` [PATCH 8/8] sata_mv re-enable hotplug, update TODO list Mark Lord
2008-04-19 19:09   ` Mark Lord
2008-04-23 13:32 ` [PATCH 0/8] sata_mv interrupt/eh fixes etc Mark Lord

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=480A3FC3.9070006@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 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.