[PATCH 1/3] libata: call ata_check_atapi_dma() with qc better prepared

[PATCH 1/3] libata: call ata_check_atapi_dma() with qc better prepared

[Tejun Heo]

In atapi_xlat(), prepare qc better before calling
ata_check_atapi_dma() such that ata_check_atapi_dma() can use info
from qc.  While at it, reformat weird looking if/else block in the
function.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-scsi.c |   20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

Index: work/drivers/ata/libata-scsi.c
===================================================================
--- work.orig/drivers/ata/libata-scsi.c
+++ work/drivers/ata/libata-scsi.c
@@ -2384,11 +2384,6 @@ static unsigned int atapi_xlat(struct at
 	int using_pio = (dev->flags & ATA_DFLAG_PIO);
 	int nodata = (scmd->sc_data_direction == DMA_NONE);
 
-	if (!using_pio)
-		/* Check whether ATAPI DMA is safe */
-		if (ata_check_atapi_dma(qc))
-			using_pio = 1;
-
 	memset(qc->cdb, 0, dev->cdb_len);
 	memcpy(qc->cdb, scmd->cmnd, scmd->cmd_len);
 
@@ -2401,19 +2396,22 @@ static unsigned int atapi_xlat(struct at
 	}
 
 	qc->tf.command = ATA_CMD_PACKET;
+	qc->nbytes = scmd->request_bufflen;
+
+	/* check whether ATAPI DMA is safe */
+	if (!using_pio && ata_check_atapi_dma(qc))
+		using_pio = 1;
 
-	/* no data, or PIO data xfer */
 	if (using_pio || nodata) {
+		/* no data, or PIO data xfer */
 		if (nodata)
 			qc->tf.protocol = ATA_PROT_ATAPI_NODATA;
 		else
 			qc->tf.protocol = ATA_PROT_ATAPI;
 		qc->tf.lbam = (8 * 1024) & 0xff;
 		qc->tf.lbah = (8 * 1024) >> 8;
-	}
-
-	/* DMA data xfer */
-	else {
+	} else {
+		/* DMA data xfer */
 		qc->tf.protocol = ATA_PROT_ATAPI_DMA;
 		qc->tf.feature |= ATAPI_PKT_DMA;
 
@@ -2422,8 +2420,6 @@ static unsigned int atapi_xlat(struct at
 			qc->tf.feature |= ATAPI_DMADIR;
 	}
 
-	qc->nbytes = scmd->request_bufflen;
-
 	return 0;
 }
 

[PATCH] libata: use PIO for non-16 byte aligned ATAPI commands

[Tejun Heo]

The IDE driver used DMA for ATAPI commands if READ/WRITE command is
multiple of sector size or sg command is multiple of 16 bytes.  For
libata, READ/WRITE sector alignment is guaranteed by the high level
driver (sr), so we only have to worry about the 16 byte alignment.

This patch makes ata_check_atapi_dma() always request PIO for all data
transfer commands which are not multiple of 16 bytes.

The following reports are related to this problem.

http://bugzilla.kernel.org/show_bug.cgi?id=8605		(confirmed)
http://thread.gmane.org/gmane.linux.kernel/476620	(confirmed)
https://bugzilla.novell.com/show_bug.cgi?id=229260	(probably)

Albert first pointed out the difference between IDE and libata.  Kudos
to him.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Albert Lee <albertcc@tw.ibm.com>
---
 drivers/ata/libata-core.c |   31 +++++++++----------------------
 1 file changed, 9 insertions(+), 22 deletions(-)

Index: work/drivers/ata/libata-core.c
===================================================================
--- work.orig/drivers/ata/libata-core.c
+++ work/drivers/ata/libata-core.c
@@ -4109,6 +4109,7 @@ static void ata_fill_sg(struct ata_queue
 	if (idx)
 		ap->prd[idx - 1].flags_len |= cpu_to_le32(ATA_PRD_EOT);
 }
+
 /**
  *	ata_check_atapi_dma - Check whether ATAPI DMA can be supported
  *	@qc: Metadata associated with taskfile to check
@@ -4126,33 +4127,19 @@ static void ata_fill_sg(struct ata_queue
 int ata_check_atapi_dma(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
-	int rc = 0; /* Assume ATAPI DMA is OK by default */
 
-	/* some drives can only do ATAPI DMA on read/write */
-	if (unlikely(qc->dev->horkage & ATA_HORKAGE_DMA_RW_ONLY)) {
-		struct scsi_cmnd *cmd = qc->scsicmd;
-		u8 *scsicmd = cmd->cmnd;
-
-		switch (scsicmd[0]) {
-		case READ_10:
-		case WRITE_10:
-		case READ_12:
-		case WRITE_12:
-		case READ_6:
-		case WRITE_6:
-			/* atapi dma maybe ok */
-			break;
-		default:
-			/* turn off atapi dma */
-			return 1;
-		}
-	}
+	/* Don't allow DMA if it isn't multiple of 16 bytes.  Quite a
+	 * few ATAPI devices choke on such DMA requests.
+	 */
+	if (unlikely(qc->nbytes & 15))
+		return 1;
 
 	if (ap->ops->check_atapi_dma)
-		rc = ap->ops->check_atapi_dma(qc);
+		return ap->ops->check_atapi_dma(qc);
 
-	return rc;
+	return 0;
 }
+
 /**
  *	ata_qc_prep - Prepare taskfile for submission
  *	@qc: Metadata associated with taskfile to be prepared

[PATCH 3/3] libata: kill ATA_HORKAGE_DMA_RW_ONLY

[Tejun Heo]

ATA_HORKAGE_DMA_RW_ONLY for TORiSAN is verified to be subset of using
DMA for ATAPI commands which aren't aligned to 16 bytes.  As libata
now doesn't use DMA for unaligned ATAPI commands, the horkage is
redundant.  Kill it.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
In case it wasn't clear.  The one before this one was 2/3 and all
three patches are against 2.6.22-rc5.  Thanks.

 drivers/ata/libata-core.c |    7 +------
 include/linux/libata.h    |    1 -
 2 files changed, 1 insertion(+), 7 deletions(-)

Index: work/drivers/ata/libata-core.c
===================================================================
--- work.orig/drivers/ata/libata-core.c
+++ work/drivers/ata/libata-core.c
@@ -2046,10 +2046,6 @@ int ata_dev_configure(struct ata_device 
 		dev->max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128,
 					 dev->max_sectors);
 
-	/* limit ATAPI DMA to R/W commands only */
-	if (ata_device_blacklisted(dev) & ATA_HORKAGE_DMA_RW_ONLY)
-		dev->horkage |= ATA_HORKAGE_DMA_RW_ONLY;
-
 	if (ap->ops->dev_config)
 		ap->ops->dev_config(dev);
 
@@ -3780,8 +3776,7 @@ static const struct ata_blacklist_entry 
 	{ "IOMEGA  ZIP 250       ATAPI", NULL,	ATA_HORKAGE_NODMA }, /* temporary fix */
 
 	/* Weird ATAPI devices */
-	{ "TORiSAN DVD-ROM DRD-N216", NULL,	ATA_HORKAGE_MAX_SEC_128 |
-						ATA_HORKAGE_DMA_RW_ONLY },
+	{ "TORiSAN DVD-ROM DRD-N216", NULL,	ATA_HORKAGE_MAX_SEC_128 },
 
 	/* Devices we expect to fail diagnostics */
 
Index: work/include/linux/libata.h
===================================================================
--- work.orig/include/linux/libata.h
+++ work/include/linux/libata.h
@@ -298,7 +298,6 @@ enum {
 	ATA_HORKAGE_NODMA	= (1 << 1),	/* DMA problems */
 	ATA_HORKAGE_NONCQ	= (1 << 2),	/* Don't use NCQ */
 	ATA_HORKAGE_MAX_SEC_128	= (1 << 3),	/* Limit max sects to 128 */
-	ATA_HORKAGE_DMA_RW_ONLY	= (1 << 4),	/* ATAPI DMA for RW only */
 };
 
 enum hsm_task_states {

Re: [PATCH 1/3] libata: call ata_check_atapi_dma() with qc better prepared

[Jeff Garzik]

Tejun Heo wrote:
> In atapi_xlat(), prepare qc better before calling
> ata_check_atapi_dma() such that ata_check_atapi_dma() can use info
> from qc.  While at it, reformat weird looking if/else block in the
> function.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-scsi.c |   20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)

applied 1-3