Re: [PATCH] ntb: Use pci_enable_msix_range() instead of pci_enable_msix()

From: Jon Mason
Date: Wed Feb 19 2014 - 13:09:55 EST


On Wed, Feb 19, 2014 at 11:15:21AM +0100, Alexander Gordeev wrote:
> As result of 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@xxxxxxxxxx>
> Cc: Jon Mason <jon.mason@xxxxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx
> ---
> drivers/ntb/ntb_hw.c | 43 ++++++++++++++++---------------------------
> drivers/ntb/ntb_hw.h | 2 --
> 2 files changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> index 170e8e6..fda37eb 100644
> --- a/drivers/ntb/ntb_hw.c
> +++ b/drivers/ntb/ntb_hw.c
> @@ -1085,21 +1085,15 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> struct msix_entry *msix;
> int msix_entries;
> int rc, i;
> - u16 val;
>
> - if (!pdev->msix_cap) {
> - rc = -EIO;
> - goto err;
> - }
> -
> - rc = pci_read_config_word(pdev, pdev->msix_cap + PCI_MSIX_FLAGS, &val);
> - if (rc)
> + rc = pci_msix_vec_count(pdev);

Nit, you should probably use msix_entries instead of rc in this case.

> + if (rc < 0) {
> goto err;
> -
> - msix_entries = msix_table_size(val);
> - if (msix_entries > ndev->limits.msix_cnt) {
> + } else if (rc > ndev->limits.msix_cnt) {
> rc = -EINVAL;
> goto err;
> + } else {
> + msix_entries = rc;
> }

Style Nit. Braces with only one line. If you make the change above,
then it is moot.

>
> ndev->msix_entries = kmalloc(sizeof(struct msix_entry) * msix_entries,
> @@ -1112,26 +1106,21 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> for (i = 0; i < msix_entries; i++)
> ndev->msix_entries[i].entry = i;
>
> - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> - if (rc < 0)
> - goto err1;
> - if (rc > 0) {
> + if (ndev->hw_type != BWD_HW)
> /* On SNB, the link interrupt is always tied to 4th vector. If
> * we can't get all 4, then we can't use MSI-X.
> */
> - if (ndev->hw_type != BWD_HW) {
> - rc = -EIO;
> - goto err1;
> - }
> -
> - dev_warn(&pdev->dev,
> - "Only %d MSI-X vectors. Limiting the number of queues to that number.\n",
> - rc);
> + rc = pci_enable_msix_range(pdev, ndev->msix_entries,
> + msix_entries, msix_entries);
> + else
> + rc = pci_enable_msix_range(pdev, ndev->msix_entries,
> + 1, msix_entries);

Actually, this must be 2 for the min. One for the Data and one for
the Link. If you look a few lines after this in the original code,
there is a grabbing of the last vector for the link. I realize there
is currently no check for this in the driver and a potential error
case occurs. I can make a separate patch to correct this issue if
this patch is not going through my tree.

Thanks,
Jon


> + if (rc < 0)
> + goto err1;
> + else if (rc < msix_entries) {
> + dev_warn(&pdev->dev, "Only %d MSI-X vectors. "
> + "Limiting the number of queues to that number.\n", rc);
> msix_entries = rc;
> -
> - rc = pci_enable_msix(pdev, ndev->msix_entries, msix_entries);
> - if (rc)
> - goto err1;
> }
>
> for (i = 0; i < msix_entries; i++) {
> diff --git a/drivers/ntb/ntb_hw.h b/drivers/ntb/ntb_hw.h
> index bbdb7ed..d307107 100644
> --- a/drivers/ntb/ntb_hw.h
> +++ b/drivers/ntb/ntb_hw.h
> @@ -60,8 +60,6 @@
> #define PCI_DEVICE_ID_INTEL_NTB_SS_HSX 0x2F0F
> #define PCI_DEVICE_ID_INTEL_NTB_B2B_BWD 0x0C4E
>
> -#define msix_table_size(control) ((control & PCI_MSIX_FLAGS_QSIZE)+1)
> -
> #ifndef readq
> static inline u64 readq(void __iomem *addr)
> {
> --
> 1.7.7.6
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/