From 513daadd152ddbf32cb6d0447ddba3427ce5b8e8 Mon Sep 17 00:00:00 2001 From: Suleiman Souhlal Date: Mon, 26 Mar 2007 23:03:20 +0200 Subject: ide: use correct IDE error recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IDE error recovery is using IDLE IMMEDIATE if the drive is busy or has DRQ set. This violates the ATA spec (can only send IDLE IMMEDIATE when drive is not busy) and really hoses up some drives (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 command. 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. Bart: * port the patch over the current tree * undo the recalibration code removal * send SET FEATURES command after checking for good drive status * don't check whether the current request is of REQ_TYPE_ATA_{CMD,TASK} type because we need to send SET FEATURES before handling any requests * some pre-ATA4 drives require INITIALIZE DEVICE PARAMETERS command before other commands (except IDENTIFY) so send SET FEATURES only if there are no pending drive->special requests * update comments and patch description * any bugs introduced by this patch are mine and not Suleiman's :-) Signed-off-by: Suleiman Souhlal Acked-by: Alan Cox Signed-off-by: Bartlomiej Zolnierkiewicz --- include/linux/ide.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/linux/ide.h') diff --git a/include/linux/ide.h b/include/linux/ide.h index 34f2676b3c62..58564a199862 100644 --- a/include/linux/ide.h +++ b/include/linux/ide.h @@ -615,6 +615,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 */ -- cgit v1.2.3 From 23450319e2890986c247ec0aa1442f060e657e6d Mon Sep 17 00:00:00 2001 From: Suleiman Souhlal Date: Tue, 10 Apr 2007 22:38:37 +0200 Subject: ide: correctly prevent IDE timer expiry function to run if request was already handled It is possible for the timer expiry function to run even though the request has already been handled: ide_timer_expiry() only checks that the handler is not NULL, but it is possible that we have handled a request (thus clearing the handler) and then started a new request (thus starting the timer again, and setting a handler). A simple way to exhibit this is to set the DMA timeout to 1 jiffy and run dd: The kernel will panic after a few minutes because ide_timer_expiry() tries to add a timer when it's already active. To fix this, we simply add a request generation count that gets incremented at every interrupt, and check in ide_timer_expiry() that we have not already handled a new interrupt before running the expiry function. Signed-off-by: Suleiman Souhlal Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/ide/ide-io.c | 6 +++++- drivers/ide/ide-iops.c | 2 ++ include/linux/ide.h | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) (limited to 'include/linux/ide.h') diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c index 0e0280076fcd..8670112f1d39 100644 --- a/drivers/ide/ide-io.c +++ b/drivers/ide/ide-io.c @@ -1226,6 +1226,7 @@ static void ide_do_request (ide_hwgroup_t *hwgroup, int masked_irq) #endif /* so that ide_timer_expiry knows what to do */ hwgroup->sleeping = 1; + hwgroup->req_gen_timer = hwgroup->req_gen; mod_timer(&hwgroup->timer, sleep); /* we purposely leave hwgroup->busy==1 * while sleeping */ @@ -1411,7 +1412,8 @@ void ide_timer_expiry (unsigned long data) spin_lock_irqsave(&ide_lock, flags); - if ((handler = hwgroup->handler) == NULL) { + if (((handler = hwgroup->handler) == NULL) || + (hwgroup->req_gen != hwgroup->req_gen_timer)) { /* * Either a marginal timeout occurred * (got the interrupt just as timer expired), @@ -1439,6 +1441,7 @@ void ide_timer_expiry (unsigned long data) if ((wait = expiry(drive)) > 0) { /* reset timer */ hwgroup->timer.expires = jiffies + wait; + hwgroup->req_gen_timer = hwgroup->req_gen; add_timer(&hwgroup->timer); spin_unlock_irqrestore(&ide_lock, flags); return; @@ -1653,6 +1656,7 @@ irqreturn_t ide_intr (int irq, void *dev_id) printk(KERN_ERR "%s: ide_intr: hwgroup->busy was 0 ??\n", drive->name); } hwgroup->handler = NULL; + hwgroup->req_gen++; del_timer(&hwgroup->timer); spin_unlock(&ide_lock); diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c index 1ee53a551c3a..3caa176b3155 100644 --- a/drivers/ide/ide-iops.c +++ b/drivers/ide/ide-iops.c @@ -889,6 +889,7 @@ static void __ide_set_handler (ide_drive_t *drive, ide_handler_t *handler, hwgroup->handler = handler; hwgroup->expiry = expiry; hwgroup->timer.expires = jiffies + timeout; + hwgroup->req_gen_timer = hwgroup->req_gen; add_timer(&hwgroup->timer); } @@ -929,6 +930,7 @@ void ide_execute_command(ide_drive_t *drive, task_ioreg_t cmd, ide_handler_t *ha hwgroup->handler = handler; hwgroup->expiry = expiry; hwgroup->timer.expires = jiffies + timeout; + hwgroup->req_gen_timer = hwgroup->req_gen; add_timer(&hwgroup->timer); hwif->OUTBSYNC(drive, cmd, IDE_COMMAND_REG); /* Drive takes 400nS to respond, we must avoid the IRQ being diff --git a/include/linux/ide.h b/include/linux/ide.h index 58564a199862..d3bbc7188b6a 100644 --- a/include/linux/ide.h +++ b/include/linux/ide.h @@ -861,6 +861,8 @@ typedef struct hwgroup_s { int (*expiry)(ide_drive_t *); /* ide_system_bus_speed */ int pio_clock; + int req_gen; + int req_gen_timer; unsigned char cmd_buf[4]; } ide_hwgroup_t; -- cgit v1.2.3