Messages in this thread | | | From | Suleiman Souhlal <> | Subject | Re: [PATCH 3/3] Use correct IDE error recovery | Date | Thu, 8 Mar 2007 12:53:07 -0800 |
| |
On Mar 8, 2007, at 12:34 PM, Bartlomiej Zolnierkiewicz wrote:
> > Hi, > > On Thursday 08 March 2007, Suleiman Souhlal wrote: >> >> On Mar 7, 2007, at 1:16 PM, Bartlomiej Zolnierkiewicz wrote: >> >>> >>> Hi, >>> >>> (sorry for the long delay) >>> >>> On Wednesday 21 February 2007, Suleiman Souhlal wrote: >>>> 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. >>> >>> This change looks fine, indeed we are better of using SRST + >>> SET_FEATURES than IDLE_IMMEDIATE. >>> >>>> Signed-off-by: Suleiman Souhlal <suleiman@google.com> >>>> >>>> --- >>>> 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; >>> >>> Is the removal of ERROR_RECAL handling intentional? >>> There is nothing about it in the patch description... >> >> Yes, it was intentional, but I forgot to add "while there remove some > > Why is it useless? What am I missing?
I thought the recalibration code didn't do anything, but upon rereading the code I'm not so sure anymore..
-- Suleiman
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |