On Wednesday 21 February 2007 02:21, Suleiman Souhlal wrote:
> Use ide_wait_cmd() in ide_config_drive_speed() if the drive has been
> initialized and we're not in an interrupt, to avoid changing the
> xfer speed while requests are in flight.
Many devices have problems with SETFEATURES_XFER if the WIN_SETFEATURES
command is driven by IRQ so we must resort to the polling mode.
This also implies that ide_config_drive_speed() generally does its
job right and the problem lies in the higher layers.
set_using_dma() should queue special driver specific request (same goes
for some other options from /proc/ide/hdx/settings / ioctls).
I have a patch re-writting /proc/ide/hdx/settings so that there are
->get/->set methods for each setting. This should help in designing
special driver request but I need some more time to sync the patch
with the recent IDE changes. I will get back to you on this one.
Thanks,
Bart
> An easy way to trigger the problem is to dd the disk while doing
> while :; do hdparm -d 1 /dev/hdc> /dev/null; done
>
> While there, remove some commented-out code.
>
> Signed-off-by: Suleiman Souhlal <suleiman@xxxxxxxxxx>
>
> ---
> drivers/ide/ide-iops.c | 35 ++++++++++++++++++++---------------
> 1 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
> index c67b3b1..35ab3af 100644
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -748,32 +748,36 @@ int ide_config_drive_speed (ide_drive_t
> int i, error = 1;
> u8 stat;
>
> -// while (HWGROUP(drive)->busy)
> -// msleep(50);
> + /*
> + * 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);
> #endif
>
> - /*
> - * Don't use ide_wait_cmd here - it will
> - * attempt to set_geometry and recalibrate,
> - * but for some reason these don't work at
> - * this point (lost interrupt).
> - */
> /*
> * Select the drive, and issue the SETFEATURES command
> */
> disable_irq_nosync(hwif->irq);
>
> - /*
> - * FIXME: we race against the running IRQ here if
> - * this is called from non IRQ context. If we use
> - * disable_irq() we hang on the error path. Work
> - * is needed.
> - */
> -
> udelay(1);
> SELECT_DRIVE(drive);
> SELECT_MASK(drive, 0);
> @@ -835,6 +839,7 @@ #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;
>
>
-
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
