Re: [PATCH] fix issues with loading PCI ide drivers as modules(linux 2.6.0)

[Davin McCall]

Ok - fourth try! Hopefully I'm getting better.

On Mon, 5 Jan 2004 15:16:03 +0100
Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@xxxxxxxxxxxxxx> wrote:

> You don't need to export "initializing" variable from ide.c,
> just use "pre_init" variable from setup-pci.c :-).
> 

Yes - of course.

But actually I have found an even better solution - "hwif->present" tells us if the hwif is currently being controlled by some chipset driver (that's what it's there for!) so I check that instead.

> > Plus, everything works as before - including "idex=..." parameters.
> 
> Except when using them for IDE PCI modules with non default ports:
> - hwif->chipset is set to ide_generic during boot
> - main IDE driver initialization
> - module load fails (because hwif->chipset == ide_generic && !initializing)
> 
> You can fix it by replacing all current occurrences of ide_generic by some
> new type (ide_forced).  It will also clear confusion about ide_generic name.

I've changed all occurrences of "ide_generic" to "ide_forced" and removed "ide_generic" altogether.

Now, "ide_forced" means what it should. The chipset is changed to "ide_unknown" or "ide_pci" when some chipset driver takes over (be it the standard ide, or some PCI specific). So no hwif which is actually present will be left as "ide_forced".

Also I have slightly modified match_hwif to check hwif->present when it is trying to find a free entry (otherwise, there is a chance that it will return one that cannot be used, even when there is a truly free one available).

This doesn't *exactly* match what you said, but it seems to me to be very clean and solve all the issues at once. If you can see any problems let me know and I will revert it to the previous way (with your improvements incorporated).

regards
Davin


diff -urN linux-2.6.0-vanilla/drivers/ide/ide-probe.c linux-2.6.0/drivers/ide/ide-probe.c
--- linux-2.6.0-vanilla/drivers/ide/ide-probe.c	Thu Nov 27 07:44:24 2003
+++ linux-2.6.0/drivers/ide/ide-probe.c	Tue Jan  6 12:54:50 2004
@@ -1343,6 +1343,8 @@
 			int unit;
 			if (!hwif->present)
 				continue;
+			if (hwif->chipset == ide_forced)
+				hwif->chipset = ide_unknown;
 			for (unit = 0; unit < MAX_DRIVES; ++unit)
 				if (hwif->drives[unit].present)
 					ata_attach(&hwif->drives[unit]);
diff -urN linux-2.6.0-vanilla/drivers/ide/ide-proc.c linux-2.6.0/drivers/ide/ide-proc.c
--- linux-2.6.0-vanilla/drivers/ide/ide-proc.c	Thu Nov 27 07:44:17 2003
+++ linux-2.6.0/drivers/ide/ide-proc.c	Tue Jan  6 12:40:58 2004
@@ -348,9 +348,13 @@
 	int		len;
 	const char	*name;
 
+	/*
+	 * Note: Don't need to handle "ide_forced" as that should never be set at
+	 * this stage.
+	 */
+	
 	switch (hwif->chipset) {
 		case ide_unknown:	name = "(none)";	break;
-		case ide_generic:	name = "generic";	break;
 		case ide_pci:		name = "pci";		break;
 		case ide_cmd640:	name = "cmd640";	break;
 		case ide_dtc2278:	name = "dtc2278";	break;
diff -urN linux-2.6.0-vanilla/drivers/ide/ide.c linux-2.6.0/drivers/ide/ide.c
--- linux-2.6.0-vanilla/drivers/ide/ide.c	Thu Nov 27 07:43:29 2003
+++ linux-2.6.0/drivers/ide/ide.c	Tue Jan  6 12:41:19 2004
@@ -2185,7 +2185,7 @@
 				memcpy(hwif->io_ports, hwif->hw.io_ports, sizeof(hwif->io_ports));
 				hwif->irq      = vals[2];
 				hwif->noprobe  = 0;
-				hwif->chipset  = ide_generic;
+				hwif->chipset  = ide_forced;
 				goto done;
 
 			case 0: goto bad_option;
diff -urN linux-2.6.0-vanilla/drivers/ide/pci/cmd640.c linux-2.6.0/drivers/ide/pci/cmd640.c
--- linux-2.6.0-vanilla/drivers/ide/pci/cmd640.c	Thu Nov 27 07:45:36 2003
+++ linux-2.6.0/drivers/ide/pci/cmd640.c	Tue Jan  6 12:42:20 2004
@@ -419,7 +419,7 @@
 	cmd_hwif1 = &ide_hwifs[1]; /* default, if not found below */
 	for (i = 0; i < MAX_HWIFS; i++) {
 		ide_hwif_t *hwif = &ide_hwifs[i];
-		if (hwif->chipset == ide_unknown || hwif->chipset == ide_generic) {
+		if (hwif->chipset == ide_unknown || hwif->chipset == ide_forced) {
 			if (hwif->io_ports[IDE_DATA_OFFSET] == 0x1f0)
 				cmd_hwif0 = hwif;
 			else if (hwif->io_ports[IDE_DATA_OFFSET] == 0x170)
diff -urN linux-2.6.0-vanilla/drivers/ide/setup-pci.c linux-2.6.0/drivers/ide/setup-pci.c
--- linux-2.6.0-vanilla/drivers/ide/setup-pci.c	Thu Nov 27 07:43:39 2003
+++ linux-2.6.0/drivers/ide/setup-pci.c	Tue Jan  6 13:44:45 2004
@@ -59,7 +59,7 @@
 	for (h = 0; h < MAX_HWIFS; ++h) {
 		hwif = &ide_hwifs[h];
 		if (hwif->io_ports[IDE_DATA_OFFSET] == io_base) {
-			if (hwif->chipset == ide_generic)
+			if (hwif->chipset == ide_forced)
 				return hwif; /* a perfect match */
 		}
 	}
@@ -91,19 +91,19 @@
 	if (bootable) {
 		for (h = 0; h < MAX_HWIFS; ++h) {
 			hwif = &ide_hwifs[h];
-			if (hwif->chipset == ide_unknown)
+			if (hwif->chipset == ide_unknown && !hwif->present)
 				return hwif;	/* pick an unused entry */
 		}
 	} else {
 		for (h = 2; h < MAX_HWIFS; ++h) {
 			hwif = ide_hwifs + h;
-			if (hwif->chipset == ide_unknown)
+			if (hwif->chipset == ide_unknown && !hwif->present)
 				return hwif;	/* pick an unused entry */
 		}
 	}
 	for (h = 0; h < 2; ++h) {
 		hwif = ide_hwifs + h;
-		if (hwif->chipset == ide_unknown)
+		if (hwif->chipset == ide_unknown && !hwif->present)
 			return hwif;	/* pick an unused entry */
 	}
 	printk(KERN_ERR "%s: too many IDE interfaces, no room in table\n", name);
@@ -441,6 +441,8 @@
 	}
 	if ((hwif = ide_match_hwif(base, d->bootable, d->name)) == NULL)
 		return NULL;	/* no room in ide_hwifs[] */
+	if (hwif->present)
+		return NULL;
 	if (hwif->io_ports[IDE_DATA_OFFSET] != base) {
 fixup_address:
 		ide_init_hwif_ports(&hwif->hw, base, (ctl | 2), NULL);
diff -urN linux-2.6.0-vanilla/include/linux/ide.h linux-2.6.0/include/linux/ide.h
--- linux-2.6.0-vanilla/include/linux/ide.h	Thu Nov 27 07:43:36 2003
+++ linux-2.6.0/include/linux/ide.h	Tue Jan  6 12:36:46 2004
@@ -276,7 +276,7 @@
  * hwif_chipset_t is used to keep track of the specific hardware
  * chipset used by each IDE interface, if known.
  */
-typedef enum {	ide_unknown,	ide_generic,	ide_pci,
+typedef enum {	ide_unknown,	ide_forced,	ide_pci,
 		ide_cmd640,	ide_dtc2278,	ide_ali14xx,
 		ide_qd65xx,	ide_umc8672,	ide_ht6560b,
 		ide_pdc4030,	ide_rz1000,	ide_trm290,

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/