Re: [dpdk-dev] [PATCH v2 02/11] net/octeontx_ep: add ethdev probe and remove
From: Ferruh Yigit <hidden>
Date: 2021-01-26 15:27:38
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?