Re: [dpdk-dev] [PATCH v7 3/5] iFPGA: Add Intel FPGA BUS Rawdev Driver

["Zhang, Tianfei" <tianfei.zhang@intel.com>]

> -----Original Message-----
> From: Shreyansh Jain [mailto:shreyansh.jain@nxp.com]
> Sent: Sunday, May 6, 2018 3:10 AM
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Zhang, Tianfei
> <tianfei.zhang@intel.com>; Liu, Song <song.liu@intel.com>; Wu, Hao
> <hao.wu@intel.com>; gaetan.rivet@6wind.com; Wu, Yanglong
> <yanglong.wu@intel.com>
> Subject: RE: [PATCH v7 3/5] iFPGA: Add Intel FPGA BUS Rawdev Driver
> 
> Though I had already acked this, I had a quick comment:
> 
> > -----Original Message-----
> > From: Xu, Rosen [mailto:rosen.xu@intel.com]
> > Sent: Friday, May 4, 2018 7:41 PM
> > To: dev@dpdk.org
> > Cc: rosen.xu@intel.com; declan.doherty@intel.com;
> > bruce.richardson@intel.com; Shreyansh Jain <shreyansh.jain@nxp.com>;
> > ferruh.yigit@intel.com; konstantin.ananyev@intel.com;
> > tianfei.zhang@intel.com; song.liu@intel.com; hao.wu@intel.com;
> > gaetan.rivet@6wind.com; Yanglong Wu <yanglong.wu@intel.com>
> > Subject: [PATCH v7 3/5] iFPGA: Add Intel FPGA BUS Rawdev Driver
> >
> > From: Rosen Xu <rosen.xu@intel.com>
> >
> > Add Intel FPGA BUS Rawdev Driver which is based on librte_rawdev
> > library.
> >
> > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > Signed-off-by: Yanglong Wu <yanglong.wu@intel.com>
> > Signed-off-by: Figo zhang <tianfei.zhang@intel.com>
> > ---
> 
> [...]
> 
> > diff --git a/mk/rte.app.mk b/mk/rte.app.mk index f47bbe8..b0a994f
> > 100644
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -253,6 +253,7 @@ endif # CONFIG_RTE_LIBRTE_EVENTDEV
> >
> >  ifeq ($(CONFIG_RTE_LIBRTE_RAWDEV),y)
> >  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_RAWDEV) += -
> > lrte_pmd_skeleton_rawdev
> > +_LDLIBS-$(CONFIG_RTE_LIBRTE_IFPGA_RAWDEV)   +=
> -lrte_ifpga_rawdev
> >  endif # CONFIG_RTE_LIBRTE_RAWDEV
> 
> This driver is dependent on CONFIG_RTE_LIBRTE_IFPGA_BUS and if
> someone disables that, it would lead to build issues. I think you should
> enclose the IFPGA_RAWDEV compilation into conditional for
> CONFIG_RTE_LIBRTE_IFPGA_BUS=y.
> 
> Though, I do faintly remember a discussion in past that such thing
> should/can be left to individual environment configuration and it can be
> safely assumed that such erroneous configuration would be
> user-responsibility (at least until a dependency based configuration system is
> available in DPDK). So, it is a good-to-have rather a necessity.

Good point, we will add RAWDEV condition in our Patch 1:

ifeq ($(CONFIG_RTE_LIBRTE_RAWDEV),y)
_LDLIBS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS)      += -lrte_bus_ifpga
endif # CONFIG_RTE_LIBRTE_RAWDEV


and add IFGA_BUS condition checking in Patch 3, like this:

ifeq ($(CONFIG_RTE_LIBRTE_RAWDEV),y)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SKELETON_RAWDEV) += -lrte_pmd_skeleton_rawdevi
_LDLIBS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS)      += -lrte_bus_ifpga
ifeq($(CONFIG_RTE_LIBRTE_IFPGA_BUS),y)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_IFPGA_RAWDEV)   += -lrte_ifpga_rawdev
endif# CONFIG_RTE_LIBRTE_IFPGA_BUS
endif # CONFIG_RTE_LIBRTE_RAWDEV