<snip> > > I think we need to split this further. Few suggestions below. > > > > > > > > * Rename variables to have names that better describe what the > > > variables store > > This should be a separate commit. > > > > > * Remove unused or superfluous variables > > Same here > > > > > * Change a list to dictionary where key lookup is needed > > Same here > > > > > * Add informatory comments in the code > > > > > * Minor code restructure and reformatting > > Same for this > > > > Ok, hopefully I'll be able to separate these cleanly. > > > > > > > Signed-off-by: Juraj Linkeš <juraj.lin...@pantheon.tech> > > > --- > > > config/arm/arm64_armada_linux_gcc | 2 +- > > > config/arm/arm64_armv8_linux_gcc | 8 +- > > > config/arm/arm64_bluefield_linux_gcc | 4 +- > > > config/arm/arm64_dpaa_linux_gcc | 2 +- > > > config/arm/arm64_emag_linux_gcc | 2 +- > > > config/arm/arm64_n1sdp_linux_gcc | 4 +- > > > config/arm/arm64_octeontx2_linux_gcc | 4 +- > > > config/arm/arm64_stingray_linux_gcc | 4 +- > > > config/arm/arm64_thunderx2_linux_gcc | 4 +- > > > config/arm/arm64_thunderx_linux_gcc | 2 +- > > > config/arm/meson.build | 247 +++++++++++++++------------ > > > 11 files changed, 153 insertions(+), 130 deletions(-) > > > > > > diff --git a/config/arm/arm64_armada_linux_gcc > > > b/config/arm/arm64_armada_linux_gcc > > > index fa40c0398..52c5f4476 100644 > > > --- a/config/arm/arm64_armada_linux_gcc > > > +++ b/config/arm/arm64_armada_linux_gcc > > > @@ -14,4 +14,4 @@ cpu = 'armv8-a' > > > endian = 'little' > > > > > > [properties] > > > -implementor_id = '0x56' > > > +implementer_id = '0x56' > > Implementor and implementer mean the same. Looked at Arm specs, they > > use 'implementer'. So, I am fine. > > > > That's where I got this and some of the other variable name changes from. > > > > diff --git a/config/arm/arm64_armv8_linux_gcc > > > b/config/arm/arm64_armv8_linux_gcc > > > index 88f0ff9da..13ee8b223 100644 > > > --- a/config/arm/arm64_armv8_linux_gcc > > > +++ b/config/arm/arm64_armv8_linux_gcc > > > @@ -13,10 +13,10 @@ cpu = 'armv8-a' > > > endian = 'little' > > > > > > [properties] > > > -implementor_id = 'generic' > > > +implementer_id = 'generic' > > > > > > -# Valid options for Arm's implementor_pn: > > > -# 'default': valid for all armv8-a architectures (default value) > > > +# Valid options for Arm's part_number: > > > +# 'generic': valid for all armv8-a architectures (default value) > > > # '0xd03': cortex-a53 > > > # '0xd04': cortex-a35 > > > # '0xd05': cortex-a55 > > > @@ -25,4 +25,4 @@ implementor_id = 'generic' > > > # '0xd09': cortex-a73 > > > # '0xd0a': cortex-a75 > > > # '0xd0b': cortex-a76 > > > -implementor_pn = 'default' > > > +part_number = 'generic' > > Same here, Arm specs refer to this as 'PartNumber'. So, this should be fine. > > I like 'generic' for part_number here. > > > > > diff --git a/config/arm/arm64_bluefield_linux_gcc > > > b/config/arm/arm64_bluefield_linux_gcc > > > index 86797d23c..b79389d85 100644 > > > --- a/config/arm/arm64_bluefield_linux_gcc > > > +++ b/config/arm/arm64_bluefield_linux_gcc > > > @@ -13,5 +13,5 @@ cpu = 'armv8-a' > > > endian = 'little' > > >
[...] > > > diff --git a/config/arm/meson.build b/config/arm/meson.build index > > > 491842cad..6c31ab167 100644 > > > --- a/config/arm/meson.build > > > +++ b/config/arm/meson.build > > > @@ -3,12 +3,12 @@ > > > # Copyright(c) 2017 Cavium, Inc > > > # Copyright(c) 2020 PANTHEON.tech s.r.o. > > > > > > -# for checking defines we need to use the correct compiler flags > > > -march_opt = '-march=@0@'.format(machine) > > > - [...] > > > > > > +# implementer specific aarch64 flags, with middle priority # (will > > > +overwrite common flags) > > > flags_generic = [ > > > ['RTE_MACHINE', '"armv8a"'], > > > ['RTE_MAX_LCORE', 256], > > > ['RTE_USE_C11_MEM_MODEL', true], > > > - ['RTE_CACHE_LINE_SIZE', 128]] > > > + ['RTE_CACHE_LINE_SIZE', 128] > > > +] > > Any particular reason for this change? (and similar changes below) > > > > The first bracket is split from the second bracket so I did the same for the > last > two brackets. It makes it more apparent which brackets are paired, it's more > consistent and also in line with how flags_common_default is formatted. Ok > [...] > > > flags_dpaa = [ > > > ['RTE_MACHINE', '"dpaa"'], > > > ['RTE_USE_C11_MEM_MODEL', true], > > > ['RTE_CACHE_LINE_SIZE', 64], > > > ['RTE_MAX_NUMA_NODES', 1], > > > ['RTE_MAX_LCORE', 16], > > > - ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]] > > > + ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false] ] > > This is not needed > > > > Do you mean the space? It should be a line break. I'll check the exact I meant the space. I guess it needs to be on the next line. > characters, but I see this as adding a space in my local patch. Or do you mean > the config option? It's set to true by default in config/meson.build and > according to [1] it should be disabled. > > [1] http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-dpaa-linuxapp- > gcc?h=v20.08 > > > > flags_emag = [ > > > ['RTE_MACHINE', '"emag"'], > > > - ['RTE_CACHE_LINE_SIZE', 64], > > > ['RTE_MAX_NUMA_NODES', 1], > > > - ['RTE_MAX_LCORE', 32]] > > > + ['RTE_MAX_LCORE', 32], > > > + ['RTE_CACHE_LINE_SIZE', 64] > > > +] > > > flags_armada = [ > > > ['RTE_MACHINE', '"armv8a"'], > > > - ['RTE_CACHE_LINE_SIZE', 64], > > > ['RTE_MAX_NUMA_NODES', 1], > > > - ['RTE_MAX_LCORE', 16]] > > > + ['RTE_MAX_LCORE', 16], > > > + ['RTE_CACHE_LINE_SIZE', 64] > > > +] > > Any reason for this change? > > > > The default (from flags_common_default) is 128 and I found here [0] that it > should be set to 64 so I added it here. Should this also be in a separate > patch > (apart from those 4 already mention above)? The RTE_CACHE_LINE_SIZE is already set to 64 for 'flags_armada'. It looks like you have moved the line down. > > [0] http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-armada- > linuxapp-gcc?h=v20.08 > > > > > > > -flags_default_extra = [] > > > +# part number specific aarch64 flags, with highest priority # (will > > > +overwrite both common and implementer specific flags) > > > flags_n1sdp_extra = [ > > > ['RTE_MACHINE', '"n1sdp"'], > > > ['RTE_MAX_NUMA_NODES', 1], > > > ['RTE_MAX_LCORE', 4], > > > ['RTE_EAL_NUMA_AWARE_HUGEPAGES', false], > > > - ['RTE_LIBRTE_VHOST_NUMA', false]] > > > + ['RTE_LIBRTE_VHOST_NUMA', false] > > > +] <snip> > > > - > > > -machine_args_emag = [ > > > - ['default', ['-march=armv8-a+crc+crypto', '-mtune=emag']], > > > - ['native', ['-march=native']]] > > > + ['RTE_USE_C11_MEM_MODEL', true] > > > +] > > > +# arm config (implementer 0x41) is the default config > > > +pn_config_default > > 'pn' here for 'part_number' is not consistent. > > > > Ok, I can rename it to part_number_config_default. Same for the other two > pn variables. Ok > <snip> > > > -- > > > 2.20.1
