Bartlomiej Zolnierkiewicz wrote:
Hello.
Bartlomiej Zolnierkiewicz wrote:
[PATCH] sl82c105: add ->speedproc support
* add sl82c105_tunepio() wrapper for sl82c105_tune_drive()
(just to get the error value)
* add sl82c105_tune_chipset() (->speedproc method) for setting
transfer mode
Thanks for the patch!
> Index: b/drivers/ide/pci/sl82c105.c
===================================================================
--- a/drivers/ide/pci/sl82c105.c
+++ b/drivers/ide/pci/sl82c105.c
[...]
@@ -91,7 +91,7 @@ static void sl82c105_tune_drive(ide_driv
xfer_mode = ide_get_best_pio_mode(drive, pio, 5, &p) + XFER_PIO_0;
if (ide_config_drive_speed(drive, xfer_mode))
- return;
+ return 1;
drive->drive_data = drv_ctrl = get_timing_sl82c105(&p);
@@ -114,17 +114,45 @@ static void sl82c105_tune_drive(ide_driv
*/
drive->io_32bit = 1;
drive->unmask = 1;
+
+ return 0;
+}
+
+static void sl82c105_tune_drive(ide_drive_t *drive, u8 pio)
+{
+ /*
+ * TODO: find best PIO mode and set device speed here
+ * (requires adding helper function for getting PIO cycle time)
+ */
I thought we were doing it by calling ide_get_best_pio_mode() above...
We are also using ide_get_best_pio_mode() to get PIO cycle time
so we can't move it here ATM.
I've found/used quite convenient workaround for that -- return PIO mode
actually selected from xxx_tune_pio(), then call ide_config_drive_speed() from
the real tuneproc() method.
+ (void)sl82c105_tunepio(drive, pio);
Erm, I thought afterwards that I vainly folded one into another. I think
it's worth moving those io_32bit and unmask flag assignments above back
there... May also recast my patch. :-)
Moving them to ->init_hwif where they belong would be even better... ;-)
Well, I wasn't sure where they belong... :-)
So, OK to recast that patch?
+static int sl82c105_tune_chipset(ide_drive_t *drive, u8 mode)
+{
+ mode = ide_rate_filter(drive, mode);
+
+ if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
+ return sl82c105_tunepio(drive, mode - XFER_PIO_0);
+
+ /*
+ * TODO: add MWDMA0/1 support
+ */
+ BUG_ON(mode != XFER_MW_DMA_2);
Well, the other drivers just return non-zero in this case...
They are are buggy and need fixing.
Ugh, that'll be a lot of fixing... :-)
Why BUG on being passed unsupported mode from userspace though (when
there's no filtering involved before that)?
IDE driver itself should never ask for an unsupported mode and if user passes
such through ->speedproc interface OOPSing is IMO better than possible data
corruption. On the second thought ide_rate_filter() takes care of filtering
UDMA modes, so it may as well take care of filtering SWDMA/MWDMA/PIO and the
problem will be solved in more gracefull way... but more work is needed...
Yeah. :-)
Thanks,
Bart
MBR, Sergei
-
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
