Re: [dpdk-dev] [PATCH v10 1/3] bus/ifpga: Add Intel FPGA BUS Library

["Xu, Rosen" <rosen.xu@intel.com>]

Hi Jingjing,

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, May 11, 2018 6:40
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org; thomas@monjalon.net
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; shreyansh.jain@nxp.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
> Subject: RE: [dpdk-dev] [PATCH v10 1/3] bus/ifpga: Add Intel FPGA BUS
> Library
> 
> 
> 
> > -----Original Message-----
> > From: Xu, Rosen
> > Sent: Thursday, May 10, 2018 8:21 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org;
> > thomas@monjalon.net
> > Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Doherty, Declan
> > <declan.doherty@intel.com>; Richardson, Bruce
> > <bruce.richardson@intel.com>; shreyansh.jain@nxp.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
> > Subject: RE: [dpdk-dev] [PATCH v10 1/3] bus/ifpga: Add Intel FPGA BUS
> > Library
> >
> > Hi Jingjing,
> >
> > > -----Original Message-----
> > > From: Wu, Jingjing
> > > Sent: Thursday, May 10, 2018 16:44
> > > To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org;
> > > thomas@monjalon.net
> > > Cc: Xu, Rosen <rosen.xu@intel.com>; Zhang, Roy Fan
> > > <roy.fan.zhang@intel.com>; Doherty, Declan
> > > <declan.doherty@intel.com>; Richardson, Bruce
> > > <bruce.richardson@intel.com>; shreyansh.jain@nxp.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
> > > Subject: RE: [dpdk-dev] [PATCH v10 1/3] bus/ifpga: Add Intel FPGA
> > > BUS Library
> > >
> > > Hi, Rosen
> > >
> > > Few comments below.
> >
> > Thanks a lot Jingjing.
> >
> > >
> > > Thanks
> > > Jingjing
> > >
> > > [......]
> > > > +static struct rte_ifpga_device *
> > > > +ifpga_find_ifpga_dev(const struct rte_rawdev *rdev) {
> > > > +	struct rte_ifpga_device *ifpga_dev = NULL;
> > > > +
> > > > +	TAILQ_FOREACH(ifpga_dev, &ifpga_device_list, next) {
> > > > +		if (rdev &&
> > > rdev -> ifpage_dev  ??
> >
> > Rdev doesn't has this variable.
> I mean "if (ifpag_dev" & to replace "rdev"

I will fixed it in patch v12.

> >
> > > > +			ifpga_dev->rdev &&
> > > > +			ifpga_dev->rdev == rdev)
> > > > +			return ifpga_dev;
> > > > +	}
> > > > +	return NULL;
> > > > +}
> > > > +
> > > > +static struct rte_afu_device *
> > > > +ifpga_find_afu_dev(const struct rte_ifpga_device *ifpga_dev,
> > > > +	const struct rte_afu_id *afu_id) {
> > > > +	struct rte_afu_device *afu_dev = NULL;
> > > > +
> > > > +	TAILQ_FOREACH(afu_dev, &ifpga_dev->afu_list, next) {
> > > > +		if (!ifpga_afu_id_cmp(&afu_dev->id, afu_id))
> > > Add checking afu_dev?
> >
> > Fixed.
> >
> > > [...]
> > >
> > > > +static int
> > > > +ifpga_parse(const char *name, void *addr) {
> > > > +	int *out = addr;
> > > > +	struct rte_rawdev *rawdev = NULL;
> > > > +	char rawdev_name[RTE_RAWDEV_NAME_MAX_LEN];
> > > > +	char *c1 = NULL, *c2 = NULL;
> > > According to coding style, we need to two lines for the definition like:
> > > char *c1 = NULL;
> > > char *c2 = NULL;
> >
> > Fixed
> >
> > > > +	int port = IFPGA_BUS_DEV_PORT_MAX;
> > > > +	char str_port[8];
> > > > +	int str_port_len = 0;
> > > > +	int ret;
> > > > +
> > > > +	memset(str_port, 0, 8);
> > > > +	c1 = strchr(name, '|');
> > > > +	if (c1 != NULL) {
> > > > +		str_port_len = c1-name;
> > > According to coding style, spaces are required around opreations.
> >
> > Fixed.
> >
> > > > +		c2 = c1+1;
> > > > +	}