Hi Jerin,

Thanks for reviewing the series, my comments inline. 

-----Original Message-----
From: Jerin Jacob <redacted>
Sent: Thursday, September 12, 2019 11:49 PM
To: Gavin Hu (Arm Technology China) <redacted>
Cc: dev@dpdk.org; nd <redacted>; thomas@monjalon.net;
stephen@networkplumber.org; hemant.agrawal@nxp.com;
jerinj@marvell.com; Pavan Nikhilesh [off-list ref];
Honnappa Nagarahalli [off-list ref]
Subject: Re: [dpdk-dev] [PATCH v5 1/8] config: add WFE config entry for
aarch64

On Thu, Sep 12, 2019 at 5:05 PM Gavin Hu [off-list ref] wrote:

quoted

Add the RTE_USE_WFE configuration entry for aarch64, disabled by default.

s/RTE_USE_WFE/RTE_ARM_USE_WFE

Will fix in next version.

quoted

It can be enabled selectively based on the performance benchmarking.

Signed-off-by: Gavin Hu <redacted>
Reviewed-by: Ruifeng Wang <redacted>
Reviewed-by: Steve Capper <redacted>
Reviewed-by: Honnappa Nagarahalli <redacted>
Reviewed-by: Phil Yang <redacted>

Does it make sense to add the Reviewed-by without CCing the people?

I will cc all the internal reviewers for the future patches.

I understand, There may be an internal review before sending out to
mailing list, IMO, it better to give Reviewed-By in the mailing list.
Not sure about general practice and other people view.

If there is a guideline here, we will strictly follow it. Currently both ways are ok? 

quoted

Acked-by: Pavan Nikhilesh <redacted>
---
 config/arm/meson.build | 1 +
 config/common_base     | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 979018e..18ecd53 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build

@@ -116,6 +116,7 @@ impl_dpaa = ['NXP DPAA', flags_dpaa,

machine_args_generic]

quoted

 impl_dpaa2 = ['NXP DPAA2', flags_dpaa2, machine_args_generic]

 dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
+dpdk_conf.set('RTE_ARM_USE_WFE', 0)

 if not dpdk_conf.get('RTE_ARCH_64')
        dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)

diff --git a/config/common_base b/config/common_base
index 8ef75c2..d4cf974 100644
--- a/config/common_base
+++ b/config/common_base

@@ -570,6 +570,12 @@ CONFIG_RTE_CRYPTO_MAX_DEVS=64
 CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO=n
 CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO_DEBUG=n

+# Use WFE instructions to implement the rte_wait_for_equal_xxx APIs,
+# calling these APIs put the cores in low power state while waiting
+# for the memory address to become equal to the expected value.
+# This is supported only by aarch64.
+CONFIG_RTE_ARM_USE_WFE=n

Since this comes as EAL config, IMO, it is better to move this to end
of EAL section.
i.e move after CONFIG_RTE_USE_LIBBSD

Will fix it in next version.

And I think, we should squash this patch to  "eal: add the APIs to
wait until equall" as it single logical change.

Will fix it in next version.

quoted

+
 #
 # Compile NXP CAAM JR crypto Driver
 #
--
2.7.4