make the returned err_mask more accurate
Changes:
- add ac_err_drq() and ac_err_idle() to map err_mask from drv_status for data xfer and idle states.
- fix ata_hsm_move() to use the functions for err_mask mapping.
Signed-off-by: Albert Lee <albertcc@xxxxxxxxxx>
---
The current err_mask returned by HSM doesn't look right.
e.g. When we found DRQ=0 when data transfer is expected, AC_ERR_HSM is returned.
However, DRQ=0 maybe caused by device error. So, during data transfer,
if we see DRQ=0 and ERR=1, returning AC_ERR_DEV should be more accurate.
e.g. In HSM_ST_LAST state, AC_ERR_OTHER is returned for BSY=0 DRQ=1 currently.
AC_ERR_HSM shoule be more accurate.
Patch against irq-pio (79fa1b677be3a985cc66b9218a4dd09818f1051b).
For your review, thanks.
--- irq-pio0/drivers/scsi/libata-core.c 2006-04-06 10:08:44.000000000 +0800
+++ irq-pio1/drivers/scsi/libata-core.c 2006-04-06 11:20:15.000000000 +0800
@@ -3906,7 +3906,7 @@ fsm_start:
/* check device status */
if (unlikely((status & (ATA_BUSY | ATA_DRQ)) != ATA_DRQ)) {
/* Wrong status. Let EH handle this */
- qc->err_mask |= AC_ERR_HSM;
+ qc->err_mask |= ac_err_drq(status);
ap->hsm_task_state = HSM_ST_ERR;
goto fsm_start;
}
@@ -3920,7 +3920,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;
}
@@ -3976,7 +3976,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;
}
@@ -3991,7 +3991,7 @@ fsm_start:
/* ATA PIO protocol */
if (unlikely((status & ATA_DRQ) == 0)) {
/* handle BSY=0, DRQ=0 as error */
- qc->err_mask |= AC_ERR_HSM;
+ qc->err_mask |= ac_err_drq(status);
ap->hsm_task_state = HSM_ST_ERR;
goto fsm_start;
}
@@ -4007,13 +4007,16 @@ fsm_start:
* transferred to the device.
*/
if (unlikely(status & (ATA_ERR | ATA_DF))) {
- /* data might be corrputed */
- qc->err_mask |= AC_ERR_DEV;
- if (!(qc->tf.flags & ATA_TFLAG_WRITE)) {
+ if (qc->tf.flags & ATA_TFLAG_WRITE)
+ qc->err_mask |= AC_ERR_HSM;
+ else {
+ /* data might be corrputed */
+ qc->err_mask |= AC_ERR_DEV;
ata_pio_sectors(qc);
ata_altstatus(ap);
status = ata_wait_idle(ap);
+ qc->err_mask |= ac_err_idle(status);
}
/* ata_pio_sectors() might change the
@@ -4041,7 +4044,7 @@ fsm_start:
case HSM_ST_LAST:
if (unlikely(!ata_ok(status))) {
- qc->err_mask |= __ac_err_mask(status);
+ qc->err_mask |= ac_err_idle(status);
ap->hsm_task_state = HSM_ST_ERR;
goto fsm_start;
}
--- irq-pio0/include/linux/libata.h 2006-04-06 10:08:48.000000000 +0800
+++ irq-pio1/include/linux/libata.h 2006-04-06 13:44:25.000000000 +0800
@@ -942,6 +942,46 @@ static inline int ata_try_flush_cache(co
ata_id_has_flush_ext(dev->id);
}
+static inline unsigned int ac_err_drq(u8 status)
+{
+ /* BSY = 1*/
+ if (status & ATA_BUSY)
+ return AC_ERR_HSM;
+
+ /* BSY = 0, DRQ = 1 */
+ if (status & ATA_DRQ)
+ return 0;
+
+ /* BSY = 0, DRQ = 0, device err */
+ if (status & (ATA_ERR | ATA_DF))
+ return AC_ERR_DEV;
+
+ /* BSY = 0, DRQ = 0, no device err
+ * when we are expecting data transfer.
+ */
+ return AC_ERR_HSM;
+}
+
+static inline unsigned int ac_err_idle(u8 status)
+{
+ /* BSY = 1 or DRQ = 1 */
+ if (status & (ATA_BUSY | ATA_DRQ))
+ return AC_ERR_HSM;
+
+ /* BSY = 0, DRQ = 0, device err */
+ if (status & (ATA_ERR | ATA_DF))
+ return AC_ERR_DEV;
+
+ /* BSY = 0, DRQ = 0, no device err. DRDY = 1 */
+ if (status & ATA_DRDY)
+ return 0;
+
+ /* thing looks good.
+ * don't know why DRDY not set.
+ */
+ return AC_ERR_OTHER
+}
+
static inline unsigned int ac_err_mask(u8 status)
{
if (status & ATA_BUSY)
-
: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
