Jeff Garzik wrote:

Albert Lee wrote:

quoted

Only one minor addition for your review:

* ata_pio_block() changed to go to PIO_ST_LAST state, instead of
  going to PIO_ST_IDLE state directly and calling 
ata_poll_qc_complete().

i.e.

@@ -2845,9 +2852,7 @@
    if (is_atapi_taskfile(&qc->tf)) {
        /* no more data to transfer or unsupported ATAPI command */
        if ((status & ATA_DRQ) == 0) {
-            ap->pio_task_state = PIO_ST_IDLE;
-
-            ata_poll_qc_complete(qc, status);
+            ap->pio_task_state = PIO_ST_LAST;
            return;
        }

hmmmm.  I think that should be PIO_ST_ERR not PIO_ST_LAST.  Comments?

Just tested it. Changing to PIO_ST_ERR will break REQUEST_SENSE and 
MODE_SENSE. :(
For REQUEST_SENSE and MODE_SENSE, the data returned from the device might be
less than the buffer provided and those two commands relies on DRQ == 0 
to mark the end of
the data transfer.

e.g. a sample REQUST SENSE transaction:
 1. ata_pio_task() entered with PIO_ST.
 2. ata_pio_block() called, 24 bytes received from the device.
 3. The buffer size is 96 bytes, so the state is kept as PIO_ST.
 4. ata_pio_task() entered with PIO_ST.
 5. ata_pio_block() called, DRQ == 0 to mark the end of data transfer.
 6. state changed to PIO_ST_LAST and command completed as successful.

DRQ == 0 in step 5 seems to be normal and should be treated as OK.

Albert