[PATCH 00/07] sata_mv: ATAPI patchset

[PATCH 00/07] sata_mv: ATAPI patchset

[Mark Lord]

The following patchset adds ATAPI capability to sata_mv.
These patches are all for #upstream.

01_sata_mv_cleanup_flags.patch
02_sata_mv_edma_cfg.patch
03_sata_mv_qc_issue.patch
04_sata_mv_get_active_qc.patch
05_sata_mv_fill_sg.patch
06_sata_mv_atapi_dma.patch
07_sata_mv_optimize_edma_cfg_use.patch

Cheers

[PATCH 01/07] sata_mv: cleanup chipset GENeration FLAGS

[Mark Lord]

Clean up the chipset GENeration FLAGS, and rename them
for consistency with other uses of GEN_XX within sata_mv.

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

--- old/drivers/ata/sata_mv.c	2009-01-30 16:09:07.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 16:27:13.000000000 -0500
@@ -120,14 +120,15 @@
 	MV_FLAG_IRQ_COALESCE	= (1 << 29),  /* IRQ coalescing capability */
 
 	MV_COMMON_FLAGS		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI |
-				  ATA_FLAG_PIO_POLLING,
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_POLLING,
 
-	MV_6XXX_FLAGS		= MV_FLAG_IRQ_COALESCE,
+	MV_GEN_I_FLAGS		= MV_COMMON_FLAGS | ATA_FLAG_NO_ATAPI,
 
-	MV_GENIIE_FLAGS		= MV_COMMON_FLAGS | MV_6XXX_FLAGS |
+	MV_GEN_II_FLAGS		= MV_COMMON_FLAGS | MV_FLAG_IRQ_COALESCE |
 				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ | ATA_FLAG_AN,
+				  ATA_FLAG_NCQ | ATA_FLAG_NO_ATAPI,
+
+	MV_GEN_IIE_FLAGS	= MV_GEN_II_FLAGS | ATA_FLAG_AN,
 
 	CRQB_FLAG_READ		= (1 << 0),
 	CRQB_TAG_SHIFT		= 1,
@@ -603,53 +604,49 @@
 
 static const struct ata_port_info mv_port_info[] = {
 	{  /* chip_504x */
-		.flags		= MV_COMMON_FLAGS,
+		.flags		= MV_GEN_I_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv5_ops,
 	},
 	{  /* chip_508x */
-		.flags		= MV_COMMON_FLAGS | MV_FLAG_DUAL_HC,
+		.flags		= MV_GEN_I_FLAGS | MV_FLAG_DUAL_HC,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv5_ops,
 	},
 	{  /* chip_5080 */
-		.flags		= MV_COMMON_FLAGS | MV_FLAG_DUAL_HC,
+		.flags		= MV_GEN_I_FLAGS | MV_FLAG_DUAL_HC,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv5_ops,
 	},
 	{  /* chip_604x */
-		.flags		= MV_COMMON_FLAGS | MV_6XXX_FLAGS |
-				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ,
+		.flags		= MV_GEN_II_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv6_ops,
 	},
 	{  /* chip_608x */
-		.flags		= MV_COMMON_FLAGS | MV_6XXX_FLAGS |
-				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ | MV_FLAG_DUAL_HC,
+		.flags		= MV_GEN_II_FLAGS | MV_FLAG_DUAL_HC,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv6_ops,
 	},
 	{  /* chip_6042 */
-		.flags		= MV_GENIIE_FLAGS,
+		.flags		= MV_GEN_IIE_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv_iie_ops,
 	},
 	{  /* chip_7042 */
-		.flags		= MV_GENIIE_FLAGS,
+		.flags		= MV_GEN_IIE_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv_iie_ops,
 	},
 	{  /* chip_soc */
-		.flags		= MV_GENIIE_FLAGS,
+		.flags		= MV_GEN_IIE_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &mv_iie_ops,

[PATCH 02/07] sata_mv: rearrange mv_start_dma() and friends

[Mark Lord]

Rearrange mv_start_dma() and friends, in preparation for adding
non-EDMA DMA modes, and non-EDMA interrupts, to the driver.

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

--- old/drivers/ata/sata_mv.c	2009-01-30 16:45:20.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 16:44:12.000000000 -0500
@@ -536,7 +536,7 @@
 			     unsigned int port_no);
 static int mv_stop_edma(struct ata_port *ap);
 static int mv_stop_edma_engine(void __iomem *port_mmio);
-static void mv_edma_cfg(struct ata_port *ap, int want_ncq);
+static void mv_edma_cfg(struct ata_port *ap, int want_ncq, int want_edma);
 
 static void mv_pmp_select(struct ata_port *ap, int pmp);
 static int mv_pmp_hardreset(struct ata_link *link, unsigned int *class,
@@ -849,8 +849,32 @@
 	mv_set_main_irq_mask(ap->host, disable_bits, enable_bits);
 }
 
+static void mv_clear_and_enable_port_irqs(struct ata_port *ap,
+					  void __iomem *port_mmio,
+					  unsigned int port_irqs)
+{
+	struct mv_host_priv *hpriv = ap->host->private_data;
+	int hardport = mv_hardport_from_port(ap->port_no);
+	void __iomem *hc_mmio = mv_hc_base_from_port(
+				mv_host_base(ap->host), ap->port_no);
+	u32 hc_irq_cause;
+
+	/* clear EDMA event indicators, if any */
+	writelfl(0, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
+
+	/* clear pending irq events */
+	hc_irq_cause = ~((DEV_IRQ | DMA_IRQ) << hardport);
+	writelfl(hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
+
+	/* clear FIS IRQ Cause */
+	if (IS_GEN_IIE(hpriv))
+		writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+
+	mv_enable_port_irqs(ap, port_irqs);
+}
+
 /**
- *      mv_start_dma - Enable eDMA engine
+ *      mv_start_edma - Enable eDMA engine
  *      @base: port base address
  *      @pp: port private data
  *
@@ -860,7 +884,7 @@
  *      LOCKING:
  *      Inherited from caller.
  */
-static void mv_start_dma(struct ata_port *ap, void __iomem *port_mmio,
+static void mv_start_edma(struct ata_port *ap, void __iomem *port_mmio,
 			 struct mv_port_priv *pp, u8 protocol)
 {
 	int want_ncq = (protocol == ATA_PROT_NCQ);
@@ -872,26 +896,11 @@
 	}
 	if (!(pp->pp_flags & MV_PP_FLAG_EDMA_EN)) {
 		struct mv_host_priv *hpriv = ap->host->private_data;
-		int hardport = mv_hardport_from_port(ap->port_no);
-		void __iomem *hc_mmio = mv_hc_base_from_port(
-					mv_host_base(ap->host), ap->port_no);
-		u32 hc_irq_cause;
-
-		/* clear EDMA event indicators, if any */
-		writelfl(0, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
-
-		/* clear pending irq events */
-		hc_irq_cause = ~((DEV_IRQ | DMA_IRQ) << hardport);
-		writelfl(hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
 
-		mv_edma_cfg(ap, want_ncq);
-
-		/* clear FIS IRQ Cause */
-		if (IS_GEN_IIE(hpriv))
-			writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+		mv_edma_cfg(ap, want_ncq, 1);
 
 		mv_set_edma_ptrs(port_mmio, hpriv, pp);
-		mv_enable_port_irqs(ap, DONE_IRQ|ERR_IRQ);
+		mv_clear_and_enable_port_irqs(ap, port_mmio, DONE_IRQ|ERR_IRQ);
 
 		writelfl(EDMA_EN, port_mmio + EDMA_CMD_OFS);
 		pp->pp_flags |= MV_PP_FLAG_EDMA_EN;
@@ -1173,7 +1182,7 @@
 		writel(new, hpriv->base + MV_GPIO_PORT_CTL_OFS);
 }
 
-static void mv_edma_cfg(struct ata_port *ap, int want_ncq)
+static void mv_edma_cfg(struct ata_port *ap, int want_ncq, int want_edma)
 {
 	u32 cfg;
 	struct mv_port_priv *pp    = ap->private_data;
@@ -1182,7 +1191,7 @@
 
 	/* set up non-NCQ EDMA configuration */
 	cfg = EDMA_CFG_Q_DEPTH;		/* always 0x1f for *all* chips */
-	pp->pp_flags &= ~MV_PP_FLAG_FBS_EN;
+	pp->pp_flags &= ~(MV_PP_FLAG_FBS_EN | MV_PP_FLAG_NCQ_EN);
 
 	if (IS_GEN_I(hpriv))
 		cfg |= (1 << 8);	/* enab config burst size mask */
@@ -1211,9 +1220,11 @@
 		}
 
 		cfg |= (1 << 23);	/* do not mask PM field in rx'd FIS */
-		cfg |= (1 << 22);	/* enab 4-entry host queue cache */
-		if (!IS_SOC(hpriv))
-			cfg |= (1 << 18);	/* enab early completion */
+		if (want_edma) {
+			cfg |= (1 << 22); /* enab 4-entry host queue cache */
+			if (!IS_SOC(hpriv))
+				cfg |= (1 << 18); /* enab early completion */
+		}
 		if (hpriv->hp_flags & MV_HP_CUT_THROUGH)
 			cfg |= (1 << 17); /* enab cut-thru (dis stor&forwrd) */
 	}
@@ -1221,8 +1232,7 @@
 	if (want_ncq) {
 		cfg |= EDMA_CFG_NCQ;
 		pp->pp_flags |=  MV_PP_FLAG_NCQ_EN;
-	} else
-		pp->pp_flags &= ~MV_PP_FLAG_NCQ_EN;
+	}
 
 	writelfl(cfg, port_mmio + EDMA_CFG_OFS);
 }
@@ -1591,7 +1601,7 @@
 		return ata_sff_qc_issue(qc);
 	}
 
-	mv_start_dma(ap, port_mmio, pp, qc->tf.protocol);
+	mv_start_edma(ap, port_mmio, pp, qc->tf.protocol);
 
 	pp->req_idx = (pp->req_idx + 1) & MV_MAX_Q_DEPTH_MASK;
 	in_index = pp->req_idx << EDMA_REQ_Q_PTR_SHIFT;

[PATCH 03/07] sata_mv: restructure mv_qc_issue

[Mark Lord]

Rearrange logic in mv_qc_issue() to handle protocols
other than ATA_PROT_DMA, ATA_PROT_NCQ, and ATA_PROT_PIO.
This is in preparation for later enabling ATAPI support.

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

--- old/drivers/ata/sata_mv.c	2009-01-30 18:21:49.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 18:23:22.000000000 -0500
@@ -1565,14 +1565,26 @@
  */
 static unsigned int mv_qc_issue(struct ata_queued_cmd *qc)
 {
+	static int limit_warnings = 10;
 	struct ata_port *ap = qc->ap;
 	void __iomem *port_mmio = mv_ap_base(ap);
 	struct mv_port_priv *pp = ap->private_data;
 	u32 in_index;
+	unsigned int port_irqs = DONE_IRQ | ERR_IRQ;
 
-	if ((qc->tf.protocol != ATA_PROT_DMA) &&
-	    (qc->tf.protocol != ATA_PROT_NCQ)) {
-		static int limit_warnings = 10;
+	switch (qc->tf.protocol) {
+	case ATA_PROT_DMA:
+	case ATA_PROT_NCQ:
+		mv_start_edma(ap, port_mmio, pp, qc->tf.protocol);
+		pp->req_idx = (pp->req_idx + 1) & MV_MAX_Q_DEPTH_MASK;
+		in_index = pp->req_idx << EDMA_REQ_Q_PTR_SHIFT;
+
+		/* Write the request in pointer to kick the EDMA to life */
+		writelfl((pp->crqb_dma & EDMA_REQ_Q_BASE_LO_MASK) | in_index,
+					port_mmio + EDMA_REQ_Q_IN_PTR_OFS);
+		return 0;
+
+	case ATA_PROT_PIO:
 		/*
 		 * Errata SATA#16, SATA#24: warn if multiple DRQs expected.
 		 *
@@ -1590,27 +1602,22 @@
 					": attempting PIO w/multiple DRQ: "
 					"this may fail due to h/w errata\n");
 		}
+		/* drop through */
+	case ATAPI_PROT_PIO:
+		port_irqs = ERR_IRQ;	/* leave DONE_IRQ masked for PIO */
+		/* drop through */
+	default:
 		/*
 		 * We're about to send a non-EDMA capable command to the
 		 * port.  Turn off EDMA so there won't be problems accessing
 		 * shadow block, etc registers.
 		 */
 		mv_stop_edma(ap);
-		mv_enable_port_irqs(ap, ERR_IRQ);
+		mv_edma_cfg(ap, 0, 0);
+		mv_clear_and_enable_port_irqs(ap, mv_ap_base(ap), port_irqs);
 		mv_pmp_select(ap, qc->dev->link->pmp);
 		return ata_sff_qc_issue(qc);
 	}
-
-	mv_start_edma(ap, port_mmio, pp, qc->tf.protocol);
-
-	pp->req_idx = (pp->req_idx + 1) & MV_MAX_Q_DEPTH_MASK;
-	in_index = pp->req_idx << EDMA_REQ_Q_PTR_SHIFT;
-
-	/* and write the request in pointer to kick the EDMA to life */
-	writelfl((pp->crqb_dma & EDMA_REQ_Q_BASE_LO_MASK) | in_index,
-		 port_mmio + EDMA_REQ_Q_IN_PTR_OFS);
-
-	return 0;
 }
 
 static struct ata_queued_cmd *mv_get_active_qc(struct ata_port *ap)

[PATCH 04/07] sata_mv: update ata_qc_from_tag

[Mark Lord]

Update the logic in ata_qc_from_tag() to match that used
in similar places elsewhere in libata.

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

--- old/drivers/ata/sata_mv.c	2009-01-30 18:23:22.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 18:26:55.000000000 -0500
@@ -1628,6 +1628,12 @@
 	if (pp->pp_flags & MV_PP_FLAG_NCQ_EN)
 		return NULL;
 	qc = ata_qc_from_tag(ap, ap->link.active_tag);
+	if (qc) {
+		if (qc->tf.flags & ATA_TFLAG_POLLING)
+			qc = NULL;
+		else if (!(qc->flags & ATA_QCFLAG_ACTIVE))
+			qc = NULL;
+	}
 	if (qc && (qc->tf.flags & ATA_TFLAG_POLLING))
 		qc = NULL;
 	return qc;

[PATCH 05/07] sata_mv: mv_fill_sg fixes

[Mark Lord]

Fix mv_fill_sg() to zero out the reserved word (required for ATAPI),
and to include a memory barrier.  This may also help with problems
reported by Jens on the PPC platform.

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

--- old/drivers/ata/sata_mv.c	2009-01-30 18:31:03.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 18:32:08.000000000 -0500
@@ -1364,12 +1364,13 @@
 			u32 offset = addr & 0xffff;
 			u32 len = sg_len;
 
-			if ((offset + sg_len > 0x10000))
+			if ((offset + len > 0x10000))
 				len = 0x10000 - offset;
 
 			mv_sg->addr = cpu_to_le32(addr & 0xffffffff);
 			mv_sg->addr_hi = cpu_to_le32((addr >> 16) >> 16);
 			mv_sg->flags_size = cpu_to_le32(len & 0xffff);
+			mv_sg->reserved = 0;
 
 			sg_len -= len;
 			addr += len;
@@ -1381,6 +1382,7 @@
 
 	if (likely(last_sg))
 		last_sg->flags_size |= cpu_to_le32(EPRD_FLAG_END_OF_TBL);
+	mb(); /* ensure data structure is visible to the chipset */
 }
 
 static void mv_crqb_pack_cmd(__le16 *cmdw, u8 data, u8 addr, unsigned last)

[PATCH 06/07] sata_mv: introduce support for ATAPI devices

[Mark Lord]

Add ATAPI support to sata_mv, using sff DMA for GEN_II chipsets,
and plain old PIO for GEN_IIE.

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

--- old/drivers/ata/sata_mv.c	2009-01-30 18:32:08.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 18:33:04.000000000 -0500
@@ -31,8 +31,6 @@
  *
  * --> Complete a full errata audit for all chipsets to identify others.
  *
- * --> ATAPI support (Marvell claims the 60xx/70xx chips can do it).
- *
  * --> Develop a low-power-consumption strategy, and implement it.
  *
  * --> [Experiment, low priority] Investigate interrupt coalescing.
@@ -68,7 +66,7 @@
 #include <linux/libata.h>
 
 #define DRV_NAME	"sata_mv"
-#define DRV_VERSION	"1.25"
+#define DRV_VERSION	"1.26"
 
 enum {
 	/* BAR's are enumerated in terms of pci_resource_start() terms */
@@ -126,7 +124,7 @@
 
 	MV_GEN_II_FLAGS		= MV_COMMON_FLAGS | MV_FLAG_IRQ_COALESCE |
 				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ | ATA_FLAG_NO_ATAPI,
+				  ATA_FLAG_NCQ,
 
 	MV_GEN_IIE_FLAGS	= MV_GEN_II_FLAGS | ATA_FLAG_AN,
 
@@ -348,6 +346,12 @@
 
 	EDMA_HALTCOND_OFS	= 0x60,		/* GenIIe halt conditions */
 
+
+	BMDMA_CMD_OFS		= 0x224,	/* bmdma command register */
+	BMDMA_STATUS_OFS	= 0x228,	/* bmdma status register */
+	BMDMA_PRD_LOW_OFS	= 0x22c,	/* bmdma PRD addr 31:0 */
+	BMDMA_PRD_HIGH_OFS	= 0x230,	/* bmdma PRD addr 63:32 */
+
 	/* Host private flags (hp_flags) */
 	MV_HP_FLAG_MSI		= (1 << 0),
 	MV_HP_ERRATA_50XXB0	= (1 << 1),
@@ -547,6 +551,15 @@
 static void mv_process_crpb_entries(struct ata_port *ap,
 					struct mv_port_priv *pp);
 
+static unsigned long mv_mode_filter(struct ata_device *dev,
+				    unsigned long xfer_mask);
+static void mv_sff_irq_clear(struct ata_port *ap);
+static int mv_check_atapi_dma(struct ata_queued_cmd *qc);
+static void mv_bmdma_setup(struct ata_queued_cmd *qc);
+static void mv_bmdma_start(struct ata_queued_cmd *qc);
+static void mv_bmdma_stop(struct ata_queued_cmd *qc);
+static u8   mv_bmdma_status(struct ata_port *ap);
+
 /* .sg_tablesize is (MV_MAX_SG_CT / 2) in the structures below
  * because we have to allow room for worst case splitting of
  * PRDs for 64K boundaries in mv_fill_sg().
@@ -594,6 +607,14 @@
 	.pmp_softreset		= mv_softreset,
 	.softreset		= mv_softreset,
 	.error_handler		= mv_pmp_error_handler,
+
+	.sff_irq_clear		= mv_sff_irq_clear,
+	.check_atapi_dma	= mv_check_atapi_dma,
+	.bmdma_setup		= mv_bmdma_setup,
+	.bmdma_start		= mv_bmdma_start,
+	.bmdma_stop		= mv_bmdma_stop,
+	.bmdma_status		= mv_bmdma_status,
+	.mode_filter		= mv_mode_filter,
 };
 
 static struct ata_port_operations mv_iie_ops = {
@@ -1393,6 +1414,167 @@
 }
 
 /**
+ *	mv_mode_filter - Allow ATAPI DMA only on GenII chips.
+ *	@dev: device whose xfer modes are being configured.
+ *
+ *	Only the GenII hardware can use DMA with ATAPI drives.
+ */
+static unsigned long mv_mode_filter(struct ata_device *adev,
+				    unsigned long xfer_mask)
+{
+	if (adev->class == ATA_DEV_ATAPI) {
+		struct mv_host_priv *hpriv = adev->link->ap->host->private_data;
+		if (!IS_GEN_II(hpriv)) {
+			xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
+			ata_dev_printk(adev, KERN_INFO,
+				"ATAPI DMA not supported on this chipset\n");
+		}
+	}
+	return xfer_mask;
+}
+
+/**
+ *	mv_sff_irq_clear - Clear hardware interrupt after DMA.
+ *	@ap: Port associated with this ATA transaction.
+ *
+ *	We need this only for ATAPI bmdma transactions,
+ *	as otherwise we experience spurious interrupts
+ *	after libata-sff handles the bmdma interrupts.
+ */
+static void mv_sff_irq_clear(struct ata_port *ap)
+{
+	mv_clear_and_enable_port_irqs(ap, mv_ap_base(ap), ERR_IRQ);
+}
+
+/**
+ *	mv_check_atapi_dma - Filter ATAPI cmds which are unsuitable for DMA.
+ *	@qc: queued command to check for chipset/DMA compatibility.
+ *
+ *	The bmdma engines cannot handle speculative data sizes
+ *	(bytecount under/over flow).  So only allow DMA for
+ *	data transfer commands with known data sizes.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+static int mv_check_atapi_dma(struct ata_queued_cmd *qc)
+{
+	struct scsi_cmnd *scmd = qc->scsicmd;
+
+	if (scmd) {
+		switch (scmd->cmnd[0]) {
+		case READ_6:
+		case READ_10:
+		case READ_12:
+		case WRITE_6:
+		case WRITE_10:
+		case WRITE_12:
+		case GPCMD_READ_CD:
+		case GPCMD_SEND_DVD_STRUCTURE:
+		case GPCMD_SEND_CUE_SHEET:
+			return 0; /* DMA is safe */
+		}
+	}
+	return -EOPNOTSUPP; /* use PIO instead */
+}
+
+/**
+ *	mv_bmdma_setup - Set up BMDMA transaction
+ *	@qc: queued command to prepare DMA for.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+static void mv_bmdma_setup(struct ata_queued_cmd *qc)
+{
+	struct ata_port *ap = qc->ap;
+	void __iomem *port_mmio = mv_ap_base(ap);
+	struct mv_port_priv *pp = ap->private_data;
+
+	mv_fill_sg(qc);
+
+	/* clear all DMA cmd bits */
+	writel(0, port_mmio + BMDMA_CMD_OFS);
+
+	/* load PRD table addr. */
+	writel((pp->sg_tbl_dma[qc->tag] >> 16) >> 16,
+		port_mmio + BMDMA_PRD_HIGH_OFS);
+	writelfl(pp->sg_tbl_dma[qc->tag],
+		port_mmio + BMDMA_PRD_LOW_OFS);
+
+	/* issue r/w command */
+	ap->ops->sff_exec_command(ap, &qc->tf);
+}
+
+/**
+ *	mv_bmdma_start - Start a BMDMA transaction
+ *	@qc: queued command to start DMA on.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+static void mv_bmdma_start(struct ata_queued_cmd *qc)
+{
+	struct ata_port *ap = qc->ap;
+	void __iomem *port_mmio = mv_ap_base(ap);
+	unsigned int rw = (qc->tf.flags & ATA_TFLAG_WRITE);
+	u32 cmd = (rw ? 0 : ATA_DMA_WR) | ATA_DMA_START;
+
+	/* start host DMA transaction */
+	writelfl(cmd, port_mmio + BMDMA_CMD_OFS);
+}
+
+/**
+ *	mv_bmdma_stop - Stop BMDMA transfer
+ *	@qc: queued command to stop DMA on.
+ *
+ *	Clears the ATA_DMA_START flag in the bmdma control register
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+static void mv_bmdma_stop(struct ata_queued_cmd *qc)
+{
+	struct ata_port *ap = qc->ap;
+	void __iomem *port_mmio = mv_ap_base(ap);
+	u32 cmd;
+
+	/* clear start/stop bit */
+	cmd = readl(port_mmio + BMDMA_CMD_OFS);
+	cmd &= ~ATA_DMA_START;
+	writelfl(cmd, port_mmio + BMDMA_CMD_OFS);
+
+	/* one-PIO-cycle guaranteed wait, per spec, for HDMA1:0 transition */
+	ata_sff_dma_pause(ap);
+}
+
+/**
+ *	mv_bmdma_status - Read BMDMA status
+ *	@ap: port for which to retrieve DMA status.
+ *
+ *	Read and return equivalent of the sff BMDMA status register.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+static u8 mv_bmdma_status(struct ata_port *ap)
+{
+	void __iomem *port_mmio = mv_ap_base(ap);
+	u32 reg, status;
+
+	/*
+	 * Other bits are valid only if ATA_DMA_ACTIVE==0,
+	 * and the ATA_DMA_INTR bit doesn't exist.
+	 */
+	reg = readl(port_mmio + BMDMA_STATUS_OFS);
+	if (reg & ATA_DMA_ACTIVE)
+		status = ATA_DMA_ACTIVE;
+	else
+		status = (reg & ATA_DMA_ERR) | ATA_DMA_INTR;
+	return status;
+}
+
+/**
  *      mv_qc_prep - Host specific command preparation.
  *      @qc: queued command to prepare
  *

[PATCH 07/07] sata_mv: optimize use of mv_edma_cfg

[Mark Lord]

Try and avoid unnecessary reconfiguration of the EDMA config register
on every single non-EDMA I/O operation, by moving the call to
mv_edma_cfg() into mv_stop_edma().  It must then also be invoked
from mv_hardreset() and from mv_port_start().

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

--- old/drivers/ata/sata_mv.c	2009-01-30 18:33:04.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 18:34:05.000000000 -0500
@@ -979,6 +979,7 @@
 {
 	void __iomem *port_mmio = mv_ap_base(ap);
 	struct mv_port_priv *pp = ap->private_data;
+	int err = 0;
 
 	if (!(pp->pp_flags & MV_PP_FLAG_EDMA_EN))
 		return 0;
@@ -986,9 +987,10 @@
 	mv_wait_for_edma_empty_idle(ap);
 	if (mv_stop_edma_engine(port_mmio)) {
 		ata_port_printk(ap, KERN_ERR, "Unable to stop eDMA\n");
-		return -EIO;
+		err = -EIO;
 	}
-	return 0;
+	mv_edma_cfg(ap, 0, 0);
+	return err;
 }
 
 #ifdef ATA_DEBUG
@@ -1337,6 +1339,7 @@
 			pp->sg_tbl_dma[tag] = pp->sg_tbl_dma[0];
 		}
 	}
+	mv_edma_cfg(ap, 0, 0);
 	return 0;
 
 out_port_free_dma_mem:
@@ -1797,7 +1800,6 @@
 		 * shadow block, etc registers.
 		 */
 		mv_stop_edma(ap);
-		mv_edma_cfg(ap, 0, 0);
 		mv_clear_and_enable_port_irqs(ap, mv_ap_base(ap), port_irqs);
 		mv_pmp_select(ap, qc->dev->link->pmp);
 		return ata_sff_qc_issue(qc);
@@ -2996,6 +2998,7 @@
 				extra = HZ; /* only extend it once, max */
 		}
 	} while (sstatus != 0x0 && sstatus != 0x113 && sstatus != 0x123);
+	mv_edma_cfg(ap, 0, 0);
 
 	return rc;
 }

Re: [PATCH 06/07] sata_mv: introduce support for ATAPI devices

[Jeff Garzik]

Mark Lord wrote:
> Add ATAPI support to sata_mv, using sff DMA for GEN_II chipsets,
> and plain old PIO for GEN_IIE.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

applied 5-8

Re: [PATCH 05/07] sata_mv: mv_fill_sg fixes

[Grant Grundler]

On Fri, Jan 30, 2009 at 3:50 PM, Mark Lord <liml@rtr.ca> wrote:
> Fix mv_fill_sg() to zero out the reserved word (required for ATAPI),
> and to include a memory barrier.  This may also help with problems
> reported by Jens on the PPC platform.
>
> Signed-off-by: Mark Lord <mlord@pobox.com>
>
> --- old/drivers/ata/sata_mv.c   2009-01-30 18:31:03.000000000 -0500
> +++ linux/drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500
> @@ -1364,12 +1364,13 @@
>                        u32 offset = addr & 0xffff;
>                        u32 len = sg_len;
>
> -                       if ((offset + sg_len > 0x10000))
> +                       if ((offset + len > 0x10000))

Why the double parens?

>                                len = 0x10000 - offset;
>
>                        mv_sg->addr = cpu_to_le32(addr & 0xffffffff);
>                        mv_sg->addr_hi = cpu_to_le32((addr >> 16) >> 16);
>                        mv_sg->flags_size = cpu_to_le32(len & 0xffff);
> +                       mv_sg->reserved = 0;
>
>                        sg_len -= len;
>                        addr += len;
> @@ -1381,6 +1382,7 @@
>
>        if (likely(last_sg))
>                last_sg->flags_size |= cpu_to_le32(EPRD_FLAG_END_OF_TBL);
> +       mb(); /* ensure data structure is visible to the chipset */

It's not obvious to me what you are racing against here.
Normally the mb() is to prevent the above store from getting executed
*after* some MMIO read or write that would tell the chip to read the
flags_size field (or anything recently stored in that data structure).

I guess I'm asking for the comment to indicate which MMIO write it's
racing with.

thanks,
grant

> }
>
> static void mv_crqb_pack_cmd(__le16 *cmdw, u8 data, u8 addr, unsigned last)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 05/07] sata_mv: mv_fill_sg fixes

[Mark Lord]

Grant Grundler wrote:
> On Fri, Jan 30, 2009 at 3:50 PM, Mark Lord <liml@rtr.ca> wrote:
..
>>
>>        if (likely(last_sg))
>>                last_sg->flags_size |= cpu_to_le32(EPRD_FLAG_END_OF_TBL);
>> +       mb(); /* ensure data structure is visible to the chipset */
> 
> It's not obvious to me what you are racing against here.
> Normally the mb() is to prevent the above store from getting executed
> *after* some MMIO read or write that would tell the chip to read the
> flags_size field (or anything recently stored in that data structure).
> 
> I guess I'm asking for the comment to indicate which MMIO write it's
> racing with.
..

It's exactly the same as the generic routine in libata-sff,
which has a mb() in the same place for the same reason:
To ensure the PRD table is visible to the chipset
before we trigger the I/O operation.

Pretty standard.

[PATCH 05/07] sata_mv: mv_fill_sg fixes v2

[Mark Lord]

Grant Grundler wrote:
> On Fri, Jan 30, 2009 at 3:50 PM, Mark Lord <liml@rtr.ca> wrote:
>> Fix mv_fill_sg() to zero out the reserved word (required for ATAPI),
>> and to include a memory barrier.  This may also help with problems
>> reported by Jens on the PPC platform.
>>
>> Signed-off-by: Mark Lord <mlord@pobox.com>
>>
>> --- old/drivers/ata/sata_mv.c   2009-01-30 18:31:03.000000000 -0500
>> +++ linux/drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500
>> @@ -1364,12 +1364,13 @@
>>                        u32 offset = addr & 0xffff;
>>                        u32 len = sg_len;
>>
>> -                       if ((offset + sg_len > 0x10000))
>> +                       if ((offset + len > 0x10000))
> 
> Why the double parens?
..

No reason, no harm -- they were there already.

But if we're being pedantic, here it is again without them:

(REPOST, without the redundant parenthesis)

Fix mv_fill_sg() to zero out the reserved word (required for ATAPI),
and to include a memory barrier.  This may also help with problems
reported by Jens on the PPC platform.

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

--- old/drivers/ata/sata_mv.c	2009-01-30 18:31:03.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 18:32:08.000000000 -0500
@@ -1364,12 +1364,13 @@
 			u32 offset = addr & 0xffff;
 			u32 len = sg_len;
 
-			if ((offset + sg_len > 0x10000))
+			if (offset + len > 0x10000)
 				len = 0x10000 - offset;
 
 			mv_sg->addr = cpu_to_le32(addr & 0xffffffff);
 			mv_sg->addr_hi = cpu_to_le32((addr >> 16) >> 16);
 			mv_sg->flags_size = cpu_to_le32(len & 0xffff);
+			mv_sg->reserved = 0;
 
 			sg_len -= len;
 			addr += len;
@@ -1381,6 +1382,7 @@
 
 	if (likely(last_sg))
 		last_sg->flags_size |= cpu_to_le32(EPRD_FLAG_END_OF_TBL);
+	mb(); /* ensure data structure is visible to the chipset */
 }
 
 static void mv_crqb_pack_cmd(__le16 *cmdw, u8 data, u8 addr, unsigned last)

[PATCH 08/07] sata_mv: remove leftovers

[Mark Lord]

Remove redundant code left over from the earlier patch 04/07.

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

Gets rid of a harmless artifact from an earlier patch.
Posted as an addendum, rather than reworking the entire sequence.

--- old/drivers/ata/sata_mv.c	2009-01-30 18:38:17.000000000 -0500
+++ linux/drivers/ata/sata_mv.c	2009-01-30 21:37:56.000000000 -0500
@@ -1820,8 +1820,6 @@
 		else if (!(qc->flags & ATA_QCFLAG_ACTIVE))
 			qc = NULL;
 	}
-	if (qc && (qc->tf.flags & ATA_TFLAG_POLLING))
-		qc = NULL;
 	return qc;
 }
 

Re: [PATCH 04/07] sata_mv: update ata_qc_from_tag

[Grant Grundler]

On Fri, Jan 30, 2009 at 3:49 PM, Mark Lord <liml@rtr.ca> wrote:
> Update the logic in ata_qc_from_tag() to match that used
> in similar places elsewhere in libata.
>
> Signed-off-by: Mark Lord <mlord@pobox.com>
>
> --- old/drivers/ata/sata_mv.c   2009-01-30 18:23:22.000000000 -0500
> +++ linux/drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500
> @@ -1628,6 +1628,12 @@
>        if (pp->pp_flags & MV_PP_FLAG_NCQ_EN)
>                return NULL;
>        qc = ata_qc_from_tag(ap, ap->link.active_tag);
> +       if (qc) {
> +               if (qc->tf.flags & ATA_TFLAG_POLLING)
> +                       qc = NULL;
> +               else if (!(qc->flags & ATA_QCFLAG_ACTIVE))
> +                       qc = NULL;
> +       }
>        if (qc && (qc->tf.flags & ATA_TFLAG_POLLING))
>                qc = NULL;

Mark,
isn't the second test redudant now and can be removed?
(refering to the test immediately following the code you added).

grant

>        return qc;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 04/07] sata_mv: update ata_qc_from_tag

[Mark Lord]

Grant Grundler wrote:
> On Fri, Jan 30, 2009 at 3:49 PM, Mark Lord <liml@rtr.ca> wrote:
>> Update the logic in ata_qc_from_tag() to match that used
>> in similar places elsewhere in libata.
>>
>> Signed-off-by: Mark Lord <mlord@pobox.com>
>>
>> --- old/drivers/ata/sata_mv.c   2009-01-30 18:23:22.000000000 -0500
>> +++ linux/drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500
>> @@ -1628,6 +1628,12 @@
>>        if (pp->pp_flags & MV_PP_FLAG_NCQ_EN)
>>                return NULL;
>>        qc = ata_qc_from_tag(ap, ap->link.active_tag);
>> +       if (qc) {
>> +               if (qc->tf.flags & ATA_TFLAG_POLLING)
>> +                       qc = NULL;
>> +               else if (!(qc->flags & ATA_QCFLAG_ACTIVE))
>> +                       qc = NULL;
>> +       }
>>        if (qc && (qc->tf.flags & ATA_TFLAG_POLLING))
>>                qc = NULL;
> 
> Mark,
> isn't the second test redudant now and can be removed?
..

Yes, and it has been.  Keep reading.

-ml

Re: [PATCH 03/07] sata_mv: restructure mv_qc_issue

[Jeff Garzik]

Mark Lord wrote:
> Rearrange logic in mv_qc_issue() to handle protocols
> other than ATA_PROT_DMA, ATA_PROT_NCQ, and ATA_PROT_PIO.
> This is in preparation for later enabling ATAPI support.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

applied 3-4

Re: [PATCH 01/07] sata_mv: cleanup chipset GENeration FLAGS

[Jeff Garzik]

Mark Lord wrote:
> Clean up the chipset GENeration FLAGS, and rename them
> for consistency with other uses of GEN_XX within sata_mv.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

applied 1-2

Re: [PATCH 00/07] sata_mv: ATAPI patchset

[Mark Lord]

Mark Lord wrote:
> The following patchset adds ATAPI capability to sata_mv.
> These patches are all for #upstream.
> 
> 01_sata_mv_cleanup_flags.patch
> 02_sata_mv_edma_cfg.patch
> 03_sata_mv_qc_issue.patch
> 04_sata_mv_get_active_qc.patch
> 05_sata_mv_fill_sg.patch
> 06_sata_mv_atapi_dma.patch
> 07_sata_mv_optimize_edma_cfg_use.patch
..

As you can see, I decided to do a little more cleanup in the driver
prior to tacking on the actual ATAPI stuff.  :)

All of these are on top of the earlier submitted patch
which corrects the chipset IDs for the Highpoint RocketRAID 174x cards.

ATAPI DMA works for GEN_II chips, but not for GEN_IIE.
Marvell may eventually find a way to make it work for those, as well.

Patch 05 above could be a candidate for 2.6.29
as well as #upstream.

Cheers