Hi Jingjing,

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, May 10, 2018 16:44
> To: Xu, Rosen <[email protected]>; [email protected]; [email protected]
> Cc: Xu, Rosen <[email protected]>; Zhang, Roy Fan
> <[email protected]>; Doherty, Declan <[email protected]>;
> Richardson, Bruce <[email protected]>; [email protected];
> Yigit, Ferruh <[email protected]>; Ananyev, Konstantin
> <[email protected]>; Zhang, Tianfei <[email protected]>;
> Liu, Song <[email protected]>; Wu, Hao <[email protected]>;
> [email protected]
> 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.

> > +                   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;
> > +   }

Reply via email to