IDE error recovery is using WIN_IDLEIMMEDIATE which was only valid for IDE V1 and IDE V2. Modern drives will not be able to recover using this error handling. The correct thing to do is issue a SRST followed by a SET_FEATURES. Signed-off-by: Suleiman Souhlal <suleiman@xxxxxxxxxx> --- drivers/ide/ide-io.c | 35 +++++++++++----- drivers/ide/ide-iops.c | 105 ++++++++++++++++++++++++++++-------------------- include/linux/ide.h | 2 + 3 files changed, 88 insertions(+), 54 deletions(-) diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c index c193553..2f05b4d 100644 --- a/drivers/ide/ide-io.c +++ b/drivers/ide/ide-io.c @@ -519,21 +519,21 @@ static ide_startstop_t ide_ata_error(ide if ((stat & DRQ_STAT) && rq_data_dir(rq) == READ && hwif->err_stops_fifo == 0) try_to_flush_leftover_data(drive); + if (rq->errors >= ERROR_MAX || blk_noretry_request(rq)) { + ide_kill_rq(drive, rq); + return ide_stopped; + } + if (hwif->INB(IDE_STATUS_REG) & (BUSY_STAT|DRQ_STAT)) - /* force an abort */ - hwif->OUTB(WIN_IDLEIMMEDIATE, IDE_COMMAND_REG); + rq->errors |= ERROR_RESET; - if (rq->errors >= ERROR_MAX || blk_noretry_request(rq)) - ide_kill_rq(drive, rq); - else { - if ((rq->errors & ERROR_RESET) == ERROR_RESET) { - ++rq->errors; - return ide_do_reset(drive); - } - if ((rq->errors & ERROR_RECAL) == ERROR_RECAL) - drive->special.b.recalibrate = 1; + if ((rq->errors & ERROR_RESET) == ERROR_RESET) { ++rq->errors; + return ide_do_reset(drive); } + + ++rq->errors; + return ide_stopped; } @@ -586,6 +586,13 @@ EXPORT_SYMBOL_GPL(__ide_error); * both new-style (taskfile) and old style command handling here. * In the case of taskfile command handling there is work left to * do + * This used to send a idle immediate to the drive if the drive was + * busy or had drq set. This violates the ATA spec (can only send IDLE + * immediate when drive is not busy) and really hoses up some drives. + * We've changed it to just do a SRST followed by a set features (set + * udma mode) it those cases. This is what Western Digital recommends + * for error recovery and what Western Digital says Windows does. It + * also does not violate the ATA spec as far as I can tell. */ ide_startstop_t ide_error (ide_drive_t *drive, const char *msg, u8 stat) @@ -1004,6 +1011,12 @@ #endif goto kill_rq; } + /* We reset the drive so we need to issue a SETFEATURES. */ + if ((drive->current_speed == 0xff) && + ((rq->cmd_type == REQ_TYPE_ATA_CMD) || + (rq->cmd_type == REQ_TYPE_ATA_TASK))) + ide_config_drive_speed_irq(drive, drive->desired_speed); + block = rq->sector; if (blk_fs_request(rq) && (drive->media == ide_disk || drive->media == ide_floppy)) { diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c index 35ab3af..e0573cb 100644 --- a/drivers/ide/ide-iops.c +++ b/drivers/ide/ide-iops.c @@ -731,6 +731,30 @@ #else #endif } +static void ide_drive_speed_changed(ide_drive_t *drive, u8 speed) +{ + switch(speed) { + case XFER_UDMA_7: drive->id->dma_ultra |= 0x8080; break; + case XFER_UDMA_6: drive->id->dma_ultra |= 0x4040; break; + case XFER_UDMA_5: drive->id->dma_ultra |= 0x2020; break; + case XFER_UDMA_4: drive->id->dma_ultra |= 0x1010; break; + case XFER_UDMA_3: drive->id->dma_ultra |= 0x0808; break; + case XFER_UDMA_2: drive->id->dma_ultra |= 0x0404; break; + case XFER_UDMA_1: drive->id->dma_ultra |= 0x0202; break; + case XFER_UDMA_0: drive->id->dma_ultra |= 0x0101; break; + case XFER_MW_DMA_2: drive->id->dma_mword |= 0x0404; break; + case XFER_MW_DMA_1: drive->id->dma_mword |= 0x0202; break; + case XFER_MW_DMA_0: drive->id->dma_mword |= 0x0101; break; + case XFER_SW_DMA_2: drive->id->dma_1word |= 0x0404; break; + case XFER_SW_DMA_1: drive->id->dma_1word |= 0x0202; break; + case XFER_SW_DMA_0: drive->id->dma_1word |= 0x0101; break; + default: break; + } + if (!drive->init_speed) + drive->init_speed = speed; + drive->current_speed = speed; +} + /* * Similar to ide_wait_stat(), except it never calls ide_error internally. * This is a kludge to handle the new ide_config_drive_speed() function, @@ -742,32 +766,12 @@ #endif * * const char *msg == consider adding for verbose errors. */ -int ide_config_drive_speed (ide_drive_t *drive, u8 speed) +int ide_config_drive_speed_irq(ide_drive_t *drive, u8 speed) { ide_hwif_t *hwif = HWIF(drive); int i, error = 1; u8 stat; - /* - * Just use ide_wait_cmd() if the drive has been initialized and we - * aren't in an interrupt handler, to avoid changing the xfer speed - * while requests are in flight. - * - * If we are in an interrupt, it should be safe to issue - * SETFEATURES manually, since there shouldn't be any requests in - * flight. - */ - if (drive->queue != NULL && !in_interrupt()) { - error = ide_wait_cmd(drive, WIN_SETFEATURES, speed, - SETFEATURES_XFER, 0, NULL); - if (error) { - stat = hwif->INB(IDE_STATUS_REG); - ide_dump_status(drive, "set_drive_speed_status", stat); - return (error); - } - goto done; - } - #ifdef CONFIG_BLK_DEV_IDEDMA if (hwif->ide_dma_check) /* check if host supports DMA */ hwif->dma_host_off(drive); @@ -839,28 +843,39 @@ #ifdef CONFIG_BLK_DEV_IDEDMA hwif->dma_off_quietly(drive); #endif -done: - switch(speed) { - case XFER_UDMA_7: drive->id->dma_ultra |= 0x8080; break; - case XFER_UDMA_6: drive->id->dma_ultra |= 0x4040; break; - case XFER_UDMA_5: drive->id->dma_ultra |= 0x2020; break; - case XFER_UDMA_4: drive->id->dma_ultra |= 0x1010; break; - case XFER_UDMA_3: drive->id->dma_ultra |= 0x0808; break; - case XFER_UDMA_2: drive->id->dma_ultra |= 0x0404; break; - case XFER_UDMA_1: drive->id->dma_ultra |= 0x0202; break; - case XFER_UDMA_0: drive->id->dma_ultra |= 0x0101; break; - case XFER_MW_DMA_2: drive->id->dma_mword |= 0x0404; break; - case XFER_MW_DMA_1: drive->id->dma_mword |= 0x0202; break; - case XFER_MW_DMA_0: drive->id->dma_mword |= 0x0101; break; - case XFER_SW_DMA_2: drive->id->dma_1word |= 0x0404; break; - case XFER_SW_DMA_1: drive->id->dma_1word |= 0x0202; break; - case XFER_SW_DMA_0: drive->id->dma_1word |= 0x0101; break; - default: break; - } - if (!drive->init_speed) - drive->init_speed = speed; - drive->current_speed = speed; - return error; + ide_drive_speed_changed(drive, speed); + + return (error); +} + +int ide_config_drive_speed (ide_drive_t *drive, u8 speed) +{ + ide_hwif_t *hwif = HWIF(drive); + int error; + u8 stat; + + /* + * Just use ide_wait_cmd() if the drive has been initialized and we + * aren't in an interrupt handler, to avoid changing the xfer speed + * while requests are in flight. + * + * If we are in an interrupt, it should be safe to issue + * SETFEATURES manually, since there shouldn't be any requests in + * flight. + */ + if (drive->queue != NULL && !in_interrupt()) { + error = ide_wait_cmd(drive, WIN_SETFEATURES, speed, + SETFEATURES_XFER, 0, NULL); + if (error) { + stat = hwif->INB(IDE_STATUS_REG); + ide_dump_status(drive, "set_drive_speed_status", stat); + return (error); + } + ide_drive_speed_changed(drive, speed); + } else + error = ide_config_drive_speed_irq(drive, speed); + + return (error); } EXPORT_SYMBOL(ide_config_drive_speed); @@ -1093,6 +1108,10 @@ static void pre_reset(ide_drive_t *drive if (HWIF(drive)->pre_reset != NULL) HWIF(drive)->pre_reset(drive); + /* Make sure we issue a SETFEATURES before the next request. */ + if (drive->current_speed != 0xff) + drive->desired_speed = drive->current_speed; + drive->current_speed = 0xff; } /* diff --git a/include/linux/ide.h b/include/linux/ide.h index 3861753..c7f6027 100644 --- a/include/linux/ide.h +++ b/include/linux/ide.h @@ -616,6 +616,7 @@ typedef struct ide_drive_s { u8 init_speed; /* transfer rate set at boot */ u8 pio_speed; /* unused by core, used by some drivers for fallback from DMA */ u8 current_speed; /* current transfer rate set */ + u8 desired_speed; /* desired transfer rate set */ u8 dn; /* now wide spread use */ u8 wcache; /* status of write cache */ u8 acoustic; /* acoustic management */ @@ -1184,6 +1185,7 @@ extern int system_bus_clock(void); extern int ide_driveid_update(ide_drive_t *); extern int ide_ata66_check(ide_drive_t *, ide_task_t *); extern int ide_config_drive_speed(ide_drive_t *, u8); +extern int ide_config_drive_speed_irq(ide_drive_t *, u8); extern u8 eighty_ninty_three (ide_drive_t *); extern int set_transfer(ide_drive_t *, ide_task_t *); extern int taskfile_lib_get_identify(ide_drive_t *drive, u8 *); - To unsubscribe from this list: 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