* [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines
@ 2017-05-24 22:47 Myron Stowe
2017-05-24 22:47 ` [PATCH 1/2] PCI: Add Mellanox device IDs Myron Stowe
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Myron Stowe @ 2017-05-24 22:47 UTC (permalink / raw)
To: linux-pci, netdev; +Cc: bhelgaas, saeedm, noaos, tariqt
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(-)
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] PCI: Add Mellanox device IDs 2017-05-24 22:47 [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines Myron Stowe @ 2017-05-24 22:47 ` Myron Stowe 2017-05-24 22:47 ` [PATCH 2/2] net/mlx5e: Use device ID defines Myron Stowe 2017-05-25 0:02 ` [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines David Miller 2 siblings, 0 replies; 7+ messages in thread From: Myron Stowe @ 2017-05-24 22:47 UTC (permalink / raw) To: linux-pci, netdev; +Cc: bhelgaas, saeedm, noaos, tariqt Add Mellanox device IDs for controllers covered by the mlx5 driver. Signed-off-by: Myron Stowe <myron.stowe@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_LX 0x1015 +#define PCI_DEVICE_ID_MELLANOX_CONNECTX5 0x1017 +#define PCI_DEVICE_ID_MELLANOX_CONNECTX5_EX 0x1019 +#define PCI_DEVICE_ID_MELLANOX_CONNECTX6 0x101b #define PCI_DEVICE_ID_MELLANOX_TAVOR 0x5a44 #define PCI_DEVICE_ID_MELLANOX_TAVOR_BRIDGE 0x5a46 #define PCI_DEVICE_ID_MELLANOX_SINAI_OLD 0x5e8c ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] net/mlx5e: Use device ID defines 2017-05-24 22:47 [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines Myron Stowe 2017-05-24 22:47 ` [PATCH 1/2] PCI: Add Mellanox device IDs Myron Stowe @ 2017-05-24 22:47 ` Myron Stowe 2017-05-25 0:02 ` [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines David Miller 2 siblings, 0 replies; 7+ messages in thread From: Myron Stowe @ 2017-05-24 22:47 UTC (permalink / raw) To: linux-pci, netdev; +Cc: bhelgaas, saeedm, noaos, tariqt 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.stowe@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, } }; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines 2017-05-24 22:47 [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines Myron Stowe 2017-05-24 22:47 ` [PATCH 1/2] PCI: Add Mellanox device IDs Myron Stowe 2017-05-24 22:47 ` [PATCH 2/2] net/mlx5e: Use device ID defines Myron Stowe @ 2017-05-25 0:02 ` David Miller 2017-05-25 15:56 ` Myron Stowe 2 siblings, 1 reply; 7+ messages in thread From: David Miller @ 2017-05-25 0:02 UTC (permalink / raw) To: myron.stowe; +Cc: linux-pci, netdev, bhelgaas, saeedm, noaos, tariqt From: Myron Stowe <myron.stowe@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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines 2017-05-25 0:02 ` [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines David Miller @ 2017-05-25 15:56 ` Myron Stowe 2017-06-16 20:08 ` Bjorn Helgaas 0 siblings, 1 reply; 7+ messages in thread From: Myron Stowe @ 2017-05-25 15:56 UTC (permalink / raw) To: David Miller Cc: myron.stowe, linux-pci, netdev, bhelgaas, saeedm, noaos, tariqt On Wed, 24 May 2017 20:02:49 -0400 (EDT) David Miller <davem@davemloft.net> wrote: > From: Myron Stowe <myron.stowe@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines 2017-05-25 15:56 ` Myron Stowe @ 2017-06-16 20:08 ` Bjorn Helgaas 2017-06-16 20:21 ` Myron Stowe 0 siblings, 1 reply; 7+ messages in thread From: Bjorn Helgaas @ 2017-06-16 20:08 UTC (permalink / raw) To: Myron Stowe Cc: David Miller, myron.stowe, linux-pci, netdev, bhelgaas, saeedm, noaos, tariqt 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 <davem@davemloft.net> wrote: > > > From: Myron Stowe <myron.stowe@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. The patch below doesn't touch PCI, so it would be just for netdev. 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 */ ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines 2017-06-16 20:08 ` Bjorn Helgaas @ 2017-06-16 20:21 ` Myron Stowe 0 siblings, 0 replies; 7+ messages in thread From: Myron Stowe @ 2017-06-16 20:21 UTC (permalink / raw) To: Bjorn Helgaas Cc: Myron Stowe, David Miller, Myron Stowe, linux-pci, netdev, Bjorn Helgaas, saeedm, noaos, tariqt On Fri, Jun 16, 2017 at 2:08 PM, Bjorn Helgaas <helgaas@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 <davem@davemloft.net> wrote: >> >> > From: Myron Stowe <myron.stowe@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 */ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-16 20:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-24 22:47 [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines Myron Stowe 2017-05-24 22:47 ` [PATCH 1/2] PCI: Add Mellanox device IDs Myron Stowe 2017-05-24 22:47 ` [PATCH 2/2] net/mlx5e: Use device ID defines Myron Stowe 2017-05-25 0:02 ` [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines David Miller 2017-05-25 15:56 ` Myron Stowe 2017-06-16 20:08 ` Bjorn Helgaas 2017-06-16 20:21 ` Myron Stowe
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).