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.lenda...@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.lenda...@amd.com> > Signed-off-by: Christoph Hellwig <h...@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; >
