Bartlomiej Zolnierkiewicz wrote:
A lot to argue about here...
* Add sil_tuneproc() wrapper for siimage_tuneproc() which also sets
PIO mode on the device.
Planning on the global prefix change? :-)
* Add code limiting maximum PIO mode according to the pair device capabilities
to sil_tuneproc().
Ugh... that part is terrible. :-/
Actually, we only need to limit the taskfile, not the data transfers --
unlike it was done before.
* Remove no longer needed siimage_taskfile_timing()
and config_siimage_chipset_for_pio().
Yeah, time to get rid of that garbage. :-)
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
Note that PIO setup keeps being somewhat borken IODRY-wise even with this
patch as sil_tune_pio() only controls taskfile IORDY sampling -- the Right
Thing could only be done via speedproc() method...
Index: b/drivers/ide/pci/siimage.c
===================================================================
--- a/drivers/ide/pci/siimage.c
+++ b/drivers/ide/pci/siimage.c
[...]
-/**
- * simmage_tuneproc - tune a drive
+ * sil_tune_pio - tune a drive
* @drive: drive to tune
- * @mode_wanted: the target operating mode
+ * @pio: the desired PIO mode
*
* Load the timing settings for this device mode into the
* controller. If we are in PIO mode 3 or 4 turn on IORDY
* monitoring (bit 9). The TF timing is bits 31:16
*/
-
-static void siimage_tuneproc (ide_drive_t *drive, byte mode_wanted)
+
+static void sil_tune_pio(ide_drive_t *drive, u8 pio)
{
ide_hwif_t *hwif = HWIF(drive);
+ ide_drive_t *pair = &hwif->drives[1 - drive->select.b.unit];
I'd suggest simpler [drive->dn ^ 1]...
u32 speedt = 0;
u16 speedp = 0;
unsigned long addr = siimage_seldev(drive, 0x04);
unsigned long tfaddr = siimage_selreg(hwif, 0x02);
-
+
+ /*
+ * Compute the best PIO mode we can for a given device. We must
+ * pick a speed that does not cause problems with the other device
+ * on the cable.
+ */
+ if (pair) {
Huh? It can *not* really be NULL.
+ u8 pair_pio = ide_get_best_pio_mode(pair, 255, 4, NULL);
I'm not quite sure it's safe enough to trim to the maximum supported mode
of the other drive -- the current mode would be the safest bet... well, after
referring to the ATA spec., it's considered safe enough there.
+
+ /* trim PIO to the slowest of the master/slave */
+ if (pair_pio < pio)
+ pio = pair_pio;
No need to trim the *data* PIO mode.
+ }
+
/* cheat for now and use the docs */
- switch (mode_wanted) {
+ switch (pio) {
case 4:
speedp = 0x10c1;
speedt = 0x10c1;
What I envisioned was putting speedt into drive->drive_data, calculating
the maximum value for 2 drives and using it to actually program the taskfile
timing. Either that or put PIO mode there, and add the second switch to
calculate the taskfile timings after getting the minimum PIO mode for 2 drives
(but that's not as neat).
@@ -235,7 +224,7 @@ static void siimage_tuneproc (ide_drive_
hwif->OUTW(speedp, addr);
hwif->OUTW(speedt, tfaddr);
/* Now set up IORDY */
- if(mode_wanted == 3 || mode_wanted == 4)
+ if (pio == 3 || pio == 4)
Why not just (pio > 2)?
hwif->OUTW(hwif->INW(tfaddr-2)|0x200, tfaddr-2);
Erm, the same comments about taskfile IORDY as before: it should be
selected if the drive supports it. In fact, if either of 2 drives do.
else
hwif->OUTW(hwif->INW(tfaddr-2)&~0x200, tfaddr-2);
This is wrong logic: thus we may turn off IORDY although the 2nd drive may
support it.
@@ -245,42 +234,17 @@ static void siimage_tuneproc (ide_drive_
pci_read_config_word(hwif->pci_dev, tfaddr-2, &speedp);
speedp &= ~0x200;
/* Set IORDY for mode 3 or 4 */
- if(mode_wanted == 3 || mode_wanted == 4)
+ if (pio == 3 || pio == 4)
speedp |= 0x200;
pci_write_config_word(hwif->pci_dev, tfaddr-2, speedp);
}
}
Same here...
[...]
@@ -335,7 +299,7 @@ static int siimage_tune_chipset (ide_dri
case XFER_PIO_2:
case XFER_PIO_1:
case XFER_PIO_0:
- siimage_tuneproc(drive, (speed - XFER_PIO_0));
+ sil_tune_pio(drive, speed - XFER_PIO_0);
mode |= ((unit) ? 0x10 : 0x01);
The last line enables IORDY sampling for data transfers.
break;
case XFER_MW_DMA_2:
@@ -343,7 +307,7 @@ static int siimage_tune_chipset (ide_dri
case XFER_MW_DMA_0:
multi = dma[speed - XFER_MW_DMA_0];
mode |= ((unit) ? 0x20 : 0x02);
... and this line also enables IORDY. And the one in UltraDMA case group too.
- config_siimage_chipset_for_pio(drive, 0);
+ sil_tune_pio(drive, 4); /* FIXME */
Why we still need this nonsense here...
break;
case XFER_UDMA_6:
case XFER_UDMA_5:
@@ -356,7 +320,7 @@ static int siimage_tune_chipset (ide_dri
ultra |= ((scsc) ? (ultra6[speed - XFER_UDMA_0]) :
(ultra5[speed - XFER_UDMA_0]));
mode |= ((unit) ? 0x30 : 0x03);
- config_siimage_chipset_for_pio(drive, 0);
+ sil_tune_pio(drive, 4); /* FIXME */
... and here? If we so desperately want to setup PIO data/taskfile
timings, it's better to do via setting the 'autotune' field unconditionally.
break;
default:
return 1;
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
