pci, msi: Rework error path of msix_capability_init() v2

Commit Message

Hidetoshi Seto May 18, 2009, 4:42 a.m. UTC

Using msi_free_irqs() in error path of msix_capability_init() has
an problem - It does writel to Vector Control register.  Since reading
the original value of the register (that includes preserved bits) was
moved to later by previous patch for NIU problem, write before the read
will violate PCI spec.

And there was no iounmap if no msi_disc is allocated to the device.

This patch restructures the error path of msix_capability_init() to
fix above problems.

v2: Add dropped arch_teardown_msi_irqs()

Signed-off-by: Hidetoshi Seto <[e-mail]>
---
 drivers/pci/msi.c |   57 ++++++++++++++++++++++++++++++++--------------------
 1 files changed, 35 insertions(+), 22 deletions(-)

Patch

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d55db55..f8e41f6 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -446,10 +446,8 @@  static int msix_capability_init(struct pci_dev *dev,
 	for (i = 0; i < nvec; i++) {
 		entry = alloc_msi_entry(dev);
 		if (!entry) {
-			if (!i)
-				return -ENOMEM;
-			msi_free_irqs(dev);
-			return i;
+			ret = (i > 0) ? i : -ENOMEM;
+			goto error1;
 		}
 
  		j = entries[i].entry;
@@ -465,24 +463,8 @@  static int msix_capability_init(struct pci_dev *dev,
 	}
 
 	ret = arch_setup_msi_irqs(dev, nvec, PCI_CAP_ID_MSIX);
-	if (ret < 0) {
-		/* If we had some success report the number of irqs
-		 * we succeeded in setting up. */
-		int avail = 0;
-		list_for_each_entry(entry, &dev->msi_list, list) {
-			if (entry->irq != 0) {
-				avail++;
-			}
-		}
-
-		if (avail != 0)
-			ret = avail;
-	}
-
-	if (ret) {
-		msi_free_irqs(dev);
-		return ret;
-	}
+	if (ret)
+		goto error2;
 
 	i = 0;
 	list_for_each_entry(entry, &dev->msi_list, list) {
@@ -502,6 +484,37 @@  static int msix_capability_init(struct pci_dev *dev,
 	}
 
 	return 0;
+
+error2:
+	if (ret < 0) {
+		/*
+		 * If we had some success report the number of irqs
+		 * we succeeded in setting up.
+		 */
+		int avail = 0;
+
+		list_for_each_entry(entry, &dev->msi_list, list) {
+			if (entry->irq != 0)
+				avail++;
+		}
+
+		if (avail != 0)
+			ret = avail;
+	}
+	arch_teardown_msi_irqs(dev);
+error1:
+	{
+		struct msi_desc *tmp;
+
+		list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
+			list_del(&entry->list);
+			kfree(entry);
+		}
+	}
+
+	iounmap(base);
+
+	return ret;
 }
 
 /**