[v1,1/2] meson: update meson build for armada drivers

Message ID 20201202130529.7332-2-lironh@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series armada: introduce musdk pkg-config |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Liron Himi Dec. 2, 2020, 1:05 p.m. UTC
  From: Liron Himi <lironh@marvell.com>

With pkg-config support available within musdk library,
meson option 'lib_musdk_dir' can be removed.
PKG_CONFIG_PATH environment variable should be set appropriately
to use the musdk library.

Signed-off-by: Liron Himi <lironh@marvell.com>
Reviewed-by: Liron Himi <lironh@marvell.com>
---
 drivers/common/mvep/meson.build    | 14 +++++---------
 drivers/crypto/mvsam/meson.build   | 15 +++++----------
 drivers/net/mvneta/meson.build     | 19 +++++--------------
 drivers/net/mvneta/mvneta_ethdev.h |  1 +
 drivers/net/mvpp2/meson.build      | 15 +++++----------
 meson_options.txt                  |  2 --
 6 files changed, 21 insertions(+), 45 deletions(-)
  

Comments

Ferruh Yigit Dec. 9, 2020, 10:49 a.m. UTC | #1
On 12/2/2020 1:05 PM, lironh@marvell.com wrote:
> From: Liron Himi <lironh@marvell.com>
> 
> With pkg-config support available within musdk library,
> meson option 'lib_musdk_dir' can be removed.
> PKG_CONFIG_PATH environment variable should be set appropriately
> to use the musdk library.
> 
> Signed-off-by: Liron Himi <lironh@marvell.com>
> Reviewed-by: Liron Himi <lironh@marvell.com>
> ---
>   drivers/common/mvep/meson.build    | 14 +++++---------
>   drivers/crypto/mvsam/meson.build   | 15 +++++----------
>   drivers/net/mvneta/meson.build     | 19 +++++--------------
>   drivers/net/mvneta/mvneta_ethdev.h |  1 +
>   drivers/net/mvpp2/meson.build      | 15 +++++----------
>   meson_options.txt                  |  2 --
>   6 files changed, 21 insertions(+), 45 deletions(-)
> 

Hi Liron,

+1 to this change, but can you please make a new version to include build 
related document changes into this same patch?

Second patch updates document, both for this build related issues and other 
issues like new musdk version and supported features etc.. Instead you can move 
build related changes to this patch, and separate second patch for crypto and 
net, so that it can have proper commit logs for each and can be reviewed easier.

--
Thanks,
ferruh
  
Liron Himi Dec. 13, 2020, 8:06 p.m. UTC | #2
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Wednesday, 9 December 2020 12:50
To: Liron Himi <lironh@marvell.com>; bruce.richardson@intel.com
Cc: dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>
Subject: [EXT] Re: [dpdk-dev] [PATCH v1 1/2] meson: update meson build for armada drivers

External Email

----------------------------------------------------------------------
On 12/2/2020 1:05 PM, lironh@marvell.com wrote:
> From: Liron Himi <lironh@marvell.com>
> 
> With pkg-config support available within musdk library, meson option 
> 'lib_musdk_dir' can be removed.
> PKG_CONFIG_PATH environment variable should be set appropriately to 
> use the musdk library.
> 
> Signed-off-by: Liron Himi <lironh@marvell.com>
> Reviewed-by: Liron Himi <lironh@marvell.com>
> ---
>   drivers/common/mvep/meson.build    | 14 +++++---------
>   drivers/crypto/mvsam/meson.build   | 15 +++++----------
>   drivers/net/mvneta/meson.build     | 19 +++++--------------
>   drivers/net/mvneta/mvneta_ethdev.h |  1 +
>   drivers/net/mvpp2/meson.build      | 15 +++++----------
>   meson_options.txt                  |  2 --
>   6 files changed, 21 insertions(+), 45 deletions(-)
> 

Hi Liron,

+1 to this change, but can you please make a new version to include 
+build
related document changes into this same patch?

Second patch updates document, both for this build related issues and other issues like new musdk version and supported features etc.. Instead you can move build related changes to this patch, and separate second patch for crypto and net, so that it can have proper commit logs for each and can be reviewed easier.

[L.H.] the support for the pkg-config, which meson build relay on, requires newer musdk version. Can I create a platform doc for all Marvell ARMADA platforms? 
--
Thanks,
ferruh
  
Ferruh Yigit Dec. 14, 2020, 9:57 a.m. UTC | #3
On 12/13/2020 8:06 PM, Liron Himi wrote:
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, 9 December 2020 12:50
> To: Liron Himi <lironh@marvell.com>; bruce.richardson@intel.com
> Cc: dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v1 1/2] meson: update meson build for armada drivers
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 12/2/2020 1:05 PM, lironh@marvell.com wrote:
>> From: Liron Himi <lironh@marvell.com>
>>
>> With pkg-config support available within musdk library, meson option
>> 'lib_musdk_dir' can be removed.
>> PKG_CONFIG_PATH environment variable should be set appropriately to
>> use the musdk library.
>>
>> Signed-off-by: Liron Himi <lironh@marvell.com>
>> Reviewed-by: Liron Himi <lironh@marvell.com>
>> ---
>>    drivers/common/mvep/meson.build    | 14 +++++---------
>>    drivers/crypto/mvsam/meson.build   | 15 +++++----------
>>    drivers/net/mvneta/meson.build     | 19 +++++--------------
>>    drivers/net/mvneta/mvneta_ethdev.h |  1 +
>>    drivers/net/mvpp2/meson.build      | 15 +++++----------
>>    meson_options.txt                  |  2 --
>>    6 files changed, 21 insertions(+), 45 deletions(-)
>>
> 
> Hi Liron,
> 
> +1 to this change, but can you please make a new version to include
> +build
> related document changes into this same patch?
> 
> Second patch updates document, both for this build related issues and other issues like new musdk version and supported features etc.. Instead you can move build related changes to this patch, and separate second patch for crypto and net, so that it can have proper commit logs for each and can be reviewed easier.
> 
> [L.H.] the support for the pkg-config, which meson build relay on, requires newer musdk version. Can I create a platform doc for all Marvell ARMADA platforms?

Not sure if a platform doc is required just to document external library dependency.
But if there is more to add to platform document, can you sent it as separate 
patch so it can be reviewed without blocking this set?
  
Liron Himi Dec. 15, 2020, 10:04 a.m. UTC | #4
-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Monday, 14 December 2020 11:57
To: Liron Himi <lironh@marvell.com>; bruce.richardson@intel.com
Cc: dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>
Subject: Re: [EXT] Re: [dpdk-dev] [PATCH v1 1/2] meson: update meson build for armada drivers

On 12/13/2020 8:06 PM, Liron Himi wrote:
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, 9 December 2020 12:50
> To: Liron Himi <lironh@marvell.com>; bruce.richardson@intel.com
> Cc: dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v1 1/2] meson: update meson build 
> for armada drivers
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 12/2/2020 1:05 PM, lironh@marvell.com wrote:
>> From: Liron Himi <lironh@marvell.com>
>>
>> With pkg-config support available within musdk library, meson option 
>> 'lib_musdk_dir' can be removed.
>> PKG_CONFIG_PATH environment variable should be set appropriately to 
>> use the musdk library.
>>
>> Signed-off-by: Liron Himi <lironh@marvell.com>
>> Reviewed-by: Liron Himi <lironh@marvell.com>
>> ---
>>    drivers/common/mvep/meson.build    | 14 +++++---------
>>    drivers/crypto/mvsam/meson.build   | 15 +++++----------
>>    drivers/net/mvneta/meson.build     | 19 +++++--------------
>>    drivers/net/mvneta/mvneta_ethdev.h |  1 +
>>    drivers/net/mvpp2/meson.build      | 15 +++++----------
>>    meson_options.txt                  |  2 --
>>    6 files changed, 21 insertions(+), 45 deletions(-)
>>
> 
> Hi Liron,
> 
> +1 to this change, but can you please make a new version to include 
> +build
> related document changes into this same patch?
> 
> Second patch updates document, both for this build related issues and other issues like new musdk version and supported features etc.. Instead you can move build related changes to this patch, and separate second patch for crypto and net, so that it can have proper commit logs for each and can be reviewed easier.
> 
> [L.H.] the support for the pkg-config, which meson build relay on, requires newer musdk version. Can I create a platform doc for all Marvell ARMADA platforms?

Not sure if a platform doc is required just to document external library dependency.
But if there is more to add to platform document, can you sent it as separate patch so it can be reviewed without blocking this set?

[L.H.] i can split the doc patch and add the meson part to this patch, but I will also need to include the musdk version as only in the new version the pkg-config support is included.
Is that fine?
  
Ferruh Yigit Dec. 15, 2020, 10:29 a.m. UTC | #5
On 12/15/2020 10:04 AM, Liron Himi wrote:
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, 14 December 2020 11:57
> To: Liron Himi <lironh@marvell.com>; bruce.richardson@intel.com
> Cc: dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: Re: [EXT] Re: [dpdk-dev] [PATCH v1 1/2] meson: update meson build for armada drivers
> 
> On 12/13/2020 8:06 PM, Liron Himi wrote:
>>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, 9 December 2020 12:50
>> To: Liron Himi <lironh@marvell.com>; bruce.richardson@intel.com
>> Cc: dev@dpdk.org; Akhil Goyal <akhil.goyal@nxp.com>
>> Subject: [EXT] Re: [dpdk-dev] [PATCH v1 1/2] meson: update meson build
>> for armada drivers
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> On 12/2/2020 1:05 PM, lironh@marvell.com wrote:
>>> From: Liron Himi <lironh@marvell.com>
>>>
>>> With pkg-config support available within musdk library, meson option
>>> 'lib_musdk_dir' can be removed.
>>> PKG_CONFIG_PATH environment variable should be set appropriately to
>>> use the musdk library.
>>>
>>> Signed-off-by: Liron Himi <lironh@marvell.com>
>>> Reviewed-by: Liron Himi <lironh@marvell.com>
>>> ---
>>>     drivers/common/mvep/meson.build    | 14 +++++---------
>>>     drivers/crypto/mvsam/meson.build   | 15 +++++----------
>>>     drivers/net/mvneta/meson.build     | 19 +++++--------------
>>>     drivers/net/mvneta/mvneta_ethdev.h |  1 +
>>>     drivers/net/mvpp2/meson.build      | 15 +++++----------
>>>     meson_options.txt                  |  2 --
>>>     6 files changed, 21 insertions(+), 45 deletions(-)
>>>
>>
>> Hi Liron,
>>
>> +1 to this change, but can you please make a new version to include
>> +build
>> related document changes into this same patch?
>>
>> Second patch updates document, both for this build related issues and other issues like new musdk version and supported features etc.. Instead you can move build related changes to this patch, and separate second patch for crypto and net, so that it can have proper commit logs for each and can be reviewed easier.
>>
>> [L.H.] the support for the pkg-config, which meson build relay on, requires newer musdk version. Can I create a platform doc for all Marvell ARMADA platforms?
> 
> Not sure if a platform doc is required just to document external library dependency.
> But if there is more to add to platform document, can you sent it as separate patch so it can be reviewed without blocking this set?
> 
> [L.H.] i can split the doc patch and add the meson part to this patch, but I will also need to include the musdk version as only in the new version the pkg-config support is included.
> Is that fine?
> 

Got it, that is fine but can you please highlight this in the commit log? (That 
only new specific version of the musdk supports pkg-config)
  

Patch

diff --git a/drivers/common/mvep/meson.build b/drivers/common/mvep/meson.build
index 8df4bc6e0..863a20ab9 100644
--- a/drivers/common/mvep/meson.build
+++ b/drivers/common/mvep/meson.build
@@ -3,18 +3,14 @@ 
 # Copyright(c) 2018 Semihalf.
 # All rights reserved.
 #
-path = get_option('lib_musdk_dir')
-lib_dir = path + '/lib'
-inc_dir = path + '/include'
 
-lib = cc.find_library('libmusdk', dirs: [lib_dir], required: false)
-if not lib.found()
+dep = dependency('libmusdk', required: false)
+if not dep.found()
 	build = false
 	reason = 'missing dependency, "libmusdk"'
-else
-	ext_deps += lib
-	includes += include_directories(inc_dir)
-	cflags += ['-DMVCONF_TYPES_PUBLIC', '-DMVCONF_DMA_PHYS_ADDR_T_PUBLIC']
+	subdir_done()
 endif
 
+ext_deps += dep
+
 sources = files('mvep_common.c')
diff --git a/drivers/crypto/mvsam/meson.build b/drivers/crypto/mvsam/meson.build
index 6d97dc8a2..384eacff0 100644
--- a/drivers/crypto/mvsam/meson.build
+++ b/drivers/crypto/mvsam/meson.build
@@ -3,20 +3,15 @@ 
 # Copyright(c) 2018 Semihalf.
 # All rights reserved.
 
-path = get_option('lib_musdk_dir')
-lib_dir = path + '/lib'
-inc_dir = path + '/include'
-
-lib = cc.find_library('libmusdk', dirs: [lib_dir], required: false)
-if not lib.found()
+dep = dependency('libmusdk', required: false)
+if not dep.found()
 	build = false
 	reason = 'missing dependency, "libmusdk"'
-else
-	ext_deps += lib
-	includes += include_directories(inc_dir)
-	cflags += ['-DMVCONF_TYPES_PUBLIC', '-DMVCONF_DMA_PHYS_ADDR_T_PUBLIC']
+	subdir_done()
 endif
 
+ext_deps += dep
+
 sources = files('rte_mrvl_pmd.c', 'rte_mrvl_pmd_ops.c')
 
 deps += ['bus_vdev', 'common_mvep']
diff --git a/drivers/net/mvneta/meson.build b/drivers/net/mvneta/meson.build
index 8d7202788..c887ddc10 100644
--- a/drivers/net/mvneta/meson.build
+++ b/drivers/net/mvneta/meson.build
@@ -3,24 +3,15 @@ 
 # Copyright(c) 2018 Semihalf.
 # All rights reserved.
 
-path = get_option('lib_musdk_dir')
-lib_dir = path + '/lib'
-inc_dir = path + '/include'
-
-lib = cc.find_library('libmusdk', dirs : [lib_dir], required: false)
-if not lib.found()
+dep = dependency('libmusdk', required: false)
+if not dep.found()
 	build = false
 	reason = 'missing dependency, "libmusdk"'
-else
-	ext_deps += lib
-	includes += include_directories(inc_dir)
-	cflags += [
-	  '-DMVCONF_TYPES_PUBLIC',
-	  '-DMVCONF_DMA_PHYS_ADDR_T_PUBLIC',
-	  '-DMVCONF_DMA_PHYS_ADDR_T_SIZE=64'
-	]
+	subdir_done()
 endif
 
+ext_deps += dep
+
 sources = files(
 	'mvneta_ethdev.c',
 	'mvneta_rxtx.c'
diff --git a/drivers/net/mvneta/mvneta_ethdev.h b/drivers/net/mvneta/mvneta_ethdev.h
index ef8067790..e090abc25 100644
--- a/drivers/net/mvneta/mvneta_ethdev.h
+++ b/drivers/net/mvneta/mvneta_ethdev.h
@@ -21,6 +21,7 @@ 
 #undef container_of
 #endif
 
+#include <env/mv_autogen_comp_flags.h>
 #include <drivers/mv_neta.h>
 #include <drivers/mv_neta_ppio.h>
 
diff --git a/drivers/net/mvpp2/meson.build b/drivers/net/mvpp2/meson.build
index e06eddaac..3015a5559 100644
--- a/drivers/net/mvpp2/meson.build
+++ b/drivers/net/mvpp2/meson.build
@@ -3,20 +3,15 @@ 
 # Copyright(c) 2018 Semihalf.
 # All rights reserved.
 
-path = get_option('lib_musdk_dir')
-lib_dir = path + '/lib'
-inc_dir = path + '/include'
-
-lib = cc.find_library('libmusdk', dirs : [lib_dir], required: false)
-if not lib.found()
+dep = dependency('libmusdk', required: false)
+if not dep.found()
 	build = false
 	reason = 'missing dependency, "libmusdk"'
-else
-	ext_deps += lib
-	includes += include_directories(inc_dir)
-	cflags += ['-DMVCONF_TYPES_PUBLIC', '-DMVCONF_DMA_PHYS_ADDR_T_PUBLIC']
+	subdir_done()
 endif
 
+ext_deps += dep
+
 sources = files(
 	'mrvl_ethdev.c',
 	'mrvl_flow.c',
diff --git a/meson_options.txt b/meson_options.txt
index 9bf18ab6b..63245b95d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -20,8 +20,6 @@  option('include_subdir_arch', type: 'string', value: '',
 	description: 'subdirectory where to install arch-dependent headers')
 option('kernel_dir', type: 'string', value: '',
 	description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
-option('lib_musdk_dir', type: 'string', value: '',
-	description: 'path to the MUSDK library installation directory')
 option('machine', type: 'string', value: 'native',
 	description: 'set the target machine type')
 option('max_ethports', type: 'integer', value: 32,