From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ferruh.yigit@intel.com>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by dpdk.org (Postfix) with ESMTP id 25A61567C
 for <dev@dpdk.org>; Tue, 24 Jan 2017 11:49:16 +0100 (CET)
Received: from orsmga005.jf.intel.com ([10.7.209.41])
 by orsmga102.jf.intel.com with ESMTP; 24 Jan 2017 02:49:15 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.33,278,1477983600"; d="scan'208";a="56572254"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38])
 ([10.237.220.38])
 by orsmga005.jf.intel.com with ESMTP; 24 Jan 2017 02:49:13 -0800
To: Shreyansh Jain <shreyansh.jain@nxp.com>
References: <1484832240-2048-1-git-send-email-hemant.agrawal@nxp.com>
 <1485172803-17288-1-git-send-email-hemant.agrawal@nxp.com>
 <1485172803-17288-17-git-send-email-hemant.agrawal@nxp.com>
 <7c521c9c-e868-81c1-39ec-ae26db8fbf69@intel.com>
 <33f05835-47aa-39d3-0338-01ab165be6d9@nxp.com>
Cc: Hemant Agrawal <hemant.agrawal@nxp.com>, dev@dpdk.org,
 thomas.monjalon@6wind.com, bruce.richardson@intel.com,
 john.mcnamara@intel.com, jerin.jacob@caviumnetworks.com
From: Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <fff0af67-36a5-c125-d3d5-1eb9c7693e46@intel.com>
Date: Tue, 24 Jan 2017 10:49:12 +0000
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.6.0
MIME-Version: 1.0
In-Reply-To: <33f05835-47aa-39d3-0338-01ab165be6d9@nxp.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [PATCHv6 16/33] drivers/pool/dpaa2: adding hw
	offloaded mempool
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://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: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 24 Jan 2017 10:49:16 -0000

On 1/24/2017 9:12 AM, Shreyansh Jain wrote:
> On Monday 23 January 2017 11:04 PM, Ferruh Yigit wrote:
>> On 1/23/2017 11:59 AM, Hemant Agrawal wrote:
>>> Adding NXP DPAA2 architecture specific mempool support
>>> Each mempool instance is represented by a DPBP object
>>> from the FSL-MC bus.
>>>
>>> This patch also registers a dpaa2 type MEMPOOL OPS
>>>
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>> ---
>> <...>
>>
>>> diff --git a/drivers/common/Makefile b/drivers/common/Makefile
>>> index b52931c..0bb75b5 100644
>>> --- a/drivers/common/Makefile
>>> +++ b/drivers/common/Makefile
>>> @@ -35,7 +35,11 @@ ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_PMD),y)
>>>  CONFIG_RTE_LIBRTE_DPAA2_COMMON = $(CONFIG_RTE_LIBRTE_DPAA2_PMD)
>>>  endif
>>>
>>> -ifeq ($(CONFIG_RTE_LIBRTE_FSLMC_BUS),y)
>>> +ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_POOL),y)
>>> +CONFIG_RTE_LIBRTE_DPAA2_COMMON = $(CONFIG_RTE_LIBRTE_DPAA2_POOL)
>>> +endif
>>> +
>>> +ifneq ($(CONFIG_RTE_LIBRTE_FSLMC_BUS),y)
>>
>> I guess this is a typo, but this prevents DPAA2_COMMON to be compiled !!
> 
> It should be 'ifeq' rather than 'ifneq'. 

> And it will prevent COMMON
> compilation only if CONFIG_RTE_LIBRTE_FSLMC_BUS=n which is not the case
> right now.

It was the case for me for x86 config, but you are right it is not the
default case for arm.

> 
> We will fix it.
> 
>>
>>>  CONFIG_RTE_LIBRTE_DPAA2_COMMON = $(CONFIG_RTE_LIBRTE_FSLMC_BUS)
>>>  endif
>>>
>>
>> <...>
>>> +# library dependencies
>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_eal
>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_mempool
>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_common_dpaa2_qbman
>>
>> This dependeny doesn not looks correct, there is no folder like that.
> 
> This is something even I need to understand. From the DEPDIRS what I
> understood was that though it refers to a directory, it essentially
> links libraries in build/lib/*.
> 
> Further, somehow the development is deploying drivers/bus,
> drivers/common and drivers/pool in lib/* under the name specified as
> LIB in Makefile. My understanding was that it is expected behavior and
> not special because of drivers folder.
> 
> Thus, above line only links lib/librte_common_dpaa2_qbman generated by
> drivers/common/dpaa2/qbman code.
> 
> In fact, I think, this might also one of the issues why a parallel 
> shared build fails for DPAA2 PMD (added in Cover letter).
> The dependency graph cannot create a graph for drivers/common
> as dependency for drivers/net or drivers/bus and hence parallel build
> fails because of missing libraries which are being parallely compiled.

DEPDIRS-y is mainly to resolve dependencies for compilation order, and
should point to the folder,

Following line will cause "librte_eal" to be compiled before driver:
DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_eal

So "lib/librte_common_dpaa2_qbman" does not makes more sense, since
there is no folder like that.


Somewhere in the history, with following commit, DEPDIRS-y gained a side
effect, it has been used to set dynamic linking dependencies, to fix
underlinking issue:
 bf5a46fa5972 ("mk: generate internal library dependencies")

I guess you are having that line to benefit from this side effect, but
this can be done with following more properly:
LDLIBS += lib/librte_common_dpaa2_qbman


To resolve the drivers/net to drivers/common dependency, following line
in this Makefile should work:
DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD) += drivers/common/dpaa2

This adds following, which will cause "drivers/common" compiled before
any "drivers/net":
LOCAL_DEPDIRS-drivers/net += drivers/common

> 
>>
>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_bus_fslmc
>>> +
>>> +include $(RTE_SDK)/mk/rte.lib.mk
>>
>> <...>
>>
>