[PATCH 0/15] ide: fixes/cleanups for IDE devices probing

[PATCH 0/15] ide: fixes/cleanups for IDE devices probing

[Bartlomiej Zolnierkiewicz]

This is a preparation for a more sophisticated changes.

diffstat:

 drivers/ide/ide-iops.c            |    2
 drivers/ide/ide-probe.c           |  199 ++++++++++++++++----------------------
 drivers/ide/ide.c                 |  109 ++++++++++----------
 drivers/ide/legacy/ide_platform.c |   34 +-----
 drivers/ide/pci/ns87415.c         |    4
 include/linux/ide.h               |   15 --
 6 files changed, 157 insertions(+), 206 deletions(-)

Bart

[PATCH 1/15] ide: fix handling of busy I/O resources in probe_hwif()

[Bartlomiej Zolnierkiewicz]

It could be that I/O resources are busy because some other host driver
has already claimed them so don't unregister the devices.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
-94 bytes
 drivers/ide/ide-probe.c |   13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -727,18 +727,7 @@ static void probe_hwif(ide_hwif_t *hwif)
 
 	if ((hwif->chipset != ide_4drives || !hwif->mate || !hwif->mate->present) &&
 	    (ide_hwif_request_regions(hwif))) {
-		u16 msgout = 0;
-		for (unit = 0; unit < MAX_DRIVES; ++unit) {
-			ide_drive_t *drive = &hwif->drives[unit];
-			if (drive->present) {
-				drive->present = 0;
-				printk(KERN_ERR "%s: ERROR, PORTS ALREADY IN USE\n",
-					drive->name);
-				msgout = 1;
-			}
-		}
-		if (!msgout)
-			printk(KERN_ERR "%s: ports already in use, skipping probe\n",
+		printk(KERN_ERR "%s: ports already in use, skipping probe\n",
 				hwif->name);
 		return;	
 	}

[PATCH 2/15] ide: move wait_hwif_ready() documentation in the right place

[Bartlomiej Zolnierkiewicz]

* Move wait_hwif_ready() documentation before this function.

* Fix trailing whitespaces.

* s/wait-for-busy/wait-for-non-busy/

* Remove no longer valid comment about the current code behavior.

* Docbook-ize it.

* Rename wait_hwif_ready() to ide_port_wait_ready().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-probe.c |   49 +++++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -634,7 +634,33 @@ static void hwif_register (ide_hwif_t *h
 			__FUNCTION__, ret);
 }
 
-static int wait_hwif_ready(ide_hwif_t *hwif)
+/**
+ *	ide_port_wait_ready	-	wait for port to become ready
+ *	@hwif: IDE port
+ *
+ *	This is needed on some PPCs and a bunch of BIOS-less embedded
+ *	platforms.  Typical cases are:
+ *
+ *	- The firmware hard reset the disk before booting the kernel,
+ *	  the drive is still doing it's poweron-reset sequence, that
+ *	  can take up to 30 seconds.
+ *
+ *	- The firmware does nothing (or no firmware), the device is
+ *	  still in POST state (same as above actually).
+ *
+ *	- Some CD/DVD/Writer combo drives tend to drive the bus during
+ *	  their reset sequence even when they are non-selected slave
+ *	  devices, thus preventing discovery of the main HD.
+ *
+ *	Doing this wait-for-non-busy should not harm any existing
+ *	configuration and fix some issues like the above.
+ *
+ *	BenH.
+ *
+ *	Returns 0 on success, error code (< 0) otherwise.
+ */
+
+static int ide_port_wait_ready(ide_hwif_t *hwif)
 {
 	int unit, rc;
 
@@ -742,26 +768,7 @@ static void probe_hwif(ide_hwif_t *hwif)
 
 	local_irq_set(flags);
 
-	/* This is needed on some PPCs and a bunch of BIOS-less embedded
-	 * platforms. Typical cases are:
-	 * 
-	 *  - The firmware hard reset the disk before booting the kernel,
-	 *    the drive is still doing it's poweron-reset sequence, that
-	 *    can take up to 30 seconds
-	 *  - The firmware does nothing (or no firmware), the device is
-	 *    still in POST state (same as above actually).
-	 *  - Some CD/DVD/Writer combo drives tend to drive the bus during
-	 *    their reset sequence even when they are non-selected slave
-	 *    devices, thus preventing discovery of the main HD
-	 *    
-	 *  Doing this wait-for-busy should not harm any existing configuration
-	 *  (at least things won't be worse than what current code does, that
-	 *  is blindly go & talk to the drive) and fix some issues like the
-	 *  above.
-	 *  
-	 *  BenH.
-	 */
-	if (wait_hwif_ready(hwif) == -EBUSY)
+	if (ide_port_wait_ready(hwif) == -EBUSY)
 		printk(KERN_DEBUG "%s: Wait for ready failed before probe !\n", hwif->name);
 
 	/*

[PATCH 3/15] ide: remove hwif->present manipulations from hwif_init()

[Bartlomiej Zolnierkiewicz]

* Call hwif_init() only if hwif->present is set.

* If hwif_init() fails clear hwif->present.

* Remove hwif->present manipulations from hwif_init().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
-24 bytes
 drivers/ide/ide-probe.c |   18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1245,28 +1245,21 @@ static int hwif_init(ide_hwif_t *hwif)
 {
 	int old_irq;
 
-	/* Return success if no device is connected */
-	if (!hwif->present)
-		return 1;
-
 	if (!hwif->irq) {
 		if (!(hwif->irq = ide_default_irq(hwif->io_ports[IDE_DATA_OFFSET])))
 		{
 			printk("%s: DISABLED, NO IRQ\n", hwif->name);
-			return (hwif->present = 0);
+			return 0;
 		}
 	}
 #ifdef CONFIG_BLK_DEV_HD
 	if (hwif->irq == HD_IRQ && hwif->io_ports[IDE_DATA_OFFSET] != HD_DATA) {
 		printk("%s: CANNOT SHARE IRQ WITH OLD "
 			"HARDDISK DRIVER (hd.c)\n", hwif->name);
-		return (hwif->present = 0);
+		return 0;
 	}
 #endif /* CONFIG_BLK_DEV_HD */
 
-	/* we set it back to 1 if all is ok below */	
-	hwif->present = 0;
-
 	if (register_blkdev(hwif->major, hwif->name))
 		return 0;
 
@@ -1305,10 +1298,7 @@ static int hwif_init(ide_hwif_t *hwif)
 
 done:
 	init_gendisk(hwif);
-
 	ide_acpi_init(hwif);
-
-	hwif->present = 1;	/* success */
 	return 1;
 
 out:
@@ -1352,9 +1342,13 @@ int ide_device_add_all(u8 *idx)
 
 		hwif = &ide_hwifs[idx[i]];
 
+		if (!hwif->present)
+			continue;
+
 		if (hwif_init(hwif) == 0) {
 			printk(KERN_INFO "%s: failed to initialize IDE "
 					 "interface\n", hwif->name);
+			hwif->present = 0;
 			rc = -1;
 			continue;
 		}

[PATCH 4/15] ide_platform: remove struct hwif_prop

[Bartlomiej Zolnierkiewicz]

* Remove 'if (hwif != hwif_prop.hwif)' check from plat_ide_remove()
  (pdev->dev.driver_data and hwif_prop.hwif are set at the same time
   in plat_ide_probe() and are identical).

* Use hwif->index for ide_unregister() in plat_ide_remove().

* Use local variables instead of hwif_prop.plat_ide[_alt]_mapbase in
  plat_ide_probe() and remove no longer needed struct hwif_prop.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
-150 bytes
 drivers/ide/legacy/ide_platform.c |   34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

Index: b/drivers/ide/legacy/ide_platform.c
===================================================================
--- a/drivers/ide/legacy/ide_platform.c
+++ b/drivers/ide/legacy/ide_platform.c
@@ -21,13 +21,6 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 
-static struct {
-	void __iomem *plat_ide_mapbase;
-	void __iomem *plat_ide_alt_mapbase;
-	ide_hwif_t *hwif;
-	int index;
-} hwif_prop;
-
 static void __devinit plat_ide_setup_ports(hw_regs_t *hw,
 					   void __iomem *base,
 					   void __iomem *ctrl,
@@ -54,6 +47,7 @@ static void __devinit plat_ide_setup_por
 static int __devinit plat_ide_probe(struct platform_device *pdev)
 {
 	struct resource *res_base, *res_alt, *res_irq;
+	void __iomem *base, *alt_base;
 	ide_hwif_t *hwif;
 	struct pata_platform_info *pdata;
 	u8 idx[4] = { 0xff, 0xff, 0xff, 0xff };
@@ -84,27 +78,25 @@ static int __devinit plat_ide_probe(stru
 	}
 
 	if (mmio) {
-		hwif_prop.plat_ide_mapbase = devm_ioremap(&pdev->dev,
+		base = devm_ioremap(&pdev->dev,
 			res_base->start, res_base->end - res_base->start + 1);
-		hwif_prop.plat_ide_alt_mapbase = devm_ioremap(&pdev->dev,
+		alt_base = devm_ioremap(&pdev->dev,
 			res_alt->start, res_alt->end - res_alt->start + 1);
 	} else {
-		hwif_prop.plat_ide_mapbase = devm_ioport_map(&pdev->dev,
+		base = devm_ioport_map(&pdev->dev,
 			res_base->start, res_base->end - res_base->start + 1);
-		hwif_prop.plat_ide_alt_mapbase = devm_ioport_map(&pdev->dev,
+		alt_base = devm_ioport_map(&pdev->dev,
 			res_alt->start, res_alt->end - res_alt->start + 1);
 	}
 
-	hwif = ide_find_port((unsigned long)hwif_prop.plat_ide_mapbase);
+	hwif = ide_find_port((unsigned long)base);
 	if (!hwif) {
 		ret = -ENODEV;
 		goto out;
 	}
 
 	memset(&hw, 0, sizeof(hw));
-	plat_ide_setup_ports(&hw, hwif_prop.plat_ide_mapbase,
-			     hwif_prop.plat_ide_alt_mapbase,
-			     pdata, res_irq->start);
+	plat_ide_setup_ports(&hw, base, alt_base, pdata, res_irq->start);
 	hw.dev = &pdev->dev;
 
 	ide_init_port_hw(hwif, &hw);
@@ -114,9 +106,6 @@ static int __devinit plat_ide_probe(stru
 		default_hwif_mmiops(hwif);
 	}
 
-	hwif_prop.hwif = hwif;
-	hwif_prop.index = hwif->index;
-
 	idx[0] = hwif->index;
 
 	ide_device_add(idx);
@@ -133,14 +122,7 @@ static int __devexit plat_ide_remove(str
 {
 	ide_hwif_t *hwif = pdev->dev.driver_data;
 
-	if (hwif != hwif_prop.hwif) {
-		dev_printk(KERN_DEBUG, &pdev->dev, "%s: hwif value error",
-		           pdev->name);
-	} else {
-		ide_unregister(hwif_prop.index);
-		hwif_prop.index = 0;
-		hwif_prop.hwif = NULL;
-	}
+	ide_unregister(hwif->index);
 
 	return 0;
 }

[PATCH 5/15] ide: remove unused ide_hwgroup_t fields

[Bartlomiej Zolnierkiewicz]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 include/linux/ide.h |   14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -628,8 +628,7 @@ typedef int (ide_expiry_t)(ide_drive_t *
 typedef struct hwgroup_s {
 		/* irq handler, if active */
 	ide_startstop_t	(*handler)(ide_drive_t *);
-		/* irq handler, suspended if active */
-	ide_startstop_t	(*handler_save)(ide_drive_t *);
+
 		/* BOOL: protects all fields below */
 	volatile int busy;
 		/* BOOL: wake us up on timer expiry */
@@ -644,25 +643,18 @@ typedef struct hwgroup_s {
 		/* ptr to current hwif in linked-list */
 	ide_hwif_t *hwif;
 
-		/* for pci chipsets */
-	struct pci_dev *pci_dev;
-
 		/* current request */
 	struct request *rq;
+
 		/* failsafe timer */
 	struct timer_list timer;
-		/* local copy of current write rq */
-	struct request wrq;
 		/* timeout value during long polls */
 	unsigned long poll_timeout;
 		/* queried upon timeouts */
 	int (*expiry)(ide_drive_t *);
-		/* ide_system_bus_speed */
-	int pio_clock;
+
 	int req_gen;
 	int req_gen_timer;
-
-	unsigned char cmd_buf[4];
 } ide_hwgroup_t;
 
 typedef struct ide_driver_s ide_driver_t;

[PATCH 6/15] ide: remove needless zeroing of hwgroup fields from init_irq()

[Bartlomiej Zolnierkiewicz]

Since kmalloc_node() is called with __GFP_ZERO flag there is no need to
explicitly zero hwgroup fields.

While at it:

* Use 'hwif' instead of 'hwif->drives[0].hwif' for kmalloc_node() call.

* Fix whitespace damage.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
-27 bytes
 drivers/ide/ide-probe.c |   15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -998,19 +998,14 @@ static int init_irq (ide_hwif_t *hwif)
 		hwgroup->hwif->next = hwif;
 		spin_unlock_irq(&ide_lock);
 	} else {
-		hwgroup = kmalloc_node(sizeof(ide_hwgroup_t),
-					GFP_KERNEL | __GFP_ZERO,
-					hwif_to_node(hwif->drives[0].hwif));
-		if (!hwgroup)
-	       		goto out_up;
+		hwgroup = kmalloc_node(sizeof(*hwgroup), GFP_KERNEL|__GFP_ZERO,
+				       hwif_to_node(hwif));
+		if (hwgroup == NULL)
+			goto out_up;
 
 		hwif->hwgroup = hwgroup;
+		hwgroup->hwif = hwif->next = hwif;
 
-		hwgroup->hwif     = hwif->next = hwif;
-		hwgroup->rq       = NULL;
-		hwgroup->handler  = NULL;
-		hwgroup->drive    = NULL;
-		hwgroup->busy     = 0;
 		init_timer(&hwgroup->timer);
 		hwgroup->timer.function = &ide_timer_expiry;
 		hwgroup->timer.data = (unsigned long) hwgroup;

[PATCH 7/15] ide: cleanup ide_system_bus_speed()

[Bartlomiej Zolnierkiewicz]

Use the facts that:

* When ide_init() is called 'system_bus_speed' always equals zero.

* system_bus_clock() is never called before ide_init().

and:

* Move printk() from ide_system_bus_speed() to ide_init().

* Don't assign 'system_bus_speed' in ide_system_bus_speed().

* Don't call ide_system_bus_speed() in system_bus_clock().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
-39 bytes
 drivers/ide/ide.c |   28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -240,22 +240,12 @@ static int ide_system_bus_speed(void)
 #define pci_default 0
 #endif /* CONFIG_PCI */
 
-	if (!system_bus_speed) {
-		if (idebus_parameter) {
-			/* user supplied value */
-			system_bus_speed = idebus_parameter;
-		} else if (pci_dev_present(pci_default)) {
-			/* safe default value for PCI */
-			system_bus_speed = 33;
-		} else {
-			/* safe default value for VESA and PCI */
-			system_bus_speed = 50;
-		}
-		printk(KERN_INFO "ide: Assuming %dMHz system bus speed "
-			"for PIO modes%s\n", system_bus_speed,
-			idebus_parameter ? "" : "; override with idebus=xx");
-	}
-	return system_bus_speed;
+	/* user supplied value */
+	if (idebus_parameter)
+		return idebus_parameter;
+
+	/* safe default value for PCI or VESA and PCI*/
+	return pci_dev_present(pci_default) ? 33 : 50;
 }
 
 ide_hwif_t * ide_find_port(unsigned long base)
@@ -912,7 +902,7 @@ static int set_unmaskirq(ide_drive_t *dr
 
 int system_bus_clock (void)
 {
-	return((int) ((!system_bus_speed) ? ide_system_bus_speed() : system_bus_speed ));
+	return system_bus_speed;
 }
 
 EXPORT_SYMBOL(system_bus_clock);
@@ -1667,6 +1657,10 @@ static int __init ide_init(void)
 	printk(KERN_INFO "Uniform Multi-Platform E-IDE driver " REVISION "\n");
 	system_bus_speed = ide_system_bus_speed();
 
+	printk(KERN_INFO "ide: Assuming %dMHz system bus speed "
+			 "for PIO modes%s\n", system_bus_speed,
+			idebus_parameter ? "" : "; override with idebus=xx");
+
 	ret = bus_register(&ide_bus_type);
 	if (ret < 0) {
 		printk(KERN_WARNING "IDE: bus_register error: %d\n", ret);

[PATCH 8/15] ide: I/O resources are released too early in ide_unregister()

[Bartlomiej Zolnierkiewicz]

Release I/O resources after releasing DMA.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -529,13 +529,6 @@ void ide_unregister(unsigned int index)
 
 	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.
-	 */
-	ide_hwif_release_regions(hwif);
-
-	/*
 	 * Remove us from the hwgroup, and free
 	 * the hwgroup if we were the only member
 	 */
@@ -589,6 +582,13 @@ void ide_unregister(unsigned int index)
 		hwif->extra_ports = 0;
 	}
 
+	/*
+	 * Note that we only release the standard ports,
+	 * and do not even try to handle any extra ports
+	 * allocated for weird IDE interface chipsets.
+	 */
+	ide_hwif_release_regions(hwif);
+
 	/* copy original settings */
 	tmp_hwif = *hwif;
 

[PATCH 9/15] ide: factor out code removing port from hwgroup from ide_unregister()

[Bartlomiej Zolnierkiewicz]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide.c |   65 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 29 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -461,6 +461,41 @@ static void ide_hwif_restore(ide_hwif_t 
 	hwif->hwif_data			= tmp_hwif->hwif_data;
 }
 
+static void ide_remove_port_from_hwgroup(ide_hwif_t *hwif)
+{
+	ide_hwgroup_t *hwgroup = hwif->hwgroup;
+
+	spin_lock_irq(&ide_lock);
+	/*
+	 * Remove us from the hwgroup, and free
+	 * the hwgroup if we were the only member
+	 */
+	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.
+		 */
+		ide_hwif_t *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);
+	}
+	spin_unlock_irq(&ide_lock);
+}
+
 /**
  *	ide_unregister		-	free an IDE interface
  *	@index: index of interface (will change soon to a pointer)
@@ -528,36 +563,8 @@ void ide_unregister(unsigned int index)
 	if (irq_count == 1)
 		free_irq(hwif->irq, hwgroup);
 
-	spin_lock_irq(&ide_lock);
-	/*
-	 * Remove us from the hwgroup, and free
-	 * the hwgroup if we were the only member
-	 */
-	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.
-			 */
-			BUG_ON(hwgroup->drive != NULL);
-			hwgroup->hwif = g;
-		}
-		BUG_ON(hwgroup->hwif == hwif);
-	}
+	ide_remove_port_from_hwgroup(hwif);
 
-	/* More messed up locking ... */
-	spin_unlock_irq(&ide_lock);
 	device_unregister(&hwif->gendev);
 	wait_for_completion(&hwif->gendev_rel_comp);
 

[PATCH 10/15] ide: prepare init_irq() for using ide_remove_port_from_hwgroup()

[Bartlomiej Zolnierkiewicz]

* BUG_ON() early if 'hwif->next == hwif' and there is a 'match' hwgroup.

* Remove printk() for impossible condition and add a BUG_ON(hwgroup->drive)
  to match code in ide_unregister().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
-52 bytes
 drivers/ide/ide-probe.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -996,6 +996,7 @@ static int init_irq (ide_hwif_t *hwif)
 		spin_lock_irq(&ide_lock);
 		hwif->next = hwgroup->hwif->next;
 		hwgroup->hwif->next = hwif;
+		BUG_ON(hwif->next == hwif);
 		spin_unlock_irq(&ide_lock);
 	} else {
 		hwgroup = kmalloc_node(sizeof(*hwgroup), GFP_KERNEL|__GFP_ZERO,
@@ -1075,7 +1076,6 @@ static int init_irq (ide_hwif_t *hwif)
 out_unlink:
 	spin_lock_irq(&ide_lock);
 	if (hwif->next == hwif) {
-		BUG_ON(match);
 		BUG_ON(hwgroup->hwif != hwif);
 		kfree(hwgroup);
 	} else {
@@ -1085,8 +1085,7 @@ out_unlink:
 			g = g->next;
 		g->next = hwif->next;
 		if (hwgroup->hwif == hwif) {
-			/* Impossible. */
-			printk(KERN_ERR "Duh. Uninitialized hwif listed as active hwif.\n");
+			BUG_ON(hwgroup->drive);
 			hwgroup->hwif = g;
 		}
 		BUG_ON(hwgroup->hwif == hwif);

[PATCH 11/15] ide: use ide_remove_port_from_hwgroup in init_irq()

[Bartlomiej Zolnierkiewicz]

There should be no functionality changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
-77 bytes
 drivers/ide/ide-probe.c |   18 +-----------------
 drivers/ide/ide.c       |    2 +-
 include/linux/ide.h     |    1 +
 3 files changed, 3 insertions(+), 18 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1074,23 +1074,7 @@ static int init_irq (ide_hwif_t *hwif)
 	mutex_unlock(&ide_cfg_mtx);
 	return 0;
 out_unlink:
-	spin_lock_irq(&ide_lock);
-	if (hwif->next == hwif) {
-		BUG_ON(hwgroup->hwif != hwif);
-		kfree(hwgroup);
-	} else {
-		ide_hwif_t *g;
-		g = hwgroup->hwif;
-		while (g->next != hwif)
-			g = g->next;
-		g->next = hwif->next;
-		if (hwgroup->hwif == hwif) {
-			BUG_ON(hwgroup->drive);
-			hwgroup->hwif = g;
-		}
-		BUG_ON(hwgroup->hwif == hwif);
-	}
-	spin_unlock_irq(&ide_lock);
+	ide_remove_port_from_hwgroup(hwif);
 out_up:
 	mutex_unlock(&ide_cfg_mtx);
 	return 1;
Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -461,7 +461,7 @@ static void ide_hwif_restore(ide_hwif_t 
 	hwif->hwif_data			= tmp_hwif->hwif_data;
 }
 
-static void ide_remove_port_from_hwgroup(ide_hwif_t *hwif)
+void ide_remove_port_from_hwgroup(ide_hwif_t *hwif)
 {
 	ide_hwgroup_t *hwgroup = hwif->hwgroup;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1194,6 +1194,7 @@ static inline void ide_acpi_init(ide_hwi
 static inline void ide_acpi_set_state(ide_hwif_t *hwif, int on) {}
 #endif
 
+void ide_remove_port_from_hwgroup(ide_hwif_t *);
 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);

[PATCH 12/15] ide: make probe_hwif() return an error value

[Bartlomiej Zolnierkiewicz]

Make probe_hwif() return an error value.

While at it:

* Remove comment about MAX_DRIVES == 2 limitation (it is not special to
  probe_hwif(), it is a general assumption taken by a lot of IDE code).

* Rename probe_hwif() to ide_probe_port().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-iops.c    |    2 +-
 drivers/ide/ide-probe.c   |   16 +++++++---------
 drivers/ide/pci/ns87415.c |    4 ++--
 3 files changed, 10 insertions(+), 12 deletions(-)

Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -1166,7 +1166,7 @@ EXPORT_SYMBOL(ide_do_reset);
 
 /*
  * ide_wait_not_busy() waits for the currently selected device on the hwif
- * to report a non-busy status, see comments in probe_hwif().
+ * to report a non-busy status, see comments in ide_probe_port().
  */
 int ide_wait_not_busy(ide_hwif_t *hwif, unsigned long timeout)
 {
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -738,24 +738,20 @@ void ide_undecoded_slave(ide_drive_t *dr
 
 EXPORT_SYMBOL_GPL(ide_undecoded_slave);
 
-/*
- * This routine only knows how to look for drive units 0 and 1
- * on an interface, so any setting of MAX_DRIVES > 2 won't work here.
- */
-static void probe_hwif(ide_hwif_t *hwif)
+static int ide_probe_port(ide_hwif_t *hwif)
 {
 	unsigned long flags;
 	unsigned int irqd;
 	int unit;
 
 	if (hwif->noprobe)
-		return;
+		return -EACCES;
 
 	if ((hwif->chipset != ide_4drives || !hwif->mate || !hwif->mate->present) &&
 	    (ide_hwif_request_regions(hwif))) {
 		printk(KERN_ERR "%s: ports already in use, skipping probe\n",
 				hwif->name);
-		return;	
+		return -EBUSY;
 	}
 
 	/*
@@ -804,7 +800,7 @@ static void probe_hwif(ide_hwif_t *hwif)
 
 	if (!hwif->present) {
 		ide_hwif_release_regions(hwif);
-		return;
+		return -ENODEV;
 	}
 
 	for (unit = 0; unit < MAX_DRIVES; unit++) {
@@ -840,6 +836,8 @@ static void probe_hwif(ide_hwif_t *hwif)
 		else
 			drive->no_io_32bit = drive->id->dword_io ? 1 : 0;
 	}
+
+	return 0;
 }
 
 #if MAX_HWIFS > 1
@@ -1311,7 +1309,7 @@ int ide_device_add_all(u8 *idx)
 		if (idx[i] == 0xff)
 			continue;
 
-		probe_hwif(&ide_hwifs[idx[i]]);
+		(void)ide_probe_port(&ide_hwifs[idx[i]]);
 	}
 
 	for (i = 0; i < MAX_HWIFS; i++) {
Index: b/drivers/ide/pci/ns87415.c
===================================================================
--- a/drivers/ide/pci/ns87415.c
+++ b/drivers/ide/pci/ns87415.c
@@ -229,8 +229,8 @@ static void __devinit init_hwif_ns87415 
 
 #ifdef __sparc_v9__
 		/*
-		 * XXX: Reset the device, if we don't it will not respond
-		 *      to SELECT_DRIVE() properly during first probe_hwif().
+		 * XXX: Reset the device, if we don't it will not respond to
+		 *      SELECT_DRIVE() properly during first ide_probe_port().
 		 */
 		timeout = 10000;
 		outb(12, hwif->io_ports[IDE_CONTROL_OFFSET]);

[PATCH 13/15] ide: move handling of I/O resources out of ide_probe_port()

[Bartlomiej Zolnierkiewicz]

Reserve/release I/O resources in ide_device_add_all() instead of
ide_probe_port().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-probe.c |   23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -747,13 +747,6 @@ static int ide_probe_port(ide_hwif_t *hw
 	if (hwif->noprobe)
 		return -EACCES;
 
-	if ((hwif->chipset != ide_4drives || !hwif->mate || !hwif->mate->present) &&
-	    (ide_hwif_request_regions(hwif))) {
-		printk(KERN_ERR "%s: ports already in use, skipping probe\n",
-				hwif->name);
-		return -EBUSY;
-	}
-
 	/*
 	 * We must always disable IRQ, as probe_for_drive will assert IRQ, but
 	 * we'll install our IRQ driver much later...
@@ -798,10 +791,8 @@ static int ide_probe_port(ide_hwif_t *hw
 	if (irqd)
 		enable_irq(irqd);
 
-	if (!hwif->present) {
-		ide_hwif_release_regions(hwif);
+	if (!hwif->present)
 		return -ENODEV;
-	}
 
 	for (unit = 0; unit < MAX_DRIVES; unit++) {
 		ide_drive_t *drive = &hwif->drives[unit];
@@ -1309,7 +1300,17 @@ int ide_device_add_all(u8 *idx)
 		if (idx[i] == 0xff)
 			continue;
 
-		(void)ide_probe_port(&ide_hwifs[idx[i]]);
+		hwif = &ide_hwifs[idx[i]];
+
+		if ((hwif->chipset != ide_4drives || !hwif->mate ||
+		     !hwif->mate->present) && ide_hwif_request_regions(hwif)) {
+			printk(KERN_ERR "%s: ports already in use, "
+					"skipping probe\n", hwif->name);
+			continue;
+		}
+
+		if (ide_probe_port(hwif) < 0)
+			ide_hwif_release_regions(hwif);
 	}
 
 	for (i = 0; i < MAX_HWIFS; i++) {

[PATCH 14/15] ide: factor out code for tuning devices from ide_probe_port()

[Bartlomiej Zolnierkiewicz]

* Factor out code for tuning devices from ide_probe_port()
  to ide_port_tune_devices().

* Move ide_port_tune_devices() call from ide_probe_port()
  to ide_device_add_all().

There should be no functionality changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-probe.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -794,6 +794,13 @@ static int ide_probe_port(ide_hwif_t *hw
 	if (!hwif->present)
 		return -ENODEV;
 
+	return 0;
+}
+
+static void ide_port_tune_devices(ide_hwif_t *hwif)
+{
+	int unit;
+
 	for (unit = 0; unit < MAX_DRIVES; unit++) {
 		ide_drive_t *drive = &hwif->drives[unit];
 
@@ -827,8 +834,6 @@ static int ide_probe_port(ide_hwif_t *hw
 		else
 			drive->no_io_32bit = drive->id->dword_io ? 1 : 0;
 	}
-
-	return 0;
 }
 
 #if MAX_HWIFS > 1
@@ -1309,8 +1314,12 @@ int ide_device_add_all(u8 *idx)
 			continue;
 		}
 
-		if (ide_probe_port(hwif) < 0)
+		if (ide_probe_port(hwif) < 0) {
 			ide_hwif_release_regions(hwif);
+			continue;
+		}
+
+		ide_port_tune_devices(hwif);
 	}
 
 	for (i = 0; i < MAX_HWIFS; i++) {

[PATCH 15/15] ide: move hwif_register() call out of ide_probe_port()

[Bartlomiej Zolnierkiewicz]

* Add BUG_ON(hwif->present) at the start of ide_probe_port().

* Move hwif_register() call (along with setting hwif->present) from
  ide_probe_port() to ide_device_add_all().

  As a result the port will be registered with the device tree _after_:
  - probing both devices (if both are present)
  - port reset (if hwif->reset is set)
  - restoring local IRQs state and re-enabling port IRQ

While at it:

* Rename hwif_register() to ide_register_port().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
+17 bytes
 drivers/ide/ide-probe.c |   27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -613,7 +613,7 @@ static void hwif_release_dev (struct dev
 	complete(&hwif->gendev_rel_comp);
 }
 
-static void hwif_register (ide_hwif_t *hwif)
+static void ide_register_port(ide_hwif_t *hwif)
 {
 	int ret;
 
@@ -742,7 +742,9 @@ static int ide_probe_port(ide_hwif_t *hw
 {
 	unsigned long flags;
 	unsigned int irqd;
-	int unit;
+	int unit, rc = -ENODEV;
+
+	BUG_ON(hwif->present);
 
 	if (hwif->noprobe)
 		return -EACCES;
@@ -767,14 +769,8 @@ static int ide_probe_port(ide_hwif_t *hw
 		ide_drive_t *drive = &hwif->drives[unit];
 		drive->dn = (hwif->channel ? 2 : 0) + unit;
 		(void) probe_for_drive(drive);
-		if (drive->present && !hwif->present) {
-			hwif->present = 1;
-			if (hwif->chipset != ide_4drives ||
-			    !hwif->mate || 
-			    !hwif->mate->present) {
-				hwif_register(hwif);
-			}
-		}
+		if (drive->present)
+			rc = 0;
 	}
 	if (hwif->io_ports[IDE_CONTROL_OFFSET] && hwif->reset) {
 		printk(KERN_WARNING "%s: reset\n", hwif->name);
@@ -791,10 +787,7 @@ static int ide_probe_port(ide_hwif_t *hw
 	if (irqd)
 		enable_irq(irqd);
 
-	if (!hwif->present)
-		return -ENODEV;
-
-	return 0;
+	return rc;
 }
 
 static void ide_port_tune_devices(ide_hwif_t *hwif)
@@ -1319,6 +1312,12 @@ int ide_device_add_all(u8 *idx)
 			continue;
 		}
 
+		hwif->present = 1;
+
+		if (hwif->chipset != ide_4drives || !hwif->mate ||
+		    !hwif->mate->present)
+			ide_register_port(hwif);
+
 		ide_port_tune_devices(hwif);
 	}
 

Re: [PATCH 2/15] ide: move wait_hwif_ready() documentation in the right place

[Sergei Shtylyov]

HNY. :-)

Bartlomiej Zolnierkiewicz wrote:

> * Move wait_hwif_ready() documentation before this function.

> * Fix trailing whitespaces.

> * s/wait-for-busy/wait-for-non-busy/

> * Remove no longer valid comment about the current code behavior.

> * Docbook-ize it.

> * Rename wait_hwif_ready() to ide_port_wait_ready().

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

MBR, Sergei

Re: [PATCH 2/15] ide: move wait_hwif_ready() documentation in the right place

[Sergei Shtylyov]

Hi, I wrote:

>> * Move wait_hwif_ready() documentation before this function.

>> * Fix trailing whitespaces.

>> * s/wait-for-busy/wait-for-non-busy/

>> * Remove no longer valid comment about the current code behavior.

>> * Docbook-ize it.

>> * Rename wait_hwif_ready() to ide_port_wait_ready().

>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

    Oops, forgot to stamp:

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

MBR, Sergei