Re: [dpdk-dev] [EXT] Re: [PATCH v2 02/11] net/octeontx_ep: add ethdev probe and remove
From: Pradeep Kumar Nalla <hidden>
Date: 2021-01-27 11:45:18
-----Original Message----- From: Ferruh Yigit <redacted> Sent: Tuesday, January 26, 2021 8:57 PM To: Pradeep Kumar Nalla <redacted>; Jerin Jacob Kollanukkaran <redacted>; Nithin Kumar Dabilpuram <redacted>; Radha Chintakuntla <redacted>; 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 <redacted>
<...>
quoted hunk ↗ jump to hunk
@@ -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. <...>
quoted hunk ↗ jump to hunk
+++ 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? <...>
quoted hunk ↗ jump to hunk
--- 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? <...>
quoted hunk ↗ jump to hunk
+++ 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