[dpdk-dev] [PATCH v2 02/11] net/octeontx_ep: add ethdev probe and remove

[Ferruh Yigit]

On 1/18/2021 9:35 AM, Nalla Pradeep wrote:
> add basic PCIe ethdev probe and remove.
> 
> Signed-off-by: Nalla Pradeep <pnalla at 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?