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

[Wu, Jingjing]

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