IDE initialization fix

Andre, Alan, Linus

We thought this bug was fixed for a long time, but apparently it is back! This patch makes sure we call the init for ALL chipsets that support it. Please apply this for the next 2.4.x, or let me know of problems.

thanks (more patches to come!)

Tim

Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[email protected]

diff -ruN dist-2.4.12+patches/drivers/ide/ide-pci.c cvs-2.4.12+patches/drivers/ide/ide-pci.c
@@ -584,7 +584,8 @@
 	ide_hwif_t *hwif, *mate = NULL;
 	unsigned int class_rev;
 	static int secondpdc = 0;
-
+	int pci_class_ide;
+	
 #ifdef CONFIG_IDEDMA_AUTO
 	if (!noautodma)
 		autodma = 1;

@@ -653,7 +654,8 @@
 		/* Its attached to something else, just a random bridge. 
 		   Suspect a fastrak and fall through */
 	}
-	if ((dev->class & ~(0xfa)) != ((PCI_CLASS_STORAGE_IDE << 8) | 5)) {
+	pci_class_ide = ((dev->class >> 8) == PCI_CLASS_STORAGE_IDE);
+        if (!pci_class_ide && ((dev->class >> 8) != PCI_CLASS_STORAGE_OTHER)) {
 		printk("%s: not 100%% native mode: will probe irqs later\n", d->name);
 		/*
 		 * This allows offboard ide-pci cards the enable a BIOS,

@@ -666,8 +668,19 @@
 		printk("%s: will probe irqs later\n", d->name);
 		pciirq = 0;
 	} else if (!pciirq) {
-		printk("%s: bad irq (%d): will probe later\n", d->name, pciirq);
-		pciirq = 0;
+		if (pci_class_ide) {
+			/* this is the normal path for most IDE devices */
+			if (d->init_chipset) {
+				pciirq = d->init_chipset(dev, d->name);
+			} else {
+				printk(KERN_INFO 
+					"%s standard IDE device detected\n", 
+					d->name);
+			}
+		} else {
+			printk(KERN_WARNING 
+				"%s: bad irq (0): will probe later\n", d->name);
+		}
 	} else {
 		if (d->init_chipset)
 			(void) d->init_chipset(dev, d->name);

Tim,

Why are we calling for interrupts assignments early?
Since all legacy hosts can reserve BAR's 0-3 to be 0x0000, and there are two differenct pci interrupts assigned, I am puzzled by the changes?

Obviously there are addon cards that do not fill in the interrupt line, also if one is doing SMP the value may be shifted.

The real issue is that the initialization of the card/host has a problem, so that belongs in the pci_fix_up region in the arch/<>/kernel/pc-irq.c stuff, imho. In general one should not be changing the interrupt lines at this stage of the INIT process.

Lastly, if you have a multi-host card and it does not register IO bases because there are no devices attached, then you have a problem of nonexistant resources, this is what I truly suspect is the problem w/ the Cobalt box.

Comments?

Cheers,

Andre Hedrick
Linux Disk Certification Project Linux ATA Development

Andre Hedrick wrote:
Andre, The summary of the change is this: without this change, the PCI init for chipsets does not get called. I'll speak specifically about the CSB5. The CSB5 in non-native mode has a PCI irqline register forced to 0. The PCI probe then skips it's PCI init and it never gets called.

We then see that there is no file in /proc/ide for the serverworks chipset. With this fix, there is.

Is there something else we aren't doing, instead? This seems obvious - there is NOWHERE else that calls the init_chipset() method.

I put a printk in the pci_init_svwks routine, and it doesn't get called.

Please show me what you mean - my desktop has an IRQ of zero, also. I do have some control over BIOS, too.

Please, I'd like to resolve this issue once and for all :)

Patch attached again for completeness..

Tim

Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[email protected]

diff -ruN dist-2.4.12+patches/drivers/ide/ide-pci.c cvs-2.4.12+patches/drivers/ide/ide-pci.c
@@ -584,7 +584,8 @@
 	ide_hwif_t *hwif, *mate = NULL;
 	unsigned int class_rev;
 	static int secondpdc = 0;
-
+	int pci_class_ide;
+	
 #ifdef CONFIG_IDEDMA_AUTO
 	if (!noautodma)
 		autodma = 1;

@@ -653,7 +654,8 @@
 		/* Its attached to something else, just a random bridge. 
 		   Suspect a fastrak and fall through */
 	}
-	if ((dev->class & ~(0xfa)) != ((PCI_CLASS_STORAGE_IDE << 8) | 5)) {
+	pci_class_ide = ((dev->class >> 8) == PCI_CLASS_STORAGE_IDE);
+        if (!pci_class_ide && ((dev->class >> 8) != PCI_CLASS_STORAGE_OTHER)) {
 		printk("%s: not 100%% native mode: will probe irqs later\n", d->name);
 		/*
 		 * This allows offboard ide-pci cards the enable a BIOS,

@@ -666,8 +668,19 @@
 		printk("%s: will probe irqs later\n", d->name);
 		pciirq = 0;
 	} else if (!pciirq) {
-		printk("%s: bad irq (%d): will probe later\n", d->name, pciirq);
-		pciirq = 0;
+		if (pci_class_ide) {
+			/* this is the normal path for most IDE devices */
+			if (d->init_chipset) {
+				pciirq = d->init_chipset(dev, d->name);
+			} else {
+				printk(KERN_INFO 
+					"%s standard IDE device detected\n", 
+					d->name);
+			}
+		} else {
+			printk(KERN_WARNING 
+				"%s: bad irq (0): will probe later\n", d->name);
+		}
 	} else {
 		if (d->init_chipset)
 			(void) d->init_chipset(dev, d->name);

Hi Tim,

Uniform Multi-Platform E-IDE driver Revision: 6.31
ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx ServerWorks CSB5: IDE controller on PCI bus 00 dev 79 ServerWorks CSB5: chipset revision 146
ServerWorks CSB5: not 100% native mode: will probe irqs later

    ide0: BM-DMA at 0x08b0-0x08b7, BIOS settings: hda:DMA, hdb:DMA
    ide1: BM-DMA at 0x08b8-0x08bf, BIOS settings: hdc:DMA, hdd:DMA

hda: Maxtor 5T020H2, ATA DISK drive
hdb: Maxtor 5T020H2, ATA DISK drive
hdc: _NEC DV-5800A, ATAPI CD/DVD-ROM drive
hdd: Imation Travan 20GB Tape, ATAPI TAPE drive
ide0 at 0x1f0-0x1f7,0x3f6 on irq 14
ide1 at 0x170-0x177,0x376 on irq 15
hda: 39062500 sectors (20000 MB) w/2048KiB Cache, CHS=2431/255/63, UDMA(100)
hdb: 39062500 sectors (20000 MB) w/2048KiB Cache, CHS=2431/255/63, UDMA(100)
hdc: ATAPI 48X DVD-ROM drive, 512kB Cache, UDMA(33)

Oh like this one ??

00:0f.1 IDE interface: ServerWorks: Unknown device 0212 (rev 92) 00: 66 11 12 02 05 00 00 02 92 8a 01 01 08 40 80 00 10: f1 01 00 00 f5 03 00 00 71 01 00 00 75 03 00 00 20: b1 08 00 00 01 10 00 00 00 00 00 00 66 11 12 02 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 40: 20 20 20 20 20 20 20 20 f0 50 44 44 00 00 00 00 50: 00 00 00 00 07 00 55 02 0f 04 03 00 00 00 00 00 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

beetle:/proc/ide # cat svwks

ServerWorks CSB5 Chipset.
------------------------------- General Status --------------------------------- --------------- Primary Channel ---------------- Secondary Channel ------------
enabled enabled
--------------- drive0 --------- drive1 -------- drive0 ---------- drive1 ------

DMA enabled:    yes              yes             yes               yes
UDMA enabled:   yes              yes             yes               no
UDMA enabled:   5                5               2                 0
DMA enabled:    2                2               2                 2
PIO  enabled:   4                4               4                 4

Why are you putting it in non-native mode?
Oh, are you trying to run the RAID features?

                    IDE_PCI_DEVID_EQ(d->devid, DEVID_CMD649) ||
                    IDE_PCI_DEVID_EQ(d->devid, DEVID_OSB4) ||
+                   IDE_PCI_DEVID_EQ(d->devid, DEVID_CSB5) ||
                    ((dev->class >> 8) == PCI_CLASS_STORAGE_IDE && (dev->class & 0x80))) {

Try this....

Andre Hedrick
Linux Disk Certification Project Linux ATA Development

OK! This is what I have learned. The ide-pci init code has one case that isn't caught. Perhaps you can clarify what it means, for me.

* If an IDE controller has a PCI IRQ and has class code bit 0x5 set, it is 100% native.

* CSB5 _CAN_NOT_ have a PCI IRQ assigned (RO register).

* CSB5 spec clearly indicates that you can turn on class code bits 0x5 for native mode operation.

* My system has the native mode bits turned on, but not an IRQ. The driver incorrectly drops this case into teh "bad irq" case.

* Solution: cleanup probe to something like:

if ((is_storage_ide || is_storage_other) && native_bits && pciirq) {
        /* 100% native mode */
        init_chipset();
} else if ((is_storage_ide || is_storage_other) && native_bits) {
        /* native mode */
        init_chipset();
} else if (is_storage_ide || is_storage_other) {
        /* non-native mode */
        init_chipset();
} else {
        /* what here ?!?! */
}

or simpler:

if (is_storage_ide || is_storage_other) {
	if (native_bits && pciirq) {
		/* 100% native mode */
	} else if (native_bits) {
		/* plain-old native mode */
	} else {
		/* non-native mode */
	}
	init_chipset();
} else {
	/* what here?!? */
}

What do you think of this case? The other alternative is to turn OFF native bits on any CSB5, since it can't have a PCI IRQ assigned.

Please let me know, and I'll work up and test a patch.

Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[email protected]