[dpdk-dev] [PATCH v2 01/16] net/axgbe: add minimal dev init and uninit support

[Kumar, Ravi1]

>On 1/5/2018 9:52 AM, Ravi Kumar wrote:
>> Signed-off-by: Ravi Kumar <Ravi1.kumar at amd.com>
>
>lgtm except below comments.
>
>At least share build error needs a new version of the patch.
>Can you able to complete licensing updates for next version?
>
Hi Ferruh,

Thanks a lot for reviewing our code. Good to know that it is fine other than the license and comment on this mail.

The new changes in licensing policy has introduced some delays as we work through the implications and get agreement from the third party.

We are working on it and will upload the v3 that addresses the comment below and has SPDX license tags as soon as possible. 

Regards,
Ravi


><...>
>
>> @@ -445,6 +445,12 @@ CONFIG_RTE_LIBRTE_AVP_DEBUG_DRIVER=y
>>  CONFIG_RTE_LIBRTE_AVP_DEBUG_BUFFERS=n
>>  
>>  #
>> +# Compile AMD PMD
>> +#
>> +CONFIG_RTE_LIBRTE_AXGBE_PMD=n
>
>Why disabled by default?
>
>> +CONFIG_RTE_LIBRTE_AXGBE_DEBUG_INIT=n
>
>As far as I can see dynamic logging implemented, is this config option still needed? If you will remove this please remember to clean up the documentation too.
>
><...>
>
>> +include $(RTE_SDK)/mk/rte.vars.mk
>> +
>> +#
>> +# library name
>> +#
>> +LIB = librte_pmd_axgbe.a
>> +
>> +CFLAGS += -O3
>> +CFLAGS += $(WERROR_FLAGS)
>> +
>> +EXPORT_MAP := rte_pmd_axgbe_version.map
>> +
>> +LIBABIVER := 1
>> +
>
>Need to add dependent libraries [1] or causing a build error for shared library build [2].
>
>
>[1]
>Something like:
> LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring  LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs  LDLIBS += -lrte_bus_pc
>
>
>[2]
>build error:
>.../drivers/net/axgbe/axgbe_ethdev.c:(.text+0x16): undefined reference to `rte_pci_register'
>
>To enable shared build, update config file:
>CONFIG_RTE_BUILD_SHARED_LIB=y
>
><...>
>