From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v4 01/17] net/axgbe: add minimal dev init and
 uninit support
Date: Thu, 5 Apr 2018 12:34:47 +0100
Message-ID: <ebaec22c-4ff3-0ed3-d570-cfc4675a16ac@intel.com>
References: <1520584954-130575-1-git-send-email-Ravi1.kumar@amd.com>
 <1522910389-35530-1-git-send-email-Ravi1.kumar@amd.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
To: Ravi Kumar <Ravi1.kumar@amd.com>, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id 988481BAF7
 for <dev@dpdk.org>; Thu,  5 Apr 2018 13:34:51 +0200 (CEST)
In-Reply-To: <1522910389-35530-1-git-send-email-Ravi1.kumar@amd.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://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 4/5/2018 7:39 AM, Ravi Kumar wrote:
> add ethernet poll mode driver for AMD 10G devices embedded in
> AMD EPYC™ EMBEDDED 3000 family processors
> 
> Signed-off-by: Ravi Kumar <Ravi1.kumar@amd.com>
<...>

> @@ -410,6 +410,12 @@ CONFIG_RTE_PMD_RING_MAX_TX_RINGS=16
>  CONFIG_RTE_LIBRTE_PMD_SOFTNIC=y
>  
>  #
> +# Compile AMD PMD
> +#
> +CONFIG_RTE_LIBRTE_AXGBE_DEBUG_INIT=n
> +CONFIG_RTE_LIBRTE_AXGBE_PMD=y


Please add alphabetically.

RTE_LIBRTE_AXGBE_DEBUG_INIT is used for data path logs, otherwise it should be
dynamic logging. So the name is wrong for the purpose, it is no more "init"
debug log, you have dynamic log for init already.
And in documentation this has been documented as "Toggle display of
initialization related messages" which seems wrong as well.

<...>

> @@ -12,6 +12,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += af_packet
>  DIRS-$(CONFIG_RTE_LIBRTE_ARK_PMD) += ark
>  DIRS-$(CONFIG_RTE_LIBRTE_AVF_PMD) += avf
>  DIRS-$(CONFIG_RTE_LIBRTE_AVP_PMD) += avp
> +DIRS-$(CONFIG_RTE_LIBRTE_AXGBE_PMD) += axgbe

Please consider adding meson support too. Perhaps with a separate patch to this set.

<...>

> +
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-$(CONFIG_RTE_LIBRTE_AXGBE_PMD) += axgbe_ethdev.c

Shared build causing build error, you need to add dependent libraries [1], and
please test shared library builds:

[1] something like:
 +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
 +LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
 +LDLIBS += -lrte_bus_pci

<...>

> +RTE_PMD_REGISTER_PCI(net_axgbe, rte_axgbe_pmd);
> +RTE_PMD_REGISTER_PCI_TABLE(net_axgbe, pci_id_axgbe_map);
> +RTE_PMD_REGISTER_KMOD_DEP(net_axgbe, "* igb_uio | uio_pci_generic | vfio-pci");

Is vfio-pci supported?

Documentation says:
"
AXGBE PMD works only with legacy interrupts. Load ``igb_uio`` module in legacy
interrupt mode using module params.

Bind the intended AMD device to igb_uio module
"
<...>