Re: [dpdk-dev] [PATCH v6 2/8] qede: Add base driver

[Rasesh Mody <rasesh.mody@qlogic.com>]

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Tuesday, April 26, 2016 6:02 AM
> 
> 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.

Will address this, thanks.

> 
> 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>