On Wednesday 04 July 2007, Sergei Shtylyov wrote:
> Hello.
>
> 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? :-)
>
> > Yep.
>
> Well, it didn't work out with 'ata_'... ;-)
Because of bad libata taking over our precioussssss namespace... ;-)
Fortunately pata_sil680 driver uses "sil680_" prefix.
> >>>* 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.
>
> > Fixed.
>
> Let's see... :-)
>
> >> 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...
>
> > After rehashing the datasheet I see the source of the issue:
>
> > IORDY is controlled in two registers and moreover it is always enabled
> > if MDMA or UDMA transfer modes are selected.
>
> Yeah. I drafted some patch for pata_sil.c but Alan fixed it first and in a
> more simple way -- libata calls PIO/DMA setting methods in the strict
> sequence, and that allowed to bypass some checks...
I fail to see how this helps in this case, care to explain?
> > + /* cheat for now and use the docs */
> > + speedp = data_speed[pio];
> > + speedt = tf_speed[tf_pio];
> > +
> > if (hwif->mmio) {
> > hwif->OUTW(speedp, addr);
> > hwif->OUTW(speedt, tfaddr);
> > /* Now set up IORDY */
> > - if(mode_wanted == 3 || mode_wanted == 4)
> > + if (pio > 2)
>
> Not tf_pio? This IORDY bit is for taskfile accesses...
Would this be really OK (tf_pio takes minimum PIO mode)?
> > hwif->OUTW(hwif->INW(tfaddr-2)|0x200, tfaddr-2);
> > else
> > hwif->OUTW(hwif->INW(tfaddr-2)&~0x200, tfaddr-2);
> > @@ -245,42 +213,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 > 2)
>
> Same question here.
> However... this logic should rely on both drives' PIO modes, so would be
> broken either way until this is fixed...
Yep.
Thanks,
Bart
-
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
