PATCH: straighten out the IDE layer locking and add hotplug

[Alan Cox <alan@redhat.com>]

There really isnt any sane way to break this patch down because all the
changes are interlinked so closely.

We now
	- enforce and document an actual lock order
	- use a seperate semaphore to get sleep-read/spin-write locking for
	  the lists that need it
	- Fix the driver list locking
	- Fix the hwif list locking
	- Remove the impossible to fix and unused stuff for passing
	  a device between drivers
	- Change ide_unregister into ide_unregister_hwif as it now takes
	  a hwif pointer
	- Fix various cases we wrongly dropped locks
	- Split configured/present so that we can do unregister right
	- Call ->remove() callbacks

This patch leaves pci and ide-cs broken, patches to move them to the new
locking follow 


diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.8-rc3/drivers/ide/ide.c linux-2.6.8-rc3/drivers/ide/ide.c
--- linux.vanilla-2.6.8-rc3/drivers/ide/ide.c	2004-08-09 15:51:00.000000000 +0100
+++ linux-2.6.8-rc3/drivers/ide/ide.c	2004-08-12 17:55:05.000000000 +0100
@@ -448,31 +515,46 @@
 	return -ENXIO;
 }
 
+/*
+ *	drives_lock protects the list of drives, drivers lock the
+ *	list of drivers. Currently nobody takes both at once.
+ *	drivers_sem guards the drivers_list for readers that may
+ *	sleep. It must be taken before drivers_lock. Take drivers_sem
+ *	before ide_setting_sem and idecfg_sem before either of the
+ *	others.
+ */
+ 
 static spinlock_t drives_lock = SPIN_LOCK_UNLOCKED;
+static DECLARE_MUTEX(drivers_sem);
 static spinlock_t drivers_lock = SPIN_LOCK_UNLOCKED;
+
 static LIST_HEAD(drivers);
 
-/* Iterator */
+/* Iterator for the driver list. */
+
 static void *m_start(struct seq_file *m, loff_t *pos)
 {
 	struct list_head *p;
 	loff_t l = *pos;
-	spin_lock(&drivers_lock);
+	down(&drivers_sem);
 	list_for_each(p, &drivers)
 		if (!l--)
 			return list_entry(p, ide_driver_t, drivers);
 	return NULL;
 }
+
 static void *m_next(struct seq_file *m, void *v, loff_t *pos)
 {
 	struct list_head *p = ((ide_driver_t *)v)->drivers.next;
 	(*pos)++;
 	return p==&drivers ? NULL : list_entry(p, ide_driver_t, drivers);
 }
+
 static void m_stop(struct seq_file *m, void *v)
 {
-	spin_unlock(&drivers_lock);
+	up(&drivers_sem);
 }
+
 static int show_driver(struct seq_file *m, void *v)
 {
 	ide_driver_t *driver = v;
@@ -515,6 +597,7 @@
  *	MMIO leaves it to the controller driver,
  *	PIO will migrate this way over time.
  */
+ 
 int ide_hwif_request_regions(ide_hwif_t *hwif)
 {
 	unsigned long addr;
@@ -564,6 +647,7 @@
  *	importantly our caller should be doing this so we need to 
  *	restructure this as a helper function for drivers.
  */
+
 void ide_hwif_release_regions(ide_hwif_t *hwif)
 {
 	u32 i = 0;
@@ -581,7 +665,15 @@
 			release_region(hwif->io_ports[i], 1);
 }
 
-/* restore hwif to a sane state */
+/**
+ *	ide_hwif_restore	-	restore hwif to template
+ *	@hwif: hwif to update
+ *	@tmp_hwif: template
+ *
+ *	Restore hwif to a default state by copying most settngs
+ *	from the template.
+ */
+ 
 static void ide_hwif_restore(ide_hwif_t *hwif, ide_hwif_t *tmp_hwif)
 {
 	hwif->hwgroup			= tmp_hwif->hwgroup;
@@ -678,15 +771,16 @@
 }
 
 /**
- *	ide_unregister		-	free an ide interface
- *	@index: index of interface (will change soon to a pointer)
+ *	__ide_unregister_hwif	-	free an ide interface
+ *	@hwif: interface to unregister
  *
  *	Perform the final unregister of an IDE interface. At the moment
  *	we don't refcount interfaces so this will also get split up.
  *
  *	Locking:
- *	The caller must not hold the IDE locks
- *	The drive present/vanishing is not yet properly locked
+ *	The caller must not hold the IDE locks except for ide_cfg_sem
+ *	which must be held.
+ *
  *	Take care with the callbacks. These have been split to avoid
  *	deadlocking the IDE layer. The shutdown callback is called
  *	before we take the lock and free resources. It is up to the
@@ -695,31 +789,43 @@
  *	isnt yet done btw). After we commit to the final kill we
  *	call the cleanup callback with the ide locks held.
  *
+ *	An interface can be in four states we care about
+ *	- It can be busy (drive or driver thinks its active). No unload
+ *	- It can be unconfigured - which means its already gone
+ *	- It can be configured and present - a full interface
+ *	- It can be configured and not present - pci configured but no drives
+ *						 so logically absent.
+ *
  *	Unregister restores the hwif structures to the default state.
- *	This is raving bonkers.
  */
 
-void ide_unregister(unsigned int index)
+int __ide_unregister_hwif(ide_hwif_t *hwif)
 {
 	ide_drive_t *drive;
-	ide_hwif_t *hwif, *g, *tmp_hwif;
+	ide_hwif_t *g;
 	ide_hwgroup_t *hwgroup;
 	int irq_count = 0, unit, i;
+	int index;	
+	static ide_hwif_t tmp_hwif;	/* Serialized by locking */
+	int ret = 0;
+	int was_present;
 
-	BUG_ON(index >= MAX_HWIFS);
-
-	tmp_hwif = kmalloc(sizeof(*tmp_hwif), GFP_KERNEL|__GFP_NOFAIL);
-	if (!tmp_hwif) {
-		printk(KERN_ERR "%s: unable to allocate memory\n", __FUNCTION__);
-		return;
-	}
-
+	index = hwif->index;
+	
 	BUG_ON(in_interrupt());
 	BUG_ON(irqs_disabled());
-	down(&ide_cfg_sem);
+
+	/* Make sure nobody sneaks in via the proc interface */	
+	down(&ide_setting_sem);
+	
+	/* Now ensure nobody gets in for I/O while we clean up and
+	   do the busy check. If busy is set then the device is still
+	   open and we must stop */
 	spin_lock_irq(&ide_lock);
-	hwif = &ide_hwifs[index];
-	if (!hwif->present)
+	
+	printk("Unregister %d\n", index);
+	
+	if (!hwif->configured)
 		goto abort;
 	for (unit = 0; unit < MAX_DRIVES; ++unit) {
 		drive = &hwif->drives[unit];
@@ -729,10 +835,18 @@
 			goto abort;
 		drive->dead = 1;
 	}
+	/* 
+	 * Protect against new users. From this point the hwif
+	 * is not present so cannot be opened by a new I/O source.
+	 * This also invalidates key driven access from procfs
+	 */
+
+	was_present = hwif->present;	 
 	hwif->present = 0;
 
 	spin_unlock_irq(&ide_lock);
-
+	up(&ide_setting_sem);
+	
 	for (unit = 0; unit < MAX_DRIVES; ++unit) {
 		drive = &hwif->drives[unit];
 		if (!drive->present)
@@ -743,27 +857,36 @@
 #ifdef CONFIG_PROC_FS
 	destroy_proc_ide_drives(hwif);
 #endif
-
+	spin_lock_irq(&ide_lock);
+	
 	hwgroup = hwif->hwgroup;
-	/*
-	 * free the irq if we were the only hwif using it
-	 */
-	g = hwgroup->hwif;
-	do {
-		if (g->irq == hwif->irq)
-			++irq_count;
-		g = g->next;
-	} while (g != hwgroup->hwif);
-	if (irq_count == 1)
+	if(hwgroup)
+	{
+		/*
+		 * free the irq if we were the only hwif using it
+		 */
+		g = hwgroup->hwif;
+		do {
+			if (g->irq == hwif->irq)
+				++irq_count;
+			g = g->next;
+		} while (g != hwgroup->hwif);
+	}
+	spin_unlock_irq(&ide_lock);
+	
+	if (irq_count == 1 && hwgroup)
 		free_irq(hwif->irq, hwgroup);
 
-	spin_lock_irq(&ide_lock);
 	/*
 	 * Note that we only release the standard ports,
 	 * and do not even try to handle any extra ports
 	 * allocated for weird IDE interface chipsets.
+	 *
+	 * FIXME: should defer this I think
 	 */
-	ide_hwif_release_regions(hwif);
+
+	if(was_present)
+		ide_hwif_release_regions(hwif);
 
 	/*
 	 * Remove us from the hwgroup, and free
@@ -777,6 +900,14 @@
 		}
 		if (!drive->present)
 			continue;
+			
+		/*
+		 * The hwgroup chain is IRQ touched. We must protect
+		 * walking this from an IDE event for another device
+		 * in the chain 
+		 */
+		   
+		spin_lock_irq(&ide_lock);
 		if (drive == drive->next) {
 			/* special case: last drive from hwgroup. */
 			BUG_ON(hwgroup->drive != drive);
@@ -793,51 +924,73 @@
 				hwgroup->hwif = HWIF(hwgroup->drive);
 			}
 		}
+		spin_unlock_irq(&ide_lock);
+		
+		/*
+		 * The rest of the cleanup is private
+		 */
+		 
 		BUG_ON(hwgroup->drive == drive);
 		if (drive->id != NULL) {
 			kfree(drive->id);
 			drive->id = NULL;
 		}
 		drive->present = 0;
-		/* Messed up locking ... */
-		spin_unlock_irq(&ide_lock);
 		blk_cleanup_queue(drive->queue);
 		device_unregister(&drive->gendev);
 		down(&drive->gendev_rel_sem);
-		spin_lock_irq(&ide_lock);
 		drive->queue = NULL;
 	}
-	if (hwif->next == hwif) {
-		BUG_ON(hwgroup->hwif != hwif);
-		kfree(hwgroup);
-	} else {
-		/* There is another interface in hwgroup.
-		 * Unlink us, and set hwgroup->drive and ->hwif to
-		 * something sane.
-		 */
-		g = hwgroup->hwif;
-		while (g->next != hwif)
-			g = g->next;
-		g->next = hwif->next;
-		if (hwgroup->hwif == hwif) {
-			/* Chose a random hwif for hwgroup->hwif.
-			 * It's guaranteed that there are no drives
-			 * left in the hwgroup.
+	
+	/*
+	 * Lock against hwgroup walkers including interrupts off other
+	 * IDE devices wile we unhook ourselves.
+	 */
+	 
+	spin_lock_irq(&ide_lock);
+	
+	if (hwgroup)
+	{
+		if (hwif->next == hwif) {
+			BUG_ON(hwgroup->hwif != hwif);
+			kfree(hwgroup);
+		} else {
+			/* There is another interface in hwgroup.
+			 * Unlink us, and set hwgroup->drive and ->hwif to
+			 * something sane.
 			 */
-			BUG_ON(hwgroup->drive != NULL);
-			hwgroup->hwif = g;
+			g = hwgroup->hwif;
+			while (g->next != hwif)
+				g = g->next;
+			g->next = hwif->next;
+			if (hwgroup->hwif == hwif) {
+				/* Chose a random hwif for hwgroup->hwif.
+				 * It's guaranteed that there are no drives
+				 * left in the hwgroup.
+				 */
+				BUG_ON(hwgroup->drive != NULL);
+				hwgroup->hwif = g;
+			}
+			BUG_ON(hwgroup->hwif == hwif);
 		}
-		BUG_ON(hwgroup->hwif == hwif);
 	}
-
-	/* More messed up locking ... */
 	spin_unlock_irq(&ide_lock);
-	device_unregister(&hwif->gendev);
-	down(&hwif->gendev_rel_sem);
+
+	/*
+	 * PCI interfaces with no devices don't exist in the device
+	 * tree so don't unregister them.
+	 */
+	 
+	if(was_present)
+	{
+		device_unregister(&hwif->gendev);
+		down(&hwif->gendev_rel_sem);
+	}
 
 	/*
 	 * Remove us from the kernel's knowledge
 	 */
+
 	blk_unregister_region(MKDEV(hwif->major, 0), MAX_DRIVES<<PARTN_BITS);
 	for (i = 0; i < MAX_DRIVES; i++) {
 		struct gendisk *disk = hwif->drives[i].disk;
@@ -845,8 +998,16 @@
 		put_disk(disk);
 	}
 	unregister_blkdev(hwif->major, hwif->name);
+
 	spin_lock_irq(&ide_lock);
 
+	/*
+	 * Let the driver free up private objects
+	 */
+
+	if(hwif->remove)
+		hwif->remove(hwif);
+		
 	if (hwif->dma_base) {
 		(void) ide_release_dma(hwif);
 
@@ -858,24 +1019,70 @@
 		hwif->dma_vendor3 = 0;
 		hwif->dma_prdtable = 0;
 	}
+	
+	hwif->chipset = ide_unknown;
 
 	/* copy original settings */
-	*tmp_hwif = *hwif;
+	tmp_hwif = *hwif;
 
 	/* restore hwif data to pristine status */
 	init_hwif_data(hwif, index);
 	init_hwif_default(hwif, index);
+	
+	hwif->key++;
+	hwif->configured = 0;
 
-	ide_hwif_restore(hwif, tmp_hwif);
+	ide_hwif_restore(hwif, &tmp_hwif);
+	
+	spin_unlock_irq(&ide_lock);
+	return 0;
 
 abort:
+	if(hwif->configured)
+	{
+		printk("Unregister %d fail %d %d\n", index, drive->usage, DRIVER(drive)->busy);
+		ret = -EBUSY;
+	}
+	else
+	{
+		printk("No such hwif!\n");
+		ret = -ENOENT;
+	}
 	spin_unlock_irq(&ide_lock);
-	up(&ide_cfg_sem);
+	up(&ide_setting_sem);
+	return ret;
+}
+
+EXPORT_SYMBOL_GPL(__ide_unregister_hwif);
 
-	kfree(tmp_hwif);
+
+/**
+ *	ide_unregister_hwif	-	free an ide interface
+ *	@hwif: interface to unregister
+ *
+ *	Perform the final unregister of an IDE interface. At the moment
+ *	we don't refcount interfaces so this will also get split up.
+ *	Unregister restores the hwif structures to the default state.
+ *
+ *	No locks should be held on entry. When an unregister must
+ *	be done atomically with a register see __ide_unregister_hwif
+ *	and hold the ide_cfg_sem yourself.
+ */
+
+int ide_unregister_hwif(ide_hwif_t *hwif)
+{
+	int ret;
+	
+	/* This protects two things. Firstly it serializes the 
+	   shutdown sequence, secondly it protects us from
+	   races while we are killing off a device */
+	down(&ide_cfg_sem);
+	ret = __ide_unregister_hwif(hwif);
+	up(&ide_cfg_sem);
+	return ret;
 }
 
-EXPORT_SYMBOL(ide_unregister);
+EXPORT_SYMBOL_GPL(ide_unregister_hwif);
 
 
 /**
@@ -931,15 +1138,25 @@
  */
 }
 
-/*
- * Register an IDE interface, specifying exactly the registers etc
- * Set init=1 iff calling before probes have taken place.
+/**
+ *	ide_register_hw		-	register IDE interface
+ *	@hw: hardware registers
+ *	@hwifp: pointer to returned hwif
+ *
+ *	Register an IDE interface, specifying exactly the registers etc
+ *	Set init=1 iff calling before probes have taken place. The 
+ *	ide_cfg_sem protects this against races.
+ *
+ *	Returns -1 on error.
  */
+ 
 int ide_register_hw (hw_regs_t *hw, ide_hwif_t **hwifp)
 {
 	int index, retry = 1;
 	ide_hwif_t *hwif;
 
+	down(&ide_cfg_sem);
+	
 	do {
 		for (index = 0; index < MAX_HWIFS; ++index) {
 			hwif = &ide_hwifs[index];
@@ -950,28 +1167,37 @@
 			hwif = &ide_hwifs[index];
 			if (hwif->hold)
 				continue;
-			if ((!hwif->present && !hwif->mate && !initializing) ||
+			if ((!hwif->configured && !hwif->mate && !initializing) ||
 			    (!hwif->hw.io_ports[IDE_DATA_OFFSET] && initializing))
 				goto found;
 		}
+		/* FIXME- this check should die as should the retry loop */
 		for (index = 0; index < MAX_HWIFS; index++)
-			ide_unregister(index);
+		{
+			hwif = &ide_hwifs[index];
+			__ide_unregister_hwif(hwif);
+		}
 	} while (retry--);
+	
+	up(&ide_cfg_sem);
 	return -1;
 found:
-	if (hwif->present)
-		ide_unregister(index);
+	/* FIXME: do we really need this case */
+	if (hwif->configured)
+		__ide_unregister_hwif(hwif);
 	else if (!hwif->hold) {
 		init_hwif_data(hwif, index);
 		init_hwif_default(hwif, index);
 	}
-	if (hwif->present)
+	if (hwif->configured)
 		return -1;
+	hwif->configured = 1;
 	memcpy(&hwif->hw, hw, sizeof(*hw));
 	memcpy(hwif->io_ports, hwif->hw.io_ports, sizeof(hwif->hw.io_ports));
 	hwif->irq = hw->irq;
 	hwif->noprobe = 0;
 	hwif->chipset = hw->chipset;
+	up(&ide_cfg_sem);
 
 	if (!initializing) {
 		probe_hwif_init(hwif);
@@ -1008,21 +1234,21 @@
  *	@set: setting
  *
  *	Removes the setting named from the device if it is present.
- *	The function takes the settings_lock to protect against 
- *	parallel changes. This function must not be called from IRQ
- *	context. Returns 0 on success or -1 on failure.
+ *	This function must not be called from IRQ context. Returns 0
+ *	on success or -1 on failure.
  *
  *	BUGS: This code is seriously over-engineered. There is also
  *	magic about how the driver specific features are setup. If
  *	a driver is attached we assume the driver settings are auto
  *	remove.
+ *
+ *	The caller must hold settings_lock
  */
  
 int ide_add_setting (ide_drive_t *drive, const char *name, int rw, int read_ioctl, int write_ioctl, int data_type, int min, int max, int mul_factor, int div_factor, void *data, ide_procset_t *set)
 {
 	ide_settings_t **p = (ide_settings_t **) &drive->settings, *setting = NULL;
 
-	down(&ide_setting_sem);
 	while ((*p) && strcmp((*p)->name, name) < 0)
 		p = &((*p)->next);
 	if ((setting = kmalloc(sizeof(*setting), GFP_KERNEL)) == NULL)
@@ -1046,10 +1272,8 @@
 	if (drive->driver != &idedefault_driver)
 		setting->auto_remove = 1;
 	*p = setting;
-	up(&ide_setting_sem);
 	return 0;
 abort:
-	up(&ide_setting_sem);
 	if (setting)
 		kfree(setting);
 	return -1;
@@ -1058,7 +1282,7 @@
 EXPORT_SYMBOL(ide_add_setting);
 
 /**
- *	__ide_remove_setting	-	remove an ide setting option
+ *	ide_remove_setting	-	remove an ide setting option
  *	@drive: drive to use
  *	@name: setting name
  *
@@ -1066,7 +1290,7 @@
  *	The caller must hold the setting semaphore.
  */
  
-static void __ide_remove_setting (ide_drive_t *drive, char *name)
+static void ide_remove_setting (ide_drive_t *drive, char *name)
 {
 	ide_settings_t **p, *setting;
 
@@ -1144,7 +1368,7 @@
 	setting = drive->settings;
 	while (setting) {
 		if (setting->auto_remove) {
-			__ide_remove_setting(drive, setting->name);
+			ide_remove_setting(drive, setting->name);
 			goto repeat;
 		}
 		setting = setting->next;
@@ -1322,25 +1556,19 @@
 	return err;
 }
 
-int ide_atapi_to_scsi (ide_drive_t *drive, int arg)
-{
-	if (drive->media == ide_disk) {
-		drive->scsi = 0;
-		return 0;
-	}
-
-	if (DRIVER(drive)->cleanup(drive)) {
-		drive->scsi = 0;
-		return 0;
-	}
-
-	drive->scsi = (u8) arg;
-	ata_attach(drive);
-	return 0;
-}
 
+/**
+ *	ide_add_generic_settings	-	generic /proc settings
+ *	@drive: drive being configured
+ *
+ *	Add the generic parts of the system settings to the /proc files
+ *	for this IDE device. The caller must not be holding the settings_sem
+ *	.lock
+ */
+ 
 void ide_add_generic_settings (ide_drive_t *drive)
 {
+	down(&ide_setting_sem);
 /*
  *			drive	setting name		read/write access				read ioctl		write ioctl		data type	min	max				mul_factor	div_factor	data pointer			set function
  */
@@ -1353,10 +1581,17 @@
 	ide_add_setting(drive,	"init_speed",		SETTING_RW,					-1,			-1,			TYPE_BYTE,	0,	70,				1,		1,		&drive->init_speed,		NULL);
 	ide_add_setting(drive,	"current_speed",	SETTING_RW,					-1,			-1,			TYPE_BYTE,	0,	70,				1,		1,		&drive->current_speed,		set_xfer_rate);
 	ide_add_setting(drive,	"number",		SETTING_RW,					-1,			-1,			TYPE_BYTE,	0,	3,				1,		1,		&drive->dn,			NULL);
-	if (drive->media != ide_disk)
-		ide_add_setting(drive,	"ide-scsi",		SETTING_RW,					-1,		HDIO_SET_IDE_SCSI,		TYPE_BYTE,	0,	1,				1,		1,		&drive->scsi,			ide_atapi_to_scsi);
+	
+	up(&ide_setting_sem);
 }
 
+/**
+ *	system_bus_clock	-	clock guess
+ *
+ *	External version of the bus clock guess used by old old IDE drivers
+ *	for things like VLB timings. Should not be used.
+ */
+ 
 int system_bus_clock (void)
 {
 	return((int) ((!system_bus_speed) ? ide_system_bus_speed() : system_bus_speed ));
@@ -1364,52 +1599,42 @@
 
 EXPORT_SYMBOL(system_bus_clock);
 
-/*
- *	Locking is badly broken here - since way back.  That sucker is
- * root-only, but that's not an excuse...  The real question is what
- * exclusion rules do we want here.
+/**
+ *	ata_attach		-	attach an ATA/ATAPI device
+ *	@drive: drive to attach
+ *
+ *	Takes a drive that is as yet not assigned to any midlayer IDE
+ *	module and figures out which driver would like to own it. If 
+ *	nobody claims the driver then it is automatically attached
+ *	to the default driver used for unclaimed objects.
+ *
+ *	A return of zero indicates attachment to a driver, of one
+ *	attachment to the default driver
+ *
+ *	Takes the driver list lock and the ide_settings semaphore.
  */
-int ide_replace_subdriver (ide_drive_t *drive, const char *driver)
-{
-	if (!drive->present || drive->usage || drive->dead)
-		goto abort;
-	if (DRIVER(drive)->cleanup(drive))
-		goto abort;
-	strlcpy(drive->driver_req, driver, sizeof(drive->driver_req));
-	if (ata_attach(drive)) {
-		spin_lock(&drives_lock);
-		list_del_init(&drive->list);
-		spin_unlock(&drives_lock);
-		drive->driver_req[0] = 0;
-		ata_attach(drive);
-	} else {
-		drive->driver_req[0] = 0;
-	}
-	if (DRIVER(drive)!= &idedefault_driver && !strcmp(DRIVER(drive)->name, driver))
-		return 0;
-abort:
-	return 1;
-}
-
+ 
 int ata_attach(ide_drive_t *drive)
 {
 	struct list_head *p;
-	spin_lock(&drivers_lock);
+	down(&drivers_sem);
+	down(&ide_setting_sem);
 	list_for_each(p, &drivers) {
 		ide_driver_t *driver = list_entry(p, ide_driver_t, drivers);
 		if (!try_module_get(driver->owner))
 			continue;
-		spin_unlock(&drivers_lock);
 		if (driver->attach(drive) == 0) {
 			module_put(driver->owner);
 			drive->gendev.driver = &driver->gen_driver;
+			up(&ide_setting_sem);
+			up(&drivers_sem);
 			return 0;
 		}
-		spin_lock(&drivers_lock);
 		module_put(driver->owner);
 	}
 	drive->gendev.driver = &idedefault_driver.gen_driver;
-	spin_unlock(&drivers_lock);
+	up(&ide_setting_sem);
+	up(&drivers_sem);
 	if(idedefault_driver.attach(drive) != 0)
 		panic("ide: default attach failed");
 	return 1;
@@ -1549,9 +1774,11 @@
 		}
 	        case HDIO_UNREGISTER_HWIF:
 			if (!capable(CAP_SYS_RAWIO)) return -EACCES;
-			/* (arg > MAX_HWIFS) checked in function */
-			ide_unregister(arg);
-			return 0;
+			if(arg > MAX_HWIFS || arg < 0)
+				return -EINVAL;
+			if(ide_hwifs[arg].pci_dev)
+				return -EINVAL;
+			return ide_unregister_hwif(&ide_hwifs[arg]);
 		case HDIO_SET_NICE:
 			if (!capable(CAP_SYS_ADMIN)) return -EACCES;
 			if (arg != (arg & ((1 << IDE_NICE_DSC_OVERLAP) | (1 << IDE_NICE_1))))
@@ -2199,9 +2426,23 @@
 
 EXPORT_SYMBOL(ide_register_subdriver);
 
+/**
+ *	ide_unregister_subdriver	-	disconnect drive from driver
+ *	@drive: drive to unplug
+ *
+ *	Disconnect a drive from the driver it was attached to and then
+ *	clean up the various proc files and other objects attached to it.
+ *	Takes ide_sem, ide_lock, and drive_lock. Caller must hold none of
+ *	the locks.
+ *
+ *	No locking versus subdriver unload because we are moving to the
+ *	default driver anyway. Wants double checking.
+ */
+ 
 int ide_unregister_subdriver (ide_drive_t *drive)
 {
 	unsigned long flags;
+	ide_proc_entry_t *dir;
 	
 	down(&ide_setting_sem);
 	spin_lock_irqsave(&ide_lock, flags);
@@ -2210,13 +2451,14 @@
 		up(&ide_setting_sem);
 		return 1;
 	}
+	dir = DRIVER(drive)->proc;
+	drive->driver = &idedefault_driver;
+	spin_unlock_irqrestore(&ide_lock, flags);
 #ifdef CONFIG_PROC_FS
-	ide_remove_proc_entries(drive->proc, DRIVER(drive)->proc);
+	ide_remove_proc_entries(drive->proc, dir);
 	ide_remove_proc_entries(drive->proc, generic_subdriver_entries);
 #endif
 	auto_remove_settings(drive);
-	drive->driver = &idedefault_driver;
-	spin_unlock_irqrestore(&ide_lock, flags);
 	up(&ide_setting_sem);
 	spin_lock(&drives_lock);
 	list_del_init(&drive->list);
@@ -2234,6 +2476,18 @@
 	return 0;
 }
 
+/**
+ *	ide_register_driver	-	new driver loaded
+ *	@driver: the IDE driver module
+ *
+ *	Register a new driver module and then scan the devices
+ *	on the IDE bus in case any should be attached to the 
+ *	driver we have just attached. If so attach them
+ *
+ *	Takes the drivers and drives lock. Should take the
+ *	ide_sem but doesn't - FIXME
+ */
+ 
 int ide_register_driver(ide_driver_t *driver)
 {
 	struct list_head list;
@@ -2242,9 +2496,11 @@
 
 	setup_driver_defaults(driver);
 
+	down(&drivers_sem);
 	spin_lock(&drivers_lock);
 	list_add(&driver->drivers, &drivers);
 	spin_unlock(&drivers_lock);
+	up(&drivers_sem);
 
 	INIT_LIST_HEAD(&list);
 	spin_lock(&drives_lock);
@@ -2265,13 +2521,25 @@
 
 EXPORT_SYMBOL(ide_register_driver);
 
+/**
+ *	ide_unregister_driver	-	IDE module unload
+ *	@driver: IDE driver module
+ *
+ *	Unload a driver module and reattach any devices to whatever
+ *	driver claims them next (typically the default driver). Takes
+ *	the drivers lock, and called functions will take the 
+ *	IDE setting semaphore.
+ */
+ 
 void ide_unregister_driver(ide_driver_t *driver)
 {
 	ide_drive_t *drive;
 
+	down(&drivers_sem);
 	spin_lock(&drivers_lock);
 	list_del(&driver->drivers);
 	spin_unlock(&drivers_lock);
+	up(&drivers_sem);
 
 	driver_unregister(&driver->gen_driver);
 
@@ -2385,7 +2653,8 @@
 	int index;
 
 	for (index = 0; index < MAX_HWIFS; ++index) {
-		ide_unregister(index);
+		if(ide_unregister_hwif(&ide_hwifs[index]))
+			printk(KERN_ERR "ide: unload yet busy!\n");
 		if (ide_hwifs[index].dma_base)
 			(void) ide_release_dma(&ide_hwifs[index]);
 	}
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.8-rc3/include/linux/ide.h linux-2.6.8-rc3/include/linux/ide.h
--- linux.vanilla-2.6.8-rc3/include/linux/ide.h	2004-08-09 15:50:59.000000000 +0100
+++ linux-2.6.8-rc3/include/linux/ide.h	2004-08-12 16:45:17.000000000 +0100
@@ -1503,9 +1509,11 @@
 extern void ide_pci_unregister_driver(struct pci_driver *driver);
 extern void ide_pci_setup_ports(struct pci_dev *dev, struct ide_pci_device_s *d, int autodma, int pciirq, ata_index_t *index);
 extern void ide_setup_pci_noise (struct pci_dev *dev, struct ide_pci_device_s *d);
+extern void ide_pci_remove_hwifs(struct pci_dev *dev);
 
 extern void default_hwif_iops(ide_hwif_t *);
 extern void default_hwif_mmiops(ide_hwif_t *);
+extern void removed_hwif_iops(ide_hwif_t *);
 extern void default_hwif_transport(ide_hwif_t *);
 
 int ide_register_driver(ide_driver_t *driver);
@@ -1603,8 +1611,9 @@
 #endif
 
 extern int ide_hwif_request_regions(ide_hwif_t *hwif);
-extern void ide_hwif_release_regions(ide_hwif_t* hwif);
-extern void ide_unregister (unsigned int index);
+extern void ide_hwif_release_regions(ide_hwif_t *hwif);
+extern int ide_unregister_hwif(ide_hwif_t *hwif);
+extern int __ide_unregister_hwif(ide_hwif_t *hwif);
 
 extern int probe_hwif_init(ide_hwif_t *);


Signed-off-by: Alan Cox <alan@redhat.com>