[PATCH 0/4] libata: CHS follow-up patches (revise #6)

[PATCH 0/4] libata: CHS follow-up patches (revise #6)

[Albert Lee]

Dear all,

CHS follow-up patches. (Keep sync with the scsi error handling patch. Also added patch 3/4.)

patch 1/4: move the initialization of ATA_TFLAG_LBA and ATA_TFLAG_LBA48 flags to the SCSI translation functions

patch 2/4: calculate read/write commands and protocols on the fly

patch 3/4: reread device identify info after initializing device CHS settings

patch 4/4: LBA28/LBA48 optimization
          (revised to check ending block number per Mark's advice)

Patch against libata-dev upstream branch
(edea3ab58f8edd5f72d31f891ab4f34382e97e3a).

For your review, thanks.

Albert

[PATCH 1/4] CHS: move the initialization of taskfile LBA flags (revise #6)

[Albert Lee]

Patch 1/4:

   move the initialization of taskfile LBA flags
     "ATA_TFLAG_LBA" and "ATA_TFLAG_LBA48 flags"
   to the SCSI translation functions

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>

=============

--- upstream/drivers/scsi/libata-core.c	2005-10-11 11:40:03.000000000 +0800
+++ ch1/drivers/scsi/libata-core.c	2005-10-11 12:44:53.000000000 +0800
@@ -3195,13 +3195,6 @@ struct ata_queued_cmd *ata_qc_new_init(s
 		qc->nbytes = qc->curbytes = 0;
 
 		ata_tf_init(ap, &qc->tf, dev->devno);
-
-		if (dev->flags & ATA_DFLAG_LBA) {
-			qc->tf.flags |= ATA_TFLAG_LBA;
-
-			if (dev->flags & ATA_DFLAG_LBA48)
-				qc->tf.flags |= ATA_TFLAG_LBA48;
-		}
 	}
 
 	return qc;
--- upstream/drivers/scsi/libata-scsi.c	2005-10-11 11:40:03.000000000 +0800
+++ ch1/drivers/scsi/libata-scsi.c	2005-10-11 12:53:36.000000000 +0800
@@ -492,7 +492,7 @@ static unsigned int ata_scsi_flush_xlat(
 	tf->flags |= ATA_TFLAG_DEVICE;
 	tf->protocol = ATA_PROT_NODATA;
 
-	if ((tf->flags & ATA_TFLAG_LBA48) &&
+	if ((qc->dev->flags & ATA_DFLAG_LBA48) &&
 	    (ata_id_has_flush_ext(qc->dev->id)))
 		tf->command = ATA_CMD_FLUSH_EXT;
 	else
@@ -612,8 +612,6 @@ static unsigned int ata_scsi_verify_xlat
 {
 	struct ata_taskfile *tf = &qc->tf;
 	struct ata_device *dev = qc->dev;
-	unsigned int lba   = tf->flags & ATA_TFLAG_LBA;
-	unsigned int lba48 = tf->flags & ATA_TFLAG_LBA48;
 	u64 dev_sectors = qc->dev->n_sectors;
 	u64 block;
 	u32 n_block;
@@ -634,16 +632,16 @@ static unsigned int ata_scsi_verify_xlat
 		goto out_of_range;
 	if ((block + n_block) > dev_sectors)
 		goto out_of_range;
-	if (lba48) {
-		if (n_block > (64 * 1024))
-			goto invalid_fld;
-	} else {
-		if (n_block > 256)
-			goto invalid_fld;
-	}
 
-	if (lba) {
-		if (lba48) {
+	if (dev->flags & ATA_DFLAG_LBA) {
+		tf->flags |= ATA_TFLAG_LBA;
+
+		if (dev->flags & ATA_DFLAG_LBA48) {
+			if (n_block > (64 * 1024))
+				goto invalid_fld;
+
+			/* use LBA48 */
+			tf->flags |= ATA_TFLAG_LBA48;
 			tf->command = ATA_CMD_VERIFY_EXT;
 
 			tf->hob_nsect = (n_block >> 8) & 0xff;
@@ -652,6 +650,10 @@ static unsigned int ata_scsi_verify_xlat
 			tf->hob_lbam = (block >> 32) & 0xff;
 			tf->hob_lbal = (block >> 24) & 0xff;
 		} else {
+			if (n_block > 256)
+				goto invalid_fld;
+
+			/* use LBA28 */
 			tf->command = ATA_CMD_VERIFY;
 
 			tf->device |= (block >> 24) & 0xf;
@@ -668,6 +670,9 @@ static unsigned int ata_scsi_verify_xlat
 		/* CHS */
 		u32 sect, head, cyl, track;
 
+		if (n_block > 256)
+			goto invalid_fld;
+
 		/* Convert LBA to CHS */
 		track = (u32)block / dev->sectors;
 		cyl   = track / dev->heads;
@@ -733,8 +738,6 @@ static unsigned int ata_scsi_rw_xlat(str
 {
 	struct ata_taskfile *tf = &qc->tf;
 	struct ata_device *dev = qc->dev;
-	unsigned int lba   = tf->flags & ATA_TFLAG_LBA;
-	unsigned int lba48 = tf->flags & ATA_TFLAG_LBA48;
 	u64 block;
 	u32 n_block;
 
@@ -783,19 +786,24 @@ static unsigned int ata_scsi_rw_xlat(str
 		 */
 		goto nothing_to_do;
 
-	if (lba) {
-		if (lba48) {
+	if (dev->flags & ATA_DFLAG_LBA) {
+		tf->flags |= ATA_TFLAG_LBA;
+
+		if (dev->flags & ATA_DFLAG_LBA48) {
 			/* The request -may- be too large for LBA48. */
 			if ((block >> 48) || (n_block > 65536))
 				goto out_of_range;
 
+			/* use LBA48 */
+			tf->flags |= ATA_TFLAG_LBA48;
+
 			tf->hob_nsect = (n_block >> 8) & 0xff;
 
 			tf->hob_lbah = (block >> 40) & 0xff;
 			tf->hob_lbam = (block >> 32) & 0xff;
 			tf->hob_lbal = (block >> 24) & 0xff;
 		} else { 
-			/* LBA28 */
+			/* use LBA28 */
 
 			/* The request -may- be too large for LBA28. */
 			if ((block >> 28) || (n_block > 256))

[PATCH 2/4] CHS: calculate read/write commands and protocol on the fly (revise #6)

[Albert Lee]

Patch 2/4: calculate read/write commands and protocols on the fly

changes:
     - merge ata_prot_to_cmd() and ata_dev_set_protocol() as
       ata_rwcmd_protocol()
     - pave road for read/write multiple support
     - remove usage of pre-cached command and protocol values and call
       ata_rwcmd_protocol() instead

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>

==============

--- upstream/include/linux/ata.h	2005-10-11 11:40:08.000000000 +0800
+++ ch2/include/linux/ata.h	2005-10-11 12:56:56.000000000 +0800
@@ -128,6 +128,10 @@ enum {
 	ATA_CMD_PIO_READ_EXT	= 0x24,
 	ATA_CMD_PIO_WRITE	= 0x30,
 	ATA_CMD_PIO_WRITE_EXT	= 0x34,
+	ATA_CMD_READ_MULTI	= 0xC4,
+	ATA_CMD_READ_MULTI_EXT	= 0x29,
+	ATA_CMD_WRITE_MULTI	= 0xC5,
+	ATA_CMD_WRITE_MULTI_EXT	= 0x39,
 	ATA_CMD_SET_FEATURES	= 0xEF,
 	ATA_CMD_PACKET		= 0xA0,
 	ATA_CMD_VERIFY		= 0x40,
--- upstream/include/linux/libata.h	2005-10-11 11:40:09.000000000 +0800
+++ ch2/include/linux/libata.h	2005-10-11 12:58:12.000000000 +0800
@@ -283,10 +283,8 @@ struct ata_device {
 	u8			xfer_mode;
 	unsigned int		xfer_shift;	/* ATA_SHIFT_xxx */
 
-	/* cache info about current transfer mode */
-	u8			xfer_protocol;	/* taskfile xfer protocol */
-	u8			read_cmd;	/* opcode to use on read */
-	u8			write_cmd;	/* opcode to use on write */
+	unsigned int		multi_count;	/* sectors count for
+						   READ/WRITE MULTIPLE */
 
 	/* for CHS addressing */
 	u16			cylinders;	/* Number of cylinders */
--- upstream/drivers/scsi/libata.h	2005-10-11 11:40:03.000000000 +0800
+++ ch2/drivers/scsi/libata.h	2005-10-11 12:59:09.000000000 +0800
@@ -42,6 +42,7 @@ extern int atapi_enabled;
 extern int ata_qc_complete_noop(struct ata_queued_cmd *qc, u8 drv_stat);
 extern struct ata_queued_cmd *ata_qc_new_init(struct ata_port *ap,
 				      struct ata_device *dev);
+extern void ata_rwcmd_protocol(struct ata_queued_cmd *qc);
 extern void ata_qc_free(struct ata_queued_cmd *qc);
 extern int ata_qc_issue(struct ata_queued_cmd *qc);
 extern int ata_check_atapi_dma(struct ata_queued_cmd *qc);
--- ch1/drivers/scsi/libata-core.c	2005-10-11 12:44:53.000000000 +0800
+++ ch2/drivers/scsi/libata-core.c	2005-10-11 13:01:05.000000000 +0800
@@ -616,79 +616,53 @@ void ata_tf_from_fis(u8 *fis, struct ata
 	tf->hob_nsect	= fis[13];
 }
 
-/**
- *	ata_prot_to_cmd - determine which read/write opcodes to use
- *	@protocol: ATA_PROT_xxx taskfile protocol
- *	@lba48: true is lba48 is present
- *
- *	Given necessary input, determine which read/write commands
- *	to use to transfer data.
- *
- *	LOCKING:
- *	None.
- */
-static int ata_prot_to_cmd(int protocol, int lba48)
-{
-	int rcmd = 0, wcmd = 0;
-
-	switch (protocol) {
-	case ATA_PROT_PIO:
-		if (lba48) {
-			rcmd = ATA_CMD_PIO_READ_EXT;
-			wcmd = ATA_CMD_PIO_WRITE_EXT;
-		} else {
-			rcmd = ATA_CMD_PIO_READ;
-			wcmd = ATA_CMD_PIO_WRITE;
-		}
-		break;
-
-	case ATA_PROT_DMA:
-		if (lba48) {
-			rcmd = ATA_CMD_READ_EXT;
-			wcmd = ATA_CMD_WRITE_EXT;
-		} else {
-			rcmd = ATA_CMD_READ;
-			wcmd = ATA_CMD_WRITE;
-		}
-		break;
-
-	default:
-		return -1;
-	}
-
-	return rcmd | (wcmd << 8);
-}
+static const u8 ata_rw_cmds[] = {
+	/* pio multi */
+	ATA_CMD_READ_MULTI,
+	ATA_CMD_WRITE_MULTI,
+	ATA_CMD_READ_MULTI_EXT,
+	ATA_CMD_WRITE_MULTI_EXT,
+	/* pio */
+	ATA_CMD_PIO_READ,
+	ATA_CMD_PIO_WRITE,
+	ATA_CMD_PIO_READ_EXT,
+	ATA_CMD_PIO_WRITE_EXT,
+	/* dma */
+	ATA_CMD_READ,
+	ATA_CMD_WRITE,
+	ATA_CMD_READ_EXT,
+	ATA_CMD_WRITE_EXT
+};
 
 /**
- *	ata_dev_set_protocol - set taskfile protocol and r/w commands
- *	@dev: device to examine and configure
+ *	ata_rwcmd_protocol - set taskfile r/w commands and protocol
+ *	@qc: command to examine and configure
  *
- *	Examine the device configuration, after we have
- *	read the identify-device page and configured the
- *	data transfer mode.  Set internal state related to
- *	the ATA taskfile protocol (pio, pio mult, dma, etc.)
- *	and calculate the proper read/write commands to use.
+ *	Examine the device configuration and tf->flags to calculate 
+ *	the proper read/write commands and protocol to use.
  *
  *	LOCKING:
  *	caller.
  */
-static void ata_dev_set_protocol(struct ata_device *dev)
+void ata_rwcmd_protocol(struct ata_queued_cmd *qc)
 {
-	int pio = (dev->flags & ATA_DFLAG_PIO);
-	int lba48 = (dev->flags & ATA_DFLAG_LBA48);
-	int proto, cmd;
-
-	if (pio)
-		proto = dev->xfer_protocol = ATA_PROT_PIO;
-	else
-		proto = dev->xfer_protocol = ATA_PROT_DMA;
+	struct ata_taskfile *tf = &qc->tf;
+	struct ata_device *dev = qc->dev;
 
-	cmd = ata_prot_to_cmd(proto, lba48);
-	if (cmd < 0)
-		BUG();
+	int index, lba48, write;
+ 
+	lba48 = (tf->flags & ATA_TFLAG_LBA48) ? 2 : 0;
+	write = (tf->flags & ATA_TFLAG_WRITE) ? 1 : 0;
+
+	if (dev->flags & ATA_DFLAG_PIO) {
+		tf->protocol = ATA_PROT_PIO;
+		index = dev->multi_count ? 0 : 4;
+	} else {
+		tf->protocol = ATA_PROT_DMA;
+		index = 8;
+	}
 
-	dev->read_cmd = cmd & 0xff;
-	dev->write_cmd = (cmd >> 8) & 0xff;
+	tf->command = ata_rw_cmds[index + lba48 + write];
 }
 
 static const char * xfer_mode_str[] = {
@@ -1641,7 +1615,7 @@ static void ata_host_set_dma(struct ata_
  */
 static void ata_set_mode(struct ata_port *ap)
 {
-	unsigned int i, xfer_shift;
+	unsigned int xfer_shift;
 	u8 xfer_mode;
 	int rc;
 
@@ -1670,11 +1644,6 @@ static void ata_set_mode(struct ata_port
 	if (ap->ops->post_set_mode)
 		ap->ops->post_set_mode(ap);
 
-	for (i = 0; i < 2; i++) {
-		struct ata_device *dev = &ap->device[i];
-		ata_dev_set_protocol(dev);
-	}
-
 	return;
 
 err_out:
--- ch1/drivers/scsi/libata-scsi.c	2005-10-11 12:53:36.000000000 +0800
+++ ch2/drivers/scsi/libata-scsi.c	2005-10-11 13:03:10.000000000 +0800
@@ -742,15 +742,10 @@ static unsigned int ata_scsi_rw_xlat(str
 	u32 n_block;
 
 	tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
-	tf->protocol = qc->dev->xfer_protocol;
 
-	if (scsicmd[0] == READ_10 || scsicmd[0] == READ_6 ||
-	    scsicmd[0] == READ_16) {
-		tf->command = qc->dev->read_cmd;
-	} else {
-		tf->command = qc->dev->write_cmd;
+	if (scsicmd[0] == WRITE_10 || scsicmd[0] == WRITE_6 ||
+	    scsicmd[0] == WRITE_16)
 		tf->flags |= ATA_TFLAG_WRITE;
-	}
 
 	/* Calculate the SCSI LBA and transfer length. */
 	switch (scsicmd[0]) {
@@ -812,6 +807,8 @@ static unsigned int ata_scsi_rw_xlat(str
 			tf->device |= (block >> 24) & 0xf;
 		}
 
+		ata_rwcmd_protocol(qc);
+
 		qc->nsect = n_block;
 		tf->nsect = n_block & 0xff;
 
@@ -828,6 +825,8 @@ static unsigned int ata_scsi_rw_xlat(str
 		if ((block >> 28) || (n_block > 256))
 			goto out_of_range;
 
+		ata_rwcmd_protocol(qc);
+
 		/* Convert LBA to CHS */
 		track = (u32)block / dev->sectors;
 		cyl   = track / dev->heads;

[PATCH 4/4] CHS: LBA28/LBA48 optimization (revise #6)

[Albert Lee]

Patch 4/4: LBA28/LBA48 optimization per Mark's advice.

Changes:
     - add lba_28_ok() and lba_48_ok() to ata.h.
     - check ending block number instead of staring block number.
     - use lba_28_ok() for CHS range check
     - LBA28/LBA48 optimization

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>

=====

--- ch2/include/linux/ata.h	2005-10-11 12:56:56.000000000 +0800
+++ ch4/include/linux/ata.h	2005-10-12 14:26:59.000000000 +0800
@@ -291,4 +291,16 @@ static inline int ata_ok(u8 status)
 			== ATA_DRDY);
 }
 
+static inline int lba_28_ok(u64 block, u32 n_block)
+{
+	/* check the ending block number */
+	return ((block + n_block - 1) < ((u64)1 << 28)) && (n_block <= 256);
+}
+
+static inline int lba_48_ok(u64 block, u32 n_block)
+{
+	/* check the ending block number */
+	return ((block + n_block - 1) < ((u64)1 << 48)) && (n_block <= 65536);
+}
+
 #endif /* __LINUX_ATA_H__ */
--- ch2/drivers/scsi/libata-scsi.c	2005-10-11 13:03:10.000000000 +0800
+++ ch4/drivers/scsi/libata-scsi.c	2005-10-11 13:13:19.000000000 +0800
@@ -636,9 +636,13 @@ static unsigned int ata_scsi_verify_xlat
 	if (dev->flags & ATA_DFLAG_LBA) {
 		tf->flags |= ATA_TFLAG_LBA;
 
-		if (dev->flags & ATA_DFLAG_LBA48) {
-			if (n_block > (64 * 1024))
-				goto invalid_fld;
+		if (lba_28_ok(block, n_block)) {
+			/* use LBA28 */
+			tf->command = ATA_CMD_VERIFY;
+			tf->device |= (block >> 24) & 0xf;
+		} else if (lba_48_ok(block, n_block)) {
+			if (!(dev->flags & ATA_DFLAG_LBA48))
+				goto out_of_range;
 
 			/* use LBA48 */
 			tf->flags |= ATA_TFLAG_LBA48;
@@ -649,15 +653,9 @@ static unsigned int ata_scsi_verify_xlat
 			tf->hob_lbah = (block >> 40) & 0xff;
 			tf->hob_lbam = (block >> 32) & 0xff;
 			tf->hob_lbal = (block >> 24) & 0xff;
-		} else {
-			if (n_block > 256)
-				goto invalid_fld;
-
-			/* use LBA28 */
-			tf->command = ATA_CMD_VERIFY;
-
-			tf->device |= (block >> 24) & 0xf;
-		}
+		} else
+			/* request too large even for LBA48 */
+			goto out_of_range;
 
 		tf->nsect = n_block & 0xff;
 
@@ -670,8 +668,8 @@ static unsigned int ata_scsi_verify_xlat
 		/* CHS */
 		u32 sect, head, cyl, track;
 
-		if (n_block > 256)
-			goto invalid_fld;
+		if (!lba_28_ok(block, n_block))
+			goto out_of_range;
 
 		/* Convert LBA to CHS */
 		track = (u32)block / dev->sectors;
@@ -784,9 +782,11 @@ static unsigned int ata_scsi_rw_xlat(str
 	if (dev->flags & ATA_DFLAG_LBA) {
 		tf->flags |= ATA_TFLAG_LBA;
 
-		if (dev->flags & ATA_DFLAG_LBA48) {
-			/* The request -may- be too large for LBA48. */
-			if ((block >> 48) || (n_block > 65536))
+		if (lba_28_ok(block, n_block)) {
+			/* use LBA28 */
+			tf->device |= (block >> 24) & 0xf;
+		} else if (lba_48_ok(block, n_block)) {
+			if (!(dev->flags & ATA_DFLAG_LBA48))
 				goto out_of_range;
 
 			/* use LBA48 */
@@ -797,15 +797,9 @@ static unsigned int ata_scsi_rw_xlat(str
 			tf->hob_lbah = (block >> 40) & 0xff;
 			tf->hob_lbam = (block >> 32) & 0xff;
 			tf->hob_lbal = (block >> 24) & 0xff;
-		} else { 
-			/* use LBA28 */
-
-			/* The request -may- be too large for LBA28. */
-			if ((block >> 28) || (n_block > 256))
-				goto out_of_range;
-
-			tf->device |= (block >> 24) & 0xf;
-		}
+		} else
+			/* request too large even for LBA48 */
+			goto out_of_range;
 
 		ata_rwcmd_protocol(qc);
 
@@ -822,7 +816,7 @@ static unsigned int ata_scsi_rw_xlat(str
 		u32 sect, head, cyl, track;
 
 		/* The request -may- be too large for CHS addressing. */
-		if ((block >> 28) || (n_block > 256))
+		if (!lba_28_ok(block, n_block))
 			goto out_of_range;
 
 		ata_rwcmd_protocol(qc);

Re: [PATCH 1/4] CHS: move the initialization of taskfile LBA flags (revise #6)

[Jeff Garzik]

applied patches 1-3 to upstream branch

Re: [PATCH 4/4] CHS: LBA28/LBA48 optimization (revise #6)

[Jeff Garzik]

Albert Lee wrote:
> Patch 4/4: LBA28/LBA48 optimization per Mark's advice.
> 
> Changes:
>      - add lba_28_ok() and lba_48_ok() to ata.h.
>      - check ending block number instead of staring block number.
>      - use lba_28_ok() for CHS range check
>      - LBA28/LBA48 optimization
> 
> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>

applied to the new lba48-opt branch of libata-dev.

We'll probably merge this into the 'upstream' branch after 2.6.15-rc1 is 
released, to enable lots of testing.

	Jeff