[v2,02/11] net/octeontx_ep: add ethdev probe and remove

Message ID 20210118093602.5449-2-pnalla@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [v2,01/11] net/octeontx_ep: add build and doc infrastructure |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Pradeep Nalla Jan. 18, 2021, 9:35 a.m. UTC
  add basic PCIe ethdev probe and remove.

Signed-off-by: Nalla Pradeep <pnalla@marvell.com>
---
 drivers/common/octeontx2/otx2_common.h    |  5 +-
 drivers/net/octeontx_ep/meson.build       | 13 +++++
 drivers/net/octeontx_ep/otx_ep_common.h   | 14 +++++
 drivers/net/octeontx_ep/otx_ep_ethdev.c   | 62 +++++++++++++++++++++++
 drivers/net/octeontx_ep/otx_ep_vf.h       |  9 ++++
 drivers/raw/octeontx2_ep/otx2_ep_rawdev.c |  6 +--
 6 files changed, 105 insertions(+), 4 deletions(-)
 create mode 100644 drivers/net/octeontx_ep/otx_ep_common.h
 create mode 100644 drivers/net/octeontx_ep/otx_ep_vf.h
  

Comments

Jerin Jacob Jan. 19, 2021, 11:55 a.m. UTC | #1
On Mon, Jan 18, 2021 at 3:07 PM Nalla Pradeep <pnalla@marvell.com> wrote:
>
> add basic PCIe ethdev probe and remove.
>
> Signed-off-by: Nalla Pradeep <pnalla@marvell.com>
> ---
>  drivers/common/octeontx2/otx2_common.h    |  5 +-
>  drivers/net/octeontx_ep/meson.build       | 13 +++++
>  drivers/net/octeontx_ep/otx_ep_common.h   | 14 +++++
>  drivers/net/octeontx_ep/otx_ep_ethdev.c   | 62 +++++++++++++++++++++++
>  drivers/net/octeontx_ep/otx_ep_vf.h       |  9 ++++
>  drivers/raw/octeontx2_ep/otx2_ep_rawdev.c |  6 +--
>  6 files changed, 105 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/net/octeontx_ep/otx_ep_common.h
>  create mode 100644 drivers/net/octeontx_ep/otx_ep_vf.h
>
> diff --git a/drivers/common/octeontx2/otx2_common.h b/drivers/common/octeontx2/otx2_common.h
> index b6779f7104..cd52e098e6 100644
> --- a/drivers/common/octeontx2/otx2_common.h
> +++ b/drivers/common/octeontx2/otx2_common.h
> @@ -136,7 +136,10 @@ extern int otx2_logtype_ree;
>  #define PCI_DEVID_OCTEONTX2_RVU_CPT_VF         0xA0FE
>  #define PCI_DEVID_OCTEONTX2_RVU_AF_VF          0xA0f8
>  #define PCI_DEVID_OCTEONTX2_DPI_VF             0xA081
> -#define PCI_DEVID_OCTEONTX2_EP_VF              0xB203 /* OCTEON TX2 EP mode */
> +#define PCI_DEVID_OCTEONTX2_EP_NET_VF          0xB203 /* OCTEON TX2 EP mode */
> +/* OCTEON TX2 98xx EP mode */
> +#define PCI_DEVID_CN98XX_EP_NET_VF             0xB103
> +#define PCI_DEVID_OCTEONTX2_EP_RAW_VF          0xB204 /* OCTEON TX2 EP mode */
>  #define PCI_DEVID_OCTEONTX2_RVU_SDP_PF         0xA0f6
>  #define PCI_DEVID_OCTEONTX2_RVU_SDP_VF         0xA0f7
>  #define PCI_DEVID_OCTEONTX2_RVU_REE_PF         0xA0f4
> diff --git a/drivers/net/octeontx_ep/meson.build b/drivers/net/octeontx_ep/meson.build
> index 83ffbad7b6..06663de4e2 100644
> --- a/drivers/net/octeontx_ep/meson.build
> +++ b/drivers/net/octeontx_ep/meson.build
> @@ -6,3 +6,16 @@ sources = files(
>                 'otx_ep_ethdev.c',
>                 )
>
> +extra_flags = []
> +# This integrated controller runs only on a arm64 machine, remove 32bit warnings
> +if not dpdk_conf.get('RTE_ARCH_64')
> +        extra_flags += ['-Wno-int-to-pointer-cast', '-Wno-pointer-to-int-cast']
> +endif

This was from net/octeontx2 driver due to NPA complication. Please
check, Is this really required
as this driver can run in x86 32bit mode unlike net/octeontx2.
  
Ferruh Yigit Jan. 26, 2021, 3:27 p.m. UTC | #2
On 1/18/2021 9:35 AM, Nalla Pradeep wrote:
> add basic PCIe ethdev probe and remove.
> 
> Signed-off-by: Nalla Pradeep <pnalla@marvell.com>

<...>

> @@ -136,7 +136,10 @@ extern int otx2_logtype_ree;
>   #define PCI_DEVID_OCTEONTX2_RVU_CPT_VF		0xA0FE
>   #define PCI_DEVID_OCTEONTX2_RVU_AF_VF		0xA0f8
>   #define PCI_DEVID_OCTEONTX2_DPI_VF		0xA081
> -#define PCI_DEVID_OCTEONTX2_EP_VF		0xB203 /* OCTEON TX2 EP mode */
> +#define PCI_DEVID_OCTEONTX2_EP_NET_VF		0xB203 /* OCTEON TX2 EP mode */

You can considering doing this rename on its own patch, to reduce the noise for 
the actual patch.

<...>

> +++ b/drivers/net/octeontx_ep/meson.build
> @@ -6,3 +6,16 @@ sources = files(
>                  'otx_ep_ethdev.c',
>                  )
>   
> +extra_flags = []
> +# This integrated controller runs only on a arm64 machine, remove 32bit warnings
> +if not dpdk_conf.get('RTE_ARCH_64')
> +        extra_flags += ['-Wno-int-to-pointer-cast', '-Wno-pointer-to-int-cast']
> +endif

There is almost any code at this stage, are above compiler flags really needed? 
Can you add then only when they are really needed?

<...>

> --- a/drivers/net/octeontx_ep/otx_ep_ethdev.c
> +++ b/drivers/net/octeontx_ep/otx_ep_ethdev.c
> @@ -1,3 +1,65 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
>    * Copyright(C) 2020 Marvell.
>    */
> +
> +#include <rte_ethdev_pci.h>
> +#include <rte_malloc.h>
> +#include <rte_io.h>
> +

Are all these headers needed, no io or mem allocation yet, can you please add 
them as they are needed?

<...>

> +++ b/drivers/net/octeontx_ep/otx_ep_vf.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Marvell.
> + */
> +#ifndef _OTX_EP_VF_H_
> +#define _OTX_EP_VF_H_
> +
> +#define PCI_DEVID_OCTEONTX_EP_VF 0xa303
> +

Why this PCI device id defined different place than all the others did, won't it 
be easier to keep them all in same place?
  
Pradeep Nalla Jan. 27, 2021, 11:45 a.m. UTC | #3
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Tuesday, January 26, 2021 8:57 PM
To: Pradeep Kumar Nalla <pnalla@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Radha Chintakuntla <radhac@marvell.com>; Veerasenareddy Burru <vburru@marvell.com>
Cc: dev@dpdk.org; Satananda Burla <sburla@marvell.com>
Subject: [EXT] Re: [dpdk-dev] [PATCH v2 02/11] net/octeontx_ep: add ethdev probe and remove

External Email

----------------------------------------------------------------------
On 1/18/2021 9:35 AM, Nalla Pradeep wrote:
> add basic PCIe ethdev probe and remove.
> 
> Signed-off-by: Nalla Pradeep <pnalla@marvell.com>

<...>

> @@ -136,7 +136,10 @@ extern int otx2_logtype_ree;
>   #define PCI_DEVID_OCTEONTX2_RVU_CPT_VF		0xA0FE
>   #define PCI_DEVID_OCTEONTX2_RVU_AF_VF		0xA0f8
>   #define PCI_DEVID_OCTEONTX2_DPI_VF		0xA081
> -#define PCI_DEVID_OCTEONTX2_EP_VF		0xB203 /* OCTEON TX2 EP mode */
> +#define PCI_DEVID_OCTEONTX2_EP_NET_VF		0xB203 /* OCTEON TX2 EP mode */

You can considering doing this rename on its own patch, to reduce the noise for the actual patch.

<...>

> +++ b/drivers/net/octeontx_ep/meson.build
> @@ -6,3 +6,16 @@ sources = files(
>                  'otx_ep_ethdev.c',
>                  )
>   
> +extra_flags = []
> +# This integrated controller runs only on a arm64 machine, remove 
> +32bit warnings if not dpdk_conf.get('RTE_ARCH_64')
> +        extra_flags += ['-Wno-int-to-pointer-cast', 
> +'-Wno-pointer-to-int-cast'] endif

There is almost any code at this stage, are above compiler flags really needed? 
Can you add then only when they are really needed?

<...>

> --- a/drivers/net/octeontx_ep/otx_ep_ethdev.c
> +++ b/drivers/net/octeontx_ep/otx_ep_ethdev.c
> @@ -1,3 +1,65 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
>    * Copyright(C) 2020 Marvell.
>    */
> +
> +#include <rte_ethdev_pci.h>
> +#include <rte_malloc.h>
> +#include <rte_io.h>
> +

Are all these headers needed, no io or mem allocation yet, can you please add them as they are needed?

<...>

> +++ b/drivers/net/octeontx_ep/otx_ep_vf.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Marvell.
> + */
> +#ifndef _OTX_EP_VF_H_
> +#define _OTX_EP_VF_H_
> +
> +#define PCI_DEVID_OCTEONTX_EP_VF 0xa303
> +

>Why this PCI device id defined different place than all the others did, won't it be easier to keep them all in same >place?
This device id is from octeontx family, whereas rest all are from octeontx2 family. All the device ids of octeontx2 family are maintained in  drivers/common/octeontx2/otx2_common.h and this particular octeon tx device will be in drivers/net/octeontx_ep/otx_ep_vf.h
  

Patch

diff --git a/drivers/common/octeontx2/otx2_common.h b/drivers/common/octeontx2/otx2_common.h
index b6779f7104..cd52e098e6 100644
--- a/drivers/common/octeontx2/otx2_common.h
+++ b/drivers/common/octeontx2/otx2_common.h
@@ -136,7 +136,10 @@  extern int otx2_logtype_ree;
 #define PCI_DEVID_OCTEONTX2_RVU_CPT_VF		0xA0FE
 #define PCI_DEVID_OCTEONTX2_RVU_AF_VF		0xA0f8
 #define PCI_DEVID_OCTEONTX2_DPI_VF		0xA081
-#define PCI_DEVID_OCTEONTX2_EP_VF		0xB203 /* OCTEON TX2 EP mode */
+#define PCI_DEVID_OCTEONTX2_EP_NET_VF		0xB203 /* OCTEON TX2 EP mode */
+/* OCTEON TX2 98xx EP mode */
+#define PCI_DEVID_CN98XX_EP_NET_VF		0xB103
+#define PCI_DEVID_OCTEONTX2_EP_RAW_VF		0xB204 /* OCTEON TX2 EP mode */
 #define PCI_DEVID_OCTEONTX2_RVU_SDP_PF		0xA0f6
 #define PCI_DEVID_OCTEONTX2_RVU_SDP_VF		0xA0f7
 #define PCI_DEVID_OCTEONTX2_RVU_REE_PF		0xA0f4
diff --git a/drivers/net/octeontx_ep/meson.build b/drivers/net/octeontx_ep/meson.build
index 83ffbad7b6..06663de4e2 100644
--- a/drivers/net/octeontx_ep/meson.build
+++ b/drivers/net/octeontx_ep/meson.build
@@ -6,3 +6,16 @@  sources = files(
                'otx_ep_ethdev.c',
                )
 
+extra_flags = []
+# This integrated controller runs only on a arm64 machine, remove 32bit warnings
+if not dpdk_conf.get('RTE_ARCH_64')
+        extra_flags += ['-Wno-int-to-pointer-cast', '-Wno-pointer-to-int-cast']
+endif
+
+foreach flag: extra_flags
+        if cc.has_argument(flag)
+                cflags += flag
+        endif
+endforeach
+
+includes += include_directories('../../common/octeontx2')
diff --git a/drivers/net/octeontx_ep/otx_ep_common.h b/drivers/net/octeontx_ep/otx_ep_common.h
new file mode 100644
index 0000000000..3fa2de9ab3
--- /dev/null
+++ b/drivers/net/octeontx_ep/otx_ep_common.h
@@ -0,0 +1,14 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2020 Marvell.
+ */
+#ifndef _OTX_EP_COMMON_H_
+#define _OTX_EP_COMMON_H_
+
+/* OTX_EP EP VF device data structure */
+struct otx_ep_device {
+	/* PCI device pointer */
+	struct rte_pci_device *pdev;
+
+	struct rte_eth_dev *eth_dev;
+};
+#endif  /* _OTX_EP_COMMON_H_ */
diff --git a/drivers/net/octeontx_ep/otx_ep_ethdev.c b/drivers/net/octeontx_ep/otx_ep_ethdev.c
index 2b2d684a0b..4eb75a2765 100644
--- a/drivers/net/octeontx_ep/otx_ep_ethdev.c
+++ b/drivers/net/octeontx_ep/otx_ep_ethdev.c
@@ -1,3 +1,65 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
  * Copyright(C) 2020 Marvell.
  */
+
+#include <rte_ethdev_pci.h>
+#include <rte_malloc.h>
+#include <rte_io.h>
+
+#include "otx2_common.h"
+#include "otx_ep_common.h"
+#include "otx_ep_vf.h"
+
+static int
+otx_ep_eth_dev_uninit(struct rte_eth_dev *eth_dev)
+{
+	RTE_SET_USED(eth_dev);
+
+	return -ENODEV;
+}
+
+static int
+otx_ep_eth_dev_init(struct rte_eth_dev *eth_dev)
+{
+	RTE_SET_USED(eth_dev);
+
+	return -ENODEV;
+}
+
+static int
+otx_ep_eth_dev_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
+		      struct rte_pci_device *pci_dev)
+{
+	return rte_eth_dev_pci_generic_probe(pci_dev,
+					     sizeof(struct otx_ep_device),
+					     otx_ep_eth_dev_init);
+}
+
+static int
+otx_ep_eth_dev_pci_remove(struct rte_pci_device *pci_dev)
+{
+	return rte_eth_dev_pci_generic_remove(pci_dev,
+					      otx_ep_eth_dev_uninit);
+}
+
+
+/* Set of PCI devices this driver supports */
+static const struct rte_pci_id pci_id_otx_ep_map[] = {
+	{ RTE_PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_OCTEONTX_EP_VF) },
+	{ RTE_PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_OCTEONTX2_EP_NET_VF) },
+	{ RTE_PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, PCI_DEVID_CN98XX_EP_NET_VF) },
+	{ .vendor_id = 0, /* sentinel */ }
+};
+
+
+
+static struct rte_pci_driver rte_otx_ep_pmd = {
+	.id_table	= pci_id_otx_ep_map,
+	.drv_flags      = RTE_PCI_DRV_NEED_MAPPING,
+	.probe		= otx_ep_eth_dev_pci_probe,
+	.remove		= otx_ep_eth_dev_pci_remove,
+};
+
+RTE_PMD_REGISTER_PCI(net_otx_ep, rte_otx_ep_pmd);
+RTE_PMD_REGISTER_PCI_TABLE(net_otx_ep, pci_id_otx_ep_map);
+RTE_PMD_REGISTER_KMOD_DEP(net_otx_ep, "* igb_uio | vfio-pci");
diff --git a/drivers/net/octeontx_ep/otx_ep_vf.h b/drivers/net/octeontx_ep/otx_ep_vf.h
new file mode 100644
index 0000000000..0498dd0cf5
--- /dev/null
+++ b/drivers/net/octeontx_ep/otx_ep_vf.h
@@ -0,0 +1,9 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2020 Marvell.
+ */
+#ifndef _OTX_EP_VF_H_
+#define _OTX_EP_VF_H_
+
+#define PCI_DEVID_OCTEONTX_EP_VF 0xa303
+
+#endif /*_OTX_EP_VF_H_ */
diff --git a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c
index 2b78a7941d..b2ccdda83e 100644
--- a/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c
+++ b/drivers/raw/octeontx2_ep/otx2_ep_rawdev.c
@@ -22,7 +22,7 @@ 
 static const struct rte_pci_id pci_sdp_vf_map[] = {
 	{
 		RTE_PCI_DEVICE(PCI_VENDOR_ID_CAVIUM,
-			       PCI_DEVID_OCTEONTX2_EP_VF)
+			       PCI_DEVID_OCTEONTX2_EP_RAW_VF)
 	},
 	{
 		.vendor_id = 0,
@@ -109,8 +109,8 @@  sdp_chip_specific_setup(struct sdp_device *sdpvf)
 	int ret;
 
 	switch (dev_id) {
-	case PCI_DEVID_OCTEONTX2_EP_VF:
-		sdpvf->chip_id = PCI_DEVID_OCTEONTX2_EP_VF;
+	case PCI_DEVID_OCTEONTX2_EP_RAW_VF:
+		sdpvf->chip_id = PCI_DEVID_OCTEONTX2_EP_RAW_VF;
 		ret = sdp_vf_setup_device(sdpvf);
 
 		break;