* [PATCH 02/13] sata_mv ncq Mask transient IRQs
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
@ 2008-01-26 23:30 ` Mark Lord
2008-01-26 23:31 ` [PATCH 03/13] sata_mv ncq Rename base to port mmio Mark Lord
` (10 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:30 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
The chips can handle many transient errors internally without a software IRQ.
We now mask/ignore those interrupts here. This is necessary for NCQ, later on.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-24 11:11:26.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-24 11:17:42.000000000 -0500
@@ -170,7 +170,7 @@
PCIE_IRQ_CAUSE_OFS = 0x1900,
PCIE_IRQ_MASK_OFS = 0x1910,
- PCIE_UNMASK_ALL_IRQS = 0x70a, /* assorted bits */
+ PCIE_UNMASK_ALL_IRQS = 0x40a, /* assorted bits */
HC_MAIN_IRQ_CAUSE_OFS = 0x1d60,
HC_MAIN_IRQ_MASK_OFS = 0x1d64,
@@ -241,17 +241,36 @@
EDMA_ERR_BIST_ASYNC = (1 << 8), /* BIST FIS or Async Notify */
EDMA_ERR_TRANS_IRQ_7 = (1 << 8), /* Gen IIE transprt layer irq */
EDMA_ERR_CRQB_PAR = (1 << 9), /* CRQB parity error */
- EDMA_ERR_CRPB_PAR = (1 << 10), /* CRPB parity error */
- EDMA_ERR_INTRL_PAR = (1 << 11), /* internal parity error */
- EDMA_ERR_IORDY = (1 << 12), /* IORdy timeout */
- EDMA_ERR_LNK_CTRL_RX = (0xf << 13), /* link ctrl rx error */
- EDMA_ERR_LNK_CTRL_RX_2 = (1 << 15),
- EDMA_ERR_LNK_DATA_RX = (0xf << 17), /* link data rx error */
- EDMA_ERR_LNK_CTRL_TX = (0x1f << 21), /* link ctrl tx error */
- EDMA_ERR_LNK_DATA_TX = (0x1f << 26), /* link data tx error */
- EDMA_ERR_TRANS_PROTO = (1 << 31), /* transport protocol error */
- EDMA_ERR_OVERRUN_5 = (1 << 5),
- EDMA_ERR_UNDERRUN_5 = (1 << 6),
+ EDMA_ERR_CRPB_PAR = (1 << 10), /* CRPB parity error */
+ EDMA_ERR_INTRL_PAR = (1 << 11), /* internal parity error */
+ EDMA_ERR_IORDY = (1 << 12), /* IORdy timeout */
+
+ EDMA_ERR_LNK_CTRL_RX = (0xf << 13), /* link ctrl rx error */
+ EDMA_ERR_LNK_CTRL_RX_0 = (1 << 13), /* transient: CRC err */
+ EDMA_ERR_LNK_CTRL_RX_1 = (1 << 14), /* transient: FIFO err */
+ EDMA_ERR_LNK_CTRL_RX_2 = (1 << 15), /* fatal: caught SYNC */
+ EDMA_ERR_LNK_CTRL_RX_3 = (1 << 16), /* transient: FIS rx err */
+
+ EDMA_ERR_LNK_DATA_RX = (0xf << 17), /* link data rx error */
+
+ EDMA_ERR_LNK_CTRL_TX = (0x1f << 21), /* link ctrl tx error */
+ EDMA_ERR_LNK_CTRL_TX_0 = (1 << 21), /* transient: CRC err */
+ EDMA_ERR_LNK_CTRL_TX_1 = (1 << 22), /* transient: FIFO err */
+ EDMA_ERR_LNK_CTRL_TX_2 = (1 << 23), /* transient: caught SYNC */
+ EDMA_ERR_LNK_CTRL_TX_3 = (1 << 24), /* transient: caught DMAT */
+ EDMA_ERR_LNK_CTRL_TX_4 = (1 << 25), /* transient: FIS collision */
+
+ EDMA_ERR_LNK_DATA_TX = (0x1f << 26), /* link data tx error */
+
+ EDMA_ERR_TRANS_PROTO = (1 << 31), /* transport protocol error */
+ EDMA_ERR_OVERRUN_5 = (1 << 5),
+ EDMA_ERR_UNDERRUN_5 = (1 << 6),
+
+ EDMA_ERR_IRQ_TRANSIENT = EDMA_ERR_LNK_CTRL_RX_0 |
+ EDMA_ERR_LNK_CTRL_RX_1 |
+ EDMA_ERR_LNK_CTRL_RX_3 |
+ EDMA_ERR_LNK_CTRL_TX,
+
EDMA_EH_FREEZE = EDMA_ERR_D_PAR |
EDMA_ERR_PRD_PAR |
EDMA_ERR_DEV_DCON |
@@ -1716,18 +1735,19 @@
struct ata_host *host = dev_instance;
unsigned int hc, handled = 0, n_hcs;
void __iomem *mmio = host->iomap[MV_PRIMARY_BAR];
- u32 irq_stat;
+ u32 irq_stat, irq_mask;
+ spin_lock(&host->lock);
irq_stat = readl(mmio + HC_MAIN_IRQ_CAUSE_OFS);
+ irq_mask = readl(mmio + HC_MAIN_IRQ_MASK_OFS);
/* check the cases where we either have nothing pending or have read
* a bogus register value which can indicate HW removal or PCI fault
*/
- if (!irq_stat || (0xffffffffU == irq_stat))
- return IRQ_NONE;
+ if (!(irq_stat & irq_mask) || (0xffffffffU == irq_stat))
+ goto out_unlock;
n_hcs = mv_get_hc_count(host->ports[0]->flags);
- spin_lock(&host->lock);
if (unlikely(irq_stat & PCI_ERR)) {
mv_pci_error(host, mmio);
@@ -2428,8 +2448,8 @@
writelfl(readl(port_mmio + serr_ofs), port_mmio + serr_ofs);
writelfl(0, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
- /* unmask all EDMA error interrupts */
- writelfl(~0, port_mmio + EDMA_ERR_IRQ_MASK_OFS);
+ /* unmask all non-transient EDMA error interrupts */
+ writelfl(~EDMA_ERR_IRQ_TRANSIENT, port_mmio + EDMA_ERR_IRQ_MASK_OFS);
VPRINTK("EDMA cfg=0x%08x EDMA IRQ err cause/mask=0x%08x/0x%08x\n",
readl(port_mmio + EDMA_CFG_OFS),
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 03/13] sata_mv ncq Rename base to port mmio
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
2008-01-26 23:30 ` [PATCH 02/13] sata_mv ncq Mask transient IRQs Mark Lord
@ 2008-01-26 23:31 ` Mark Lord
2008-01-26 23:31 ` [PATCH 04/13] sata_mv ncq Fix EDMA configuration Mark Lord
` (9 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:31 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
Use naming consistent with elsewhere in this driver.
This will keep things less confusing when we later add "hc_mmio" in this function.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-24 11:23:05.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-24 11:26:33.000000000 -0500
@@ -834,19 +834,19 @@
* LOCKING:
* Inherited from caller.
*/
-static void mv_start_dma(void __iomem *base, struct mv_host_priv *hpriv,
+static void mv_start_dma(void __iomem *port_mmio, struct mv_host_priv *hpriv,
struct mv_port_priv *pp)
{
if (!(pp->pp_flags & MV_PP_FLAG_EDMA_EN)) {
/* clear EDMA event indicators, if any */
- writelfl(0, base + EDMA_ERR_IRQ_CAUSE_OFS);
+ writelfl(0, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
- mv_set_edma_ptrs(base, hpriv, pp);
+ mv_set_edma_ptrs(port_mmio, hpriv, pp);
- writelfl(EDMA_EN, base + EDMA_CMD_OFS);
+ writelfl(EDMA_EN, port_mmio + EDMA_CMD_OFS);
pp->pp_flags |= MV_PP_FLAG_EDMA_EN;
}
- WARN_ON(!(EDMA_EN & readl(base + EDMA_CMD_OFS)));
+ WARN_ON(!(EDMA_EN & readl(port_mmio + EDMA_CMD_OFS)));
}
/**
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 04/13] sata_mv ncq Fix EDMA configuration
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
2008-01-26 23:30 ` [PATCH 02/13] sata_mv ncq Mask transient IRQs Mark Lord
2008-01-26 23:31 ` [PATCH 03/13] sata_mv ncq Rename base to port mmio Mark Lord
@ 2008-01-26 23:31 ` Mark Lord
2008-01-26 23:31 ` [PATCH 05/13] sata_mv ncq Add want ncq parameter for " Mark Lord
` (8 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:31 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
Simplify and fix EDMA configuration setup to match Marvell specificiations.
The chip documentation gives a specific (re)init sequence, which we now follow.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-24 12:06:25.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-24 12:12:37.000000000 -0500
@@ -210,6 +210,7 @@
/* SATA registers */
SATA_STATUS_OFS = 0x300, /* ctrl, err regs follow status */
SATA_ACTIVE_OFS = 0x350,
+ SATA_FIS_IRQ_CAUSE_OFS = 0x364,
PHY_MODE3 = 0x310,
PHY_MODE4 = 0x314,
PHY_MODE2 = 0x330,
@@ -222,11 +223,11 @@
/* Port registers */
EDMA_CFG_OFS = 0,
- EDMA_CFG_Q_DEPTH = 0, /* queueing disabled */
- EDMA_CFG_NCQ = (1 << 5),
- EDMA_CFG_NCQ_GO_ON_ERR = (1 << 14), /* continue on error */
- EDMA_CFG_RD_BRST_EXT = (1 << 11), /* read burst 512B */
- EDMA_CFG_WR_BUFF_LEN = (1 << 13), /* write buffer 512B */
+ EDMA_CFG_Q_DEPTH = 0x1f, /* max device queue depth */
+ EDMA_CFG_NCQ = (1 << 5), /* for R/W FPDMA queued */
+ EDMA_CFG_NCQ_GO_ON_ERR = (1 << 14), /* continue on error */
+ EDMA_CFG_RD_BRST_EXT = (1 << 11), /* read burst 512B */
+ EDMA_CFG_WR_BUFF_LEN = (1 << 13), /* write buffer 512B */
EDMA_ERR_IRQ_CAUSE_OFS = 0x8,
EDMA_ERR_IRQ_MASK_OFS = 0xc,
@@ -470,6 +471,8 @@
static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio);
static void mv_channel_reset(struct mv_host_priv *hpriv, void __iomem *mmio,
unsigned int port_no);
+static void mv_edma_cfg(struct ata_port *ap, struct mv_host_priv *hpriv,
+ void __iomem *port_mmio);
static struct scsi_host_template mv5_sht = {
.module = THIS_MODULE,
@@ -834,13 +837,33 @@
* LOCKING:
* Inherited from caller.
*/
-static void mv_start_dma(void __iomem *port_mmio, struct mv_host_priv *hpriv,
+static void mv_start_dma(struct ata_port *ap, void __iomem *port_mmio,
struct mv_port_priv *pp)
{
if (!(pp->pp_flags & MV_PP_FLAG_EDMA_EN)) {
+ struct mv_host_priv *hpriv = ap->host->private_data;
+ int hard_port = mv_hardport_from_port(ap->port_no);
+ void __iomem *hc_mmio = mv_hc_base_from_port(
+ ap->host->iomap[MV_PRIMARY_BAR], hard_port);
+ u32 hc_irq_cause, ipending;
+
/* clear EDMA event indicators, if any */
writelfl(0, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
+ /* clear EDMA interrupt indicator, if any */
+ hc_irq_cause = readl(hc_mmio + HC_IRQ_CAUSE_OFS);
+ ipending = (DEV_IRQ << hard_port) |
+ (CRPB_DMA_DONE << hard_port);
+ if (hc_irq_cause & ipending) {
+ writelfl(hc_irq_cause & ~ipending,
+ hc_mmio + HC_IRQ_CAUSE_OFS);
+ }
+
+ mv_edma_cfg(ap, hpriv, port_mmio);
+
+ /* clear FIS IRQ Cause */
+ writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+
mv_set_edma_ptrs(port_mmio, hpriv, pp);
writelfl(EDMA_EN, port_mmio + EDMA_CMD_OFS);
@@ -1025,30 +1048,22 @@
static void mv_edma_cfg(struct ata_port *ap, struct mv_host_priv *hpriv,
void __iomem *port_mmio)
{
- u32 cfg = readl(port_mmio + EDMA_CFG_OFS);
+ u32 cfg;
/* set up non-NCQ EDMA configuration */
- cfg &= ~(1 << 9); /* disable eQue */
+ cfg = EDMA_CFG_Q_DEPTH; /* always 0x1f for *all* chips */
- if (IS_GEN_I(hpriv)) {
- cfg &= ~0x1f; /* clear queue depth */
+ if (IS_GEN_I(hpriv))
cfg |= (1 << 8); /* enab config burst size mask */
- }
- else if (IS_GEN_II(hpriv)) {
- cfg &= ~0x1f; /* clear queue depth */
+ else if (IS_GEN_II(hpriv))
cfg |= EDMA_CFG_RD_BRST_EXT | EDMA_CFG_WR_BUFF_LEN;
- cfg &= ~(EDMA_CFG_NCQ | EDMA_CFG_NCQ_GO_ON_ERR); /* clear NCQ */
- }
else if (IS_GEN_IIE(hpriv)) {
cfg |= (1 << 23); /* do not mask PM field in rx'd FIS */
cfg |= (1 << 22); /* enab 4-entry host queue cache */
- cfg &= ~(1 << 19); /* dis 128-entry queue (for now?) */
cfg |= (1 << 18); /* enab early completion */
cfg |= (1 << 17); /* enab cut-through (dis stor&forwrd) */
- cfg &= ~(1 << 16); /* dis FIS-based switching (for now) */
- cfg &= ~(EDMA_CFG_NCQ); /* clear NCQ */
}
writelfl(cfg, port_mmio + EDMA_CFG_OFS);
@@ -1370,7 +1385,6 @@
struct ata_port *ap = qc->ap;
void __iomem *port_mmio = mv_ap_base(ap);
struct mv_port_priv *pp = ap->private_data;
- struct mv_host_priv *hpriv = ap->host->private_data;
u32 in_index;
if (qc->tf.protocol != ATA_PROT_DMA) {
@@ -1382,7 +1396,7 @@
return ata_qc_issue_prot(qc);
}
- mv_start_dma(port_mmio, hpriv, pp);
+ mv_start_dma(ap, port_mmio, pp);
in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 05/13] sata_mv ncq Add want ncq parameter for EDMA configuration
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (2 preceding siblings ...)
2008-01-26 23:31 ` [PATCH 04/13] sata_mv ncq Fix EDMA configuration Mark Lord
@ 2008-01-26 23:31 ` Mark Lord
2008-01-26 23:31 ` [PATCH 06/13] sata_mv ncq Use hqtag instead of ioid Mark Lord
` (7 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:31 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
An extra EDMA config bit is required for NCQ operation.
So set/clear it as needed, and cache current setting in port_priv.
For now though, it will always be "off" (0).
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-24 12:12:37.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-24 12:07:16.000000000 -0500
@@ -331,6 +331,7 @@
/* Port private flags (pp_flags) */
MV_PP_FLAG_EDMA_EN = (1 << 0), /* is EDMA engine enabled? */
+ MV_PP_FLAG_NCQ_EN = (1 << 1), /* is EDMA set up for NCQ? */
MV_PP_FLAG_HAD_A_RESET = (1 << 2), /* 1st hard reset complete? */
};
@@ -471,8 +472,9 @@
static void mv_reset_pci_bus(struct pci_dev *pdev, void __iomem *mmio);
static void mv_channel_reset(struct mv_host_priv *hpriv, void __iomem *mmio,
unsigned int port_no);
-static void mv_edma_cfg(struct ata_port *ap, struct mv_host_priv *hpriv,
- void __iomem *port_mmio);
+static void mv_edma_cfg(struct mv_port_priv *pp, struct mv_host_priv *hpriv,
+ void __iomem *port_mmio, int want_ncq);
+static int __mv_stop_dma(struct ata_port *ap);
static struct scsi_host_template mv5_sht = {
.module = THIS_MODULE,
@@ -838,8 +840,15 @@
* Inherited from caller.
*/
static void mv_start_dma(struct ata_port *ap, void __iomem *port_mmio,
- struct mv_port_priv *pp)
+ struct mv_port_priv *pp, u8 protocol)
{
+ int want_ncq = (protocol == ATA_PROT_NCQ);
+
+ if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
+ int using_ncq = ((pp->pp_flags & MV_PP_FLAG_NCQ_EN) != 0);
+ if (want_ncq != using_ncq)
+ __mv_stop_dma(ap);
+ }
if (!(pp->pp_flags & MV_PP_FLAG_EDMA_EN)) {
struct mv_host_priv *hpriv = ap->host->private_data;
int hard_port = mv_hardport_from_port(ap->port_no);
@@ -859,7 +868,7 @@
hc_mmio + HC_IRQ_CAUSE_OFS);
}
- mv_edma_cfg(ap, hpriv, port_mmio);
+ mv_edma_cfg(pp, hpriv, port_mmio, want_ncq);
/* clear FIS IRQ Cause */
writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
@@ -1045,8 +1054,8 @@
return -EINVAL;
}
-static void mv_edma_cfg(struct ata_port *ap, struct mv_host_priv *hpriv,
- void __iomem *port_mmio)
+static void mv_edma_cfg(struct mv_port_priv *pp, struct mv_host_priv *hpriv,
+ void __iomem *port_mmio, int want_ncq)
{
u32 cfg;
@@ -1066,6 +1075,12 @@
cfg |= (1 << 17); /* enab cut-through (dis stor&forwrd) */
}
+ 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);
}
@@ -1128,7 +1143,7 @@
spin_lock_irqsave(&ap->host->lock, flags);
- mv_edma_cfg(ap, hpriv, port_mmio);
+ mv_edma_cfg(pp, hpriv, port_mmio, 0);
mv_set_edma_ptrs(port_mmio, hpriv, pp);
@@ -1396,7 +1411,7 @@
return ata_qc_issue_prot(qc);
}
- mv_start_dma(ap, port_mmio, pp);
+ mv_start_dma(ap, port_mmio, pp, qc->tf.protocol);
in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 06/13] sata_mv ncq Use hqtag instead of ioid
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (3 preceding siblings ...)
2008-01-26 23:31 ` [PATCH 05/13] sata_mv ncq Add want ncq parameter for " Mark Lord
@ 2008-01-26 23:31 ` Mark Lord
2008-01-26 23:32 ` [PATCH 07/13] sata_mv ncq Ignore response status LSB on NCQ Mark Lord
` (6 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:31 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
Simplify tag handling by using the cid/hqtag field instead of ioid,
as recommended by Marvell.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-26 10:46:58.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-26 10:47:35.000000000 -0500
@@ -1252,7 +1252,6 @@
flags |= CRQB_FLAG_READ;
WARN_ON(MV_MAX_Q_DEPTH <= qc->tag);
flags |= qc->tag << CRQB_TAG_SHIFT;
- flags |= qc->tag << CRQB_IOID_SHIFT; /* 50xx appears to ignore this*/
/* get current queue index from software */
in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
@@ -1345,8 +1344,7 @@
WARN_ON(MV_MAX_Q_DEPTH <= qc->tag);
flags |= qc->tag << CRQB_TAG_SHIFT;
- flags |= qc->tag << CRQB_IOID_SHIFT; /* "I/O Id" is -really-
- what we use as our tag */
+ flags |= qc->tag << CRQB_HOSTQ_SHIFT;
/* get current queue index from software */
in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
@@ -1587,13 +1585,8 @@
* support for queueing. this works transparently for
* queued and non-queued modes.
*/
- else if (IS_GEN_II(hpriv))
- tag = (le16_to_cpu(pp->crpb[out_index].id)
- >> CRPB_IOID_SHIFT_6) & 0x3f;
-
- else /* IS_GEN_IIE */
- tag = (le16_to_cpu(pp->crpb[out_index].id)
- >> CRPB_IOID_SHIFT_7) & 0x3f;
+ else
+ tag = le16_to_cpu(pp->crpb[out_index].id) & 0x1f;
qc = ata_qc_from_tag(ap, tag);
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 07/13] sata_mv ncq Ignore response status LSB on NCQ
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (4 preceding siblings ...)
2008-01-26 23:31 ` [PATCH 06/13] sata_mv ncq Use hqtag instead of ioid Mark Lord
@ 2008-01-26 23:32 ` Mark Lord
2008-01-26 23:32 ` [PATCH 08/13] sata_mv ncq Restrict max sectors to 8-bits on GenII NCQ Mark Lord
` (5 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:32 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
The lower 8 bits of response status are not valid for NCQ.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-24 12:35:14.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-24 12:35:43.000000000 -0500
@@ -1587,13 +1587,12 @@
qc = ata_qc_from_tag(ap, tag);
- /* lower 8 bits of status are EDMA_ERR_IRQ_CAUSE_OFS
- * bits (WARNING: might not necessarily be associated
- * with this command), which -should- be clear
- * if all is well
+ /* For non-NCQ mode, the lower 8 bits of status
+ * are from EDMA_ERR_IRQ_CAUSE_OFS,
+ * which should be zero if all went well.
*/
status = le16_to_cpu(pp->crpb[out_index].flags);
- if (unlikely(status & 0xff)) {
+ if ((status & 0xff) && !(pp->pp_flags & MV_PP_FLAG_NCQ_EN)) {
mv_err_intr(ap, qc);
return;
}
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 08/13] sata_mv ncq Restrict max sectors to 8-bits on GenII NCQ
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (5 preceding siblings ...)
2008-01-26 23:32 ` [PATCH 07/13] sata_mv ncq Ignore response status LSB on NCQ Mark Lord
@ 2008-01-26 23:32 ` Mark Lord
2008-01-26 23:32 ` [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables Mark Lord
` (4 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:32 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
The GenII chips have only 8-bits for the sector_count field when performing NCQ.
Add a dev_config method to restrict this when necessary, taking care not to
override any other restriction already in place (likely none, but someday.. ?).
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-24 12:35:43.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-24 12:40:10.000000000 -0500
@@ -446,6 +446,7 @@
static void mv_post_int_cmd(struct ata_queued_cmd *qc);
static void mv_eh_freeze(struct ata_port *ap);
static void mv_eh_thaw(struct ata_port *ap);
+static void mv6_dev_config(struct ata_device *dev);
static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
@@ -538,6 +539,7 @@
};
static const struct ata_port_operations mv6_ops = {
+ .dev_config = mv6_dev_config,
.tf_load = ata_tf_load,
.tf_read = ata_tf_read,
.check_status = ata_check_status,
@@ -1051,6 +1053,17 @@
return -EINVAL;
}
+static void mv6_dev_config(struct ata_device *adev)
+{
+ /*
+ * We don't have hob_nsect when doing NCQ commands on Gen-II.
+ * See mv_qc_prep() for more info.
+ */
+ if (adev->flags & ATA_DFLAG_NCQ)
+ if (adev->max_sectors > ATA_MAX_SECTORS)
+ adev->max_sectors = ATA_MAX_SECTORS;
+}
+
static void mv_edma_cfg(struct mv_port_priv *pp, struct mv_host_priv *hpriv,
void __iomem *port_mmio, int want_ncq)
{
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (6 preceding siblings ...)
2008-01-26 23:32 ` [PATCH 08/13] sata_mv ncq Restrict max sectors to 8-bits on GenII NCQ Mark Lord
@ 2008-01-26 23:32 ` Mark Lord
2008-01-29 17:10 ` Jeff Garzik
2008-01-26 23:32 ` [PATCH 10/13] sata_mv ncq Introduce per-tag SG tables Mark Lord
` (3 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:32 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
Create host-owned DMA memory pools, for use in allocating/freeing per-port
command/response queues and SG tables. This gives us a way to guarantee we
meet the hardware address alignment requirements, and also reduces memory that
might otherwise be wasted on alignment gaps.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-26 12:50:54.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-26 13:01:56.000000000 -0500
@@ -107,14 +107,12 @@
/* CRQB needs alignment on a 1KB boundary. Size == 1KB
* CRPB needs alignment on a 256B boundary. Size == 256B
- * SG count of 176 leads to MV_PORT_PRIV_DMA_SZ == 4KB
* ePRD (SG) entries need alignment on a 16B boundary. Size == 16B
*/
MV_CRQB_Q_SZ = (32 * MV_MAX_Q_DEPTH),
MV_CRPB_Q_SZ = (8 * MV_MAX_Q_DEPTH),
- MV_MAX_SG_CT = 176,
+ MV_MAX_SG_CT = 256,
MV_SG_TBL_SZ = (16 * MV_MAX_SG_CT),
- MV_PORT_PRIV_DMA_SZ = (MV_CRQB_Q_SZ + MV_CRPB_Q_SZ + MV_SG_TBL_SZ),
MV_PORTS_PER_HC = 4,
/* == (port / MV_PORTS_PER_HC) to determine HC from 0-7 port */
@@ -421,6 +419,14 @@
u32 irq_cause_ofs;
u32 irq_mask_ofs;
u32 unmask_all_irqs;
+ /*
+ * These consistent DMA memory pools give us guaranteed
+ * alignment for hardware-accessed data structures,
+ * and less memory waste in accomplishing the alignment.
+ */
+ struct dma_pool *crqb_pool;
+ struct dma_pool *crpb_pool;
+ struct dma_pool *sg_tbl_pool;
};
struct mv_hw_ops {
@@ -1097,6 +1103,25 @@
writelfl(cfg, port_mmio + EDMA_CFG_OFS);
}
+static void mv_port_free_dma_mem(struct ata_port *ap)
+{
+ struct mv_host_priv *hpriv = ap->host->private_data;
+ struct mv_port_priv *pp = ap->private_data;
+
+ if (pp->crqb) {
+ dma_pool_free(hpriv->crqb_pool, pp->crqb, pp->crqb_dma);
+ pp->crqb = NULL;
+ }
+ if (pp->crpb) {
+ dma_pool_free(hpriv->crpb_pool, pp->crpb, pp->crpb_dma);
+ pp->crpb = NULL;
+ }
+ if (pp->sg_tbl) {
+ dma_pool_free(hpriv->sg_tbl_pool, pp->sg_tbl, pp->sg_tbl_dma);
+ pp->sg_tbl = NULL;
+ }
+}
+
/**
* mv_port_start - Port specific init/start routine.
* @ap: ATA channel to manipulate
@@ -1113,51 +1138,36 @@
struct mv_host_priv *hpriv = ap->host->private_data;
struct mv_port_priv *pp;
void __iomem *port_mmio = mv_ap_base(ap);
- void *mem;
- dma_addr_t mem_dma;
unsigned long flags;
int rc;
pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
if (!pp)
return -ENOMEM;
-
- mem = dmam_alloc_coherent(dev, MV_PORT_PRIV_DMA_SZ, &mem_dma,
- GFP_KERNEL);
- if (!mem)
- return -ENOMEM;
- memset(mem, 0, MV_PORT_PRIV_DMA_SZ);
+ ap->private_data = pp;
rc = ata_pad_alloc(ap, dev);
if (rc)
return rc;
- /* First item in chunk of DMA memory:
- * 32-slot command request table (CRQB), 32 bytes each in size
- */
- pp->crqb = mem;
- pp->crqb_dma = mem_dma;
- mem += MV_CRQB_Q_SZ;
- mem_dma += MV_CRQB_Q_SZ;
-
- /* Second item:
- * 32-slot command response table (CRPB), 8 bytes each in size
- */
- pp->crpb = mem;
- pp->crpb_dma = mem_dma;
- mem += MV_CRPB_Q_SZ;
- mem_dma += MV_CRPB_Q_SZ;
+ pp->crqb = dma_pool_alloc(hpriv->crqb_pool, GFP_KERNEL, &pp->crqb_dma);
+ if (!pp->crqb)
+ return -ENOMEM;
+ memset(pp->crqb, 0, MV_CRQB_Q_SZ);
- /* Third item:
- * Table of scatter-gather descriptors (ePRD), 16 bytes each
- */
- pp->sg_tbl = mem;
- pp->sg_tbl_dma = mem_dma;
+ pp->crpb = dma_pool_alloc(hpriv->crpb_pool, GFP_KERNEL, &pp->crpb_dma);
+ if (!pp->crpb)
+ goto out_port_free_dma_mem;
+ memset(pp->crpb, 0, MV_CRPB_Q_SZ);
+
+ pp->sg_tbl = dma_pool_alloc(hpriv->sg_tbl_pool, GFP_KERNEL,
+ &pp->sg_tbl_dma);
+ if (!pp->sg_tbl)
+ goto out_port_free_dma_mem;
spin_lock_irqsave(&ap->host->lock, flags);
mv_edma_cfg(pp, hpriv, port_mmio, 0);
-
mv_set_edma_ptrs(port_mmio, hpriv, pp);
spin_unlock_irqrestore(&ap->host->lock, flags);
@@ -1166,8 +1176,11 @@
* we'll be unable to send non-data, PIO, etc due to restricted access
* to shadow regs.
*/
- ap->private_data = pp;
return 0;
+
+out_port_free_dma_mem:
+ mv_port_free_dma_mem(ap);
+ return -ENOMEM;
}
/**
@@ -1182,6 +1195,7 @@
static void mv_port_stop(struct ata_port *ap)
{
mv_stop_dma(ap);
+ mv_port_free_dma_mem(ap);
}
/**
@@ -2765,6 +2779,26 @@
scc_s, (MV_HP_FLAG_MSI & hpriv->hp_flags) ? "MSI" : "INTx");
}
+static int mv_create_dma_pools(struct mv_host_priv *hpriv, struct device *dev)
+{
+ hpriv->crqb_pool = dmam_pool_create("crqb_q", dev, MV_CRQB_Q_SZ,
+ MV_CRQB_Q_SZ, 0);
+ if (!hpriv->crqb_pool)
+ return -ENOMEM;
+
+ hpriv->crpb_pool = dmam_pool_create("crpb_q", dev, MV_CRPB_Q_SZ,
+ MV_CRPB_Q_SZ, 0);
+ if (!hpriv->crpb_pool)
+ return -ENOMEM;
+
+ hpriv->sg_tbl_pool = dmam_pool_create("sg_tbl", dev, MV_SG_TBL_SZ,
+ MV_SG_TBL_SZ, 0);
+ if (!hpriv->sg_tbl_pool)
+ return -ENOMEM;
+
+ return 0;
+}
+
/**
* mv_init_one - handle a positive probe of a Marvell host
* @pdev: PCI device found
@@ -2810,6 +2844,10 @@
if (rc)
return rc;
+ rc = mv_create_dma_pools(hpriv, &pdev->dev);
+ if (rc)
+ return rc;
+
/* initialize adapter */
rc = mv_init_host(host, board_idx);
if (rc)
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-26 23:32 ` [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables Mark Lord
@ 2008-01-29 17:10 ` Jeff Garzik
2008-01-29 18:24 ` Mark Lord
0 siblings, 1 reply; 26+ messages in thread
From: Jeff Garzik @ 2008-01-29 17:10 UTC (permalink / raw)
To: Mark Lord; +Cc: IDE/ATA development list
Mark Lord wrote:
> Create host-owned DMA memory pools, for use in allocating/freeing per-port
> command/response queues and SG tables. This gives us a way to guarantee we
> meet the hardware address alignment requirements, and also reduces
> memory that
> might otherwise be wasted on alignment gaps.
>
> Signed-off-by: Mark Lord <mlord@pobox.com>
ACK patches 1-13
applied patches 1-9 to #upstream. patch #10 failed, with git-am
reporting it as a corrupt patch.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-29 17:10 ` Jeff Garzik
@ 2008-01-29 18:24 ` Mark Lord
2008-01-30 9:54 ` Jeff Garzik
0 siblings, 1 reply; 26+ messages in thread
From: Mark Lord @ 2008-01-29 18:24 UTC (permalink / raw)
To: Jeff Garzik; +Cc: IDE/ATA development list
Jeff Garzik wrote:
> Mark Lord wrote:
>> Create host-owned DMA memory pools, for use in allocating/freeing
>> per-port
>> command/response queues and SG tables. This gives us a way to
>> guarantee we
>> meet the hardware address alignment requirements, and also reduces
>> memory that
>> might otherwise be wasted on alignment gaps.
>>
>> Signed-off-by: Mark Lord <mlord@pobox.com>
>
> ACK patches 1-13
>
> applied patches 1-9 to #upstream. patch #10 failed, with git-am
> reporting it as a corrupt patch.
..
That's weird. I can save the email from linux-ide here,
and apply as a patch (after 01-09) with no issues at all.
Jeff got mail reader problems?
Here it is again, in case it got corrupted in transit to you.
* * * *
In preparation for supporting NCQ, we must allocate separate SG tables
for each command tag, rather than just a single table per port as before.
Gen-I hardware cannot do NCQ, though, so we still allocate just a single
table for that, but populate it in all 32 slots to avoid special-cases
elsewhere in hotter paths of the code.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-26 14:18:05.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-26 14:19:12.000000000 -0500
@@ -398,8 +398,8 @@
dma_addr_t crqb_dma;
struct mv_crpb *crpb;
dma_addr_t crpb_dma;
- struct mv_sg *sg_tbl;
- dma_addr_t sg_tbl_dma;
+ struct mv_sg *sg_tbl[MV_MAX_Q_DEPTH];
+ dma_addr_t sg_tbl_dma[MV_MAX_Q_DEPTH];
unsigned int req_idx;
unsigned int resp_idx;
@@ -483,6 +483,10 @@
void __iomem *port_mmio, int want_ncq);
static int __mv_stop_dma(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().
+ */
static struct scsi_host_template mv5_sht = {
.module = THIS_MODULE,
.name = DRV_NAME,
@@ -1107,6 +1111,7 @@
{
struct mv_host_priv *hpriv = ap->host->private_data;
struct mv_port_priv *pp = ap->private_data;
+ int tag;
if (pp->crqb) {
dma_pool_free(hpriv->crqb_pool, pp->crqb, pp->crqb_dma);
@@ -1116,9 +1121,18 @@
dma_pool_free(hpriv->crpb_pool, pp->crpb, pp->crpb_dma);
pp->crpb = NULL;
}
- if (pp->sg_tbl) {
- dma_pool_free(hpriv->sg_tbl_pool, pp->sg_tbl, pp->sg_tbl_dma);
- pp->sg_tbl = NULL;
+ /*
+ * For GEN_I, there's no NCQ, so we have only a single sg_tbl.
+ * For later hardware, we have one unique sg_tbl per NCQ tag.
+ */
+ for (tag = 0; tag < MV_MAX_Q_DEPTH; ++tag) {
+ if (pp->sg_tbl[tag]) {
+ if (tag == 0 || !IS_GEN_I(hpriv))
+ dma_pool_free(hpriv->sg_tbl_pool,
+ pp->sg_tbl[tag],
+ pp->sg_tbl_dma[tag]);
+ pp->sg_tbl[tag] = NULL;
+ }
}
}
@@ -1139,7 +1153,7 @@
struct mv_port_priv *pp;
void __iomem *port_mmio = mv_ap_base(ap);
unsigned long flags;
- int rc;
+ int tag, rc;
pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
if (!pp)
@@ -1160,10 +1174,21 @@
goto out_port_free_dma_mem;
memset(pp->crpb, 0, MV_CRPB_Q_SZ);
- pp->sg_tbl = dma_pool_alloc(hpriv->sg_tbl_pool, GFP_KERNEL,
- &pp->sg_tbl_dma);
- if (!pp->sg_tbl)
- goto out_port_free_dma_mem;
+ /*
+ * For GEN_I, there's no NCQ, so we only allocate a single sg_tbl.
+ * For later hardware, we need one unique sg_tbl per NCQ tag.
+ */
+ for (tag = 0; tag < MV_MAX_Q_DEPTH; ++tag) {
+ if (tag == 0 || !IS_GEN_I(hpriv)) {
+ pp->sg_tbl[tag] = dma_pool_alloc(hpriv->sg_tbl_pool,
+ GFP_KERNEL, &pp->sg_tbl_dma[tag]);
+ if (!pp->sg_tbl[tag])
+ goto out_port_free_dma_mem;
+ } else {
+ pp->sg_tbl[tag] = pp->sg_tbl[0];
+ pp->sg_tbl_dma[tag] = pp->sg_tbl_dma[0];
+ }
+ }
spin_lock_irqsave(&ap->host->lock, flags);
@@ -1213,7 +1238,7 @@
struct mv_sg *mv_sg, *last_sg = NULL;
unsigned int si;
- mv_sg = pp->sg_tbl;
+ mv_sg = pp->sg_tbl[qc->tag];
for_each_sg(qc->sg, sg, qc->n_elem, si) {
dma_addr_t addr = sg_dma_address(sg);
u32 sg_len = sg_dma_len(sg);
@@ -1283,9 +1308,9 @@
in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
pp->crqb[in_index].sg_addr =
- cpu_to_le32(pp->sg_tbl_dma & 0xffffffff);
+ cpu_to_le32(pp->sg_tbl_dma[qc->tag] & 0xffffffff);
pp->crqb[in_index].sg_addr_hi =
- cpu_to_le32((pp->sg_tbl_dma >> 16) >> 16);
+ cpu_to_le32((pp->sg_tbl_dma[qc->tag] >> 16) >> 16);
pp->crqb[in_index].ctrl_flags = cpu_to_le16(flags);
cw = &pp->crqb[in_index].ata_cmd[0];
@@ -1376,8 +1401,8 @@
in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
crqb = (struct mv_crqb_iie *) &pp->crqb[in_index];
- crqb->addr = cpu_to_le32(pp->sg_tbl_dma & 0xffffffff);
- crqb->addr_hi = cpu_to_le32((pp->sg_tbl_dma >> 16) >> 16);
+ crqb->addr = cpu_to_le32(pp->sg_tbl_dma[qc->tag] & 0xffffffff);
+ crqb->addr_hi = cpu_to_le32((pp->sg_tbl_dma[qc->tag] >> 16) >> 16);
crqb->flags = cpu_to_le32(flags);
tf = &qc->tf;
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-29 18:24 ` Mark Lord
@ 2008-01-30 9:54 ` Jeff Garzik
2008-01-30 16:40 ` Mark Lord
0 siblings, 1 reply; 26+ messages in thread
From: Jeff Garzik @ 2008-01-30 9:54 UTC (permalink / raw)
To: Mark Lord; +Cc: IDE/ATA development list
Mark Lord wrote:
> Jeff Garzik wrote:
>> Mark Lord wrote:
>>> Create host-owned DMA memory pools, for use in allocating/freeing
>>> per-port
>>> command/response queues and SG tables. This gives us a way to
>>> guarantee we
>>> meet the hardware address alignment requirements, and also reduces
>>> memory that
>>> might otherwise be wasted on alignment gaps.
>>>
>>> Signed-off-by: Mark Lord <mlord@pobox.com>
>>
>> ACK patches 1-13
>>
>> applied patches 1-9 to #upstream. patch #10 failed, with git-am
>> reporting it as a corrupt patch.
> ..
>
> That's weird. I can save the email from linux-ide here,
> and apply as a patch (after 01-09) with no issues at all.
>
> Jeff got mail reader problems?
>
> Here it is again, in case it got corrupted in transit to you.
Nope, not corrupted in transit or on this side. It falls into a
familiar pattern:
* git-am(1) fails
* patch(1) succeeds
* when applying patch, patch(1) drops a .orig turd
So while patch(1) succeeds because patch(1) is highly forgiving and
git-am(1) is more strict, something was definitely strange on that
incoming email. patch(1) lets you know by giving you a .orig file,
something not normally created if the patch operation was 100% sound.
ISTR Linus or Junio explaining why git-am(1) was more strict and why it
was a good thing... As I did in this case, I usually just run patch(1),
look carefully at the result using 'git diff HEAD', and then
commit/resolve the changes.
Jeff
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-30 9:54 ` Jeff Garzik
@ 2008-01-30 16:40 ` Mark Lord
2008-01-30 17:08 ` Jeff Garzik
0 siblings, 1 reply; 26+ messages in thread
From: Mark Lord @ 2008-01-30 16:40 UTC (permalink / raw)
To: Jeff Garzik; +Cc: IDE/ATA development list
Jeff Garzik wrote:
>
> Nope, not corrupted in transit or on this side. It falls into a
> familiar pattern:
>
> * git-am(1) fails
> * patch(1) succeeds
> * when applying patch, patch(1) drops a .orig turd
..
Okay, I figured it out. There's a 1-line offset difference
between what is used in patch 10, and where stuff actually
is/was in sata_mv.c .
I probably changed an unrelated line and rediff'd an earlier patch
without rediff'ing everything that followed.
Because patch is smart and can handle it.
But since git-am plays dumb (both good and bad), I'll be more careful
about rediffing the entire series next time round.
Meanwhile, no further action required here.
Cheers
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-30 16:40 ` Mark Lord
@ 2008-01-30 17:08 ` Jeff Garzik
2008-01-30 17:19 ` Mark Lord
0 siblings, 1 reply; 26+ messages in thread
From: Jeff Garzik @ 2008-01-30 17:08 UTC (permalink / raw)
To: Mark Lord; +Cc: IDE/ATA development list
Mark Lord wrote:
> Meanwhile, no further action required here.
ACK :)
And thanks for rounding out the NCQ work. sata_mv has needed love and
attention for a while (well, really, its entire life).
Jeff
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-30 17:08 ` Jeff Garzik
@ 2008-01-30 17:19 ` Mark Lord
2008-01-30 17:45 ` Jeff Garzik
0 siblings, 1 reply; 26+ messages in thread
From: Mark Lord @ 2008-01-30 17:19 UTC (permalink / raw)
To: Jeff Garzik, Tejun Heo; +Cc: IDE/ATA development list
Jeff Garzik wrote:
> Mark Lord wrote:
>> Meanwhile, no further action required here.
>
> ACK :)
>
> And thanks for rounding out the NCQ work. sata_mv has needed love and
> attention for a while (well, really, its entire life).
..
Well, it's going to be getting plenty of TLC over the next few months.
In the short term, my plan is to submit further small patches to fix
the IRQ and error-handling in sata_mv, as bug fixes for 2.6.25.
Note that hot plug/unplug will begin to work correctly once the IRQ/EH
code gets fixed (it sort of works already, but sometimes kills the machine).
There are also some errata that need to be addressed in the 2.6.25 timeframe.
In particular, there's an NCQ EH errata for the 60x1 chips,
and a tricky issue about HOB access not working correctly on
most versions of the chips.
Bigger stuff that I'm deferring for 2.6.26:
-- Port multiplier support (though this does look rather simple..)
-- power management support
-- ATAPI
-- IRQ Coalescing
-- Target Mode support (interfaces yet to be defined)
-- TCQ support: would be good in general for libata on smart hosts,
but I'm not sure of the impact on libata EH processing.
How's that all sound to you guys?
Cheers
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-30 17:19 ` Mark Lord
@ 2008-01-30 17:45 ` Jeff Garzik
2008-01-30 18:57 ` Mark Lord
0 siblings, 1 reply; 26+ messages in thread
From: Jeff Garzik @ 2008-01-30 17:45 UTC (permalink / raw)
To: Mark Lord; +Cc: Tejun Heo, IDE/ATA development list
Mark Lord wrote:
> Jeff Garzik wrote:
>> Mark Lord wrote:
>>> Meanwhile, no further action required here.
>>
>> ACK :)
>>
>> And thanks for rounding out the NCQ work. sata_mv has needed love and
>> attention for a while (well, really, its entire life).
> ..
>
> Well, it's going to be getting plenty of TLC over the next few months.
>
> In the short term, my plan is to submit further small patches to fix
> the IRQ and error-handling in sata_mv, as bug fixes for 2.6.25.
>
> Note that hot plug/unplug will begin to work correctly once the IRQ/EH
> code gets fixed (it sort of works already, but sometimes kills the
> machine).
>
> There are also some errata that need to be addressed in the 2.6.25
> timeframe.
>
> In particular, there's an NCQ EH errata for the 60x1 chips,
> and a tricky issue about HOB access not working correctly on
> most versions of the chips.
>
> Bigger stuff that I'm deferring for 2.6.26:
>
> -- Port multiplier support (though this does look rather simple..)
> -- power management support
> -- ATAPI
I'm interested to see this :)
> -- IRQ Coalescing
Most "modern" SATA has some form of this, but I've yet to see any
benefit. I've dealt with interrupt (packet) rates of well over 500k/sec
in network land, and IMO the overhead in storage, even with tiny
operations, is really small in comparison.
So, I'm not sure its worth the latency penalty... at least as turned on
by default.
> -- Target Mode support (interfaces yet to be defined)
I would assume this would be along the lines of the SCSI target mode stuff.
> -- TCQ support: would be good in general for libata on smart hosts,
> but I'm not sure of the impact on libata EH processing.
Agreed, it would be nice to support host queueing controllers.
However, specifically for TCQ, it was rather poorly conceived. For most
controllers (mv, broadcom/svw, others) an error will stop the DMA
engine, and you perform recovery in software. All well and good, but
figuring out all the states possible during recovery is non-trivial (I
looked into it years ago). Its just messy.
Jeff
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-30 17:45 ` Jeff Garzik
@ 2008-01-30 18:57 ` Mark Lord
2008-01-31 3:23 ` Tejun Heo
0 siblings, 1 reply; 26+ messages in thread
From: Mark Lord @ 2008-01-30 18:57 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tejun Heo, IDE/ATA development list
Jeff Garzik wrote:
> Mark Lord wrote:
>> Jeff Garzik wrote:
>>> Mark Lord wrote:
..
>> Bigger stuff that I'm deferring for 2.6.26:
>>
>> -- Port multiplier support (though this does look rather simple..)
>> -- power management support
>> -- ATAPI
>
> I'm interested to see this :)
..
Heh.. me too, actually. :)
>> -- IRQ Coalescing
>
> Most "modern" SATA has some form of this, but I've yet to see any
> benefit. I've dealt with interrupt (packet) rates of well over 500k/sec
> in network land, and IMO the overhead in storage, even with tiny
> operations, is really small in comparison.
>
> So, I'm not sure its worth the latency penalty... at least as turned on
> by default.
..
I agree. It should default to off, and perhaps have some /sys/ attributes
to enable/tune it. Or something like that.
The theoretical value is apparently for situations like writing to RAID1.
The write isn't really "complete" until all drives ACK it,
so with IRQ coalescing it may behave more like NAPI than one I/O per IRQ.
And NAPI is apparently a win under heavy load for network, so.. we'll see.
The vendor wants it in the driver, and I think it will be good to have it
there so we can play with and tune it -- to find out for real whether it has
worthwhile value or not. But yes, the default should be "off", I think.
>> -- Target Mode support (interfaces yet to be defined)
>
> I would assume this would be along the lines of the SCSI target mode stuff.
..
Ah, now there's a starting point. Thanks.
>> -- TCQ support: would be good in general for libata on smart hosts,
>> but I'm not sure of the impact on libata EH processing.
>
> Agreed, it would be nice to support host queueing controllers.
>
> However, specifically for TCQ, it was rather poorly conceived. For most
> controllers (mv, broadcom/svw, others) an error will stop the DMA
> engine, and you perform recovery in software. All well and good, but
> figuring out all the states possible during recovery is non-trivial (I
> looked into it years ago). Its just messy.
..
So is NCQ EH, but we manage it. I wonder how similar (or not) the two are?
I've done host-based TCQ several times now, and EH can be as simple as:
"when something goes wrong, just reset everything, and then re-issue the
commands one at a time, doing per-command EH normally. Then resume TCQ."
That's dumb, but works extremely reliably.
Cheers
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-30 18:57 ` Mark Lord
@ 2008-01-31 3:23 ` Tejun Heo
2008-01-31 3:31 ` Tejun Heo
2008-01-31 3:59 ` Mark Lord
0 siblings, 2 replies; 26+ messages in thread
From: Tejun Heo @ 2008-01-31 3:23 UTC (permalink / raw)
To: Mark Lord; +Cc: Jeff Garzik, IDE/ATA development list
Mark Lord wrote:
>> So, I'm not sure its worth the latency penalty... at least as turned
>> on by default.
> ..
>
> I agree. It should default to off, and perhaps have some /sys/ attributes
> to enable/tune it. Or something like that.
Eeeek.. :-)
> The theoretical value is apparently for situations like writing to RAID1.
> The write isn't really "complete" until all drives ACK it,
> so with IRQ coalescing it may behave more like NAPI than one I/O per IRQ.
> And NAPI is apparently a win under heavy load for network, so.. we'll see.
>
> The vendor wants it in the driver, and I think it will be good to have it
> there so we can play with and tune it -- to find out for real whether it
> has
> worthwhile value or not. But yes, the default should be "off", I think.
I'm skeptical about the benefit of IRQ coalescing on storage
controllers. Coalescing improves performance when there are many small
requests to complete and if you put a lot of small non-consecutive
requests to a disk, it gets really really really slow and IRQ coalescing
just doesn't matter at all. The only way to achieve high number of
completions is to issue small commands to consecutive addresses which is
just silly. In storage, high volume transfer is achieved through
request coalescing not completion coalescing and this is true for even SDDs.
>>> -- Target Mode support (interfaces yet to be defined)
>>
>> I would assume this would be along the lines of the SCSI target mode
>> stuff.
> ..
>
> Ah, now there's a starting point. Thanks.
It would be great if we can make a cheap SATA analyzer out of it.
>>> -- TCQ support: would be good in general for libata on smart hosts,
>>> but I'm not sure of the impact on libata EH processing.
>>
>> Agreed, it would be nice to support host queueing controllers.
>>
>> However, specifically for TCQ, it was rather poorly conceived. For
>> most controllers (mv, broadcom/svw, others) an error will stop the DMA
>> engine, and you perform recovery in software. All well and good, but
>> figuring out all the states possible during recovery is non-trivial (I
>> looked into it years ago). Its just messy.
> ..
>
> So is NCQ EH, but we manage it. I wonder how similar (or not) the two are?
How many devices with working TCQ support are out there? Early raptors?
If the controller handles all that releasing and state transitions, I
think is shouldn't be too difficult. You'll probably need to add TCQ
support to ata_eh_autopsy or roll your own autopsy function but that
should be about it for EH. Heck, just freezing on any sign of problem
and let EH reset the drive and retry should work for most of the cases
although things like media errors won't be reported properly.
> I've done host-based TCQ several times now, and EH can be as simple as:
>
> "when something goes wrong, just reset everything, and then re-issue the
> commands one at a time, doing per-command EH normally. Then resume TCQ."
>
> That's dumb, but works extremely reliably.
Oh, you were thinking the same thing. :-) It can be made more reliable
by simply falling back to non-TCQ if error repeats itself. e.g. Media
error -> TCQ freeze -> reset -> media error -> TCQ freeze -> reset and
turn off TCQ -> media error gets handled and reported. It isn't pretty
but should work for initial implementation.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-31 3:23 ` Tejun Heo
@ 2008-01-31 3:31 ` Tejun Heo
2008-01-31 3:59 ` Mark Lord
1 sibling, 0 replies; 26+ messages in thread
From: Tejun Heo @ 2008-01-31 3:31 UTC (permalink / raw)
To: Mark Lord; +Cc: Jeff Garzik, IDE/ATA development list
Tejun Heo wrote:
> I'm skeptical about the benefit of IRQ coalescing on storage
> controllers. Coalescing improves performance when there are many small
> requests to complete and if you put a lot of small non-consecutive
> requests to a disk, it gets really really really slow and IRQ coalescing
> just doesn't matter at all. The only way to achieve high number of
> completions is to issue small commands to consecutive addresses which is
> just silly. In storage, high volume transfer is achieved through
> request coalescing not completion coalescing and this is true for even SDDs.
To add a bit, I was thinking about writes regarding SDDs. Completion
coalescing makes more sense for reads from SDDs if log based filesystem
is used on it because then read operations are scattered all over the
disk and the device doesn't have to seek. Limited queue size compared
to network interfaces will make completion coalescing less effective but
yeah, I think it will be worth playing with it on SDD + lfs.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-31 3:23 ` Tejun Heo
2008-01-31 3:31 ` Tejun Heo
@ 2008-01-31 3:59 ` Mark Lord
2008-01-31 9:00 ` Mikael Pettersson
1 sibling, 1 reply; 26+ messages in thread
From: Mark Lord @ 2008-01-31 3:59 UTC (permalink / raw)
To: Tejun Heo; +Cc: Jeff Garzik, IDE/ATA development list
Tejun Heo wrote:
>..
> I'm skeptical about the benefit of IRQ coalescing on storage
> controllers. Coalescing improves performance when there are many small
> requests to complete and if you put a lot of small non-consecutive
> requests to a disk, it gets really really really slow and IRQ coalescing
> just doesn't matter at all. The only way to achieve high number of
> completions is to issue small commands to consecutive addresses which is
> just silly. In storage, high volume transfer is achieved through
> request coalescing not completion coalescing and this is true for even SDDs.
..
One cool thing with the Marvell cores, is that they actually implement
"transaction based" IRQ coalescing, whereby a number of related I/O commands
(say, all the RAID5 member commands generated by a single R/W request)
can be tagged together, generating an interrupt only when they all complete
(or after a timeout if something goes wrong).
We don't have anything resembling an appropriate abstraction for that yet,
so I doubt that we could really take advantage of it.
I think one thought in general for IRQ coalescing, is that with largish
storage arrays it may cut down the number of IRQ entry/exit cycles
considerably under heavy load, and perhaps slightly improve L1 cache
occupancy of the IRQ handler paths as well. Dunno, but it could be fun
to wire it in there so we can experiment and (in)validate such theories.
>>>> -- Target Mode support (interfaces yet to be defined)
>>> I would assume this would be along the lines of the SCSI target mode
>>> stuff.
>> ..
>>
>> Ah, now there's a starting point. Thanks.
>
> It would be great if we can make a cheap SATA analyzer out of it.
..
Yeah. It'll have software latency added in, but the chip really looks
like it can do the analyzer job just fine. I wonder if any of the existing
analyzer products already use these chips..
..
> How many devices with working TCQ support are out there? Early raptors?
..
Raptors, everything ever made by Hitachi, some Maxtor, some Seagate (I think),
and even the odd Western Digital drive. And in the PATA space, there were
WD and IBM/Hitachi drives with it. The PDC ADMA and QStor chips were designed
for TCQ (and NCQ for QStor).
Still though, I don't think it's a huge priority, since all modern drives
now implement NCQ when it matters.
Cheers
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables
2008-01-31 3:59 ` Mark Lord
@ 2008-01-31 9:00 ` Mikael Pettersson
0 siblings, 0 replies; 26+ messages in thread
From: Mikael Pettersson @ 2008-01-31 9:00 UTC (permalink / raw)
To: Mark Lord; +Cc: Tejun Heo, Jeff Garzik, IDE/ATA development list
Mark Lord writes:
> Tejun Heo wrote:
> >..
> > I'm skeptical about the benefit of IRQ coalescing on storage
> > controllers. Coalescing improves performance when there are many small
> > requests to complete and if you put a lot of small non-consecutive
> > requests to a disk, it gets really really really slow and IRQ coalescing
> > just doesn't matter at all. The only way to achieve high number of
> > completions is to issue small commands to consecutive addresses which is
> > just silly. In storage, high volume transfer is achieved through
> > request coalescing not completion coalescing and this is true for even SDDs.
> ..
>
> One cool thing with the Marvell cores, is that they actually implement
> "transaction based" IRQ coalescing, whereby a number of related I/O commands
> (say, all the RAID5 member commands generated by a single R/W request)
> can be tagged together, generating an interrupt only when they all complete
> (or after a timeout if something goes wrong).
>
> We don't have anything resembling an appropriate abstraction for that yet,
> so I doubt that we could really take advantage of it.
Promise SATA controllers have this feature too,
though sata_promise doesn't make use of it.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 10/13] sata_mv ncq Introduce per-tag SG tables
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (7 preceding siblings ...)
2008-01-26 23:32 ` [PATCH 09/13] sata_mv ncq Use DMA memory pools for hardware memory tables Mark Lord
@ 2008-01-26 23:32 ` Mark Lord
2008-01-30 9:50 ` Jeff Garzik
2008-01-26 23:33 ` [PATCH 11/13] sata_mv ncq Enable NCQ operation Mark Lord
` (2 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:32 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
In preparation for supporting NCQ, we must allocate separate SG tables
for each command tag, rather than just a single table per port as before.
Gen-I hardware cannot do NCQ, though, so we still allocate just a single
table for that, but populate it in all 32 slots to avoid special-cases
elsewhere in hotter paths of the code.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-26 14:18:05.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-26 14:19:12.000000000 -0500
@@ -398,8 +398,8 @@
dma_addr_t crqb_dma;
struct mv_crpb *crpb;
dma_addr_t crpb_dma;
- struct mv_sg *sg_tbl;
- dma_addr_t sg_tbl_dma;
+ struct mv_sg *sg_tbl[MV_MAX_Q_DEPTH];
+ dma_addr_t sg_tbl_dma[MV_MAX_Q_DEPTH];
unsigned int req_idx;
unsigned int resp_idx;
@@ -483,6 +483,10 @@
void __iomem *port_mmio, int want_ncq);
static int __mv_stop_dma(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().
+ */
static struct scsi_host_template mv5_sht = {
.module = THIS_MODULE,
.name = DRV_NAME,
@@ -1107,6 +1111,7 @@
{
struct mv_host_priv *hpriv = ap->host->private_data;
struct mv_port_priv *pp = ap->private_data;
+ int tag;
if (pp->crqb) {
dma_pool_free(hpriv->crqb_pool, pp->crqb, pp->crqb_dma);
@@ -1116,9 +1121,18 @@
dma_pool_free(hpriv->crpb_pool, pp->crpb, pp->crpb_dma);
pp->crpb = NULL;
}
- if (pp->sg_tbl) {
- dma_pool_free(hpriv->sg_tbl_pool, pp->sg_tbl, pp->sg_tbl_dma);
- pp->sg_tbl = NULL;
+ /*
+ * For GEN_I, there's no NCQ, so we have only a single sg_tbl.
+ * For later hardware, we have one unique sg_tbl per NCQ tag.
+ */
+ for (tag = 0; tag < MV_MAX_Q_DEPTH; ++tag) {
+ if (pp->sg_tbl[tag]) {
+ if (tag == 0 || !IS_GEN_I(hpriv))
+ dma_pool_free(hpriv->sg_tbl_pool,
+ pp->sg_tbl[tag],
+ pp->sg_tbl_dma[tag]);
+ pp->sg_tbl[tag] = NULL;
+ }
}
}
@@ -1139,7 +1153,7 @@
struct mv_port_priv *pp;
void __iomem *port_mmio = mv_ap_base(ap);
unsigned long flags;
- int rc;
+ int tag, rc;
pp = devm_kzalloc(dev, sizeof(*pp), GFP_KERNEL);
if (!pp)
@@ -1160,10 +1174,21 @@
goto out_port_free_dma_mem;
memset(pp->crpb, 0, MV_CRPB_Q_SZ);
- pp->sg_tbl = dma_pool_alloc(hpriv->sg_tbl_pool, GFP_KERNEL,
- &pp->sg_tbl_dma);
- if (!pp->sg_tbl)
- goto out_port_free_dma_mem;
+ /*
+ * For GEN_I, there's no NCQ, so we only allocate a single sg_tbl.
+ * For later hardware, we need one unique sg_tbl per NCQ tag.
+ */
+ for (tag = 0; tag < MV_MAX_Q_DEPTH; ++tag) {
+ if (tag == 0 || !IS_GEN_I(hpriv)) {
+ pp->sg_tbl[tag] = dma_pool_alloc(hpriv->sg_tbl_pool,
+ GFP_KERNEL, &pp->sg_tbl_dma[tag]);
+ if (!pp->sg_tbl[tag])
+ goto out_port_free_dma_mem;
+ } else {
+ pp->sg_tbl[tag] = pp->sg_tbl[0];
+ pp->sg_tbl_dma[tag] = pp->sg_tbl_dma[0];
+ }
+ }
spin_lock_irqsave(&ap->host->lock, flags);
@@ -1213,7 +1238,7 @@
struct mv_sg *mv_sg, *last_sg = NULL;
unsigned int si;
- mv_sg = pp->sg_tbl;
+ mv_sg = pp->sg_tbl[qc->tag];
for_each_sg(qc->sg, sg, qc->n_elem, si) {
dma_addr_t addr = sg_dma_address(sg);
u32 sg_len = sg_dma_len(sg);
@@ -1283,9 +1308,9 @@
in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
pp->crqb[in_index].sg_addr =
- cpu_to_le32(pp->sg_tbl_dma & 0xffffffff);
+ cpu_to_le32(pp->sg_tbl_dma[qc->tag] & 0xffffffff);
pp->crqb[in_index].sg_addr_hi =
- cpu_to_le32((pp->sg_tbl_dma >> 16) >> 16);
+ cpu_to_le32((pp->sg_tbl_dma[qc->tag] >> 16) >> 16);
pp->crqb[in_index].ctrl_flags = cpu_to_le16(flags);
cw = &pp->crqb[in_index].ata_cmd[0];
@@ -1376,8 +1401,8 @@
in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
crqb = (struct mv_crqb_iie *) &pp->crqb[in_index];
- crqb->addr = cpu_to_le32(pp->sg_tbl_dma & 0xffffffff);
- crqb->addr_hi = cpu_to_le32((pp->sg_tbl_dma >> 16) >> 16);
+ crqb->addr = cpu_to_le32(pp->sg_tbl_dma[qc->tag] & 0xffffffff);
+ crqb->addr_hi = cpu_to_le32((pp->sg_tbl_dma[qc->tag] >> 16) >> 16);
crqb->flags = cpu_to_le32(flags);
tf = &qc->tf;
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 11/13] sata_mv ncq Enable NCQ operation
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (8 preceding siblings ...)
2008-01-26 23:32 ` [PATCH 10/13] sata_mv ncq Introduce per-tag SG tables Mark Lord
@ 2008-01-26 23:33 ` Mark Lord
2008-01-26 23:33 ` [PATCH 12/13] sata_mv ncq Remove post internal cmd op Mark Lord
2008-01-26 23:33 ` [PATCH 13/13] sata_mv ncq Comments and version bump Mark Lord
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:33 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
Final changes to actually turn on NCQ in the driver for GEN_II/IIE hardware.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-26 12:41:01.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-26 12:43:21.000000000 -0500
@@ -510,7 +510,8 @@
.name = DRV_NAME,
.ioctl = ata_scsi_ioctl,
.queuecommand = ata_scsi_queuecmd,
- .can_queue = ATA_DEF_QUEUE,
+ .change_queue_depth = ata_scsi_change_queue_depth,
+ .can_queue = MV_MAX_Q_DEPTH - 1,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = MV_MAX_SG_CT / 2,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
@@ -572,6 +573,7 @@
.post_internal_cmd = mv_post_int_cmd,
.freeze = mv_eh_freeze,
.thaw = mv_eh_thaw,
+ .qc_defer = ata_std_qc_defer,
.scr_read = mv_scr_read,
.scr_write = mv_scr_write,
@@ -600,6 +602,7 @@
.post_internal_cmd = mv_post_int_cmd,
.freeze = mv_eh_freeze,
.thaw = mv_eh_thaw,
+ .qc_defer = ata_std_qc_defer,
.scr_read = mv_scr_read,
.scr_write = mv_scr_write,
@@ -628,26 +631,29 @@
.port_ops = &mv5_ops,
},
{ /* chip_604x */
- .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS,
+ .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
+ ATA_FLAG_NCQ,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
.port_ops = &mv6_ops,
},
{ /* chip_608x */
.flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
- MV_FLAG_DUAL_HC,
+ ATA_FLAG_NCQ | MV_FLAG_DUAL_HC,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
.port_ops = &mv6_ops,
},
{ /* chip_6042 */
- .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS,
+ .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
+ ATA_FLAG_NCQ,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
.port_ops = &mv_iie_ops,
},
{ /* chip_7042 */
- .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS,
+ .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
+ ATA_FLAG_NCQ,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
.port_ops = &mv_iie_ops,
@@ -1261,7 +1267,8 @@
u16 flags = 0;
unsigned in_index;
- if (qc->tf.protocol != ATA_PROT_DMA)
+ if ((qc->tf.protocol != ATA_PROT_DMA) &&
+ (qc->tf.protocol != ATA_PROT_NCQ))
return;
/* Fill in command request block
@@ -1297,13 +1304,11 @@
case ATA_CMD_WRITE_FUA_EXT:
mv_crqb_pack_cmd(cw++, tf->hob_nsect, ATA_REG_NSECT, 0);
break;
-#ifdef LIBATA_NCQ /* FIXME: remove this line when NCQ added */
case ATA_CMD_FPDMA_READ:
case ATA_CMD_FPDMA_WRITE:
mv_crqb_pack_cmd(cw++, tf->hob_feature, ATA_REG_FEATURE, 0);
mv_crqb_pack_cmd(cw++, tf->feature, ATA_REG_FEATURE, 0);
break;
-#endif /* FIXME: remove this line when NCQ added */
default:
/* The only other commands EDMA supports in non-queued and
* non-NCQ mode are: [RW] STREAM DMA and W DMA FUA EXT, none
@@ -1352,7 +1357,8 @@
unsigned in_index;
u32 flags = 0;
- if (qc->tf.protocol != ATA_PROT_DMA)
+ if ((qc->tf.protocol != ATA_PROT_DMA) &&
+ (qc->tf.protocol != ATA_PROT_NCQ))
return;
/* Fill in Gen IIE command request block
@@ -1418,7 +1424,8 @@
struct mv_port_priv *pp = ap->private_data;
u32 in_index;
- if (qc->tf.protocol != ATA_PROT_DMA) {
+ if ((qc->tf.protocol != ATA_PROT_DMA) &&
+ (qc->tf.protocol != ATA_PROT_NCQ)) {
/* 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.
@@ -1429,12 +1436,6 @@
mv_start_dma(ap, port_mmio, pp, qc->tf.protocol);
- in_index = pp->req_idx & MV_MAX_Q_DEPTH_MASK;
-
- /* until we do queuing, the queue should be empty at this point */
- WARN_ON(in_index != ((readl(port_mmio + EDMA_REQ_Q_OUT_PTR_OFS)
- >> EDMA_REQ_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK));
-
pp->req_idx++;
in_index = (pp->req_idx & MV_MAX_Q_DEPTH_MASK) << EDMA_REQ_Q_PTR_SHIFT;
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 12/13] sata_mv ncq Remove post internal cmd op
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (9 preceding siblings ...)
2008-01-26 23:33 ` [PATCH 11/13] sata_mv ncq Enable NCQ operation Mark Lord
@ 2008-01-26 23:33 ` Mark Lord
2008-01-26 23:33 ` [PATCH 13/13] sata_mv ncq Comments and version bump Mark Lord
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:33 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
This driver currently has no need for the .post_internal_cmd op.
So get rid of it, to save unnecessary transitions between EDMA and non-EDMA modes.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-26 18:11:17.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-26 18:10:38.000000000 -0500
@@ -452,7 +452,6 @@
static void mv_qc_prep_iie(struct ata_queued_cmd *qc);
static unsigned int mv_qc_issue(struct ata_queued_cmd *qc);
static void mv_error_handler(struct ata_port *ap);
-static void mv_post_int_cmd(struct ata_queued_cmd *qc);
static void mv_eh_freeze(struct ata_port *ap);
static void mv_eh_thaw(struct ata_port *ap);
static void mv6_dev_config(struct ata_device *dev);
@@ -541,7 +540,6 @@
.irq_on = ata_irq_on,
.error_handler = mv_error_handler,
- .post_internal_cmd = mv_post_int_cmd,
.freeze = mv_eh_freeze,
.thaw = mv_eh_thaw,
@@ -570,7 +568,6 @@
.irq_on = ata_irq_on,
.error_handler = mv_error_handler,
- .post_internal_cmd = mv_post_int_cmd,
.freeze = mv_eh_freeze,
.thaw = mv_eh_thaw,
.qc_defer = ata_std_qc_defer,
@@ -599,7 +596,6 @@
.irq_on = ata_irq_on,
.error_handler = mv_error_handler,
- .post_internal_cmd = mv_post_int_cmd,
.freeze = mv_eh_freeze,
.thaw = mv_eh_thaw,
.qc_defer = ata_std_qc_defer,
@@ -2424,11 +2420,6 @@
mv_hardreset, mv_postreset);
}
-static void mv_post_int_cmd(struct ata_queued_cmd *qc)
-{
- mv_stop_dma(qc->ap);
-}
-
static void mv_eh_freeze(struct ata_port *ap)
{
void __iomem *mmio = ap->host->iomap[MV_PRIMARY_BAR];
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 13/13] sata_mv ncq Comments and version bump
2008-01-26 23:28 [PATCH 01/13] sata_mv ncq EH fixes Mark Lord
` (10 preceding siblings ...)
2008-01-26 23:33 ` [PATCH 12/13] sata_mv ncq Remove post internal cmd op Mark Lord
@ 2008-01-26 23:33 ` Mark Lord
11 siblings, 0 replies; 26+ messages in thread
From: Mark Lord @ 2008-01-26 23:33 UTC (permalink / raw)
To: Jeff Garzik, IDE/ATA development list
Remove some obsolete comments, and bump up the driver version number.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- old/drivers/ata/sata_mv.c 2008-01-26 11:05:54.000000000 -0500
+++ new/drivers/ata/sata_mv.c 2008-01-26 11:12:32.000000000 -0500
@@ -29,7 +29,13 @@
I distinctly remember a couple workarounds (one related to PCI-X)
are still needed.
- 4) Add NCQ support (easy to intermediate, once new-EH support appears)
+ 2) Improve/fix IRQ and error handling sequences.
+
+ 3) ATAPI support (Marvell claims the 60xx/70xx chips can do it).
+
+ 4) Think about TCQ support here, and for libata in general
+ with controllers that suppport it via host-queuing hardware
+ (a software-only implementation could be a nightmare).
5) Investigate problems with PCI Message Signalled Interrupts (MSI).
@@ -53,8 +59,6 @@
Target mode, for those without docs, is the ability to directly
connect two SATA controllers.
- 13) Verify that 7042 is fully supported. I only have a 6042.
-
*/
@@ -73,7 +77,7 @@
#include <linux/libata.h>
#define DRV_NAME "sata_mv"
-#define DRV_VERSION "1.01"
+#define DRV_VERSION "1.20"
enum {
/* BAR's are enumerated in terms of pci_resource_start() terms */
^ permalink raw reply [flat|nested] 26+ messages in thread