[PATCH 0/3] libata: minor pio fixes

[PATCH 0/3] libata: minor pio fixes

[Albert Lee]

Minor pio fixes:
1/3: remove unneeded ata_altstatus() from ata_hsm_qc_complete()
2/3: move the ata_altstatus() to pio data xfer functions
3/3: change the last state of pio read to HSM_ST_IDLE

Patch against the libata-dev tree for your review.

[PATCH 1/3] libata: remove unneeded ata_altstatus() from ata_hsm_qc_complete()

[Albert Lee]

Patch 1/3:
In ata_hsm_qc_complete():
Calling ata_altstatus() after the qc completed looks wrong.
Remove it.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
After checking, it is leftover of ata_pio_block( ) in 2.6.17-rc5 of the following merge:
http://git.kernel.org/?p=linux/kernel/git/jgarzik/libata-dev.git;a=commitdiff;h=bb31a8faa270beafcc51a65880c5564c6b718bd6
We can safely remove it.

diff -Nrup 00_libata-dev/drivers/ata/libata-core.c 01_remove_bad_flush/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c	2007-06-01 12:08:21.000000000 +0800
+++ 01_remove_bad_flush/drivers/ata/libata-core.c	2007-06-04 18:10:43.000000000 +0800
@@ -4782,8 +4782,6 @@ static void ata_hsm_qc_complete(struct a
 		} else
 			ata_qc_complete(qc);
 	}
-
-	ata_altstatus(ap); /* flush */
 }
 
 /**

[PATCH 2/3] libata: move ata_altstatus() to pio data xfer functions

[Albert Lee]

Patch 2/3:
 Move ata_altstatus() out from ata_hsm_move() to the pio data xfer functions
such as ata_pio_sectors() and atapi_pio_bytes() where it makes more sense.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
atapi_send_cdb() already calls ata_altstatus() inside.
This patch makes ata_pio_sectors() and atapi_pio_bytes() do the same for consistency.

diff -Nrup 01_remove_bad_flush/drivers/ata/libata-core.c 02_move_altstatus/drivers/ata/libata-core.c
--- 01_remove_bad_flush/drivers/ata/libata-core.c	2007-06-04 18:10:43.000000000 +0800
+++ 02_move_altstatus/drivers/ata/libata-core.c	2007-06-04 18:16:24.000000000 +0800
@@ -4524,6 +4524,8 @@ static void ata_pio_sectors(struct ata_q
 			ata_pio_sector(qc);
 	} else
 		ata_pio_sector(qc);
+
+	ata_altstatus(ap); /* flush */
 }
 
 /**
@@ -4698,6 +4700,7 @@ static void atapi_pio_bytes(struct ata_q
 	VPRINTK("ata%u: xfering %d bytes\n", ap->print_id, bytes);
 
 	__atapi_pio_bytes(qc, bytes);
+	ata_altstatus(ap); /* flush */
 
 	return;
 
@@ -4869,7 +4872,6 @@ fsm_start:
 			 */
 			ap->hsm_task_state = HSM_ST;
 			ata_pio_sectors(qc);
-			ata_altstatus(ap); /* flush */
 		} else
 			/* send CDB */
 			atapi_send_cdb(ap, qc);
@@ -4950,7 +4952,6 @@ fsm_start:
 
 				if (!(qc->tf.flags & ATA_TFLAG_WRITE)) {
 					ata_pio_sectors(qc);
-					ata_altstatus(ap);
 					status = ata_wait_idle(ap);
 				}
 
@@ -4970,13 +4971,11 @@ fsm_start:
 			if (ap->hsm_task_state == HSM_ST_LAST &&
 			    (!(qc->tf.flags & ATA_TFLAG_WRITE))) {
 				/* all data read */
-				ata_altstatus(ap);
 				status = ata_wait_idle(ap);
 				goto fsm_start;
 			}
 		}
 
-		ata_altstatus(ap); /* flush */
 		poll_next = 1;
 		break;
 

[PATCH 3/3] libata: change the last state of pio read to HSM_ST_IDLE

[Albert Lee]

Patch 3/3:
  After reading the last pio block, the HSM is waiting for device
to be idle, not waiting for the last interrupt.

Change the state after "PIO data-in" to HSM_ST_IDLE instead of HSM_ST_LAST for accuracy.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
diff -Nrup 02_move_altstatus/drivers/ata/libata-core.c 03_read_state/drivers/ata/libata-core.c
--- 02_move_altstatus/drivers/ata/libata-core.c	2007-06-04 18:16:24.000000000 +0800
+++ 03_read_state/drivers/ata/libata-core.c	2007-06-04 18:25:00.000000000 +0800
@@ -4462,7 +4462,7 @@ static void ata_pio_sector(struct ata_qu
 	unsigned char *buf;
 
 	if (qc->curbytes == qc->nbytes - qc->sect_size)
-		ap->hsm_task_state = HSM_ST_LAST;
+		ap->hsm_task_state = do_write ? HSM_ST_LAST : HSM_ST_IDLE;
 
 	page = sg[qc->cursg].page;
 	offset = sg[qc->cursg].offset + qc->cursg_ofs;
@@ -4811,6 +4811,8 @@ int ata_hsm_move(struct ata_port *ap, st
 	 */
 	WARN_ON(in_wq != ata_hsm_ok_in_wq(ap, qc));
 
+	WARN_ON(ap->hsm_task_state == HSM_ST_IDLE);
+
 fsm_start:
 	DPRINTK("ata%u: protocol %d task_state %d (dev_stat 0x%X)\n",
 		ap->print_id, qc->tf.protocol, ap->hsm_task_state, status);
@@ -4968,8 +4970,7 @@ fsm_start:
 
 			ata_pio_sectors(qc);
 
-			if (ap->hsm_task_state == HSM_ST_LAST &&
-			    (!(qc->tf.flags & ATA_TFLAG_WRITE))) {
+			if (ap->hsm_task_state == HSM_ST_IDLE) {
 				/* all data read */
 				status = ata_wait_idle(ap);
 				goto fsm_start;
@@ -4980,6 +4981,7 @@ fsm_start:
 		break;
 
 	case HSM_ST_LAST:
+	case HSM_ST_IDLE:
 		if (unlikely(!ata_ok(status))) {
 			qc->err_mask |= __ac_err_mask(status);
 			ap->hsm_task_state = HSM_ST_ERR;