Hi, Rosen

Few comments below.

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?
> +     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.


[...]

> +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_?

> +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>");
> +

Reply via email to