Re: [PATCH] PIIX: fix 82371MX enablebits

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

Hello.

Alan Cox wrote:

>>     According to the datasheet, Intel 82371MX (MPIIX) actually has only a
>>single IDE channel mapped to the primary or secondary ports depending on the
>>value of the bit 14 of the IDETIM register at PCI config. offset 0x6C (the
>>register at 0x6F which the driver refers to. doesn't exist). So, disguise the
>>controller as dual channel and set enablebits masks/values such that only
>>either primary or secondary channel is detected enabled. Also, preclude the
>>IDE probing code from reading PCI BARs, this controller just doesn't have them
>>(it's not the separate PCI function like the other PCI controllers), it only
>>decodes the legacy addresses.

> There are lots and lots of other things you need to fix to make MPIIX
> work with that driver. It has only a single timing register for one so
> you must switch timing as you flip drive.

    I know. All in a good time (if I have it :-)...

> Also it is not an IDE class
> device so the PCI native/legacy and simplex stuff is not valid.

    Erm, simplex stuff shouldn't be touched at all since the chip is not 
DMA-capable. The same should be true about the native/legacy mode...

> Finally the PIIX driver pokes several registers it doesn't even have.

    Hm, as I can see, it avoids touching anything at all on MPIIX. This may 
rather be said of PIIX -- this chip didn't have SIDETIM register yet, so slave 
tuning won't work on it...

> What else - oh yes the piix driver doesn't even tune the timings, so it
> doesn't work anyway.

    Of course it doesn't, because of the non-standard timing reg.
    BTW, piix_tune_drive() "forgets" to actully set the speed for drive and... 
setting UDMA modes affects PIO timing for absolutely no reason and ... all in 
all, the tuning code here is BAD.

> Thats why drivers/scsi/pata_mpiix is a separate driver. Really if you
> want to try and rescue the old PIIX driver you should split out PIIX3
> and MPIIX into their own drivers.

    There's no great need in splitting, just the separate tune_chipset() 
functions for PIIX/MPIIX and the rest of the crowd would suffice, IMHO...

MBR, Sergei