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 6/8] sata_mv more interrupt handling rework
Date: Sat, 19 Apr 2008 14:53:07 -0400	[thread overview]
Message-ID: <480A3F93.9080602@rtr.ca> (raw)
In-Reply-To: <480A3D2E.4040503@rtr.ca>

Continue fixing the interrupt handling logic.

Get rid of mv_intr_pio(), by using ata_sff_host_intr() for PIO..

Add a mv_unexpected_intr() catch-all for "impossible" scenarios,
where we get an interrupt that shouldn't have happened
(never seen in testing, but just in case..).

Rearrange the logic so that we always process completed
response queue entries before looking for other events,
This avoids having to re-issue commands that had already succeeded.

As part of this, we split out some duplicated functionality
into a new function, mv_get_active_qc().

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

--- old/drivers/ata/sata_mv.c	2008-04-19 13:36:17.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-04-19 13:47:23.000000000 -0400
@@ -1479,6 +1479,43 @@
 	return 0;
 }
 
+static struct ata_queued_cmd *mv_get_active_qc(struct ata_port *ap)
+{
+	struct mv_port_priv *pp = ap->private_data;
+	struct ata_queued_cmd *qc;
+
+	if (pp->pp_flags & MV_PP_FLAG_NCQ_EN)
+		return NULL;
+	qc = ata_qc_from_tag(ap, ap->link.active_tag);
+	if (qc && (qc->tf.flags & ATA_TFLAG_POLLING))
+		qc = NULL;
+	return qc;
+}
+
+static void mv_unexpected_intr(struct ata_port *ap)
+{
+	struct mv_port_priv *pp = ap->private_data;
+	struct ata_eh_info *ehi = &ap->link.eh_info;
+	char *when = "";
+
+	/*
+	 * We got a device interrupt from something that
+	 * was supposed to be using EDMA or polling.
+	 */
+	ata_ehi_clear_desc(ehi);
+	if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
+		when = " while EDMA enabled";
+	} else {
+		struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->link.active_tag);
+		if (qc && (qc->tf.flags & ATA_TFLAG_POLLING))
+			when = " while polling";
+	}
+	ata_ehi_push_desc(ehi, "unexpected device interrupt%s", when);
+	ehi->err_mask |= AC_ERR_OTHER;
+	ehi->action   |= ATA_EH_RESET;
+	ata_port_freeze(ap);
+}
+
 /**
  *      mv_err_intr - Handle error interrupts on the port
  *      @ap: ATA channel to manipulate
@@ -1582,28 +1619,6 @@
 		ata_port_abort(ap);
 }
 
-static void mv_intr_pio(struct ata_port *ap)
-{
-	struct ata_queued_cmd *qc;
-	u8 ata_status;
-
-	/* ignore spurious intr if drive still BUSY */
-	ata_status = readb(ap->ioaddr.status_addr);
-	if (unlikely(ata_status & ATA_BUSY))
-		return;
-
-	/* get active ATA command */
-	qc = ata_qc_from_tag(ap, ap->link.active_tag);
-	if (unlikely(!qc))			/* no active tag */
-		return;
-	if (qc->tf.flags & ATA_TFLAG_POLLING)	/* polling; we don't own qc */
-		return;
-
-	/* and finally, complete the ATA command */
-	qc->err_mask |= ac_err_mask(ata_status);
-	ata_qc_complete(qc);
-}
-
 static void mv_process_crpb_response(struct ata_port *ap,
 		struct mv_crpb *response, unsigned int tag, int ncq_enabled)
 {
@@ -1676,15 +1691,7 @@
 /**
  *      mv_host_intr - Handle all interrupts on the given host controller
  *      @host: host specific structure
- *      @relevant: port error bits relevant to this host controller
- *      @hc: which host controller we're to look at
- *
- *      Read then write clear the HC interrupt status then walk each
- *      port connected to the HC and see if it needs servicing.  Port
- *      success ints are reported in the HC interrupt status reg, the
- *      port error ints are reported in the higher level main
- *      interrupt status register and thus are passed in via the
- *      'relevant' argument.
+ *      @main_cause: Main interrupt cause register for the chip.
  *
  *      LOCKING:
  *      Inherited from caller.
@@ -1729,25 +1736,28 @@
 			writelfl(~hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
 			handled = 1;
 		}
-
-		if (unlikely(port_cause & ERR_IRQ)) {
-			struct ata_queued_cmd *qc;
-
-			qc = ata_qc_from_tag(ap, ap->link.active_tag);
-			if (qc && (qc->tf.flags & ATA_TFLAG_POLLING))
-				continue;
-
-			mv_err_intr(ap, qc);
-			continue;
-		}
-
+		/*
+		 * Process completed CRPB response(s) before other events.
+		 */
 		pp = ap->private_data;
-		if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
-			if ((DMA_IRQ << hardport) & hc_irq_cause)
+		if (hc_irq_cause & (DMA_IRQ << hardport)) {
+			if (pp->pp_flags & MV_PP_FLAG_EDMA_EN)
 				mv_process_crpb_entries(ap, pp);
-		} else {
-			if ((DEV_IRQ << hardport) & hc_irq_cause)
-				mv_intr_pio(ap);
+		}
+		/*
+		 * 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));
+		} 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);
+				if (qc) {
+					ata_sff_host_intr(ap, qc);
+					continue;
+				}
+			}
+			mv_unexpected_intr(ap);
 		}
 	}
 	return handled;

  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 ` Mark Lord [this message]
2008-04-19 18:53 ` [PATCH 7/8] sata_mv leave SError bits untouched in mv_err_intr Mark Lord
2008-04-19 19:07   ` REPOST: " 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=480A3F93.9080602@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).