[dpdk-dev,18/18] Change mk/rte.app.mk to add fm10k lib into link

Message ID 1422594454-11045-19-git-send-email-jing.d.chen@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Chen, Jing D Jan. 30, 2015, 5:07 a.m. UTC
  From: Jeff Shaw <jeffrey.b.shaw@intel.com>

Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
 mk/rte.app.mk |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
  

Comments

Neil Horman Feb. 1, 2015, 12:50 a.m. UTC | #1
On Fri, Jan 30, 2015 at 01:07:34PM +0800, Chen Jing D(Mark) wrote:
> From: Jeff Shaw <jeffrey.b.shaw@intel.com>
> 
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---
>  mk/rte.app.mk |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 4294d9a..87d8763 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -211,6 +211,10 @@ ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
>  LDLIBS += -lrte_pmd_i40e
>  endif
>  
> +ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
> +LDLIBS += -lrte_pmd_fm10k
> +endif
> +
>  ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
>  LDLIBS += -lrte_pmd_ixgbe
>  endif
> -- 
> 1.7.7.6
> 
> 
This patch should be merged with patch 17, and patch 2, and placed at the end of
your series to avoid a FTBFS issue
Neil
  
Chen, Jing D Feb. 2, 2015, 8:10 a.m. UTC | #2
Hi Neil,

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Sunday, February 01, 2015 8:51 AM
> To: Chen, Jing D
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 18/18] Change mk/rte.app.mk to add fm10k
> lib into link
> 
> On Fri, Jan 30, 2015 at 01:07:34PM +0800, Chen Jing D(Mark) wrote:
> > From: Jeff Shaw <jeffrey.b.shaw@intel.com>
> >
> > Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> > Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> > ---
> >  mk/rte.app.mk |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> > index 4294d9a..87d8763 100644
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -211,6 +211,10 @@ ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
> >  LDLIBS += -lrte_pmd_i40e
> >  endif
> >
> > +ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
> > +LDLIBS += -lrte_pmd_fm10k
> > +endif
> > +
> >  ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
> >  LDLIBS += -lrte_pmd_ixgbe
> >  endif
> > --
> > 1.7.7.6
> >
> >
> This patch should be merged with patch 17, and patch 2, and placed at the
> end of
> your series to avoid a FTBFS issue

My rationale is to make every single patch not to break the compile. So, I'd like to
add the binary library into compile and link in last 2 patches, after all the actual code
are patched.  For Patch 2, I think you are right, maybe a better way is to move it as 
patch "16". 

But I'm not sure whether I should merge these 3 together. You know, somebody may
not happy to see the changes in different directory to appear in single patch.
 
> Neil
  
Thomas Monjalon Feb. 2, 2015, 8:39 a.m. UTC | #3
2015-02-02 08:10, Chen, Jing D:
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > This patch should be merged with patch 17, and patch 2, and placed at the
> > end of
> > your series to avoid a FTBFS issue
> 
> My rationale is to make every single patch not to break the compile. So, I'd like to
> add the binary library into compile and link in last 2 patches, after all the actual code
> are patched.  For Patch 2, I think you are right, maybe a better way is to move it as 
> patch "16". 
> 
> But I'm not sure whether I should merge these 3 together. You know, somebody may
> not happy to see the changes in different directory to appear in single patch.

No, I think you are wrong. You can have modifications in different directories
in a patch. A patch must be atomic (one addition/change/feature at a time) and
buildable. In your case, it would be best to have a patch allowing compilation
of fm10k after the patch for "register". So it would mean we are able to test
the minimal driver and each feature added after.

Thanks
  
Chen, Jing D Feb. 2, 2015, 8:59 a.m. UTC | #4
Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, February 02, 2015 4:39 PM
> To: Chen, Jing D
> Cc: dev@dpdk.org; Neil Horman
> Subject: Re: [dpdk-dev] [PATCH 18/18] Change mk/rte.app.mk to add fm10k
> lib into link
> 
> 2015-02-02 08:10, Chen, Jing D:
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > This patch should be merged with patch 17, and patch 2, and placed at the
> > > end of
> > > your series to avoid a FTBFS issue
> >
> > My rationale is to make every single patch not to break the compile. So, I'd
> like to
> > add the binary library into compile and link in last 2 patches, after all the
> actual code
> > are patched.  For Patch 2, I think you are right, maybe a better way is to
> move it as
> > patch "16".
> >
> > But I'm not sure whether I should merge these 3 together. You know,
> somebody may
> > not happy to see the changes in different directory to appear in single
> patch.
> 
> No, I think you are wrong. You can have modifications in different directories
> in a patch. A patch must be atomic (one addition/change/feature at a time)
> and
> buildable. In your case, it would be best to have a patch allowing compilation
> of fm10k after the patch for "register". So it would mean we are able to test
> the minimal driver and each feature added after.

OK, got you. I'll change patch accordingly. 

> 
> Thanks
> --
> Thomas

Best Regards,
Mark
  
Neil Horman Feb. 2, 2015, 1:46 p.m. UTC | #5
On Mon, Feb 02, 2015 at 08:10:17AM +0000, Chen, Jing D wrote:
> Hi Neil,
> 
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Sunday, February 01, 2015 8:51 AM
> > To: Chen, Jing D
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 18/18] Change mk/rte.app.mk to add fm10k
> > lib into link
> > 
> > On Fri, Jan 30, 2015 at 01:07:34PM +0800, Chen Jing D(Mark) wrote:
> > > From: Jeff Shaw <jeffrey.b.shaw@intel.com>
> > >
> > > Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> > > Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> > > ---
> > >  mk/rte.app.mk |    4 ++++
> > >  1 files changed, 4 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> > > index 4294d9a..87d8763 100644
> > > --- a/mk/rte.app.mk
> > > +++ b/mk/rte.app.mk
> > > @@ -211,6 +211,10 @@ ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
> > >  LDLIBS += -lrte_pmd_i40e
> > >  endif
> > >
> > > +ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
> > > +LDLIBS += -lrte_pmd_fm10k
> > > +endif
> > > +
> > >  ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
> > >  LDLIBS += -lrte_pmd_ixgbe
> > >  endif
> > > --
> > > 1.7.7.6
> > >
> > >
> > This patch should be merged with patch 17, and patch 2, and placed at the
> > end of
> > your series to avoid a FTBFS issue
> 
> My rationale is to make every single patch not to break the compile. So, I'd like to
> add the binary library into compile and link in last 2 patches, after all the actual code
> are patched.  For Patch 2, I think you are right, maybe a better way is to move it as 
> patch "16". 
> 
> But I'm not sure whether I should merge these 3 together. You know, somebody may
> not happy to see the changes in different directory to appear in single patch.
>  
As Thomas notes, its fine to change multiple directories, its the functionality
add and ability to compile that needs to be atomic to a commit.
Neil

> > Neil
> 
>
  

Patch

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 4294d9a..87d8763 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -211,6 +211,10 @@  ifeq ($(CONFIG_RTE_LIBRTE_I40E_PMD),y)
 LDLIBS += -lrte_pmd_i40e
 endif
 
+ifeq ($(CONFIG_RTE_LIBRTE_FM10K_PMD),y)
+LDLIBS += -lrte_pmd_fm10k
+endif
+
 ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_PMD),y)
 LDLIBS += -lrte_pmd_ixgbe
 endif