[PATCH 01/04] sata_mv always do softreset

[PATCH 01/04] sata_mv always do softreset

[Mark Lord]

Always request a softreset after hardreset succeeds.
This fixes a regression reported by Martin Michlmayr <tbm@cyrius.com>.

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

For 2.6.26.

--- old/drivers/ata/sata_mv.c	2008-05-09 17:21:52.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-13 18:10:29.000000000 -0400
@@ -2728,6 +2728,7 @@
 
 		rc = sata_link_hardreset(link, timing, deadline + extra,
 					 &online, NULL);
+		rc = online ? -EAGAIN : rc;
 		if (rc)
 			return rc;
 		sata_scr_read(link, SCR_STATUS, &sstatus);

[PATCH 02/04] sata_mv fis irq register fixes

[Mark Lord]

Fix handling of the FIS_IRQ_CAUSE register in sata_mv.

This register exists *only* on GenIIe devices, so don't bother
writing to it on older chips.  Also, it has to be read/cleared
in mv_err_intr() before clearing the main ERR_IRQ_CAUSE register.

This keeps sata_mv from getting stuck forever on certain error types.

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

For 2.6.26.

--- old/drivers/ata/sata_mv.c	2008-05-14 08:48:19.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-14 08:51:49.000000000 -0400
@@ -886,7 +886,8 @@
 		mv_edma_cfg(ap, want_ncq);
 
 		/* clear FIS IRQ Cause */
-		writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+		if (IS_GEN_IIE(hpriv))
+			writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
 
 		mv_set_edma_ptrs(port_mmio, hpriv, pp);
 
@@ -1812,6 +1813,7 @@
 {
 	void __iomem *port_mmio = mv_ap_base(ap);
 	u32 edma_err_cause, eh_freeze_mask, serr = 0;
+	u32 fis_cause = 0;
 	struct mv_port_priv *pp = ap->private_data;
 	struct mv_host_priv *hpriv = ap->host->private_data;
 	unsigned int action = 0, err_mask = 0;
@@ -1821,16 +1823,19 @@
 
 	/*
 	 * Read and clear the SError and err_cause bits.
+	 * For GenIIe, if EDMA_ERR_TRANS_IRQ_7 is set, we also must read/clear
+	 * the FIS_IRQ_CAUSE register before clearing edma_err_cause.
 	 */
 	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);
+	if (IS_GEN_IIE(hpriv) && (edma_err_cause & EDMA_ERR_TRANS_IRQ_7)) {
+		fis_cause = readl(port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+		writelfl(~fis_cause, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+	}
 	writelfl(~edma_err_cause, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
 
-	ata_port_printk(ap, KERN_INFO, "%s: err_cause=%08x pp_flags=0x%x\n",
-			__func__, edma_err_cause, pp->pp_flags);
-
 	if (edma_err_cause & EDMA_ERR_DEV) {
 		/*
 		 * Device errors during FIS-based switching operation
@@ -1844,6 +1849,9 @@
 	ata_ehi_clear_desc(ehi);
 	ata_ehi_push_desc(ehi, "edma_err_cause=%08x pp_flags=%08x",
 			  edma_err_cause, pp->pp_flags);
+
+	if (IS_GEN_IIE(hpriv) && (edma_err_cause & EDMA_ERR_TRANS_IRQ_7))
+		ata_ehi_push_desc(ehi, "fis_cause=%08x", fis_cause);
 	/*
 	 * All generations share these EDMA error cause bits:
 	 */

[PATCH 03/04] sata_mv group genIIe flags

[Mark Lord]

Group all of the flags for GenIIe devices into a common definition,
to ensure that any updates to them are shared by all GenIIe devices.

This will help make future maintenance somewhat simpler.

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

For 2.6.26.

--- old/drivers/ata/sata_mv.c	2008-05-14 08:51:49.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-14 09:00:36.000000000 -0400
@@ -128,8 +128,13 @@
 	MV_COMMON_FLAGS		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI |
 				  ATA_FLAG_PIO_POLLING,
+
 	MV_6XXX_FLAGS		= MV_FLAG_IRQ_COALESCE,
 
+	MV_GENIIE_FLAGS		= MV_COMMON_FLAGS | MV_6XXX_FLAGS |
+				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
+				  ATA_FLAG_NCQ,
+
 	CRQB_FLAG_READ		= (1 << 0),
 	CRQB_TAG_SHIFT		= 1,
 	CRQB_IOID_SHIFT		= 6,	/* CRQB Gen-II/IIE IO Id shift */
@@ -640,25 +645,19 @@
 		.port_ops	= &mv6_ops,
 	},
 	{  /* chip_6042 */
-		.flags		= MV_COMMON_FLAGS | MV_6XXX_FLAGS |
-				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ,
+		.flags		= MV_GENIIE_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv_iie_ops,
 	},
 	{  /* chip_7042 */
-		.flags		= MV_COMMON_FLAGS | MV_6XXX_FLAGS |
-				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ,
+		.flags		= MV_GENIIE_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv_iie_ops,
 	},
 	{  /* chip_soc */
-		.flags		= MV_COMMON_FLAGS | MV_6XXX_FLAGS |
-				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ | MV_FLAG_SOC,
+		.flags		= MV_GENIIE_FLAGS | MV_FLAG_SOC,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv_iie_ops,

[PATCH 04/04] sata_mv async notify for genIIe only

[Mark Lord]

Now that we handle the FIS_IRQ_CAUSE register correctly,
we can also now handle SATA asynchronous notification events.

So enable them, but only for the more modern GenIIe chips.
(older chips have unaddressed errata issues related to this).

This fixes hot plug/unplug for port-muliplier ports.

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

--- old/drivers/ata/sata_mv.c	2008-05-14 09:00:36.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-14 09:04:38.000000000 -0400
@@ -133,7 +133,7 @@
 
 	MV_GENIIE_FLAGS		= MV_COMMON_FLAGS | MV_6XXX_FLAGS |
 				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ,
+				  ATA_FLAG_NCQ | ATA_FLAG_AN,
 
 	CRQB_FLAG_READ		= (1 << 0),
 	CRQB_TAG_SHIFT		= 1,
@@ -226,6 +226,7 @@
 	SATA_STATUS_OFS		= 0x300,  /* ctrl, err regs follow status */
 	SATA_ACTIVE_OFS		= 0x350,
 	SATA_FIS_IRQ_CAUSE_OFS	= 0x364,
+	SATA_FIS_IRQ_AN		= (1 << 9),	/* async notification */
 
 	LTMODE_OFS		= 0x30c,
 	LTMODE_BIT8		= (1 << 8),	/* unknown, but necessary */
@@ -1849,8 +1850,17 @@
 	ata_ehi_push_desc(ehi, "edma_err_cause=%08x pp_flags=%08x",
 			  edma_err_cause, pp->pp_flags);
 
-	if (IS_GEN_IIE(hpriv) && (edma_err_cause & EDMA_ERR_TRANS_IRQ_7))
+	if (IS_GEN_IIE(hpriv) && (edma_err_cause & EDMA_ERR_TRANS_IRQ_7)) {
 		ata_ehi_push_desc(ehi, "fis_cause=%08x", fis_cause);
+		if (fis_cause & SATA_FIS_IRQ_AN) {
+			u32 ec = edma_err_cause &
+			       ~(EDMA_ERR_TRANS_IRQ_7 | EDMA_ERR_IRQ_TRANSIENT);
+			sata_async_notification(ap);
+			if (!ec)
+				return; /* Just an AN; no need for the nukes */
+			ata_ehi_push_desc(ehi, "SDB notify");
+		}
+	}
 	/*
 	 * All generations share these EDMA error cause bits:
 	 */

[PATCH 05/09] sata_mv don't blindly enable IRQs

[Mark Lord]

These five small patches are on top of the four previously posted,
so I'm extending the numbering to be xx/09 now:

-----snip-----

Part one of simplifying/fixing handling of the main_irq_mask register
to resolve unexpected interrupt issues observed in 2.6.26-rc*.

Don't blindly enable port IRQs at host init time.
Instead, enable only the bits that we want,
which in this case is simply the PCI_ERR bit.

The per-port bits can wait until the ports are reset/probed for devices.

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

--- old/drivers/ata/sata_mv.c	2008-05-14 09:24:49.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-16 15:25:16.000000000 -0400
@@ -202,13 +202,6 @@
 	HC_MAIN_RSVD		= (0x7f << 25),	/* bits 31-25 */
 	HC_MAIN_RSVD_5		= (0x1fff << 19), /* bits 31-19 */
 	HC_MAIN_RSVD_SOC	= (0x3fffffb << 6),     /* bits 31-9, 7-6 */
-	HC_MAIN_MASKED_IRQS	= (TRAN_LO_DONE | TRAN_HI_DONE |
-				   PORTS_0_3_COAL_DONE | PORTS_4_7_COAL_DONE |
-				   PORTS_0_7_COAL_DONE | GPIO_INT | TWSI_INT |
-				   HC_MAIN_RSVD),
-	HC_MAIN_MASKED_IRQS_5	= (PORTS_0_3_COAL_DONE | PORTS_4_7_COAL_DONE |
-				   HC_MAIN_RSVD_5),
-	HC_MAIN_MASKED_IRQS_SOC = (PORTS_0_3_COAL_DONE | HC_MAIN_RSVD_SOC),
 
 	/* SATAHC registers */
 	HC_CFG_OFS		= 0,
@@ -3101,25 +3094,12 @@
 
 		/* and unmask interrupt generation for host regs */
 		writelfl(hpriv->unmask_all_irqs, mmio + hpriv->irq_mask_ofs);
-		if (IS_GEN_I(hpriv))
-			writelfl(~HC_MAIN_MASKED_IRQS_5,
-				 hpriv->main_irq_mask_addr);
-		else
-			writelfl(~HC_MAIN_MASKED_IRQS,
-				 hpriv->main_irq_mask_addr);
 
-		VPRINTK("HC MAIN IRQ cause/mask=0x%08x/0x%08x "
-			"PCI int cause/mask=0x%08x/0x%08x\n",
-			readl(hpriv->main_irq_cause_addr),
-			readl(hpriv->main_irq_mask_addr),
-			readl(mmio + hpriv->irq_cause_ofs),
-			readl(mmio + hpriv->irq_mask_ofs));
-	} else {
-		writelfl(~HC_MAIN_MASKED_IRQS_SOC,
-			 hpriv->main_irq_mask_addr);
-		VPRINTK("HC MAIN IRQ cause/mask=0x%08x/0x%08x\n",
-			readl(hpriv->main_irq_cause_addr),
-			readl(hpriv->main_irq_mask_addr));
+		/*
+		 * enable only global host interrupts for now.
+		 * The per-port interrupts get done later as ports are set up.
+		 */
+		writelfl(PCI_ERR, hpriv->main_irq_mask_addr);
 	}
 done:
 	return rc;

[PATCH 06/09] sata_mv consolidate main_irq_mask updates

[Mark Lord]

Part two of simplifying/fixing handling of the main_irq_mask register
to resolve unexpected interrupt issues observed in 2.6.26-rc*.

Consolidate all updates of the host main_irq_mask register
into a single function.  This simplifies maintenance,
and also prepares the way for caching it (later).

No functionality changes in this update.

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

--- old/drivers/ata/sata_mv.c	2008-05-16 16:03:53.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-16 16:04:45.000000000 -0400
@@ -837,6 +837,31 @@
 		 port_mmio + EDMA_RSP_Q_OUT_PTR_OFS);
 }
 
+static void mv_set_main_irq_mask(struct ata_host *host,
+				 u32 disable_bits, u32 enable_bits)
+{
+	struct mv_host_priv *hpriv = host->private_data;
+	u32 old_mask, new_mask;
+
+	old_mask = readl(hpriv->main_irq_mask_addr);
+	new_mask = (old_mask & ~disable_bits) | enable_bits;
+	if (new_mask != old_mask)
+		writelfl(new_mask, hpriv->main_irq_mask_addr);
+}
+
+static void mv_enable_port_irqs(struct ata_port *ap,
+				     unsigned int port_bits)
+{
+	unsigned int shift, hardport, port = ap->port_no;
+	u32 disable_bits, enable_bits;
+
+	MV_PORT_TO_SHIFT_AND_HARDPORT(port, shift, hardport);
+
+	disable_bits = (DONE_IRQ | ERR_IRQ) << shift;
+	enable_bits  = port_bits << shift;
+	mv_set_main_irq_mask(ap->host, disable_bits, enable_bits);
+}
+
 /**
  *      mv_start_dma - Enable eDMA engine
  *      @base: port base address
@@ -2383,7 +2408,6 @@
 	ZERO(MV_PCI_DISC_TIMER);
 	ZERO(MV_PCI_MSI_TRIGGER);
 	writel(0x000100ff, mmio + MV_PCI_XBAR_TMOUT_OFS);
-	ZERO(PCI_HC_MAIN_IRQ_MASK_OFS);
 	ZERO(MV_PCI_SERR_MASK);
 	ZERO(hpriv->irq_cause_ofs);
 	ZERO(hpriv->irq_mask_ofs);
@@ -2755,32 +2779,18 @@
 
 static void mv_eh_freeze(struct ata_port *ap)
 {
-	struct mv_host_priv *hpriv = ap->host->private_data;
-	unsigned int shift, hardport, port = ap->port_no;
-	u32 main_irq_mask;
-
-	/* FIXME: handle coalescing completion events properly */
-
 	mv_stop_edma(ap);
-	MV_PORT_TO_SHIFT_AND_HARDPORT(port, shift, hardport);
-
-	/* disable assertion of portN err, done events */
-	main_irq_mask = readl(hpriv->main_irq_mask_addr);
-	main_irq_mask &= ~((DONE_IRQ | ERR_IRQ) << shift);
-	writelfl(main_irq_mask, hpriv->main_irq_mask_addr);
+	mv_enable_port_irqs(ap, 0);
 }
 
 static void mv_eh_thaw(struct ata_port *ap)
 {
 	struct mv_host_priv *hpriv = ap->host->private_data;
-	unsigned int shift, hardport, port = ap->port_no;
+	unsigned int port = ap->port_no;
+	unsigned int hardport = mv_hardport_from_port(port);
 	void __iomem *hc_mmio = mv_hc_base_from_port(hpriv->base, port);
 	void __iomem *port_mmio = mv_ap_base(ap);
-	u32 main_irq_mask, hc_irq_cause;
-
-	/* FIXME: handle coalescing completion events properly */
-
-	MV_PORT_TO_SHIFT_AND_HARDPORT(port, shift, hardport);
+	u32 hc_irq_cause;
 
 	/* clear EDMA errors on this port */
 	writel(0, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
@@ -2790,10 +2800,7 @@
 	hc_irq_cause &= ~((DEV_IRQ | DMA_IRQ) << hardport);
 	writelfl(hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
 
-	/* enable assertion of portN err, done events */
-	main_irq_mask = readl(hpriv->main_irq_mask_addr);
-	main_irq_mask |= ((DONE_IRQ | ERR_IRQ) << shift);
-	writelfl(main_irq_mask, hpriv->main_irq_mask_addr);
+	mv_enable_port_irqs(ap, DONE_IRQ | ERR_IRQ);
 }
 
 /**
@@ -3046,7 +3053,7 @@
 	}
 
 	/* global interrupt mask: 0 == mask everything */
-	writel(0, hpriv->main_irq_mask_addr);
+	mv_set_main_irq_mask(host, ~0, 0);
 
 	n_hc = mv_get_hc_count(host->ports[0]->flags);
 
@@ -3099,7 +3106,7 @@
 		 * enable only global host interrupts for now.
 		 * The per-port interrupts get done later as ports are set up.
 		 */
-		writelfl(PCI_ERR, hpriv->main_irq_mask_addr);
+		mv_set_main_irq_mask(host, 0, PCI_ERR);
 	}
 done:
 	return rc;

[PATCH 07/09] sata_mv fix pmp drives not found

[Mark Lord]

Part three of simplifying/fixing handling of the main_irq_mask register
to resolve unexpected interrupt issues observed in 2.6.26-rc*.

Partially fix a reported bug whereby we sometimes miss seeing drives on
a port-multiplier, as reported by Gwendal Grignou <gwendal@google.com>.

The problem was that we were receiving unexpected interrupts
during EH from POLLed commands while accessing port-multiplier registers.
These unexpected interrupts can be prevented by masking the DONE_IRQ bit
for the port whenever not operating in EDMA mode.

Also fix port_stop() to mask all port interrupts.

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

--- old/drivers/ata/sata_mv.c	2008-05-16 16:04:45.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-16 16:05:48.000000000 -0400
@@ -908,6 +908,7 @@
 			writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
 
 		mv_set_edma_ptrs(port_mmio, hpriv, pp);
+		mv_enable_port_irqs(ap, DONE_IRQ|ERR_IRQ);
 
 		writelfl(EDMA_EN, port_mmio + EDMA_CMD_OFS);
 		pp->pp_flags |= MV_PP_FLAG_EDMA_EN;
@@ -1360,6 +1361,7 @@
 static void mv_port_stop(struct ata_port *ap)
 {
 	mv_stop_edma(ap);
+	mv_enable_port_irqs(ap, 0);
 	mv_port_free_dma_mem(ap);
 }
 
@@ -1601,6 +1603,7 @@
 		 * shadow block, etc registers.
 		 */
 		mv_stop_edma(ap);
+		mv_enable_port_irqs(ap, ERR_IRQ);
 		mv_pmp_select(ap, qc->dev->link->pmp);
 		return ata_sff_qc_issue(qc);
 	}
@@ -2800,7 +2803,7 @@
 	hc_irq_cause &= ~((DEV_IRQ | DMA_IRQ) << hardport);
 	writelfl(hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
 
-	mv_enable_port_irqs(ap, DONE_IRQ | ERR_IRQ);
+	mv_enable_port_irqs(ap, ERR_IRQ);
 }
 
 /**

[PATCH 08/09] sata_mv disregard masked irqs

[Mark Lord]

Part four of simplifying/fixing handling of the main_irq_mask register
to resolve unexpected interrupt issues observed in 2.6.26-rc*.

Ignore masked IRQs in mv_interrupt().
This prevents "unexpected device interrupt while idle" messages.

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

--- old/drivers/ata/sata_mv.c	2008-05-16 16:05:48.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-16 16:06:23.000000000 -0400
@@ -2200,20 +2200,21 @@
 	struct ata_host *host = dev_instance;
 	struct mv_host_priv *hpriv = host->private_data;
 	unsigned int handled = 0;
-	u32 main_irq_cause, main_irq_mask;
+	u32 main_irq_cause, main_irq_mask, pending_irqs;
 
 	spin_lock(&host->lock);
 	main_irq_cause = readl(hpriv->main_irq_cause_addr);
 	main_irq_mask  = readl(hpriv->main_irq_mask_addr);
+	pending_irqs   = main_irq_cause & main_irq_mask;
 	/*
 	 * Deal with cases where we either have nothing pending, or have read
 	 * a bogus register value which can indicate HW removal or PCI fault.
 	 */
-	if ((main_irq_cause & main_irq_mask) && (main_irq_cause != 0xffffffffU)) {
-		if (unlikely((main_irq_cause & PCI_ERR) && HAS_PCI(host)))
+	if (pending_irqs && main_irq_cause != 0xffffffffU) {
+		if (unlikely((pending_irqs & PCI_ERR) && HAS_PCI(host)))
 			handled = mv_pci_error(host, hpriv->base);
 		else
-			handled = mv_host_intr(host, main_irq_cause);
+			handled = mv_host_intr(host, pending_irqs);
 	}
 	spin_unlock(&host->lock);
 	return IRQ_RETVAL(handled);

[PATCH 09/09] sata_mv cache main_irq_mask register in hpriv

[Mark Lord]

Part five of simplifying/fixing handling of the main_irq_mask register
to resolve unexpected interrupt issues observed in 2.6.26-rc*.

Keep a cached copy of the main_irq_mask so that we don't have
to stall the CPU to read it on every pass through mv_interrupt.

This significantly speeds up interrupt handling, both for sata_mv,
and for any other driver/device sharing the same PCI IRQ line.

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

--- old/drivers/ata/sata_mv.c	2008-05-16 16:06:23.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-16 16:02:55.000000000 -0400
@@ -458,6 +458,7 @@
 
 struct mv_host_priv {
 	u32			hp_flags;
+	u32			main_irq_mask;
 	struct mv_port_signal	signal[8];
 	const struct mv_hw_ops	*ops;
 	int			n_ports;
@@ -843,10 +844,12 @@
 	struct mv_host_priv *hpriv = host->private_data;
 	u32 old_mask, new_mask;
 
-	old_mask = readl(hpriv->main_irq_mask_addr);
+	old_mask = hpriv->main_irq_mask;
 	new_mask = (old_mask & ~disable_bits) | enable_bits;
-	if (new_mask != old_mask)
+	if (new_mask != old_mask) {
+		hpriv->main_irq_mask = new_mask;
 		writelfl(new_mask, hpriv->main_irq_mask_addr);
+	}
 }
 
 static void mv_enable_port_irqs(struct ata_port *ap,
@@ -2200,12 +2203,11 @@
 	struct ata_host *host = dev_instance;
 	struct mv_host_priv *hpriv = host->private_data;
 	unsigned int handled = 0;
-	u32 main_irq_cause, main_irq_mask, pending_irqs;
+	u32 main_irq_cause, pending_irqs;
 
 	spin_lock(&host->lock);
 	main_irq_cause = readl(hpriv->main_irq_cause_addr);
-	main_irq_mask  = readl(hpriv->main_irq_mask_addr);
-	pending_irqs   = main_irq_cause & main_irq_mask;
+	pending_irqs   = main_irq_cause & hpriv->main_irq_mask;
 	/*
 	 * Deal with cases where we either have nothing pending, or have read
 	 * a bogus register value which can indicate HW removal or PCI fault.

[PATCH 10/10] sata_mv: ensure empty request queue for FBS-NCQ EH

[Mark Lord]

Check for an empty request queue before stopping EDMA after a FBS-NCQ error,
as per recommendation from the Marvell datasheet.

This ensures that the EDMA won't suddenly become active again
just after our subsequent check of the empty/idle bits.

Also bump DRV_VERSION.

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

Is DRV_VERSION even useful now?
I could do another patch to just nuke it.

--- old/drivers/ata/sata_mv.c	2008-05-16 17:02:26.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2008-05-19 08:52:17.000000000 -0400
@@ -72,7 +72,7 @@
 #include <linux/libata.h>
 
 #define DRV_NAME	"sata_mv"
-#define DRV_VERSION	"1.20"
+#define DRV_VERSION	"1.21"
 
 enum {
 	/* BAR's are enumerated in terms of pci_resource_start() terms */
@@ -1695,6 +1695,18 @@
 	}
 }
 
+static int mv_req_q_empty(struct ata_port *ap)
+{
+	void __iomem *port_mmio = mv_ap_base(ap);
+	u32 in_ptr, out_ptr;
+
+	in_ptr  = (readl(port_mmio + EDMA_REQ_Q_IN_PTR_OFS)
+			>> EDMA_REQ_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK;
+	out_ptr = (readl(port_mmio + EDMA_REQ_Q_OUT_PTR_OFS)
+			>> EDMA_REQ_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK;
+	return (in_ptr == out_ptr);	/* 1 == queue_is_empty */
+}
+
 static int mv_handle_fbs_ncq_dev_err(struct ata_port *ap)
 {
 	struct mv_port_priv *pp = ap->private_data;
@@ -1728,7 +1740,7 @@
 			ap->qc_active, failed_links,
 			ap->nr_active_links);
 
-	if (ap->nr_active_links <= failed_links) {
+	if (ap->nr_active_links <= failed_links && mv_req_q_empty(ap)) {
 		mv_process_crpb_entries(ap, pp);
 		mv_stop_edma(ap);
 		mv_eh_freeze(ap);

Re: [PATCH 10/10] sata_mv: ensure empty request queue for FBS-NCQ EH

[Jeff Garzik]

Mark Lord wrote:
> Check for an empty request queue before stopping EDMA after a FBS-NCQ 
> error,
> as per recommendation from the Marvell datasheet.
> 
> This ensures that the EDMA won't suddenly become active again
> just after our subsequent check of the empty/idle bits.
> 
> Also bump DRV_VERSION.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
> 
> Is DRV_VERSION even useful now?
> I could do another patch to just nuke it.

It is standard practice to export all driver versions via dmesg and 
modinfo for all libata drivers, so I would prefer that it stay, for 
consistency.

Is it useful?

That depends on the maintainer's definition and upkeep of the version 
number.  If you never update it, of course its useless.  If you update 
it when the driver achieves certain milestones, i.e. "2.0" when all your 
changes are complete, then it may be relevant to you and your userbase.

Version numbers tend to be quite handy because drivers get backported 
all over the place, so you might not be able to assume that sata_mv 
version 1.21 implies kernel version 2.6.26, for example.

	Jeff

Re: [PATCH 10/10] sata_mv: ensure empty request queue for FBS-NCQ EH

[Jeff Garzik]

Mark Lord wrote:
> Check for an empty request queue before stopping EDMA after a FBS-NCQ 
> error,
> as per recommendation from the Marvell datasheet.
> 
> This ensures that the EDMA won't suddenly become active again
> just after our subsequent check of the empty/idle bits.
> 
> Also bump DRV_VERSION.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
> 
> Is DRV_VERSION even useful now?
> I could do another patch to just nuke it.

applied 1-10

Re: [PATCH 09/09] sata_mv cache main_irq_mask register in hpriv

[Grant Grundler]

On Sat, May 17, 2008 at 10:38 AM, Mark Lord <liml@rtr.ca> wrote:
> Part five of simplifying/fixing handling of the main_irq_mask register
> to resolve unexpected interrupt issues observed in 2.6.26-rc*.
>
> Keep a cached copy of the main_irq_mask so that we don't have
> to stall the CPU to read it on every pass through mv_interrupt.
>
> This significantly speeds up interrupt handling, both for sata_mv,
> and for any other driver/device sharing the same PCI IRQ line.

To be precise, it saves about ~1% of cpu cycles of ~2Ghz CPU
as measured by oprofile. "fio" provided the workload by sequentially
reading 4K blocks from 4 normal SATA II disks (about 8K IO/s per disk).
This is expected to be similar to using SSDs.
So not a huge win; just an easy one.

Here is the cost of one MMIO read on a not-to-old intel PCI-e chipset:
0a:00.0 (11ab:7042): sata_mv from CPU0                   SATAHC0Cause
(20014):  1345 cycles   576 ns
0a:00.0 (11ab:7042): sata_mv from CPU0                      main_mask
(1d64):  1302 cycles   558 ns
0a:00.0 (11ab:7042): sata_mv from CPU0                     main_cause
(1d60):  1302 cycles   558 ns

I'll note this is extremely fast since there are no PCI-PCI bridges
and the 7042 controller seems to respond very quickly.
More typical is 1000ns or so.

thanks,
grant

(1) svn co http://svn.gnumonks.org/trunk/mmio_test/

Plus this patch:
    http://iou.parisc-linux.org/~grundler/diff/diff-mmio_test-02

Re: [PATCH 05/09] sata_mv don't blindly enable IRQs

[Jeff Garzik]

Mark Lord wrote:
> These five small patches are on top of the four previously posted,
> so I'm extending the numbering to be xx/09 now:
> 
> -----snip-----
> 
> Part one of simplifying/fixing handling of the main_irq_mask register
> to resolve unexpected interrupt issues observed in 2.6.26-rc*.
> 
> Don't blindly enable port IRQs at host init time.
> Instead, enable only the bits that we want,
> which in this case is simply the PCI_ERR bit.
> 
> The per-port bits can wait until the ports are reset/probed for devices.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

Two process notes for future use:

1) please include a colon (":") or other punctuation after "sata_mv" 
prefix in subject line.

2) when adding notes like "These five small patches...", we already have 
a method of doing so:  the "---" separator.  The format of the patch 
body should be:


	patch description for kernel changelog

	signed-off-by: ...
	---
	These five small patches are on top of the four previously
	posted, so I'm extending the numbering to be xx/09 now.


Both of these details are documented in Documentation/SubmittingPatches 
and http://linux.yyz.us/patch-format.html

OTOH, your numbering behavior -- extending the patch numbering as you've 
done here, when the first patchset is not yet applied to git -- is quite 
useful and should be emulated by others :)

Tejun sometimes simply posts a new patchseries, noting that that 
patchseries is dependent upon a previous series.  That's fine, too.

Thanks,

	Jeff

Re: [PATCH 05/09] sata_mv don't blindly enable IRQs

[Mark Lord]

Jeff Garzik wrote:
>
> 1) please include a colon (":") or other punctuation after "sata_mv" 
> prefix in subject line.
..

Ahh..  will do, thanks.

> 2) when adding notes like "These five small patches...", we already have 
> a method of doing so:  the "---" separator.  The format of the patch 
> body should be:
> 
> 
>     patch description for kernel changelog
> 
>     signed-off-by: ...
>     ---
>     These five small patches are on top of the four previously
>     posted, so I'm extending the numbering to be xx/09 now.
..

I wondered about that one, and decided that making it prominent at the top
might be best.  But since you're trained to look in the correct place
further down, that's where I'll put it from now on.

Thanks Jeff!

Re: [PATCH 05/09] sata_mv don't blindly enable IRQs

[Jeff Garzik]

Mark Lord wrote:
> I wondered about that one, and decided that making it prominent at the top
> might be best.  But since you're trained to look in the correct place
> further down, that's where I'll put it from now on.


Not only my brain:  Linus's standard email tools[1] automatically snip 
everything after the separator, reducing work on my end.

Thanks,

	Jeff



[1] The email tools that come with the git program, namely git-am. 
Packaged as 'git-email' in Fedora.