Skip to content

Commit

Permalink
[PATCH] libata-dev: fix the device err check sequence (respin)
Browse files Browse the repository at this point in the history
Current irq-pio checks ERR bit and stops on ERR before it does anything else.
This behavior doesn't look right.
The DRQ bit should take higher precedence than the ERR bit.

Changes:
- Let the HSM do the data transfer whenever the
  device asks for DRQ bit, even if the ERR bit is set.
- For DRQ=1 ERR=1, don't trust the data

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
  • Loading branch information
Albert Lee authored and Jeff Garzik committed Mar 29, 2006
1 parent 999bb6f commit 7160195
Showing 1 changed file with 38 additions and 6 deletions.
44 changes: 38 additions & 6 deletions drivers/scsi/libata-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -3587,12 +3587,6 @@ static int ata_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
*/
WARN_ON(in_wq != ata_hsm_ok_in_wq(ap, qc));

/* check error */
if (unlikely(status & (ATA_ERR | ATA_DF))) {
qc->err_mask |= AC_ERR_DEV;
ap->hsm_task_state = HSM_ST_ERR;
}

fsm_start:
DPRINTK("ata%u: protocol %d task_state %d (dev_stat 0x%X)\n",
ap->id, qc->tf.protocol, ap->hsm_task_state, status);
Expand All @@ -3615,6 +3609,17 @@ static int ata_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
goto fsm_start;
}

/* Device should not ask for data transfer (DRQ=1)
* when it finds something wrong.
* Anyway, we respect DRQ here and let HSM go on
* without changing hsm_task_state to HSM_ST_ERR.
*/
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;
}

/* Send the CDB (atapi) or the first data block (ata pio out).
* During the state transition, interrupt handler shouldn't
* be invoked before the data transfer is complete and
Expand Down Expand Up @@ -3657,6 +3662,17 @@ static int ata_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
goto fsm_start;
}

/* Device should not ask for data transfer (DRQ=1)
* when it finds something wrong.
* Anyway, we respect DRQ here and let HSM go on
* without changing hsm_task_state to HSM_ST_ERR.
*/
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;
}

atapi_pio_bytes(qc);

if (unlikely(ap->hsm_task_state == HSM_ST_ERR))
Expand All @@ -3672,6 +3688,22 @@ static int ata_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
goto fsm_start;
}

/* Some devices may ask for data transfer (DRQ=1)
* alone with ERR=1 for PIO reads.
* We respect DRQ here and let HSM go on without
* changing hsm_task_state to HSM_ST_ERR.
*/
if (unlikely(status & (ATA_ERR | ATA_DF))) {
/* For writes, ERR=1 DRQ=1 doesn't make
* sense since the data block has been
* transferred to the device.
*/
WARN_ON(qc->tf.flags & ATA_TFLAG_WRITE);

/* data might be corrputed */
qc->err_mask |= AC_ERR_DEV;
}

ata_pio_sectors(qc);

if (ap->hsm_task_state == HSM_ST_LAST &&
Expand Down

0 comments on commit 7160195

Please sign in to comment.