Re: kill off pci_enable_msi_{exact,range}

[Tom Lendacky]

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kill off pci_enable_msi_{exact,range}

[Christoph Hellwig]

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kill off pci_enable_msi_{exact,range}

[Bjorn Helgaas]

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=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 
> 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 
> Signed-off-by: Christoph Hellwig 
> ---
>  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++)
> - 

Re: kill off pci_enable_msi_{exact,range}

[Christoph Hellwig]

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=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 
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 
Signed-off-by: Christoph Hellwig 
---
 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;
}
 

Re: kill off pci_enable_msi_{exact,range}

[Christoph Hellwig]

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?

[1] https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kill off pci_enable_msi_{exact,range}

[Bjorn Helgaas]

On Mon, Jan 09, 2017 at 09:37:37PM +0100, Christoph Hellwig wrote:
> I had hope that we could kill these old interfaces of for 4.10-rc,
> but as of today Linus tree still has two users:
> 
>  (1) the cobalt media driver, for which I sent a patch long time ago,
>  it got missed in the merge window.
>  (2) the new xgbe driver was merged in 4.10-rc but used the old interfaces
>  anyway
> 
> This series resend the patch for (1) and adds a new one for (2), as well
> as having the final removal patch behind it.  Maybe we should just queue
> up all three together in the PCI tree for 4.11?

Applied all three (with Tom's ack on the amd-xgbe patch) to pci/msi for
v4.11, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: kill off pci_enable_msi_{exact,range}

[David Miller]

From: Christoph Hellwig 
Date: Mon,  9 Jan 2017 21:37:37 +0100

> I had hope that we could kill these old interfaces of for 4.10-rc,
> but as of today Linus tree still has two users:
> 
>  (1) the cobalt media driver, for which I sent a patch long time ago,
>  it got missed in the merge window.
>  (2) the new xgbe driver was merged in 4.10-rc but used the old interfaces
>  anyway
> 
> This series resend the patch for (1) and adds a new one for (2), as well
> as having the final removal patch behind it.  Maybe we should just queue
> up all three together in the PCI tree for 4.11?

Feel free to toss this into the PCI tree.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

kill off pci_enable_msi_{exact,range}

[Christoph Hellwig]

I had hope that we could kill these old interfaces of for 4.10-rc,
but as of today Linus tree still has two users:

 (1) the cobalt media driver, for which I sent a patch long time ago,
 it got missed in the merge window.
 (2) the new xgbe driver was merged in 4.10-rc but used the old interfaces
 anyway

This series resend the patch for (1) and adds a new one for (2), as well
as having the final removal patch behind it.  Maybe we should just queue
up all three together in the PCI tree for 4.11?
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html