Jeff Garzik wrote:
> Tejun Heo wrote:
>
>> It seems that HSM_ST needs to handle !DRQ && ERR case before the first
>> iteration (or maybe it should be pushed into HSM_ST_FIRST?). Does my
>> analysis make sense?
>
>
> Agreed, though we should check for ERR on ATAPI path first, too, IMO.
>
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@xxxxxxxxxx>
---
The device might stop the HSM by BSY=0 DRQ=1 ERR=1 when it finds something wrong.
Map this condition to AC_ERR_DEV for accuracy.
This should fix the problem of SMART reported by Nicolas.
(patch against upstream branch.)
For your review, thanks.
--- upstream0/include/linux/libata.h 2006-05-16 11:08:54.000000000 +0800
+++ 201_hsm_error/include/linux/libata.h 2006-05-19 11:33:01.000000000 +0800
@@ -1062,7 +1062,7 @@ static inline int ata_try_flush_cache(co
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;
--- upstream0/drivers/scsi/libata-core.c 2006-05-16 11:08:49.000000000 +0800
+++ 201_hsm_error/drivers/scsi/libata-core.c 2006-05-19 11:41:26.000000000 +0800
@@ -4006,9 +4006,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;
}
@@ -4022,7 +4028,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;
}
@@ -4064,7 +4070,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;
}
@@ -4078,7 +4086,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;
}
@@ -4093,7 +4101,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;
}
@@ -4118,6 +4132,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().
-
: 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
