[PATCH 2.6.24-rc1] sata_promise: ASIC PRD table bug workaround
Second-generation Promise SATA controllers have an ASIC bug
which can trigger if the last PRD entry is larger than 164 bytes,
resulting in intermittent errors and possible data corruption.
Work around this by replacing calls to ata_qc_prep() with a
private version that fills the PRD, checks the size of the
last entry, and if necessary splits it to avoid the bug.
Also reduce sg_tablesize by 1 to accommodate the new entry.
Tested on the second-generation SATA300 TX4 and SATA300 TX2plus,
and the first-generation PDC20378.
Thanks to Alexander Sabourenkov for verifying the bug by
studying the vendor driver, and for writing the initial patch
upon which this one is based.
Signed-off-by: Mikael Pettersson <[EMAIL PROTECTED]>
--
This patch is prepared against 2.6.24-rc1 but applies with
harmless minor offsets against current #upstream-fixes.
drivers/ata/sata_promise.c | 86 ++---
1 files changed, 82 insertions(+), 4 deletions(-)
diff -rupN linux-2.6.24-rc1/drivers/ata/sata_promise.c
linux-2.6.24-rc1.sata_promise-asic-sg-bug-fix/drivers/ata/sata_promise.c
--- linux-2.6.24-rc1/drivers/ata/sata_promise.c 2007-10-24 23:22:55.0
+0200
+++ linux-2.6.24-rc1.sata_promise-asic-sg-bug-fix/drivers/ata/sata_promise.c
2007-10-29 15:54:58.0 +0100
@@ -155,7 +155,7 @@ static struct scsi_host_template pdc_ata
.queuecommand = ata_scsi_queuecmd,
.can_queue = ATA_DEF_QUEUE,
.this_id= ATA_SHT_THIS_ID,
- .sg_tablesize = LIBATA_MAX_PRD,
+ .sg_tablesize = LIBATA_MAX_PRD-1,
.cmd_per_lun= ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
@@ -521,6 +521,84 @@ static void pdc_atapi_pkt(struct ata_que
memcpy(buf+31, cdb, cdb_len);
}
+/**
+ * pdc_fill_sg - Fill PCI IDE PRD table
+ * @qc: Metadata associated with taskfile to be transferred
+ *
+ * Fill PCI IDE PRD (scatter-gather) table with segments
+ * associated with the current disk command.
+ * Make sure hardware does not choke on it.
+ *
+ * LOCKING:
+ * spin_lock_irqsave(host lock)
+ *
+ */
+static void pdc_fill_sg(struct ata_queued_cmd *qc)
+{
+ struct ata_port *ap = qc->ap;
+ struct scatterlist *sg;
+ unsigned int idx;
+ const u32 SG_COUNT_ASIC_BUG = 41*4;
+
+ if (!(qc->flags & ATA_QCFLAG_DMAMAP))
+ return;
+
+ WARN_ON(qc->__sg == NULL);
+ WARN_ON(qc->n_elem == 0 && qc->pad_len == 0);
+
+ idx = 0;
+ ata_for_each_sg(sg, qc) {
+ u32 addr, offset;
+ u32 sg_len, len;
+
+ /* determine if physical DMA addr spans 64K boundary.
+* Note h/w doesn't support 64-bit, so we unconditionally
+* truncate dma_addr_t to u32.
+*/
+ addr = (u32) sg_dma_address(sg);
+ sg_len = sg_dma_len(sg);
+
+ while (sg_len) {
+ offset = addr & 0x;
+ len = sg_len;
+ if ((offset + sg_len) > 0x1)
+ len = 0x1 - offset;
+
+ ap->prd[idx].addr = cpu_to_le32(addr);
+ ap->prd[idx].flags_len = cpu_to_le32(len & 0x);
+ VPRINTK("PRD[%u] = (0x%X, 0x%X)\n", idx, addr, len);
+
+ idx++;
+ sg_len -= len;
+ addr += len;
+ }
+ }
+
+ if (idx) {
+ u32 len = le32_to_cpu(ap->prd[idx - 1].flags_len);
+
+ if (len > SG_COUNT_ASIC_BUG) {
+ u32 addr;
+
+ VPRINTK("Splitting last PRD.\n");
+
+ addr = le32_to_cpu(ap->prd[idx - 1].addr);
+ ap->prd[idx - 1].flags_len -=
cpu_to_le32(SG_COUNT_ASIC_BUG);
+ VPRINTK("PRD[%u] = (0x%X, 0x%X)\n", idx - 1, addr,
SG_COUNT_ASIC_BUG);
+
+ addr = addr + len - SG_COUNT_ASIC_BUG;
+ len = SG_COUNT_ASIC_BUG;
+ ap->prd[idx].addr = cpu_to_le32(addr);
+ ap->prd[idx].flags_len = cpu_to_le32(len);
+ VPRINTK("PRD[%u] = (0x%X, 0x%X)\n", idx, addr, len);
+
+ idx++;
+ }
+
+ ap->prd[idx - 1].flags_len |= cpu_to_le32(ATA_PRD_EOT);
+ }
+}
+
static void pdc_qc_prep(struct ata_queued_cmd *qc)
{
struct pdc_port_priv *pp = qc->ap->private_data;
@@ -530,7 +608,7 @@ static void pdc_qc_prep(struct ata_queue
switch (qc->tf.protocol) {
case ATA_PROT_DMA:
- ata_qc_prep(qc);
+ pdc_fill_sg(qc);
/* fall through */
case ATA_PROT_NODATA:
@@ -546,11 +624,11 @@ static void pdc_qc_prep(struct ata_queue
Re: [PATCH 2.6.24-rc1] sata_promise: ASIC PRD table bug workaround
Mikael Pettersson wrote:
@@ -155,7 +155,7 @@ static struct scsi_host_template pdc_ata
.queuecommand = ata_scsi_queuecmd,
.can_queue = ATA_DEF_QUEUE,
.this_id= ATA_SHT_THIS_ID,
- .sg_tablesize = LIBATA_MAX_PRD,
+ .sg_tablesize = LIBATA_MAX_PRD-1,
.cmd_per_lun= ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
IMO, this will be difficult for human eyes to see, many months from now.
I would prefer a 'PDC_MAX_PRD' constant, defined to this value.
@@ -530,7 +608,7 @@ static void pdc_qc_prep(struct ata_queue
switch (qc->tf.protocol) {
case ATA_PROT_DMA:
- ata_qc_prep(qc);
+ pdc_fill_sg(qc);
/* fall through */
case ATA_PROT_NODATA:
@@ -546,11 +624,11 @@ static void pdc_qc_prep(struct ata_queue
break;
case ATA_PROT_ATAPI:
- ata_qc_prep(qc);
+ pdc_fill_sg(qc);
break;
case ATA_PROT_ATAPI_DMA:
- ata_qc_prep(qc);
+ pdc_fill_sg(qc);
/*FALLTHROUGH*/
Note that this is not exactly an equivalent change -- you are removing
the test in ata_qc_prep() that occurs prior to ata_fill_sg() call.
Jeff
-
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
Re: [PATCH 2.6.24-rc1] sata_promise: ASIC PRD table bug workaround
Jeff Garzik wrote:
Mikael Pettersson wrote:
>> @@ -530,7 +608,7 @@ static void pdc_qc_prep(struct ata_queue
switch (qc->tf.protocol) {
case ATA_PROT_DMA:
-ata_qc_prep(qc);
+pdc_fill_sg(qc);
/* fall through */
case ATA_PROT_NODATA:
@@ -546,11 +624,11 @@ static void pdc_qc_prep(struct ata_queue
break;
case ATA_PROT_ATAPI:
-ata_qc_prep(qc);
+pdc_fill_sg(qc);
break;
case ATA_PROT_ATAPI_DMA:
-ata_qc_prep(qc);
+pdc_fill_sg(qc);
/*FALLTHROUGH*/
Note that this is not exactly an equivalent change -- you are removing
the test in ata_qc_prep() that occurs prior to ata_fill_sg() call.
Not at all. The test is there at the beginning of pdc_fill_sg().
--
./lxnt
-
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
Re: [PATCH 2.6.24-rc1] sata_promise: ASIC PRD table bug workaround
On Mon, 29 Oct 2007 15:07:06 -0400, Jeff Garzik wrote:
> Mikael Pettersson wrote:
> > @@ -155,7 +155,7 @@ static struct scsi_host_template pdc_ata
> > .queuecommand = ata_scsi_queuecmd,
> > .can_queue = ATA_DEF_QUEUE,
> > .this_id= ATA_SHT_THIS_ID,
> > - .sg_tablesize = LIBATA_MAX_PRD,
> > + .sg_tablesize = LIBATA_MAX_PRD-1,
> > .cmd_per_lun= ATA_SHT_CMD_PER_LUN,
> > .emulated = ATA_SHT_EMULATED,
> > .use_clustering = ATA_SHT_USE_CLUSTERING,
>
> IMO, this will be difficult for human eyes to see, many months from now.
>
> I would prefer a 'PDC_MAX_PRD' constant, defined to this value.
Agreed. I'll do this change.
> > @@ -530,7 +608,7 @@ static void pdc_qc_prep(struct ata_queue
> >
> > switch (qc->tf.protocol) {
> > case ATA_PROT_DMA:
> > - ata_qc_prep(qc);
> > + pdc_fill_sg(qc);
> > /* fall through */
> >
> > case ATA_PROT_NODATA:
> > @@ -546,11 +624,11 @@ static void pdc_qc_prep(struct ata_queue
> > break;
> >
> > case ATA_PROT_ATAPI:
> > - ata_qc_prep(qc);
> > + pdc_fill_sg(qc);
> > break;
> >
> > case ATA_PROT_ATAPI_DMA:
> > - ata_qc_prep(qc);
> > + pdc_fill_sg(qc);
> > /*FALLTHROUGH*/
>
> Note that this is not exactly an equivalent change -- you are removing
> the test in ata_qc_prep() that occurs prior to ata_fill_sg() call.
As Alexander replied, pdc_fill_sg() does include ata_qc_prep()'s
initial test. Unfortunately the name pdc_qc_prep() is already taken
[it switches on qc->tf.protocol], so that leaves either pdc_fill_sg()
[slightly imprecise as you noted], or uglies like __pdc_qc_prep(),
pdc_qc_prep2(), or pdc_qc_prep_and_fill_sg() as names for the new
procedure. I welcome suggestions for a better name.
/Mikael
-
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
Re: [PATCH 2.6.24-rc1] sata_promise: ASIC PRD table bug workaround
Mikael Pettersson wrote: Unfortunately the name pdc_qc_prep() is already taken [it switches on qc->tf.protocol], so that leaves either pdc_fill_sg() [slightly imprecise as you noted], or uglies like __pdc_qc_prep(), pdc_qc_prep2(), or pdc_qc_prep_and_fill_sg() as names for the new procedure. I welcome suggestions for a better name. pdc_qc_prep_backend() ? ps: I almost stuck on naming the function while writing the patch, put there first that came to mind just to not delay testing. -- ./lxnt - 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
