pci, msi: Rework error path of msix_capability_init()

Commit Message

Hidetoshi Seto May 18, 2009, 3:31 a.m. UTC

Using msi_free_irqs() in error path of msix_capability_init() has
an problem - It does writel to Vector Control register.  Doing it
before reading the original value (that includes preserved bits) 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.

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

Patch

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d55db55..e751d7b 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,36 @@  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;
+	}
+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;
 }
 
 /**