Skip to contentSkip to conversation

sata_mv: ATAPI patchset

PATCH In review Mark Lord 3 participants 18 messages Jan 30, 2009 – Feb 3, 2009 libata

lore
Share & tools
lore

Mark Lord

author
#0 lore raw

cover sata_mv: ATAPI patchset

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

author
#1 lore raw

PATCH 1/7 sata_mv: cleanup chipset GENeration FLAGS

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

drivers/ata/sata_mv.c +0 -0
@@ -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,
author
#2 lore raw

PATCH 2/7 sata_mv: rearrange mv_start_dma() and friends

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

drivers/ata/sata_mv.c +0 -0
@@ -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;
author
#3 lore raw

PATCH 3/7 sata_mv: restructure mv_qc_issue

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.

drivers/ata/sata_mv.c +0 -0
@@ -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)

author
#4 lore raw

PATCH 4/7 sata_mv: update ata_qc_from_tag

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

drivers/ata/sata_mv.c +0 -0
@@ -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;
author
#5 lore raw

PATCH 5/7 sata_mv: mv_fill_sg fixes

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.

drivers/ata/sata_mv.c +0 -0
@@ -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)

author
#6 lore raw

PATCH 6/7 sata_mv: introduce support for ATAPI devices

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

drivers/ata/sata_mv.c +0 -2
@@ -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
*

author
#7 lore raw

PATCH 7/7 sata_mv: optimize use of mv_edma_cfg

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().

drivers/ata/sata_mv.c +0 -0
@@ -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;
}
#9 lore raw

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

In reply to Mark Lord
Quoted 9 lines · 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.

drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 line 1367

Quoted 5 lines
drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:31:03.000000000 -0500 +1 -1
@@ -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?

Quoted 10 lines
drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:31:03.000000000 -0500 +1 -0
                               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;

drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 line 1385

Quoted 4 lines
drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:31:03.000000000 -0500 +1 -0
@@ -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

Quoted 8 lines
drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:31:03.000000000 -0500 +0 -1
 }
 
 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#10 lore raw

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

Quoted 1 line · Grant Grundler

On Fri, Jan 30, 2009 at 3:50 PM, Mark Lord <[email protected]> wrote:

..

Quoted 5 lines
Quoted 4 lines
       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 */

Quoted 7 lines

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.

author
#11 lore raw

PATCH 5/7 sata_mv: mv_fill_sg fixes v2

In reply to Grant Grundler
Quoted 13 lines · Grant Grundler
Quoted 12 lines · 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.

drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:31:03.000000000 -0500 +0 -0
@@ -1364,12 +1364,13 @@
                       u32 offset = addr & 0xffff;
                       u32 len = sg_len;
 
Quoted 4 lines

drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 line 1367

Quoted 2 lines
drivers/ata/sata_mv.c 2009-01-30 18:32:08.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:31:03.000000000 -0500 +1 -1
-                       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.

drivers/ata/sata_mv.c +0 -0
@@ -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)

author
#12 lore raw

[PATCH 08/07] sata_mv: remove leftovers

In reply to Mark Lord

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

---

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

drivers/ata/sata_mv.c +0 -0
@@ -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;
}
#13 lore raw

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

In reply to Mark Lord
Quoted 9 lines · Mark Lord

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

drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:23:22.000000000 -0500 +0 -0
@@ -1628,6 +1628,12 @@
       if (pp->pp_flags & MV_PP_FLAG_NCQ_EN)

drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500 line 1638

Quoted 10 lines
drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:23:22.000000000 -0500 +6 -0
               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

Quoted 6 lines
drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:23:22.000000000 -0500 +0 -1
       return qc;
--

To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#14 lore raw

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

Quoted 15 lines · Grant Grundler
Quoted 14 lines · Mark Lord

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

drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:23:22.000000000 -0500 +3 -0
@@ -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;
Quoted 8 lines

drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500 line 1638

Quoted 5 lines
drivers/ata/sata_mv.c 2009-01-30 18:26:55.000000000 -0500 renamed from drivers/ata/sata_mv.c 2009-01-30 18:23:22.000000000 -0500 +3 -0
+               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

#15 lore raw

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

In reply to Mark Lord
Quoted 5 lines · 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.

applied 3-4

author
#17 lore raw
In reply to Mark Lord
Quoted 10 lines · 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

..

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