patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Di, ChenxuX" <chenxux.di@intel.com>,
	"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>
Cc: "sachin.saxena@nxp.com" <sachin.saxena@nxp.com>,
	"stable@dpdk.org" <stable@dpdk.org>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Richardson, Bruce" <bruce.richardson@intel.com>
Subject: Re: [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync functions
Date: Thu, 17 Sep 2020 12:38:55 +0100
Message-ID: <59ceb607-4ffc-550e-45bf-293b97248ff3@intel.com> (raw)
In-Reply-To: <2ada3c33bf2b4dcf862e6fa4e6a97cbc@intel.com>

On 9/17/2020 3:03 AM, Di, ChenxuX wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, September 16, 2020 11:29 PM
>> To: Di, ChenxuX <chenxux.di@intel.com>; hemant.agrawal@nxp.com
>> Cc: sachin.saxena@nxp.com; stable@dpdk.org; dev@dpdk.org; Richardson,
>> Bruce <bruce.richardson@intel.com>
>> Subject: Re: [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync
>> functions
>>
>> On 9/15/2020 3:40 AM, Chenxu Di wrote:
>>> When the build option has '-DRTE_LIBRTE_IEEE1588=1', the announce of
>>> timesync functions will be build.
>>> However the dpdk_conf doesn't hav RTE_LIBRTE_IEEE1588 so that the file
>>> dpaa2_ptp.c will not be build.
>>> It cause the build error.
>>> This patch fixes it by adding set for dpdk_conf.
>>>
>>> Fixes: 184c39d16568 ("net/dpaa2: add DPRTC sub-module")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Chenxu Di <chenxux.di@intel.com>
>>> ---
>>>    drivers/net/dpaa2/meson.build | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/net/dpaa2/meson.build
>>> b/drivers/net/dpaa2/meson.build index 6dd0eb274..d9aadfdae 100644
>>> --- a/drivers/net/dpaa2/meson.build
>>> +++ b/drivers/net/dpaa2/meson.build
>>> @@ -17,6 +17,10 @@ sources = files('base/dpaa2_hw_dpni.c',
>>>    		'mc/dpdmux.c',
>>>    		'mc/dpni.c')
>>>
>>> +if '-DRTE_LIBRTE_IEEE1588=1' in get_option('c_args')
>>
>> The "RTE_LIBRTE_IEEE1588=1" can fail,
>> all places looking for "#ifdef RTE_LIBRTE_IEEE1588", so a "-Dc_args=-
>> DRTE_LIBRTE_IEEE1588" is more likely, but why not "-Dc_args=-
>> DRTE_LIBRTE_IEEE1588=666"
>>
> 
> Yes, I will change it
> 
>>> +	dpdk_conf.set('RTE_LIBRTE_IEEE1588', 1) endif
>>> +
>>>    if dpdk_conf.has('RTE_LIBRTE_IEEE1588')
>>>    	sources += files('mc/dprtc.c')
>>>    	sources += files('dpaa2_ptp.c')
>>>
>>
>> Can't we just remove the conditional build:
>>
>>    -if dpdk_conf.has('RTE_LIBRTE_IEEE1588')
>>    -       sources += files('mc/dprtc.c')
>>    -       sources += files('dpaa2_ptp.c')
>>    -endif
>>    +sources += files('mc/dprtc.c')
>>    +sources += files('dpaa2_ptp.c')
> 
> The announce of timesync functions are in the #define DRTE_LIBRTE_IEEE1588
> While the define of the functions are in the file 'dpaa2_ptp.c'.
> So they should be both build or not build by whether the build option -DRTE_LIBRTE_IEEE1588=1 or not.
> So it seems not a good idea that remove the conditional.
> 

timesyncs_* dev_ops functions defined but not used is not big problem, 
only can increase the library size.

I believe more concern is on:
'RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj);'
which looks like register function to run in constructor when 
'dpaa2_ptp.c' is compiled, but not sure affect of it.
It can be possible to wrap that call with 'RTE_LIBRTE_IEEE1588' ifdef, 
that should work.

It is preferred to remove the compile time flag instead of finding ways 
to make it work.

Let's wait for the dpaa2 maintainers' response, perhaps they can come 
with a smart way to remove the compile time flag.

  reply	other threads:[~2020-09-17 11:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-15  2:40 Chenxu Di
2020-09-15  3:22 ` Hemant Agrawal
2020-09-16  5:12 ` [dpdk-stable] [dpdk-dev] " Zhou, JunX W
2020-09-16 15:29 ` [dpdk-stable] " Ferruh Yigit
2020-09-17  2:03   ` Di, ChenxuX
2020-09-17 11:38     ` Ferruh Yigit [this message]
2020-09-17 11:50       ` [dpdk-stable] [dpdk-dev] " Hemant Agrawal
2020-09-17 11:59         ` Ferruh Yigit
2020-09-17 15:40           ` Hemant Agrawal
2020-09-30 15:09             ` Ferruh Yigit
2020-09-17  8:43 ` [dpdk-stable] [PATCH v2] " Chenxu Di
2020-09-17 11:48   ` [dpdk-stable] [dpdk-dev] " Hemant Agrawal
2020-10-06 17:16   ` [dpdk-stable] [PATCH v3] " Ferruh Yigit
2020-10-08  2:24     ` Sachin Saxena (OSS)
2020-10-08 13:12       ` Ferruh Yigit
2020-10-09  7:48         ` Sachin Saxena (OSS)
2020-10-09 11:22           ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=59ceb607-4ffc-550e-45bf-293b97248ff3@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=chenxux.di@intel.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=sachin.saxena@nxp.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git