-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@intel.com>
Sent: Tuesday, January 26, 2021 8:57 PM
To: Pradeep Kumar Nalla <pna...@marvell.com>; Jerin Jacob Kollanukkaran
<jer...@marvell.com>; Nithin Kumar Dabilpuram <ndabilpu...@marvell.com>; Radha
Chintakuntla <rad...@marvell.com>; Veerasenareddy Burru <vbu...@marvell.com>
Cc: dev@dpdk.org; Satananda Burla <sbu...@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 <pna...@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
