[dpdk-dev] [PATCH 30/56] net/sfc: include libefx in build

[Ferruh Yigit]

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 <Artem.Andreev at oktetlabs.ru>
>>>
>>> Implement efsys.h for the PMD.
>>>
>>> Reviewed-by: Andy Moreton <amoreton at solarflare.com>
>>> Signed-off-by: Artem Andreev <Artem.Andreev at oktetlabs.ru>
>>> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
>>> ---
>>>   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
>> <...>
> 
>