kill off pci_enable_msi_{exact,range}

  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
  

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