[PATCH v2] net/mlx5e: Use device ID defines
Use Mellanox device ID definitions in the driver's mlx5 ID table so tools such as 'grep' and 'cscope' can be used to help find correlated material (such as INTx Masking quirks: d76d2fe05fd PCI: Convert Mellanox broken INTx quirks to be for listed devices only). No functional change intended. Signed-off-by: Myron Stowe <myron.st...@redhat.com> --- drivers/net/ethernet/mellanox/mlx5/core/main.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c index 0c123d5..8a4e292 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c @@ -1508,11 +1508,11 @@ static void shutdown(struct pci_dev *pdev) } static const struct pci_device_id mlx5_core_pci_table[] = { - { PCI_VDEVICE(MELLANOX, 0x1011) }, /* Connect-IB */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTIB) }, { PCI_VDEVICE(MELLANOX, 0x1012), MLX5_PCI_DEV_IS_VF}, /* Connect-IB VF */ - { PCI_VDEVICE(MELLANOX, 0x1013) }, /* ConnectX-4 */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX4) }, { PCI_VDEVICE(MELLANOX, 0x1014), MLX5_PCI_DEV_IS_VF}, /* ConnectX-4 VF */ - { PCI_VDEVICE(MELLANOX, 0x1015) }, /* ConnectX-4LX */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX) }, { PCI_VDEVICE(MELLANOX, 0x1016), MLX5_PCI_DEV_IS_VF}, /* ConnectX-4LX VF */ { PCI_VDEVICE(MELLANOX, 0x1017) }, /* ConnectX-5, PCIe 3.0 */ { PCI_VDEVICE(MELLANOX, 0x1018), MLX5_PCI_DEV_IS_VF}, /* ConnectX-5 VF */
Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines
On Fri, Jun 16, 2017 at 2:08 PM, Bjorn Helgaas <helg...@kernel.org> wrote: > On Thu, May 25, 2017 at 09:56:55AM -0600, Myron Stowe wrote: >> On Wed, 24 May 2017 20:02:49 -0400 (EDT) >> David Miller <da...@davemloft.net> wrote: >> >> > From: Myron Stowe <myron.st...@redhat.com> >> > Date: Wed, 24 May 2017 16:47:34 -0600 >> > >> > > Noa Osherovich introduced a series of new Mellanox device ID >> > > definitions to help differentiate specific controllers that needed >> > > INTx masking quirks [1]. >> > > >> > > Bjorn Helgaas followed on, using the device ID definitions Noa >> > > provided to replace hard-coded values within the mxl4 ID table [2]. >> > > >> > > This patch continues along similar lines, adding a few additional >> > > Mellanox device ID definitions and converting the net/mlx5e >> > > driver's mlx5 ID table to use the defines so tools like 'grep' and >> > > 'cscope' can be used to help identify relationships with other >> > > aspects (such as INTx masking). >> > >> > If you're adding pci_ids.h defines, it's only valid to do so if you >> > actually use the defines in more than one location. >> > >> > This patch series is not doing that. >> >> Hi David, >> >> Yes, now that you mention that again I do vaguely remember past >> conversations stating similar constraints which is a little odd as >> Noa's series did exactly that. It was Bjorn, in a separate patch, that >> made the connection to the driver with commit c19e4b9037f >> ("net/mlx4_core: Use device ID defines") [1] and even after such, some >> of the introduced #defines are still currently singular in usage. >> >> Anyway, the part I'm interested in is creating a more transparent >> association between the Mellanox controllers that need the INTx masking >> quirk and their drivers, something that remains very opaque currently >> for a few of the remaining instances (PCI_DEVICE_ID_MELLANOX_CONNECTIB, >> PCI_DEVICE_ID_MELLANOX_CONNECTX4, and >> PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX). > > I think what you want is the patch below (your patch 2, after removing > CONNECTX5, CONNECTX5_EX, and CONNECTX6 since they're only used in one > place). > > We added definitions for CONNECTIB, CONNECTX4, and CONNECTX4_LX and uses of > them in a quirk via: > > 7254383341bc ("PCI: Add Mellanox device IDs") > d76d2fe05fd9 ("PCI: Convert Mellanox broken INTx quirks to be for listed > devices only") > > But somehow we missed using those in mlx5/core/main.c. Yes, that's the downside to not adding pci_ids.h defines originally within the driver when its written (even if they are only used once), it ends up putting the burden on subsequent patch generators to notice the association and clean things up after the fact. :) > > The patch below doesn't touch PCI, so it would be just for netdev. Yes, without the pci_ids.h #defines, the resulting patch just ends up being for netdev. That's fine, it was the association between the quirk and the driver, wanting to make such more transparent, that was the inpetus for this. > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c > b/drivers/net/ethernet/mellanox/mlx5/core/main.c > index 0c123d571b4c..8a4e292f26b8 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c > @@ -1508,11 +1508,11 @@ static void shutdown(struct pci_dev *pdev) > } > > static const struct pci_device_id mlx5_core_pci_table[] = { > - { PCI_VDEVICE(MELLANOX, 0x1011) }, /* Connect-IB > */ > + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTIB) }, > { PCI_VDEVICE(MELLANOX, 0x1012), MLX5_PCI_DEV_IS_VF}, /* Connect-IB > VF */ > - { PCI_VDEVICE(MELLANOX, 0x1013) }, /* ConnectX-4 > */ > + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX4) }, > { PCI_VDEVICE(MELLANOX, 0x1014), MLX5_PCI_DEV_IS_VF}, /* ConnectX-4 > VF */ > - { PCI_VDEVICE(MELLANOX, 0x1015) }, /* > ConnectX-4LX */ > + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX) }, > { PCI_VDEVICE(MELLANOX, 0x1016), MLX5_PCI_DEV_IS_VF}, /* > ConnectX-4LX VF */ > { PCI_VDEVICE(MELLANOX, 0x1017) }, /* > ConnectX-5, PCIe 3.0 */ > { PCI_VDEVICE(MELLANOX, 0x1018), MLX5_PCI_DEV_IS_VF}, /* ConnectX-5 > VF */
Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines
On Wed, 24 May 2017 20:02:49 -0400 (EDT) David Miller <da...@davemloft.net> wrote: > From: Myron Stowe <myron.st...@redhat.com> > Date: Wed, 24 May 2017 16:47:34 -0600 > > > Noa Osherovich introduced a series of new Mellanox device ID > > definitions to help differentiate specific controllers that needed > > INTx masking quirks [1]. > > > > Bjorn Helgaas followed on, using the device ID definitions Noa > > provided to replace hard-coded values within the mxl4 ID table [2]. > > > > This patch continues along similar lines, adding a few additional > > Mellanox device ID definitions and converting the net/mlx5e > > driver's mlx5 ID table to use the defines so tools like 'grep' and > > 'cscope' can be used to help identify relationships with other > > aspects (such as INTx masking). > > If you're adding pci_ids.h defines, it's only valid to do so if you > actually use the defines in more than one location. > > This patch series is not doing that. Hi David, Yes, now that you mention that again I do vaguely remember past conversations stating similar constraints which is a little odd as Noa's series did exactly that. It was Bjorn, in a separate patch, that made the connection to the driver with commit c19e4b9037f ("net/mlx4_core: Use device ID defines") [1] and even after such, some of the introduced #defines are still currently singular in usage. Anyway, the part I'm interested in is creating a more transparent association between the Mellanox controllers that need the INTx masking quirk and their drivers, something that remains very opaque currently for a few of the remaining instances (PCI_DEVICE_ID_MELLANOX_CONNECTIB, PCI_DEVICE_ID_MELLANOX_CONNECTX4, and PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX). I'd like to hear back from others as to whether or not there is truly concern about adding the #defines such as I submitted with singular usages and if so I can re-submit a more focused patch which would effectively be the first three substitutions in "[PATCH 2/2] net/mlx5e: Use device ID defines". [1] Perhaps Noa's submission had a similar discussion as while it was a separate series from which Bjorn then made the connection to the driver, all the patches came in via the same merge commit 25831571419 ("Merge branch 'pci/virtualization' into next"). Thanks for your feedback, Myron
[PATCH 2/2] net/mlx5e: Use device ID defines
Use Mellanox device ID definitions in the mlx5 ID table so tools such as 'grep' and 'cscope' can be used to help find related aspects. No functional change intended. Signed-off-by: Myron Stowe <myron.st...@redhat.com> --- drivers/net/ethernet/mellanox/mlx5/core/main.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c index 0c123d5..98642eb 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c @@ -1508,17 +1508,17 @@ static void shutdown(struct pci_dev *pdev) } static const struct pci_device_id mlx5_core_pci_table[] = { - { PCI_VDEVICE(MELLANOX, 0x1011) }, /* Connect-IB */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTIB) }, { PCI_VDEVICE(MELLANOX, 0x1012), MLX5_PCI_DEV_IS_VF}, /* Connect-IB VF */ - { PCI_VDEVICE(MELLANOX, 0x1013) }, /* ConnectX-4 */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX4) }, { PCI_VDEVICE(MELLANOX, 0x1014), MLX5_PCI_DEV_IS_VF}, /* ConnectX-4 VF */ - { PCI_VDEVICE(MELLANOX, 0x1015) }, /* ConnectX-4LX */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX) }, { PCI_VDEVICE(MELLANOX, 0x1016), MLX5_PCI_DEV_IS_VF}, /* ConnectX-4LX VF */ - { PCI_VDEVICE(MELLANOX, 0x1017) }, /* ConnectX-5, PCIe 3.0 */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX5) }, { PCI_VDEVICE(MELLANOX, 0x1018), MLX5_PCI_DEV_IS_VF}, /* ConnectX-5 VF */ - { PCI_VDEVICE(MELLANOX, 0x1019) }, /* ConnectX-5 Ex */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX5_EX) }, { PCI_VDEVICE(MELLANOX, 0x101a), MLX5_PCI_DEV_IS_VF}, /* ConnectX-5 Ex VF */ - { PCI_VDEVICE(MELLANOX, 0x101b) }, /* ConnectX-6 */ + { PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX6) }, { PCI_VDEVICE(MELLANOX, 0x101c), MLX5_PCI_DEV_IS_VF}, /* ConnectX-6 VF */ { 0, } };
[PATCH 1/2] PCI: Add Mellanox device IDs
Add Mellanox device IDs for controllers covered by the mlx5 driver. Signed-off-by: Myron Stowe <myron.st...@redhat.com> --- include/linux/pci_ids.h |3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 5f6b71d..5626d5a 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -2266,6 +2266,9 @@ #define PCI_DEVICE_ID_MELLANOX_CONNECTIB 0x1011 #define PCI_DEVICE_ID_MELLANOX_CONNECTX4 0x1013 #define PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX0x1015 +#define PCI_DEVICE_ID_MELLANOX_CONNECTX5 0x1017 +#define PCI_DEVICE_ID_MELLANOX_CONNECTX5_EX0x1019 +#define PCI_DEVICE_ID_MELLANOX_CONNECTX6 0x101b #define PCI_DEVICE_ID_MELLANOX_TAVOR 0x5a44 #define PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE0x5a46 #define PCI_DEVICE_ID_MELLANOX_SINAI_OLD 0x5e8c
[PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines
Noa Osherovich introduced a series of new Mellanox device ID definitions to help differentiate specific controllers that needed INTx masking quirks [1]. Bjorn Helgaas followed on, using the device ID definitions Noa provided to replace hard-coded values within the mxl4 ID table [2]. This patch continues along similar lines, adding a few additional Mellanox device ID definitions and converting the net/mlx5e driver's mlx5 ID table to use the defines so tools like 'grep' and 'cscope' can be used to help identify relationships with other aspects (such as INTx masking). [1] 7254383341b PCI: Add Mellanox device IDs d76d2fe05fd PCI: Convert Mellanox broken INTx quirks to be for listed devices only [2] c19e4b9037f net/mlx4_core: Use device ID defines Myron Stowe (2): PCI: Add Mellanox device IDs net/mlx5e: Use device ID defines drivers/net/ethernet/mellanox/mlx5/core/main.c | 12 ++-- include/linux/pci_ids.h|3 +++ 2 files changed, 9 insertions(+), 6 deletions(-) --