Messages in this thread | | | From | Bartlomiej Zolnierkiewicz <> | Subject | Re: [PATCH] Re: [PATCH][IDE] update qd65xx driver | Date | Fri, 12 Sep 2003 00:46:58 +0200 |
| |
On Thursday 11 of September 2003 23:51, Andries.Brouwer@cwi.nl wrote: > That reminds me, did I ever send you this? > > Andries
No, only similar patch for hpt366.c.
I think the (almost) correct scheme is following (some drivers get it wrong):
config_chipset_pio() style functions are used for driver auto-tuning and "255, pio" is used to find best PIO mode supported by device. "pio" argument should be set to highest mode allowed by controller.
tune_proc()/tune_drive() style functions are used for direct mode setting and "pio, max_pio" is used to set desired mode. "pio" is desired mode, "max_pio" is max mode supported by controller.
It's almost correct because we should also check if desired PIO mode is supported by device, currently we let user shoot herself/himself.
IDE driver itself calls ->tuneproc() with argument == 255, so always highest supported PIO mode is chosen. User-space can call ->tuneproc() with smaller argument. With you patch smaller than highest possible PIO mode can't be chosen.
Your patch for hpt366.c has also this side effect that you can now only set highest possible PIO mode. You should revert this change and fix hpt3xx_tune_drive() call inside hpt366_config_drive_xfer_rate() to pass 255 instead of 5.
--bartlomiej
> diff -u --recursive --new-file -X /linux/dontdiff > a/drivers/ide/legacy/qd65xx.c b/drivers/ide/legacy/qd65xx.c --- > a/drivers/ide/legacy/qd65xx.c Mon Sep 8 23:44:59 2003 > +++ b/drivers/ide/legacy/qd65xx.c Thu Sep 11 23:20:26 2003 > @@ -261,7 +261,7 @@ > int recovery_time = 415; /* worst case values from the dos driver */ > > if (drive->id && !qd_find_disk_type(drive, &active_time, &recovery_time)) > { - pio = ide_get_best_pio_mode(drive, pio, 255, &d); > + pio = ide_get_best_pio_mode(drive, 255, pio, &d); > pio = IDE_MIN(pio,4); > > switch (pio) {
- 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/
| |