Phase out pci_enable_msi_block()

In case MSI-X and MSI initialization failed the function irq_set_affinity_hint() is called with uninitialized value in dev->entry[0].vector. This update fixes the issue.

Signed-off-by Alexander Gordeev <agordeev at redhat.com>

---
 drivers/block/nvme-core.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

drivers/block/nvme-core.c +6 -7

index 26d03fa..e292450 100644
@@ -1790,15 +1790,15 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
                        vecs = 32;
                for (;;) {
                        result = pci_enable_msi_block(pdev, vecs);
-                       if (result == 0) {
-                               for (i = 0; i < vecs; i++)
-                                       dev->entry[i].vector = i + pdev->irq;
-                               break;
+                       if (result > 0) {
+                               vecs = result;
+                               continue;
                        } else if (result < 0) {
                                vecs = 1;
-                               break;
                        }
-                       vecs = result;
+                       for (i = 0; i < vecs; i++)
+                               dev->entry[i].vector = i + pdev->irq;
+                       break;
                }
        }
 
--

1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

This series is against "next" branch in Bjorn's repo:
git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git

Changes from v1 to v2:
- added a regression fix "ahci: Fix broken fallback to single

    MSI mode" as patch 1/9;
  - the series is reordered to move the regression fix in front;
  - at Bjorn's request pci_enable_msi() is un-deprecated;
  - as result, pci_enable_msi_range(pdev, 1, 1) styled calls
    rolled back to pci_enable_msi(pdev);

- nvme bug fix moved out as a separate patch 5/9 "nvme: Fix invalid call to irq_set_affinity_hint()"
- patches changelog elaborated a bit;

Bjorn,

As the release is supposedly this weekend, do you prefer the patches to go to your tree or to individual trees after the release?

Quoted 3 lines · Alexander Gordeev

In case MSI-X and MSI initialization failed the function irq_set_affinity_hint() is called with uninitialized value in dev->entry[0].vector. This update fixes the issue.

dev->entry[0].vector is initialized in nvme_dev_map(), and it's used for free_irq() above the area of your patch, so I don't think this is actually a bug, though it might be somewhat confusing.

Quoted 30 lines

Signed-off-by Alexander Gordeev <agordeev at redhat.com>

---
 drivers/block/nvme-core.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

drivers/block/nvme-core.c +6 -6

index 26d03fa..e292450 100644
@@ -1790,15 +1790,15 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
                        vecs = 32;
                for (;;) {
                        result = pci_enable_msi_block(pdev, vecs);
-                       if (result == 0) {
-                               for (i = 0; i < vecs; i++)
-                                       dev->entry[i].vector = i + pdev->irq;
-                               break;
+                       if (result > 0) {
+                               vecs = result;
+                               continue;
                        } else if (result < 0) {
                                vecs = 1;
-                               break;
                        }
-                       vecs = result;
+                       for (i = 0; i < vecs; i++)
+                               dev->entry[i].vector = i + pdev->irq;
+                       break;
                }
        }

Quoted 3 lines

As the release is supposedly this weekend, do you prefer the patches to go to your tree or to individual trees after the release?

I'd be happy to merge them, except for the fact that they probably wouldn't have any time in -next before I ask Linus to pull them. So how about if we wait until after the release, ask the area maintainers to take them, and if they don't take them, I'll put them in my tree for v3.15?

Bjorn

Quoted 9 lines · Bjorn Helgaas

Quoted 3 lines

As the release is supposedly this weekend, do you prefer the patches to go to your tree or to individual trees after the release?

I'd be happy to merge them, except for the fact that they probably wouldn't have any time in -next before I ask Linus to pull them. So how about if we wait until after the release, ask the area maintainers to take them, and if they don't take them, I'll put them in my tree for v3.15?

Patch 11 depends on patches 1-10, so I am not sure how to better handle it. Whatever works for you ;)

I am only concerned with a regression fix "ahci: Fix broken fallback to single MSI mode" which would be nice to have in 3.14. But it seems pretty much too late.

Quoted 14 lines · Alexander Gordeev

Quoted 9 lines · Bjorn Helgaas

Quoted 3 lines

As the release is supposedly this weekend, do you prefer the patches to go to your tree or to individual trees after the release?

I'd be happy to merge them, except for the fact that they probably wouldn't have any time in -next before I ask Linus to pull them. So how about if we wait until after the release, ask the area maintainers to take them, and if they don't take them, I'll put them in my tree for v3.15?

Patch 11 depends on patches 1-10, so I am not sure how to better handle it. Whatever works for you ;)

Quoted 3 lines

I am only concerned with a regression fix "ahci: Fix broken fallback to single MSI mode" which would be nice to have in 3.14. But it seems pretty much too late.

Tejun, if you want to ack that one, I can put it in either the first 3.14 pull request or a subsequent one. Either way, since it's a regression fix, we should be able to get it in 3.14.

Quoted 3 lines

dev->entry[0].vector is initialized in nvme_dev_map(), and it's used for free_irq() above the area of your patch, so I don't think this is actually a bug, though it might be somewhat confusing.

It is confusing, but there's a reason. :)

We send a single command using legacy irq to discover how many msix vectors we want. The legacy entry needs to be set some time before calling request_irq in nvme_configure_admin_queue, but also within nvme_dev_start (for power-management). I don't think there's a place to set it that won't look odd when looking at nvme_setup_io_queues. I settled on 'nvme_dev_map' was because 'nvme_dev_unmap' invalidates the entries, so this seemed to provide some amount of symmetry.

This update fixes an oddity when a device is first added and then removed from dev_list in case of initialization failure, instead of just being added in case of success.

Signed-off-by Alexander Gordeev <agordeev at redhat.com>

---
drivers/block/nvme-core.c | 19 ++++++++-----------

1 files changed, 8 insertions(+), 11 deletions(-)

drivers/block/nvme-core.c +0 -0

index e1e4ad4..e4e12be 100644

This is an attempt to make handling of admin queue in a single scope. This update also fixes a IRQ leak in case nvme_setup_io_queues() failed to allocate enough iomem and bailed out with -ENOMEM errno.

Signed-off-by Alexander Gordeev <agordeev at redhat.com>

---
drivers/block/nvme-core.c | 44 +++++++++++++++++++++++---------------------

1 files changed, 23 insertions(+), 21 deletions(-)

drivers/block/nvme-core.c +23 -22

index 3e1ae55..e1e4ad4 100644
@@ -1287,6 +1287,7 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 	if (result)
 		return result;
 
+	dev->entry[0].vector = pdev->pci_dev->irq;
 	result = queue_request_irq(dev, nvmeq, "nvme admin");
 	if (result)
 		return result;

@@ -1297,6 +1298,11 @@ static int nvme_configure_admin_queue(struct nvme_dev *dev)
 	return result;
 }
 
+static int nvme_teardown_admin_queue(struct nvme_dev *dev)
+{
+	free_irq(dev->entry[0].vector, dev->queues[0]);
+}
+
 struct nvme_iod *nvme_map_user_pages(struct nvme_dev *dev, int write,
 				unsigned long addr, unsigned length)
 {

@@ -1744,17 +1750,10 @@ static size_t db_bar_size(struct nvme_dev *dev, unsigned nr_io_queues)
 	return 4096 + ((nr_io_queues + 1) << (dev->db_stride + 3));
 }
 
-static int nvme_setup_io_queues(struct nvme_dev *dev)
+static int nvme_setup_io_queues(struct nvme_dev *dev, int nr_io_queues)
 {
 	struct pci_dev *pdev = dev->pci_dev;
-	int result, cpu, i, vecs, nr_io_queues, size, q_depth;
-
-	nr_io_queues = num_online_cpus();
-	result = set_queue_count(dev, nr_io_queues);
-	if (result < 0)
-		return result;
-	if (result < nr_io_queues)
-		nr_io_queues = result;
+	int result, cpu, i, vecs, size, q_depth;
 
 	size = db_bar_size(dev, nr_io_queues);
 	if (size > 8192) {

@@ -1771,20 +1770,15 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
 		dev->queues[0]->q_db = dev->dbs;
 	}
 
-	/* Deregister the admin queue's interrupt */
-	free_irq(dev->entry[0].vector, dev->queues[0]);
-
 	for (i = 0; i < nr_io_queues; i++)
 		dev->entry[i].entry = i;
 	vecs = pci_enable_msix_range(pdev, dev->entry, 1, nr_io_queues);
 	if (vecs < 0) {
 		vecs = pci_enable_msi_range(pdev, 1, min(nr_io_queues, 32));
-		if (vecs < 0) {
+		if (vecs < 0)
 			vecs = 1;
-		} else {
-			for (i = 0; i < vecs; i++)
-				dev->entry[i].vector = i + pdev->irq;
-		}
+		for (i = 0; i < vecs; i++)
+			dev->entry[i].vector = i + pdev->irq;
 	}
 
 	/*

@@ -1928,7 +1922,6 @@ static int nvme_dev_map(struct nvme_dev *dev)
 	if (pci_enable_device_mem(pdev))
 		return result;
 
-	dev->entry[0].vector = pdev->irq;
 	pci_set_master(pdev);
 	bars = pci_select_bars(pdev, IORESOURCE_MEM);
 	if (pci_request_selected_regions(pdev, bars, "nvme"))

@@ -2116,11 +2109,20 @@ static int nvme_dev_start(struct nvme_dev *dev)
 	list_add(&dev->node, &dev_list);
 	spin_unlock(&dev_list_lock);
 
-	result = nvme_setup_io_queues(dev);
-	if (result && result != -EBUSY)
+	result = set_queue_count(dev, num_online_cpus());
+	if (result == -EBUSY)
+		return -EBUSY;
+
+	nvme_teardown_admin_queue(dev);
+
+	if (result)
 		goto disable;
 
-	return result;
+	result = nvme_setup_io_queues(dev, result);
+	if (result)
+		goto disable;
+
+	return 0;
 
 disable:
 	spin_lock(&dev_list_lock);
--

Quoted 4 lines · Alexander Gordeev

This is an attempt to make handling of admin queue in a single scope. This update also fixes a IRQ leak in case nvme_setup_io_queues() failed to allocate enough iomem and bailed out with -ENOMEM errno.

This definitely seems to improve the code flow, but this leak was already fixed in the latest linux-nvme tree with this commit:

http://git.infradead.org/users/willy/linux-nvme.git/commit/c5dc9192d52a4a3a479f701e97386347d454af84

This is an attempt to make handling of admin queue in a single scope. This update also fixes a IRQ leak in case nvme_setup_io_queues() failed to allocate enough iomem and bailed out with -ENOMEM errno.

Signed-off-by Alexander Gordeev <agordeev at redhat.com>

---

drivers/block/nvme-core.c +5 -0

+static void nvme_teardown_admin_queue(struct nvme_dev *dev)
+{
+	nvme_disable_queue(dev, 0);
+	nvme_free_queue(dev->queues[0]);
+}

drivers/block/nvme-core.c +5 -1

-	return result;
+	result = nvme_setup_io_queues(dev, result);
+	if (result)
+		goto disable;
+
+	return 0;

As result deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() and pci_enable_msix_range() interfaces.

Signed-off-by Alexander Gordeev <[email protected]>

Quoted 6 lines · Alexander Gordeev

As result deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() and pci_enable_msix_range() interfaces.

Quoted 1 line

Signed-off-by Alexander Gordeev <[email protected]>

Brian, James,

Could you please review patches 3 and 4?

These look like they will conflict with a recent patch to the ipr driver:

http://www.spinics.net/lists/linux-scsi/msg71644.html

Do you want me to rediff your patches on top of this one, or do you want to keep the entire MSI series together and do the rediff? Otherwise the patches seem fine.

Thanks,

Brian

Signature · 3 lines

Brian King
Power Linux I/O
IBM Linux Technology Center

Quoted 5 lines

Bjorn,

As the release is supposedly this weekend, do you prefer the patches to go to your tree or to individual trees after the release?

I'd be happy to merge them, except for the fact that they probably wouldn't have any time in -next before I ask Linus to pull them. So how about if we wait until after the release, ask the area maintainers to take them, and if they don't take them, I'll put them in my tree for v3.15?

Bjorn

As result deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() and pci_enable_msix_range() interfaces.

Signed-off-by Alexander Gordeev <[email protected]>

Acked-by Alex Williamson <[email protected]>

---
 drivers/vfio/pci/vfio_pci_intrs.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

drivers/vfio/pci/vfio_pci_intrs.c +8 -4

index 641bc87..4a9db1d 100644
@@ -482,15 +482,19 @@ static int vfio_msi_enable(struct vfio_pci_device *vdev, int nvec, bool msix)
 		for (i = 0; i < nvec; i++)
 			vdev->msix[i].entry = i;
 
-		ret = pci_enable_msix(pdev, vdev->msix, nvec);
-		if (ret) {
+		ret = pci_enable_msix_range(pdev, vdev->msix, 1, nvec);
+		if (ret < nvec) {
+			if (ret > 0)
+				pci_disable_msix(pdev);
 			kfree(vdev->msix);
 			kfree(vdev->ctx);
 			return ret;
 		}
 	} else {
-		ret = pci_enable_msi_block(pdev, nvec);
-		if (ret) {
+		ret = pci_enable_msi_range(pdev, 1, nvec);
+		if (ret < nvec) {
+			if (ret > 0)
+				pci_disable_msi(pdev);
 			kfree(vdev->ctx);
 			return ret;
 		}

Signature · 7 lines

1.7.7.6

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

As result deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() and pci_enable_msix_range() interfaces.

Signed-off-by Alexander Gordeev <[email protected]>

Acked-by Vladimir Kondratiev <[email protected]>

---
 drivers/net/wireless/ath/wil6210/pcie_bus.c |   36 ++++++++++++++------------
 1 files changed, 19 insertions(+), 17 deletions(-)

drivers/net/wireless/ath/wil6210/pcie_bus.c +19 -17

index eeceab3..4e1bf54 100644
@@ -41,30 +41,32 @@ static int wil_if_pcie_enable(struct wil6210_priv *wil)
 	switch (use_msi) {
 	case 3:
 	case 1:
+		wil_dbg_misc(wil, "Setup %d MSI interrupts\n", use_msi);
+		break;
 	case 0:
+		wil_dbg_misc(wil, "MSI interrupts disabled, use INTx\n");
 		break;
 	default:
-		wil_err(wil, "Invalid use_msi=%d, default to 1\n",
-			use_msi);
+		wil_err(wil, "Invalid use_msi=%d, default to 1\n", use_msi);
 		use_msi = 1;
 	}
-	wil->n_msi = use_msi;
-	if (wil->n_msi) {
-		wil_dbg_misc(wil, "Setup %d MSI interrupts\n", use_msi);
-		rc = pci_enable_msi_block(pdev, wil->n_msi);
-		if (rc && (wil->n_msi == 3)) {
-			wil_err(wil, "3 MSI mode failed, try 1 MSI\n");
-			wil->n_msi = 1;
-			rc = pci_enable_msi_block(pdev, wil->n_msi);
-		}
-		if (rc) {
-			wil_err(wil, "pci_enable_msi failed, use INTx\n");
-			wil->n_msi = 0;
-		}
-	} else {
-		wil_dbg_misc(wil, "MSI interrupts disabled, use INTx\n");
+
+	switch (use_msi) {
+	case 3:
+		if (pci_enable_msi_range(pdev, 3, 3) > 0)
+			break;
+		wil_err(wil, "3 MSI mode failed, try 1 MSI\n");
+		use_msi = 1;
+		/* fallthrough */
+	case 1:
+		if (!pci_enable_msi(pdev))
+			break;
+		wil_err(wil, "pci_enable_msi failed, use INTx\n");
+		use_msi = 0;
 	}
 
+	wil->n_msi = use_msi;
+
 	rc = wil6210_init_irq(wil, pdev->irq);
 	if (rc)
 		goto stop_master;

Signature · 6 lines

1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 19 lines · Bjorn Helgaas

Quoted 17 lines · Alexander Gordeev

Quoted 9 lines · Bjorn Helgaas

Quoted 3 lines

As the release is supposedly this weekend, do you prefer the patches to go to your tree or to individual trees after the release?

I'd be happy to merge them, except for the fact that they probably wouldn't have any time in -next before I ask Linus to pull them. So how about if we wait until after the release, ask the area maintainers to take them, and if they don't take them, I'll put them in my tree for v3.15?

Patch 11 depends on patches 1-10, so I am not sure how to better handle it. Whatever works for you ;)

I am only concerned with a regression fix "ahci: Fix broken fallback to single MSI mode" which would be nice to have in 3.14. But it seems pretty much too late.

Quoted 3 lines

Tejun, if you want to ack that one, I can put it in either the first 3.14 pull request or a subsequent one. Either way, since it's a regression fix, we should be able to get it in 3.14.

Acked-by Tejun Heo <[email protected]>

Please feel free to route it any way you see fit.

Signed-off-by Alexander Gordeev <[email protected]>

---
 Documentation/PCI/MSI-HOWTO.txt |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

Documentation/PCI/MSI-HOWTO.txt +20 -3

index a8d0100..3c3080e 100644
@@ -82,7 +82,19 @@ Most of the hard work is done for the driver in the PCI layer.  It simply
 has to request that the PCI layer set up the MSI capability for this
 device.
 
-4.2.1 pci_enable_msi_range
+4.2.1 pci_enable_msi
+
+int pci_enable_msi(struct pci_dev *dev)
+
+A successful call allocates ONE interrupt to the device, regardless
+of how many MSIs the device supports.  The device is switched from
+pin-based interrupt mode to MSI mode.  The dev->irq number is changed
+to a new number which represents the message signaled interrupt;
+consequently, this function should be called before the driver calls
+request_irq(), because an MSI is delivered via a vector that is
+different from the vector of a pin-based interrupt.
+
+4.2.2 pci_enable_msi_range
 
 int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
 

@@ -158,7 +170,12 @@ static int foo_driver_enable_single_msi(struct pci_dev *pdev)
 	return pci_enable_msi_range(pdev, 1, 1);
 }
 
-4.2.2 pci_disable_msi
+Note, unlike pci_enable_msi() function, which could be also used to
+enable the single MSI mode, pci_enable_msi_range() returns either a
+negative errno or 1 (not negative errno or 0 - as pci_enable_msi()
+does).
+
+4.2.3 pci_disable_msi
 
 void pci_disable_msi(struct pci_dev *dev)
 

@@ -172,7 +189,7 @@ on any interrupt for which it previously called request_irq().
 Failure to do so results in a BUG_ON(), leaving the device with
 MSI enabled and thus leaking its vector.
 
-4.2.3 pci_msi_vec_count
+4.2.4 pci_msi_vec_count
 
 int pci_msi_vec_count(struct pci_dev *dev)
 

Signature · 10 lines

1.7.7.6

-- 
Regards,
Alexander Gordeev
[email protected]
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

This update obsoletes pci_enable_msi_block() function in favor of pci_enable_msi_range().

Signed-off-by Alexander Gordeev <[email protected]>

---
 drivers/pci/msi.c   |   17 +++++++++++++++--
 include/linux/pci.h |    7 ++-----
 2 files changed, 17 insertions(+), 7 deletions(-)

drivers/pci/msi.c +1 -2

index 7a0fec6..49088d0 100644
@@ -889,7 +889,7 @@ EXPORT_SYMBOL(pci_msi_vec_count);
  * updates the @dev's irq member to the lowest new interrupt number; the
  * other interrupt numbers allocated to this device are consecutive.
  */
-int pci_enable_msi_block(struct pci_dev *dev, int nvec)
+static int pci_enable_msi_block(struct pci_dev *dev, int nvec)
 {
 	int status, maxvec;
 

@@ -918,7 +918,6 @@ int pci_enable_msi_block(struct pci_dev *dev, int nvec)
 	status = msi_capability_init(dev, nvec);
 	return status;
 }
-EXPORT_SYMBOL(pci_enable_msi_block);
 
 void pci_msi_shutdown(struct pci_dev *dev)
 {

@@ -1148,6 +1147,20 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec)
 EXPORT_SYMBOL(pci_enable_msi_range);
 
 /**

drivers/pci/msi.c +4 -0

+}
+EXPORT_SYMBOL(pci_enable_msi);
+
+/**
  * pci_enable_msix_range - configure device's MSI-X capability structure
  * @dev: pointer to the pci_dev data structure of MSI-X device function
  * @entries: pointer to an array of MSI-X entries

include/linux/pci.h +2 -5

index fa959aa..5b58078 100644
@@ -1157,7 +1157,7 @@ struct msix_entry {
 
 #ifdef CONFIG_PCI_MSI
 int pci_msi_vec_count(struct pci_dev *dev);
-int pci_enable_msi_block(struct pci_dev *dev, int nvec);
+int pci_enable_msi(struct pci_dev *dev);
 void pci_msi_shutdown(struct pci_dev *dev);
 void pci_disable_msi(struct pci_dev *dev);
 int pci_msix_vec_count(struct pci_dev *dev);

@@ -1172,8 +1172,7 @@ int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
 			  int minvec, int maxvec);
 #else
 static inline int pci_msi_vec_count(struct pci_dev *dev) { return -ENOSYS; }
-static inline int pci_enable_msi_block(struct pci_dev *dev, int nvec)
-{ return -ENOSYS; }
+static inline int pci_enable_msi(struct pci_dev *dev) { return -ENOSYS; }
 static inline void pci_msi_shutdown(struct pci_dev *dev) { }
 static inline void pci_disable_msi(struct pci_dev *dev) { }
 static inline int pci_msix_vec_count(struct pci_dev *dev) { return -ENOSYS; }

@@ -1223,8 +1222,6 @@ static inline void pcie_set_ecrc_checking(struct pci_dev *dev) { }
 static inline void pcie_ecrc_get_policy(char *str) { }
 #endif
 
-#define pci_enable_msi(pdev)	pci_enable_msi_block(pdev, 1)
-
 #ifdef CONFIG_HT_IRQ
 /* The functions a driver should call */
 int  ht_create_irq(struct pci_dev *dev, int idx);

Signature · 10 lines

1.7.7.6

-- 
Regards,
Alexander Gordeev
[email protected]
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 3 lines

Do you want me to rediff your patches on top of this one, or do you want to keep the entire MSI series together and do the rediff? Otherwise the patches seem fine.

I would prefer the former.

Signed-off-by Alexander Gordeev <[email protected]>

---
 drivers/net/wireless/ath/ath10k/pci.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

drivers/net/wireless/ath/ath10k/pci.c +0 -0

index 29fd197..6525e1f 100644

Quoted 10 lines · Alexander Gordeev

Signed-off-by Alexander Gordeev <[email protected]>

---
 drivers/net/wireless/ath/ath10k/pci.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

drivers/net/wireless/ath/ath10k/pci.c +0 -0

index 29fd197..6525e1f 100644

drivers/net/wireless/ath/ath10k/pci.c static int ath10k_pci_init_irq(struct ath10k *ar) · line 2418

Quoted 5 lines

drivers/net/wireless/ath/ath10k/pci.c +0 -2

@@ -2414,8 +2414,6 @@ static int ath10k_pci_init_irq(struct ath10k *ar)
 		ret = pci_enable_msi_block(ar_pci->pdev, ar_pci->num_msi_intrs);
 		if (ret == 0)
 			return 0;
-		if (ret > 0)
-			pci_disable_msi(ar_pci->pdev);

I don't understand how this is superfluous. When I read the documentation for pci_enable_msi_block() it states that if it can't allocate all requests, it will return the number requests it could allocate. And in that case we want to fall back other modes.

Am I missing something?

Quoted 7 lines · Kalle Valo

Alexander Gordeev <[email protected]> writes:

I don't understand how this is superfluous. When I read the documentation for pci_enable_msi_block() it states that if it can't allocate all requests, it will return the number requests it could allocate. And in that case we want to fall back other modes.

Quoted 1 line

Am I missing something?

Yep. The documentation states 'could have been allocated', not 'could allocate'. IOW, MSIs are *not* enabled if a positive value returned. The code I changed tries to disable MSIs in such case, although it is not necessary, nor required. Just superfluous.

Is it ok for me to take these patches to my ath.git tree or would you prefer to route them some other way?

Quoted 2 lines · Kalle Valo

Is it ok for me to take these patches to my ath.git tree or would you prefer to route them some other way?

Yeah, Bjorn has indicated he would pull it to his tree.

I get it you are fine with 2/3 and 3/3?

As result deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() and pci_enable_msix_range() interfaces.

This update fixes a warning introduced with commit bc977ba1 ("wil6210: Use pci_enable_msi_range() instead of pci_enable_msi_block()")

drivers/net/wireless/ath/wil6210/pcie_bus.c:65 wil_if_pcie_enable()
warn: missing break? reassigning 'use_msi'

I can't reproduce this warning. What tools used to get it?

Quoted 1 line · Vladimir Kondratiev

I can't reproduce this warning. What tools used to get it?

Fengguang?

Quoted 5 lines · Alexander Gordeev

Quoted 2 lines · Kalle Valo

Is it ok for me to take these patches to my ath.git tree or would you prefer to route them some other way?

Yeah, Bjorn has indicated he would pull it to his tree.

Ok, I'll drop these from my pending branch then. I just think it would have been easier if I would take these, smaller chance of conflicts that way.

Quoted 1 line

I get it you are fine with 2/3 and 3/3?

Yes, with the addition of the commit log to 1/3 I'll give:

Acked-by Kalle Valo <[email protected]>

I haven't put these in my branch, so you can take them.

Alexander has a whole batch of network driver updates that I think David Miller is going to apply; would it make sense to include these in that batch?

There's also the wil6210 patch for drivers/net/wireless/ath/wil6210/pcie_bus.c, which seems like it maybe could be in that batch, too.

Bjorn

Quoted 6 lines · Bjorn Helgaas

I haven't put these in my branch, so you can take them.

Alexander has a whole batch of network driver updates that I think David Miller is going to apply; would it make sense to include these in that batch?

Quoted 3 lines

There's also the wil6210 patch for drivers/net/wireless/ath/wil6210/pcie_bus.c, which seems like it maybe could be in that batch, too.

Well, as this series is small I thought it could quickly go thru your tree. But since ipr had conflicts, there is no point routing all patches altogether, so up to you guys. The wil6210 patch is already in your pci/msi branch though.

Quoted 4 lines

Well, as this series is small I thought it could quickly go thru your tree. But since ipr had conflicts, there is no point routing all patches altogether, so up to you guys. The wil6210 patch is already in your pci/msi branch though.

It's in pci/msi, but that's not in my -next branch yet, so I can easily drop it. Do drivers/net/wireless patches normally follow a different path than the other drivers/net patches? The wil6210 and ath10k patches look just like the others in the 34-patch series (bnx2, bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense to include them there.

Quoted 11 lines · Bjorn Helgaas

Quoted 4 lines

Well, as this series is small I thought it could quickly go thru your tree. But since ipr had conflicts, there is no point routing all patches altogether, so up to you guys. The wil6210 patch is already in your pci/msi branch though.

It's in pci/msi, but that's not in my -next branch yet, so I can easily drop it. Do drivers/net/wireless patches normally follow a different path than the other drivers/net patches? The wil6210 and ath10k patches look just like the others in the 34-patch series (bnx2, bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense to include them there.

ath10k patches normally go through my ath.git tree to Linville and then to David Miller. To avoid conflicts I would prefer to take ath10k patches to my tree whenever possible.

Quoted 14 lines · Kalle Valo

Quoted 11 lines · Bjorn Helgaas

Quoted 4 lines

Well, as this series is small I thought it could quickly go thru your tree. But since ipr had conflicts, there is no point routing all patches altogether, so up to you guys. The wil6210 patch is already in your pci/msi branch though.

It's in pci/msi, but that's not in my -next branch yet, so I can easily drop it. Do drivers/net/wireless patches normally follow a different path than the other drivers/net patches? The wil6210 and ath10k patches look just like the others in the 34-patch series (bnx2, bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense to include them there.

Quoted 3 lines

ath10k patches normally go through my ath.git tree to Linville and then to David Miller. To avoid conflicts I would prefer to take ath10k patches to my tree whenever possible.

OK, I won't do anything with ath10k (I haven't applied it anywhere).

Quoted 11 lines · Bjorn Helgaas

Quoted 4 lines

Well, as this series is small I thought it could quickly go thru your tree. But since ipr had conflicts, there is no point routing all patches altogether, so up to you guys. The wil6210 patch is already in your pci/msi branch though.

It's in pci/msi, but that's not in my -next branch yet, so I can easily drop it. Do drivers/net/wireless patches normally follow a different path than the other drivers/net patches? The wil6210 and ath10k patches look just like the others in the 34-patch series (bnx2, bnx2x, tg3, bna, cxgb3, etc.), so I thought it would make more sense to include them there.

ath10k patches normally go through my ath.git tree to Linville and then to David Miller. To avoid conflicts I would prefer to take ath10k patches to my tree whenever possible.

Signature · 1 line

Kalle Valo

Signed-off-by Alexander Gordeev <[email protected]>

As result deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() and pci_enable_msix_range() interfaces.

Signed-off-by Alexander Gordeev <agordeev at redhat.com>

Quoted 5 lines · Alexander Gordeev

Quoted 3 lines

Do you want me to rediff your patches on top of this one, or do you want to keep the entire MSI series together and do the rediff? Otherwise the patches seem fine.

I would prefer the former.

Hi Brian,

I am sending the refreshed patches on top of "ipr: Handle early EEH".

Quoted 7 lines · Alexander Gordeev

Quoted 5 lines · Alexander Gordeev

Quoted 3 lines

Do you want me to rediff your patches on top of this one, or do you want to keep the entire MSI series together and do the rediff? Otherwise the patches seem fine.

I would prefer the former.

Quoted 3 lines

Hi Brian,

I am sending the refreshed patches on top of "ipr: Handle early EEH".

Hi Brian,

What three the two patches you ACKed are against?
Should I ask James to take them?