From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 77DF45583 for ; Fri, 25 Nov 2016 11:24:22 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP; 25 Nov 2016 02:24:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,546,1473145200"; d="scan'208";a="1064028205" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.57]) ([10.237.220.57]) by orsmga001.jf.intel.com with ESMTP; 25 Nov 2016 02:24:20 -0800 To: Andrew Rybchenko , dev@dpdk.org References: <1479740470-6723-1-git-send-email-arybchenko@solarflare.com> <1479740470-6723-31-git-send-email-arybchenko@solarflare.com> <18cbe35d-ee10-961b-f6a9-abfb0232c974@intel.com> <0cc9d168-a15f-abb7-303f-8c22132227ed@solarflare.com> Cc: Artem Andreev From: Ferruh Yigit Message-ID: <54c39a0a-0092-4f3e-bc21-e4a0bf8d36c8@intel.com> Date: Fri, 25 Nov 2016 10:24:19 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.0 MIME-Version: 1.0 In-Reply-To: <0cc9d168-a15f-abb7-303f-8c22132227ed@solarflare.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 30/56] net/sfc: include libefx in build X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Nov 2016 10:24:23 -0000 On 11/24/2016 3:44 PM, Andrew Rybchenko wrote: > See one question below. > > On 11/23/2016 06:26 PM, Ferruh Yigit wrote: >> On 11/21/2016 3:00 PM, Andrew Rybchenko wrote: >>> From: Artem Andreev >>> >>> Implement efsys.h for the PMD. >>> >>> Reviewed-by: Andy Moreton >>> Signed-off-by: Artem Andreev >>> Signed-off-by: Andrew Rybchenko >>> --- >>> drivers/net/sfc/efx/Makefile | 54 +++ >>> drivers/net/sfc/efx/efsys.h | 767 +++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 821 insertions(+) >>> create mode 100644 drivers/net/sfc/efx/efsys.h >>> >>> diff --git a/drivers/net/sfc/efx/Makefile b/drivers/net/sfc/efx/Makefile >>> index 71f07ca..de95ea8 100644 >>> --- a/drivers/net/sfc/efx/Makefile >>> +++ b/drivers/net/sfc/efx/Makefile >>> @@ -33,6 +33,8 @@ include $(RTE_SDK)/mk/rte.vars.mk >>> # >>> LIB = librte_pmd_sfc_efx.a >>> >>> +CFLAGS += -I$(SRCDIR)/base/ >>> +CFLAGS += -I$(SRCDIR) >>> CFLAGS += -O3 >>> >>> # Enable basic warnings but disable some which are accepted >>> @@ -60,6 +62,17 @@ CFLAGS += -Wstrict-prototypes >>> CFLAGS += -Wundef >>> CFLAGS += -Wwrite-strings >>> >>> +# Extra CFLAGS for base driver files >>> +CFLAGS_BASE_DRIVER += -Wno-unused-variable >>> +CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable >> clang complain about this one: >> warning: unknown warning option '-Wno-unused-but-set-variable'; did you >> mean '-Wno-unused-const-variable'? [-Wunknown-warning-option] > > Will fix in v2 > >>> + >>> +# >>> +# List of base driver object files for which >>> +# special CFLAGS above should be applied >>> +# >>> +BASE_DRIVER_OBJS=$(patsubst %.c,%.o,$(notdir $(wildcard $(SRCDIR)/base/*.c))) >>> +$(foreach obj, $(BASE_DRIVER_OBJS), $(eval CFLAGS+=$(CFLAGS_BASE_DRIVER))) >> This cause multiple "-Wno-unused-variable -Wno-unused-but-set-variable" >> params in final command, I guess the intention is: >> >> $(foreach obj, $(BASE_DRIVER_OBJS), $(eval >> CFLAGS_$(obj)+=$(CFLAGS_BASE_DRIVER))) >> >> Fixing this may generate a few compiler warnings. > > Many thanks, will fix in v2. > >> <...> >> >>> diff --git a/drivers/net/sfc/efx/efsys.h b/drivers/net/sfc/efx/efsys.h >>> new file mode 100644 >>> index 0000000..2eef996 >>> --- /dev/null >>> +++ b/drivers/net/sfc/efx/efsys.h >>> @@ -0,0 +1,767 @@ >> <...> >> >> I guess below is hardcoded compile time configuration for libefx, do you >> think does it make sense to document this default configuration? > > Yes, it is libefx configuration and more options will be enabled when > corresponding > functionality is supported in the PMD. > I'm sorry, but I don't understand what would you like to see in the > documentation. > Could you clarify, please? This is mostly a question, following defines how libefx behaves, and a little hard to find, do you think does it make sense to document these in nic documentation, guides/nics/sfc_efx.rst, to highlight default configuration. Like by default filtering capabilities and SFN7xxx family support enabled but 5xxx/6xxx family support disabled... These can be listed in a bullet listed way in two groups (default enabled / default disabled) ? > >>> + >>> +#define EFSYS_OPT_NAMES 0 >>> + >>> +#define EFSYS_OPT_SIENA 0 >>> +#define EFSYS_OPT_HUNTINGTON 1 >>> +#define EFSYS_OPT_MEDFORD 1 >>> +#ifdef RTE_LIBRTE_SFC_EFX_DEBUG >>> +#define EFSYS_OPT_CHECK_REG 1 >>> +#else >>> +#define EFSYS_OPT_CHECK_REG 0 >>> +#endif >>> + >>> +#define EFSYS_OPT_MCDI 1 >>> +#define EFSYS_OPT_MCDI_LOGGING 0 >>> +#define EFSYS_OPT_MCDI_PROXY_AUTH 0 >>> + >>> +#define EFSYS_OPT_MAC_STATS 0 >>> + >>> +#define EFSYS_OPT_LOOPBACK 0 >>> + >>> +#define EFSYS_OPT_MON_MCDI 0 >>> +#define EFSYS_OPT_MON_STATS 0 >>> + >>> +#define EFSYS_OPT_PHY_STATS 0 >>> +#define EFSYS_OPT_BIST 0 >>> +#define EFSYS_OPT_PHY_LED_CONTROL 0 >>> +#define EFSYS_OPT_PHY_FLAGS 0 >>> + >>> +#define EFSYS_OPT_VPD 0 >>> +#define EFSYS_OPT_NVRAM 0 >>> +#define EFSYS_OPT_BOOTCFG 0 >>> + >>> +#define EFSYS_OPT_DIAG 0 >>> +#define EFSYS_OPT_RX_SCALE 0 >>> +#define EFSYS_OPT_QSTATS 0 >>> +#define EFSYS_OPT_FILTER 1 >>> +#define EFSYS_OPT_RX_SCATTER 0 >>> + >>> +#define EFSYS_OPT_EV_PREFETCH 0 >>> + >>> +#define EFSYS_OPT_DECODE_INTR_FATAL 0 >>> + >>> +#define EFSYS_OPT_LICENSING 0 >>> + >>> +#define EFSYS_OPT_ALLOW_UNCONFIGURED_NIC 0 >>> + >>> +#define EFSYS_OPT_RX_PACKED_STREAM 0 >> <...> > >