On the subject of DRQ and EH...
grep for try_to_flush_leftover_data in drivers/ide.
Jeff
Quoted 2 lines · Jeff Garzik
grep for
try_to_flush_leftover_dataindrivers/ide.
Thanks for the pointer. Patch to follow.
--
Albert
[PATCH/RFC] libata-dev: handle DRQ=1 ERR=1 (revised)
Handle DRQ=1 ERR=1 situation. Revised according to what IDE try_to_flush_leftover_data() does.
Changes:
- For ATA PIO writes and ATAPI devices, just stop the HSM and let EH handle it.
- For ATA PIO reads, read only one block of junk data and then let EH handle it.
---
Thanks for the IDE try_to_flush_leftover_data() pointer.
The difference between IDE and irq-pio (before this patch):
1. IDE only does the data transfer for PIO reads.
For PIO writes and ATAPI, it just stops and abort/reset.
2. For PIO reads, only one block of data is transferred. EH then takes over if DRQ is still set after that. The behavior of irq-pio before this patch is: let HSM go on, until the device set DRQ=0 or all data read.
Te IDE behavior looks safer. (When the device is in bad status, we really should not write any data to it.)
Patch against irq-pio branch (08a556db919f67e1e4d33ae8d40f7222da34d994). For your review, thanks.
drivers/scsi/libata-core.c +32 -14
@@ -3651,13 +3651,16 @@ 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. +* We ignore DRQ here and stop the HSM by +* changing hsm_task_state to HSM_ST_ERR and +* let the EH abort the command or reset the device. */ 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 ; + ap->hsm_task_state =HSM_ST_ERR ; + goto fsm_start; }/* Send the CDB (atapi) or the first data block (ata pio out).
@@ -3704,13 +3707,16 @@ 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. +* We ignore DRQ here and stop the HSM by +* changing hsm_task_state to HSM_ST_ERR and +* let the EH abort the command or reset the device. */ 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 ; + ap->hsm_task_state =HSM_ST_ERR ; + goto fsm_start; }atapi_pio_bytes (qc);
@@ -3728,20 +3734,32 @@ fsm_start: 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. +/* For PIO reads, some devices may ask for +* data transfer (DRQ=1) alone with ERR=1. +* We respect DRQ here and transfer one +* block of junk data before changing the +* hsm_task_state to HSM_ST_ERR. +* +* For PIO writes, ERR=1 DRQ=1 doesn't make +* sense since the data block has been +* transferred to the device. */ 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 ; + +if (!(qc->tf.flags &ATA_TFLAG_WRITE )) { +ata_pio_sectors (qc); +ata_altstatus (ap); + status =ata_wait_idle (ap); + } + +/* ata_pio_sectors() might change the +* state to HSM_ST_LAST. so, the state +* is changed after ata_pio_sectors(). +*/ + ap->hsm_task_state =HSM_ST_ERR ; + goto fsm_start; }ata_pio_sectors (qc);
Jeff Garzik
AppliedRe: [PATCH/RFC] libata-dev: handle DRQ=1 ERR=1 (revised)
Quoted 15 lines · Albert Lee
Handle DRQ=1 ERR=1 situation. Revised according to what IDE
try_to_flush_leftover_data()does.Changes:
- For ATA PIO writes and ATAPI devices, just stop the HSM and let EH handle it.
- For ATA PIO reads, read only one block of junk data and then let EH handle it.Signed-off-by Albert Lee <[email protected]>---
Thanks for the IDEtry_to_flush_leftover_data()pointer.
The difference between IDE and irq-pio (before this patch):
1. IDE only does the data transfer for PIO reads.
For PIO writes and ATAPI, it just stops and abort/reset.
2. For PIO reads, only one block of data is transferred. EH then takes over if DRQ is still set after that. The behavior of irq-pio before this patch is: let HSM go on, until the device set DRQ=0 or all data read.
Quoted 5 lines
Te IDE behavior looks safer. (When the device is in bad status, we really should not write any data to it.)
Patch against irq-pio branch (
08a556db919f67e1e4d33ae8d40f7222da34d994). For your review, thanks.
applied