Hi Thomas,

On 4/11/2017 6:26 PM, Olivier MATZ wrote:
On Tue, 11 Apr 2017 14:50:14 +0200
Thomas Monjalon <thomas.monja...@6wind.com> wrote:

2017-04-11 09:39, Ferruh Yigit:
On 4/11/2017 8:50 AM, Thomas Monjalon wrote:
2017-04-11 11:28, Hemant Agrawal:
On 4/11/2017 1:28 AM, Olivier MATZ wrote:
Hemant Agrawal <hemant.agra...@nxp.com> wrote:
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -33,6 +33,10 @@ include $(RTE_SDK)/mk/rte.vars.mk

 core-libs := librte_eal librte_mbuf librte_mempool librte_ring librte_ether

+ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL),y)
+CONFIG_RTE_LIBRTE_FSLMC_BUS = $(CONFIG_RTE_LIBRTE_DPAA2_MEMPOOL)
+endif
+
 DIRS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += fslmc
 DEPDIRS-fslmc = ${core-libs}


What's the purpose of this?
Not sure we are allowed to modify the configs in the Makefiles.

DPAA2_MEMPOOL will not work without the DPAA2 mempool hw instance
detected on FSLMC_BUS.
So, it is required that if you are enabling DPAA2_MEMPOOL, FSLMC_BUS is
to be enabled.

Currently the config structure do not provide such dependency definitions.

This was done based on the suggestions on the initial patches from
Ferruh and Jerin.

Please do not do that.
We do not change the configuration in the back of the user.
This kind of dependency should be managed in the configuration step
which do not exist yet.

You can use $(error) to stop the compilation instead.

As Hemant mentioned, this was my suggestion. There is a configuration
dependency here, that we don't have a way to resolve in dpdk.

If one of the end leaf selected, it makes sense to me to auto select
dependent pieces.

A dependency must be solved at configuration time with appropriate
user notification.
For now, we just check them at compilation time and throw an error.


At present, we have removed the config dependency check from the code and sent the new patch sets.

I am not sure about error part. I will experiment on it and send patches over these patches. This should not block the existing patches.

Yes, a good reason for not doing this is because the "make config"
generates a rte_config.h file. Changing a configuration option at
one place in a Makefile makes configuration inconsistent.

I don't think it's a blocker issue for the patch integration.

Regards,
Olivier




Reply via email to