Hi Jingjing,
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, May 10, 2018 17:21
> 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]; Wu, Yanglong <[email protected]>
> Subject: RE: [dpdk-dev] [PATCH v10 3/3] iFPGA: Add Intel FPGA BUS Rawdev
> Driver
>
> Hi, Rosen
>
> Few comments below.
Thanks a lot.
> Thanks
> Jingjing
>
> [...]
>
> > +static int
> > +ifpga_rawdev_start(struct rte_rawdev *dev) {
> > + int ret = 0;
> > + struct opae_adapter *adapter;
> > +
> > + IFPGA_RAWDEV_PMD_FUNC_TRACE();
> > +
> > + RTE_FUNC_PTR_OR_ERR_RET(dev, -EINVAL);
> > +
> > + adapter = ifpga_rawdev_get_priv(dev);
> > + if (!adapter)
> > + return -ENODEV;
> > +
> Set dev->started?
Rawdev lib enable it.
> > + return ret;
> > +}
>
>
> [...]
>
> > +
> > +static const struct rte_rawdev_ops ifpga_rawdev_ops = {
> > + .dev_info_get = ifpga_rawdev_info_get,
> > + .dev_configure = NULL,
> If go the declaration of rte_rawdev_configure, you will see "This function
> must be invoked first before any other function in the API."
> So I think we need to function for it, even it does nothing.
For other Rawdev Drivers, no used function pointer is initialized with NULL.
>
> [...]
>
> > +static struct rte_pci_driver rte_ifpga_rawdev_pmd = {
> > + .id_table = pci_ifpga_map,
> > + .drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> RTE_PCI_DRV_INTR_LSC,
> Is RTE_PCI_DRV_INTR_LSC supported?
>
> [...]
>
> > +static struct rte_vdev_driver ifpga_cfg_driver = {
> > + .probe = ifpga_cfg_probe,
> > + .remove = ifpga_cfg_remove,
> > +};
> > +
> > +RTE_PMD_REGISTER_VDEV(net_ifpga_cfg, ifpga_cfg_driver);
> I think prefix net_ would mean the device is net device (eth_dev)? How about
> to change the prefix to raw_?
I have fixed it both in code and document.
> > +RTE_PMD_REGISTER_ALIAS(net_ifpga_cfg, ifpga_cfg);
> > +RTE_PMD_REGISTER_PARAM_STRING(net_ifpga_cfg,
> > + "bdf=<string> "
> ifpga=<string>?
> > + "port=<int> "
> > + "afu_bts=<path>");
> > +
