[PATCH] PIIX: fix 82371MX enablebits

[PATCH] PIIX: fix 82371MX enablebits

[Sergei Shtylyov]

[-- Attachment #1: Type: text/plain, Size: 721 bytes --]

Hello.

     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.

MBR, Sergei

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>


[-- Attachment #2: 82371MX-IDE-enablebits-fix.patch --]
[-- Type: text/plain, Size: 841 bytes --]

Index: linus/drivers/ide/pci/piix.c
===================================================================
--- linus.orig/drivers/ide/pci/piix.c
+++ linus/drivers/ide/pci/piix.c
@@ -512,13 +512,19 @@ static ide_pci_device_t piix_pci_info[] 
 	/*  0 */ DECLARE_PIIX_DEV("PIIXa"),
 	/*  1 */ DECLARE_PIIX_DEV("PIIXb"),
 
-	{	/* 2 */
+	/*  2 */
+	{	/*
+		 * 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 offset 0x6c
+		 */
 		.name		= "MPIIX",
 		.init_hwif	= init_hwif_piix,
 		.channels	= 2,
 		.autodma	= NODMA,
-		.enablebits	= {{0x6D,0x80,0x80}, {0x6F,0x80,0x80}},
+		.enablebits	= {{0x6d,0xc0,0x80}, {0x6d,0xc0,0xc0}},
 		.bootable	= ON_BOARD,
+		.flags		= IDEPCI_FLAG_ISA_PORTS
 	},
 
 	/*  3 */ DECLARE_PIIX_DEV("PIIX3"),

Re: [PATCH] PIIX: fix 82371MX enablebits

[Alan Cox]

On Iau, 2006-05-11 at 23:50 +0400, Sergei Shtylyov 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. Also it is not an IDE class
device so the PCI native/legacy and simplex stuff is not valid. Finally
the PIIX driver pokes several registers it doesn't even have.

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


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.

Re: [PATCH] PIIX: fix 82371MX enablebits

[Sergei Shtylyov]

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

Re: [PATCH] PIIX: fix 82371MX enablebits

[Alan Cox]

On Gwe, 2006-05-12 at 00:32 +0400, Sergei Shtylyov wrote:
>     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...

Native/legacy appplies to PIO too, and predates DMA stuff. Read
ide_setup_pci_baseregs and you'll see the problem. I hit that with
pata_mpiix and similar assumptions copied into libata and when I tried
the piix driver on the laptop I have.

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

It avoids it for the MPIIX but not the early PIIX chip. And you can do
slave timing just fine, you need to flip the timings when you flip drive
which is how pata_mpiix does it and pata_oldpiix.

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

Have fun 8) Having been there and done that already the list of bugs in
the piix driver is huge. Good to see someone beginning on it. However
you need more than to skip tuning and you need to reload the timings on
master/slave switches to get any performance.

If you want to fight the PIIX driver then other stuff I fixed in
pata_piix included
- Touching UDMA registers not found on earlier UDMA chips
- Setting IORDY on the wrong modes
- Setting prefetch/postwrite related options on devices it isnt safe for
(ATAPI shouldn't set PPE0 etc)

There's a nice Intel manual on the PIIX/ICH tuning rules separate to the
chip docs btw, I found it really helpful, dunno if you've already got a
copy.

Alan