Re: [dpdk-dev] [PATCH v2 02/14] net/igc: support device initialization

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 3/20/2020 2:46 AM, alvinx.zhang@intel.com wrote:
> From: Alvin Zhang <alvinx.zhang@intel.com>
> 
> Update base share codes, add readme.
> Add OS specific functions and definitions.
> Add device initialization codes.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>

<...>

>  #
> +# Add extra flags for base driver files (also known as shared code)
> +# to disable warnings
> +#
> +ifeq ($(CONFIG_RTE_TOOLCHAIN_ICC),y)
> +#
> +# CFLAGS for icc
> +#
> +CFLAGS_BASE_DRIVER  = -diag-disable 177 -diag-disable 181
> +CFLAGS_BASE_DRIVER += -diag-disable 869 -diag-disable 2259
> +else
> +#
> +# CFLAGS for gcc/clang
> +#
> +CFLAGS_BASE_DRIVER = -Wno-unused-parameter
> +CFLAGS_BASE_DRIVER += -Wno-unused-variable
> +ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> +ifeq ($(shell test $(GCC_VERSION) -ge 70 && echo 1), 1)
> +CFLAGS_BASE_DRIVER += -Wno-implicit-fallthrough

Can't we fix these in the code instead of warning disabling? As far as I can see
removing all doesn't cause any build error, at lest in this commit.

<...>

> +Intel® IGC driver
> +==================
> +
> +This directory contains source code of FreeBSD igc driver of version
> +2019.10.18 released by the team which develops basic drivers for any
> +i225 NIC.
> +The directory of base/ contains the original source package.
> +This driver is valid for the product(s) listed below
> +
> +* Intel® Ethernet Network Adapters I225
> +
> +Updating the driver
> +===================
> +
> +NOTE:
> +- To avoid namespace issues with e1000 PMD, all prefix e1000_ or E1000_
> +of the definition and macro names ware replaced with igc_ or IGC_.

What do you think doing same thing for the file names, to preven confusion, like
'e1000_phy.c' -> 'igc_phy.c', does it make sense?

<...>

> @@ -0,0 +1,28 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2020 Intel Corporation
> +
> +sources = [
> +	'e1000_api.c',
> +	'e1000_base.c',
> +	'e1000_i225.c',
> +	'e1000_mac.c',
> +	'e1000_manage.c',
> +	'e1000_nvm.c',
> +	'e1000_osdep.c',
> +	'e1000_phy.c',
> +]
> +
> +error_cflags = ['-Wno-unused-parameter', '-Wno-unused-variable']

Same comment here, can we remove these?