Re: [PATCH 3/6] siimage: PIO mode setup fixes

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Bartlomiej Zolnierkiewicz wrote:

>>>>>* 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... ;-)

    Thievessss! 8-)

> Fortunately pata_sil680 driver uses "sil680_" prefix.

    We'll make Coxsss pay. 8-)

>>>>   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?

    The whole reason for that resetproc() ceases to exist, no? At least that's 
what I was able to grasp from looing at the older versions... And it's mean to 
reset both channels just to be able to downgrade from UDMA to MWDMA (not a 
thing routinely done anyway), so this needs to go.
    Well, I'm seeing that it's also called from the ide_dma_timeout() and even 
ide_dma_lostirq() methods; not sure how much necessary it is -- at least for 
the latter case this seems just wrong...

>>>+	/* 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)?

    Well, probably not -- a slow mate drive could force IORDY disabled this 
way... So, just 'pio' seems more correct.

> Thanks,
> Bart

MBR, Sergei