Re: [dpdk-dev] [PATCH 1/3] i40e: explicit shared code naming as base driver

["Chen, Jing D" <jing.d.chen@intel.com>]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, June 25, 2014 8:22 PM
> To: dev@dpdk.org
> Cc: Chen, Jing D; Zhang, Helin; Ananyev, Konstantin; De Lara Guarch, Pablo
> Subject: [PATCH 1/3] i40e: explicit shared code naming as base driver
> 
> The PMD is built on top of the base driver which is provided by Intel and
> shouldn't be modified to allow easy batch upgrade from Intel.
> 
> The base driver is a "shared code" between many projects. But in DPDK, the
> "base driver" naming makes more sense.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  lib/librte_pmd_i40e/Makefile      | 33 ++++++++++++++++-----------------
>  lib/librte_pmd_i40e/i40e_ethdev.c | 12 ++++++------
>  2 files changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/lib/librte_pmd_i40e/Makefile b/lib/librte_pmd_i40e/Makefile
> index 09f2087..77d08fb 100644
> --- a/lib/librte_pmd_i40e/Makefile
> +++ b/lib/librte_pmd_i40e/Makefile
> @@ -39,26 +39,25 @@ LIB = librte_pmd_i40e.a  CFLAGS += -O3  CFLAGS
> += $(WERROR_FLAGS)
> 
> -ifeq ($(CC), icc)
> -CFLAGS_SHARED_DRIVERS = -wd593
> -else
> -CFLAGS_SHARED_DRIVERS =  -Wno-unused-but-set-variable
> -CFLAGS_SHARED_DRIVERS += -Wno-sign-compare
> -CFLAGS_SHARED_DRIVERS += -Wno-unused-value
> -CFLAGS_SHARED_DRIVERS += -Wno-unused-parameter
> -CFLAGS_SHARED_DRIVERS += -Wno-strict-aliasing
> -CFLAGS_SHARED_DRIVERS += -Wno-format -CFLAGS_SHARED_DRIVERS +=
> -Wno-missing-field-initializers -CFLAGS_SHARED_DRIVERS +=
> -Wno-pointer-to-int-cast -CFLAGS_SHARED_DRIVERS +=
> -Wno-format-nonliteral -CFLAGS_SHARED_DRIVERS +=
> -Wno-format-security -endif
> -
>  #
>  # Add extra flags for ND source files to disable warnings  #
> -SHARED_DRIVERS_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard
> $(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
> -$(foreach obj, $(SHARED_DRIVERS_OBJS), $(eval
> CFLAGS_$(obj)+=$(CFLAGS_SHARED_DRIVERS)))
> +ifeq ($(CC), icc)
> +CFLAGS_BASE_DRIVER = -wd593
> +else
> +CFLAGS_BASE_DRIVER  = -Wno-unused-but-set-variable
> CFLAGS_BASE_DRIVER
> ++= -Wno-sign-compare CFLAGS_BASE_DRIVER += -Wno-unused-value
> +CFLAGS_BASE_DRIVER += -Wno-unused-parameter CFLAGS_BASE_DRIVER
> +=
> +-Wno-strict-aliasing CFLAGS_BASE_DRIVER += -Wno-format
> +CFLAGS_BASE_DRIVER += -Wno-missing-field-initializers
> +CFLAGS_BASE_DRIVER += -Wno-pointer-to-int-cast CFLAGS_BASE_DRIVER
> +=
> +-Wno-format-nonliteral CFLAGS_BASE_DRIVER += -Wno-format-security
> endif
> +OBJS_BASE_DRIVER=$(patsubst %.c,%.o,$(notdir $(wildcard
> +$(RTE_SDK)/lib/librte_pmd_i40e/i40e/*.c)))
> +$(foreach obj, $(OBJS_BASE_DRIVER), $(eval
> +CFLAGS_$(obj)+=$(CFLAGS_BASE_DRIVER)))
> 
>  VPATH += $(RTE_SDK)/lib/librte_pmd_i40e/i40e
> 
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 3311d73..6bc3998 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -387,10 +387,10 @@ eth_i40e_dev_init(__rte_unused struct eth_driver
> *eth_drv,
>  		return ret;
>  	}
> 
> -	/* Initialize the shared code */
> +	/* Initialize the shared code (base driver) */
>  	ret = i40e_init_shared_code(hw);
>  	if (ret) {
> -		PMD_INIT_LOG(ERR, "Failed to init shared code: %d", ret);
> +		PMD_INIT_LOG(ERR, "Failed to init shared code (base driver): %d",
> +ret);
>  		return ret;
>  	}
> 
> @@ -1497,7 +1497,7 @@ i40e_dev_rss_reta_query(struct rte_eth_dev
> *dev,  }
> 
>  /**
> - * i40e_allocate_dma_mem_d - specific memory alloc for shared code
> + * i40e_allocate_dma_mem_d - specific memory alloc for shared code
> + (base driver)
>   * @hw:   pointer to the HW structure
>   * @mem:  pointer to mem struct to fill out
>   * @size: size of memory requested
> @@ -1531,7 +1531,7 @@
> i40e_allocate_dma_mem_d(__attribute__((unused)) struct i40e_hw *hw,  }
> 
>  /**
> - * i40e_free_dma_mem_d - specific memory free for shared code
> + * i40e_free_dma_mem_d - specific memory free for shared code (base
> + driver)
>   * @hw:   pointer to the HW structure
>   * @mem:  ptr to mem struct to free
>   **/
> @@ -1549,7 +1549,7 @@ i40e_free_dma_mem_d(__attribute__((unused))
> struct i40e_hw *hw,  }
> 
>  /**
> - * i40e_allocate_virt_mem_d - specific memory alloc for shared code
> + * i40e_allocate_virt_mem_d - specific memory alloc for shared code
> + (base driver)
>   * @hw:   pointer to the HW structure
>   * @mem:  pointer to mem struct to fill out
>   * @size: size of memory requested
> @@ -1572,7 +1572,7 @@
> i40e_allocate_virt_mem_d(__attribute__((unused)) struct i40e_hw *hw,  }
> 
>  /**
> - * i40e_free_virt_mem_d - specific memory free for shared code
> + * i40e_free_virt_mem_d - specific memory free for shared code (base
> + driver)
>   * @hw:   pointer to the HW structure
>   * @mem:  pointer to mem struct to free
>   **/
> --
> 2.0.0


I don't object the name change. But I don't see anything good. As some of the API indicates (i40e_init_shared_code), we can call it shared code as what we've done in old PMD drivers. 
BTW, won't this patch overwrite previous one that fix GCC 32bits warning?