Re: [dpdk-dev] [PATCH 03/22] net/hns3: register hns3 PMD driver

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 8/23/2019 2:46 PM, Wei Hu (Xavier) wrote:
> This patch registers hns3 PMD driver and adds the definition for log
> interfaces.
> 
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Hao Chen <chenhao164@huawei.com>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
<...>

> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> new file mode 100644
> index 0000000..0587a9c
> --- /dev/null
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -0,0 +1,141 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018-2019 Hisilicon Limited.
> + */
> +
> +#include <errno.h>
> +#include <stdarg.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <sys/queue.h>
> +#include <inttypes.h>
> +#include <unistd.h>
> +#include <arpa/inet.h>
> +#include <rte_alarm.h>
> +#include <rte_atomic.h>
> +#include <rte_bus_pci.h>
> +#include <rte_byteorder.h>
> +#include <rte_common.h>
> +#include <rte_cycles.h>
> +#include <rte_debug.h>
> +#include <rte_dev.h>
> +#include <rte_eal.h>
> +#include <rte_ether.h>
> +#include <rte_ethdev_driver.h>
> +#include <rte_ethdev_pci.h>
> +#include <rte_interrupts.h>
> +#include <rte_io.h>
> +#include <rte_log.h>
> +#include <rte_pci.h>

Are all these headers really used at this stage? Can you please clean them and
add later patches when they are required?

<...>

> +static int
> +hns3_dev_init(struct rte_eth_dev *eth_dev)
> +{
> +	struct rte_device *dev = eth_dev->device;
> +	struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev);
> +	struct hns3_adapter *hns = eth_dev->data->dev_private;
> +	struct hns3_hw *hw = &hns->hw;
> +	uint16_t device_id = pci_dev->id.device_id;
> +	int ret;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return 0;
> +
> +	eth_dev->dev_ops = &hns3_eth_dev_ops;
> +	rte_eth_copy_pci_info(eth_dev, pci_dev);

I think no need to call 'rte_eth_copy_pci_info()', it is called by
'rte_eth_dev_pci_generic_probe()' before 'hns3_dev_init()' called.

> +
> +	hns->is_vf = false;

There is a separate VF driver, is this field still needed?

> +	hw->data = eth_dev->data;
> +	hw->adapter_state = HNS3_NIC_INITIALIZED;
> +
> +	return 0;

Init should set 'RTE_ETH_DEV_CLOSE_REMOVE' flag, and '.dev_close' should free
the driver allocated resources, which there is not up until this patch:

 +eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;