kill off pci_enable_msi_{exact,range}
Commit Message
On Fri, Jan 13, 2017 at 08:55:03AM +0100, Christoph Hellwig wrote:
> On Thu, Jan 12, 2017 at 03:29:00PM -0600, Bjorn Helgaas wrote:
> > Applied all three (with Tom's ack on the amd-xgbe patch) to pci/msi for
> > v4.11, thanks!
>
> Tom had just send me an event better version of the xgbe patch. Tom,
> maybe you can resend that relative to the PCI tree [1], so that we don't
> lose it for next merge window?
Actually - Bjorn, your msi branch contains an empty commit from this
thread:
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/msi&id=7a8191de43faa9869b421a1b06075d8126ce7c0b
Maybe we should rebase it after all to avoid that? In that case please
pick up the xgbe patch from Tom below:
---
From: Tom Lendacky <thomas.lendacky@amd.com>
Subject: [PATCH] amd-xgbe: Update PCI support to use new IRQ functions
Some of the PCI MSI/MSI-X functions have been deprecated and it is
recommended to use the new pci_alloc_irq_vectors() function. Convert
the code over to use the new function. Also, modify the way in which
the IRQs are requested - try for multiple MSI-X/MSI first, then a
single MSI/legacy interrupt.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 128 +++++++++---------------------
drivers/net/ethernet/amd/xgbe/xgbe.h | 8 +-
2 files changed, 41 insertions(+), 95 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Comments
On Fri, Jan 13, 2017 at 09:05:53AM +0100, Christoph Hellwig wrote:
> On Fri, Jan 13, 2017 at 08:55:03AM +0100, Christoph Hellwig wrote:
> > On Thu, Jan 12, 2017 at 03:29:00PM -0600, Bjorn Helgaas wrote:
> > > Applied all three (with Tom's ack on the amd-xgbe patch) to pci/msi for
> > > v4.11, thanks!
> >
> > Tom had just send me an event better version of the xgbe patch. Tom,
> > maybe you can resend that relative to the PCI tree [1], so that we don't
> > lose it for next merge window?
>
> Actually - Bjorn, your msi branch contains an empty commit from this
> thread:
>
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/msi&id=7a8191de43faa9869b421a1b06075d8126ce7c0b
Yep, I botched that. Thought I'd fixed it, but guess I got distracted.
> Maybe we should rebase it after all to avoid that? In that case please
> pick up the xgbe patch from Tom below:
I dropped the empty commit and replaced the xgbe patch with the one below.
Can you take a look at [1] and make sure it's what you expected?
[1] https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/msi
Thanks!
> ---
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Subject: [PATCH] amd-xgbe: Update PCI support to use new IRQ functions
>
> Some of the PCI MSI/MSI-X functions have been deprecated and it is
> recommended to use the new pci_alloc_irq_vectors() function. Convert
> the code over to use the new function. Also, modify the way in which
> the IRQs are requested - try for multiple MSI-X/MSI first, then a
> single MSI/legacy interrupt.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 128 +++++++++---------------------
> drivers/net/ethernet/amd/xgbe/xgbe.h | 8 +-
> 2 files changed, 41 insertions(+), 95 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> index e76b7f6..e436902 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> @@ -122,104 +122,40 @@
> #include "xgbe.h"
> #include "xgbe-common.h"
>
> -static int xgbe_config_msi(struct xgbe_prv_data *pdata)
> +static int xgbe_config_multi_msi(struct xgbe_prv_data *pdata)
> {
> - unsigned int msi_count;
> + unsigned int vector_count;
> unsigned int i, j;
> int ret;
>
> - msi_count = XGBE_MSIX_BASE_COUNT;
> - msi_count += max(pdata->rx_ring_count,
> - pdata->tx_ring_count);
> - msi_count = roundup_pow_of_two(msi_count);
> + vector_count = XGBE_MSI_BASE_COUNT;
> + vector_count += max(pdata->rx_ring_count,
> + pdata->tx_ring_count);
>
> - ret = pci_enable_msi_exact(pdata->pcidev, msi_count);
> + ret = pci_alloc_irq_vectors(pdata->pcidev, XGBE_MSI_MIN_COUNT,
> + vector_count, PCI_IRQ_MSI | PCI_IRQ_MSIX);
> if (ret < 0) {
> - dev_info(pdata->dev, "MSI request for %u interrupts failed\n",
> - msi_count);
> -
> - ret = pci_enable_msi(pdata->pcidev);
> - if (ret < 0) {
> - dev_info(pdata->dev, "MSI enablement failed\n");
> - return ret;
> - }
> -
> - msi_count = 1;
> - }
> -
> - pdata->irq_count = msi_count;
> -
> - pdata->dev_irq = pdata->pcidev->irq;
> -
> - if (msi_count > 1) {
> - pdata->ecc_irq = pdata->pcidev->irq + 1;
> - pdata->i2c_irq = pdata->pcidev->irq + 2;
> - pdata->an_irq = pdata->pcidev->irq + 3;
> -
> - for (i = XGBE_MSIX_BASE_COUNT, j = 0;
> - (i < msi_count) && (j < XGBE_MAX_DMA_CHANNELS);
> - i++, j++)
> - pdata->channel_irq[j] = pdata->pcidev->irq + i;
> - pdata->channel_irq_count = j;
> -
> - pdata->per_channel_irq = 1;
> - pdata->channel_irq_mode = XGBE_IRQ_MODE_LEVEL;
> - } else {
> - pdata->ecc_irq = pdata->pcidev->irq;
> - pdata->i2c_irq = pdata->pcidev->irq;
> - pdata->an_irq = pdata->pcidev->irq;
> - }
> -
> - if (netif_msg_probe(pdata))
> - dev_dbg(pdata->dev, "MSI interrupts enabled\n");
> -
> - return 0;
> -}
> -
> -static int xgbe_config_msix(struct xgbe_prv_data *pdata)
> -{
> - unsigned int msix_count;
> - unsigned int i, j;
> - int ret;
> -
> - msix_count = XGBE_MSIX_BASE_COUNT;
> - msix_count += max(pdata->rx_ring_count,
> - pdata->tx_ring_count);
> -
> - pdata->msix_entries = devm_kcalloc(pdata->dev, msix_count,
> - sizeof(struct msix_entry),
> - GFP_KERNEL);
> - if (!pdata->msix_entries)
> - return -ENOMEM;
> -
> - for (i = 0; i < msix_count; i++)
> - pdata->msix_entries[i].entry = i;
> -
> - ret = pci_enable_msix_range(pdata->pcidev, pdata->msix_entries,
> - XGBE_MSIX_MIN_COUNT, msix_count);
> - if (ret < 0) {
> - dev_info(pdata->dev, "MSI-X enablement failed\n");
> - devm_kfree(pdata->dev, pdata->msix_entries);
> - pdata->msix_entries = NULL;
> + dev_info(pdata->dev, "multi MSI/MSI-X enablement failed\n");
> return ret;
> }
>
> pdata->irq_count = ret;
>
> - pdata->dev_irq = pdata->msix_entries[0].vector;
> - pdata->ecc_irq = pdata->msix_entries[1].vector;
> - pdata->i2c_irq = pdata->msix_entries[2].vector;
> - pdata->an_irq = pdata->msix_entries[3].vector;
> + pdata->dev_irq = pci_irq_vector(pdata->pcidev, 0);
> + pdata->ecc_irq = pci_irq_vector(pdata->pcidev, 1);
> + pdata->i2c_irq = pci_irq_vector(pdata->pcidev, 2);
> + pdata->an_irq = pci_irq_vector(pdata->pcidev, 3);
>
> - for (i = XGBE_MSIX_BASE_COUNT, j = 0; i < ret; i++, j++)
> - pdata->channel_irq[j] = pdata->msix_entries[i].vector;
> + for (i = XGBE_MSI_BASE_COUNT, j = 0; i < ret; i++, j++)
> + pdata->channel_irq[j] = pci_irq_vector(pdata->pcidev, i);
> pdata->channel_irq_count = j;
>
> pdata->per_channel_irq = 1;
> pdata->channel_irq_mode = XGBE_IRQ_MODE_LEVEL;
>
> if (netif_msg_probe(pdata))
> - dev_dbg(pdata->dev, "MSI-X interrupts enabled\n");
> + dev_dbg(pdata->dev, "multi %s interrupts enabled\n",
> + pdata->pcidev->msix_enabled ? "MSI-X" : "MSI");
>
> return 0;
> }
> @@ -228,21 +164,28 @@ static int xgbe_config_irqs(struct xgbe_prv_data *pdata)
> {
> int ret;
>
> - ret = xgbe_config_msix(pdata);
> + ret = xgbe_config_multi_msi(pdata);
> if (!ret)
> goto out;
>
> - ret = xgbe_config_msi(pdata);
> - if (!ret)
> - goto out;
> + ret = pci_alloc_irq_vectors(pdata->pcidev, 1, 1,
> + PCI_IRQ_LEGACY | PCI_IRQ_MSI);
> + if (ret < 0) {
> + dev_info(pdata->dev, "single IRQ enablement failed\n");
> + return ret;
> + }
>
> pdata->irq_count = 1;
> - pdata->irq_shared = 1;
> + pdata->channel_irq_count = 1;
> +
> + pdata->dev_irq = pci_irq_vector(pdata->pcidev, 0);
> + pdata->ecc_irq = pci_irq_vector(pdata->pcidev, 0);
> + pdata->i2c_irq = pci_irq_vector(pdata->pcidev, 0);
> + pdata->an_irq = pci_irq_vector(pdata->pcidev, 0);
>
> - pdata->dev_irq = pdata->pcidev->irq;
> - pdata->ecc_irq = pdata->pcidev->irq;
> - pdata->i2c_irq = pdata->pcidev->irq;
> - pdata->an_irq = pdata->pcidev->irq;
> + if (netif_msg_probe(pdata))
> + dev_dbg(pdata->dev, "single %s interrupt enabled\n",
> + pdata->pcidev->msi_enabled ? "MSI" : "legacy");
>
> out:
> if (netif_msg_probe(pdata)) {
> @@ -412,12 +355,15 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> /* Configure the netdev resource */
> ret = xgbe_config_netdev(pdata);
> if (ret)
> - goto err_pci_enable;
> + goto err_irq_vectors;
>
> netdev_notice(pdata->netdev, "net device enabled\n");
>
> return 0;
>
> +err_irq_vectors:
> + pci_free_irq_vectors(pdata->pcidev);
> +
> err_pci_enable:
> xgbe_free_pdata(pdata);
>
> @@ -433,6 +379,8 @@ static void xgbe_pci_remove(struct pci_dev *pdev)
>
> xgbe_deconfig_netdev(pdata);
>
> + pci_free_irq_vectors(pdata->pcidev);
> +
> xgbe_free_pdata(pdata);
> }
>
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h b/drivers/net/ethernet/amd/xgbe/xgbe.h
> index f52a9bd..99f1c87 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe.h
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h
> @@ -211,9 +211,9 @@
> #define XGBE_MAC_PROP_OFFSET 0x1d000
> #define XGBE_I2C_CTRL_OFFSET 0x1e000
>
> -/* PCI MSIx support */
> -#define XGBE_MSIX_BASE_COUNT 4
> -#define XGBE_MSIX_MIN_COUNT (XGBE_MSIX_BASE_COUNT + 1)
> +/* PCI MSI/MSIx support */
> +#define XGBE_MSI_BASE_COUNT 4
> +#define XGBE_MSI_MIN_COUNT (XGBE_MSI_BASE_COUNT + 1)
>
> /* PCI clock frequencies */
> #define XGBE_V2_DMA_CLOCK_FREQ 500000000 /* 500 MHz */
> @@ -980,14 +980,12 @@ struct xgbe_prv_data {
> unsigned int desc_ded_count;
> unsigned int desc_sec_count;
>
> - struct msix_entry *msix_entries;
> int dev_irq;
> int ecc_irq;
> int i2c_irq;
> int channel_irq[XGBE_MAX_DMA_CHANNELS];
>
> unsigned int per_channel_irq;
> - unsigned int irq_shared;
> unsigned int irq_count;
> unsigned int channel_irq_count;
> unsigned int channel_irq_mode;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 13, 2017 at 11:13:21AM -0600, Bjorn Helgaas wrote:
> I dropped the empty commit and replaced the xgbe patch with the one below.
> Can you take a look at [1] and make sure it's what you expected?
This looks great, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1/13/2017 11:15 AM, Christoph Hellwig wrote:
> On Fri, Jan 13, 2017 at 11:13:21AM -0600, Bjorn Helgaas wrote:
>> I dropped the empty commit and replaced the xgbe patch with the one below.
>> Can you take a look at [1] and make sure it's what you expected?
>
> This looks great, thanks!
>
Christoph and Bjorn, thanks for taking care of this!
Tom
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -122,104 +122,40 @@
#include "xgbe.h"
#include "xgbe-common.h"
-static int xgbe_config_msi(struct xgbe_prv_data *pdata)
+static int xgbe_config_multi_msi(struct xgbe_prv_data *pdata)
{
- unsigned int msi_count;
+ unsigned int vector_count;
unsigned int i, j;
int ret;
- msi_count = XGBE_MSIX_BASE_COUNT;
- msi_count += max(pdata->rx_ring_count,
- pdata->tx_ring_count);
- msi_count = roundup_pow_of_two(msi_count);
+ vector_count = XGBE_MSI_BASE_COUNT;
+ vector_count += max(pdata->rx_ring_count,
+ pdata->tx_ring_count);
- ret = pci_enable_msi_exact(pdata->pcidev, msi_count);
+ ret = pci_alloc_irq_vectors(pdata->pcidev, XGBE_MSI_MIN_COUNT,
+ vector_count, PCI_IRQ_MSI | PCI_IRQ_MSIX);
if (ret < 0) {
- dev_info(pdata->dev, "MSI request for %u interrupts failed\n",
- msi_count);
-
- ret = pci_enable_msi(pdata->pcidev);
- if (ret < 0) {
- dev_info(pdata->dev, "MSI enablement failed\n");
- return ret;
- }
-
- msi_count = 1;
- }
-
- pdata->irq_count = msi_count;
-
- pdata->dev_irq = pdata->pcidev->irq;
-
- if (msi_count > 1) {
- pdata->ecc_irq = pdata->pcidev->irq + 1;
- pdata->i2c_irq = pdata->pcidev->irq + 2;
- pdata->an_irq = pdata->pcidev->irq + 3;
-
- for (i = XGBE_MSIX_BASE_COUNT, j = 0;
- (i < msi_count) && (j < XGBE_MAX_DMA_CHANNELS);
- i++, j++)
- pdata->channel_irq[j] = pdata->pcidev->irq + i;
- pdata->channel_irq_count = j;
-
- pdata->per_channel_irq = 1;
- pdata->channel_irq_mode = XGBE_IRQ_MODE_LEVEL;
- } else {
- pdata->ecc_irq = pdata->pcidev->irq;
- pdata->i2c_irq = pdata->pcidev->irq;
- pdata->an_irq = pdata->pcidev->irq;
- }
-
- if (netif_msg_probe(pdata))
- dev_dbg(pdata->dev, "MSI interrupts enabled\n");
-
- return 0;
-}
-
-static int xgbe_config_msix(struct xgbe_prv_data *pdata)
-{
- unsigned int msix_count;
- unsigned int i, j;
- int ret;
-
- msix_count = XGBE_MSIX_BASE_COUNT;
- msix_count += max(pdata->rx_ring_count,
- pdata->tx_ring_count);
-
- pdata->msix_entries = devm_kcalloc(pdata->dev, msix_count,
- sizeof(struct msix_entry),
- GFP_KERNEL);
- if (!pdata->msix_entries)
- return -ENOMEM;
-
- for (i = 0; i < msix_count; i++)
- pdata->msix_entries[i].entry = i;
-
- ret = pci_enable_msix_range(pdata->pcidev, pdata->msix_entries,
- XGBE_MSIX_MIN_COUNT, msix_count);
- if (ret < 0) {
- dev_info(pdata->dev, "MSI-X enablement failed\n");
- devm_kfree(pdata->dev, pdata->msix_entries);
- pdata->msix_entries = NULL;
+ dev_info(pdata->dev, "multi MSI/MSI-X enablement failed\n");
return ret;
}
pdata->irq_count = ret;
- pdata->dev_irq = pdata->msix_entries[0].vector;
- pdata->ecc_irq = pdata->msix_entries[1].vector;
- pdata->i2c_irq = pdata->msix_entries[2].vector;
- pdata->an_irq = pdata->msix_entries[3].vector;
+ pdata->dev_irq = pci_irq_vector(pdata->pcidev, 0);
+ pdata->ecc_irq = pci_irq_vector(pdata->pcidev, 1);
+ pdata->i2c_irq = pci_irq_vector(pdata->pcidev, 2);
+ pdata->an_irq = pci_irq_vector(pdata->pcidev, 3);
- for (i = XGBE_MSIX_BASE_COUNT, j = 0; i < ret; i++, j++)
- pdata->channel_irq[j] = pdata->msix_entries[i].vector;
+ for (i = XGBE_MSI_BASE_COUNT, j = 0; i < ret; i++, j++)
+ pdata->channel_irq[j] = pci_irq_vector(pdata->pcidev, i);
pdata->channel_irq_count = j;
pdata->per_channel_irq = 1;
pdata->channel_irq_mode = XGBE_IRQ_MODE_LEVEL;
if (netif_msg_probe(pdata))
- dev_dbg(pdata->dev, "MSI-X interrupts enabled\n");
+ dev_dbg(pdata->dev, "multi %s interrupts enabled\n",
+ pdata->pcidev->msix_enabled ? "MSI-X" : "MSI");
return 0;
}
@@ -228,21 +164,28 @@ static int xgbe_config_irqs(struct xgbe_prv_data *pdata)
{
int ret;
- ret = xgbe_config_msix(pdata);
+ ret = xgbe_config_multi_msi(pdata);
if (!ret)
goto out;
- ret = xgbe_config_msi(pdata);
- if (!ret)
- goto out;
+ ret = pci_alloc_irq_vectors(pdata->pcidev, 1, 1,
+ PCI_IRQ_LEGACY | PCI_IRQ_MSI);
+ if (ret < 0) {
+ dev_info(pdata->dev, "single IRQ enablement failed\n");
+ return ret;
+ }
pdata->irq_count = 1;
- pdata->irq_shared = 1;
+ pdata->channel_irq_count = 1;
+
+ pdata->dev_irq = pci_irq_vector(pdata->pcidev, 0);
+ pdata->ecc_irq = pci_irq_vector(pdata->pcidev, 0);
+ pdata->i2c_irq = pci_irq_vector(pdata->pcidev, 0);
+ pdata->an_irq = pci_irq_vector(pdata->pcidev, 0);
- pdata->dev_irq = pdata->pcidev->irq;
- pdata->ecc_irq = pdata->pcidev->irq;
- pdata->i2c_irq = pdata->pcidev->irq;
- pdata->an_irq = pdata->pcidev->irq;
+ if (netif_msg_probe(pdata))
+ dev_dbg(pdata->dev, "single %s interrupt enabled\n",
+ pdata->pcidev->msi_enabled ? "MSI" : "legacy");
out:
if (netif_msg_probe(pdata)) {
@@ -412,12 +355,15 @@ static int xgbe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
/* Configure the netdev resource */
ret = xgbe_config_netdev(pdata);
if (ret)
- goto err_pci_enable;
+ goto err_irq_vectors;
netdev_notice(pdata->netdev, "net device enabled\n");
return 0;
+err_irq_vectors:
+ pci_free_irq_vectors(pdata->pcidev);
+
err_pci_enable:
xgbe_free_pdata(pdata);
@@ -433,6 +379,8 @@ static void xgbe_pci_remove(struct pci_dev *pdev)
xgbe_deconfig_netdev(pdata);
+ pci_free_irq_vectors(pdata->pcidev);
+
xgbe_free_pdata(pdata);
}
@@ -211,9 +211,9 @@
#define XGBE_MAC_PROP_OFFSET 0x1d000
#define XGBE_I2C_CTRL_OFFSET 0x1e000
-/* PCI MSIx support */
-#define XGBE_MSIX_BASE_COUNT 4
-#define XGBE_MSIX_MIN_COUNT (XGBE_MSIX_BASE_COUNT + 1)
+/* PCI MSI/MSIx support */
+#define XGBE_MSI_BASE_COUNT 4
+#define XGBE_MSI_MIN_COUNT (XGBE_MSI_BASE_COUNT + 1)
/* PCI clock frequencies */
#define XGBE_V2_DMA_CLOCK_FREQ 500000000 /* 500 MHz */
@@ -980,14 +980,12 @@ struct xgbe_prv_data {
unsigned int desc_ded_count;
unsigned int desc_sec_count;
- struct msix_entry *msix_entries;
int dev_irq;
int ecc_irq;
int i2c_irq;
int channel_irq[XGBE_MAX_DMA_CHANNELS];
unsigned int per_channel_irq;
- unsigned int irq_shared;
unsigned int irq_count;
unsigned int channel_irq_count;
unsigned int channel_irq_mode;