[PATCH] libata: Fix the HSM error_mask mapping (was: Re: libata-tj and SMART)

Fix the HSM error_mask mapping.

Changes:
- Better mapping in ac_err_mask()
- In HSM_ST_FIRST ans HSM_ST state, check ATA_ERR|ATA_DF and map it to AC_ERR_DEV instead of AC_ERR_HSM.
- In HSM_ST_FIRST and HSM_ST state, map DRQ=1 ERR=1 to AC_ERR_HSM.
- For PIO data in and DRQ=1 ERR=1, add check after the junk data block is read.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>

2 files changed, 25 insertions, 8 deletions

diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 00881226f8dd..aa38ed3e59a8 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -4009,9 +4009,15 @@ fsm_start:
                 poll_next = (qc->tf.flags & ATA_TFLAG_POLLING);
 
                 /* check device status */
-                if (unlikely((status & (ATA_BUSY | ATA_DRQ)) != ATA_DRQ)) {
-                        /* Wrong status. Let EH handle this */
-                        qc->err_mask |= AC_ERR_HSM;
+                if (unlikely((status & ATA_DRQ) == 0)) {
+                        /* handle BSY=0, DRQ=0 as error */
+                        if (likely(status & (ATA_ERR | ATA_DF)))
+                                /* device stops HSM for abort/error */
+                                qc->err_mask |= AC_ERR_DEV;
+                        else
+                                /* HSM violation. Let EH handle this */
+                                qc->err_mask |= AC_ERR_HSM;
+
                         ap->hsm_task_state = HSM_ST_ERR;
                         goto fsm_start;
                 }
@@ -4025,7 +4031,7 @@ fsm_start:
                 if (unlikely(status & (ATA_ERR | ATA_DF))) {
                         printk(KERN_WARNING "ata%d: DRQ=1 with device error, dev_stat 0x%X\n",
                                ap->id, status);
-                        qc->err_mask |= AC_ERR_DEV;
+                        qc->err_mask |= AC_ERR_HSM;
                         ap->hsm_task_state = HSM_ST_ERR;
                         goto fsm_start;
                 }
@@ -4067,7 +4073,9 @@ fsm_start:
                 if (qc->tf.protocol == ATA_PROT_ATAPI) {
                         /* ATAPI PIO protocol */
                         if ((status & ATA_DRQ) == 0) {
-                                /* no more data to transfer */
+                                /* No more data to transfer or device error.
+                                 * Device error will be tagged in HSM_ST_LAST.
+                                 */
                                 ap->hsm_task_state = HSM_ST_LAST;
                                 goto fsm_start;
                         }
@@ -4081,7 +4089,7 @@ fsm_start:
                         if (unlikely(status & (ATA_ERR | ATA_DF))) {
                                 printk(KERN_WARNING "ata%d: DRQ=1 with device error, dev_stat 0x%X\n",
                                        ap->id, status);
-                                qc->err_mask |= AC_ERR_DEV;
+                                qc->err_mask |= AC_ERR_HSM;
                                 ap->hsm_task_state = HSM_ST_ERR;
                                 goto fsm_start;
                         }
@@ -4096,7 +4104,13 @@ fsm_start:
                         /* ATA PIO protocol */
                         if (unlikely((status & ATA_DRQ) == 0)) {
                                 /* handle BSY=0, DRQ=0 as error */
-                                qc->err_mask |= AC_ERR_HSM;
+                                if (likely(status & (ATA_ERR | ATA_DF)))
+                                        /* device stops HSM for abort/error */
+                                        qc->err_mask |= AC_ERR_DEV;
+                                else
+                                        /* HSM violation. Let EH handle this */
+                                        qc->err_mask |= AC_ERR_HSM;
+
                                 ap->hsm_task_state = HSM_ST_ERR;
                                 goto fsm_start;
                         }
@@ -4121,6 +4135,9 @@ fsm_start:
                                         status = ata_wait_idle(ap);
                                 }
 
+                                if (status & (ATA_BUSY | ATA_DRQ))
+                                        qc->err_mask |= AC_ERR_HSM;
+
                                 /* ata_pio_sectors() might change the
                                  * state to HSM_ST_LAST. so, the state
                                  * is changed after ata_pio_sectors().
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2803ab8e9243..c51502c047a4 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1062,7 +1062,7 @@ static inline int ata_try_flush_cache(const struct ata_device *dev)
 
 static inline unsigned int ac_err_mask(u8 status)
 {
-        if (status & ATA_BUSY)
+        if (status & (ATA_BUSY | ATA_DRQ))
                 return AC_ERR_HSM;
         if (status & (ATA_ERR | ATA_DF))
                 return AC_ERR_DEV;