From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [PATCH v6 2/8] qede: Add base driver
Date: Tue, 26 Apr 2016 14:01:40 +0100
Message-ID: <20160426130140.GA6992@bricha3-MOBL3>
References: <1461647586-22234-1-git-send-email-rasesh.mody@qlogic.com>
 <1461647586-22234-2-git-send-email-rasesh.mody@qlogic.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: thomas.monjalon@6wind.com, dev@dpdk.org, ameen.rahman@qlogic.com,
 Harish Patil <harish.patil@qlogic.com>,
 Sony Chacko <sony.chacko@qlogic.com>
To: Rasesh Mody <rasesh.mody@qlogic.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id 775813208
 for <dev@dpdk.org>; Tue, 26 Apr 2016 15:02:03 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <1461647586-22234-2-git-send-email-rasesh.mody@qlogic.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Apr 25, 2016 at 10:13:00PM -0700, Rasesh Mody wrote:
> The base driver is the backend module for the QLogic FastLinQ QL4xxxx
> 25G/40G CNA family of adapters as well as their virtual functions (VF)
> in SR-IOV context.
> 
> The purpose of the base module is to:
>  - provide all the common code that will be shared between the various
>    drivers that would be used with said line of products. Flows such as
>    chip initialization and de-initialization fall under this category.
>  - abstract the protocol-specific HW & FW components, allowing the
>    protocol drivers to have clean APIs, which are detached in its
>    slowpath configuration from the actual Hardware Software Interface(HSI).
> 
> This patch adds a base module without any protocol-specific bits.
> I.e., this adds a basic implementation that almost entirely falls under
> the first category.
> 
> Signed-off-by: Harish Patil <harish.patil@qlogic.com>
> Signed-off-by: Rasesh Mody <rasesh.mody@qlogic.com>
> Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
> ---

<snip>

> +#
> +# CLANG VERSION
> +#
> +IS_CLANG_GT_362 := $(shell \
> +			CLANG_MAJOR=`echo | clang -dM -E - 2>/dev/null | grep clang_major | cut -d" " -f3`; \
> +			CLANG_MINOR=`echo | clang -dM -E - 2>/dev/null | grep clang_minor | cut -d" " -f3`; \
> +			CLANG_PATCH=`echo | clang -dM -E - 2>/dev/null | grep clang_patch | cut -d" " -f3`; \
> +			if [ "0$$CLANG_MAJOR" -gt "03" ]; then \
> +				echo 1; \
> +			elif [ "0$$CLANG_MAJOR" -eq "03" -a "0$$CLANG_MINOR" -gt "06" ]; then \
> +				echo 1; \
> +			elif [ "0$$CLANG_MAJOR" -eq "03" -a "0$$CLANG_MINOR" -eq "06" -a "0$$CLANG_PATCH" -gt "02" ]; then \
> +				echo 1; \
> +			fi)
> +

While the clang version seems something that might be generally useful, this
seems a long way of doing things just to see what compiler warning flag you need
to set. How about just testing with clang to see if you get an error with the
new flag or not.

For example, on Fedora 23 (clang 3.7):

  bruce@Fedora:dpdk-next-net$ clang -Wno-shift-negative-value -Werror -E - < /dev/null > /dev/null 2>&1
  bruce@Fedora:dpdk-next-net$ echo $?
  0

While the same commands on FreeBSD 10.3 (clang 3.4):

  bruce@bsd10:~$ clang -Wno-shift-negative-value -Werror -E - < /dev/null > /dev/null 2>&1
  bruce@bsd10:~$ echo $?
  1

> +#
> +# CFLAGS
> +#
> +CFLAGS_BASE_DRIVER = -Wno-unused-parameter
> +CFLAGS_BASE_DRIVER += -Wno-unused-value
> +CFLAGS_BASE_DRIVER += -Wno-sign-compare
> +CFLAGS_BASE_DRIVER += -Wno-missing-prototypes
> +CFLAGS_BASE_DRIVER += -Wno-cast-qual
> +CFLAGS_BASE_DRIVER += -Wno-unused-function
> +CFLAGS_BASE_DRIVER += -Wno-unused-variable
> +CFLAGS_BASE_DRIVER += -Wno-strict-aliasing
> +CFLAGS_BASE_DRIVER += -Wno-missing-prototypes
> +CFLAGS_BASE_DRIVER += -Wno-format-nonliteral
> +ifeq ($(OS_TYPE),Linux)
> +ifeq ($(IS_CLANG_GT_362),1)
> +CFLAGS_BASE_DRIVER += -Wno-shift-negative-value # Support added after clang 3.6
> +else
> +CFLAGS_BASE_DRIVER += -Wno-shift-sign-overflow
> +endif
> +endif
> +

<snip>