On 3/20/2020 2:46 AM, alvinx.zh...@intel.com wrote:
> From: Alvin Zhang <alvinx.zh...@intel.com>
>
> Implement device detection and loading.
> Add igc driver guid docs.
>
> Signed-off-by: Alvin Zhang <alvinx.zh...@intel.com>
>
> v2: Update release note. Modify codes according to comments
<...>
> @@ -0,0 +1,39 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> + Copyright(c) 2016 Intel Corporation.
Is the copyright date '2016' correct? If so you can update it as 2016-2020. This
comment is for all files.
> +
> +IGC Poll Mode Driver
> +======================
> +
> +The IGC PMD (librte_pmd_igc) provides poll mode driver support for
> +Foxville I225 Series Network Adapters.
Can you please provide some official links to the product? As much as possible
information about device is good.
<...>
> @@ -56,11 +56,16 @@ New Features
> Also, make sure to start the actual text at the margin.
> =========================================================
>
> -* **Updated Mellanox mlx5 driver.**
> + * **Updated Mellanox mlx5 driver.**
>
> - Updated Mellanox mlx5 driver with new features and improvements, including:
> + Updated Mellanox mlx5 driver with new features and improvements,
> including:
>
> - * Added support for matching on IPv4 Time To Live and IPv6 Hop Limit.
> + * Added support for matching on IPv4 Time To Live and IPv6 Hop Limit.
Above looks changed by mistake...
> +
> + * **Added a new driver for Intel Foxville I225 devices.**
> +
> + Added the new ``igc`` net driver for Intel Foxville I225 devices. See
> the
> + :doc:`../nics/igc` NIC guide for more details on this new driver.
>
>
> Removed Items
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 4a7f155..b57841d 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -61,6 +61,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += thunderx
> DIRS-$(CONFIG_RTE_LIBRTE_VDEV_NETVSC_PMD) += vdev_netvsc
> DIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio
> DIRS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += vmxnet3
> +DIRS-$(CONFIG_RTE_LIBRTE_IGC_PMD) += igc
Can you please add it alphabetically sorted?
<...>
> +static int
> +eth_igc_dev_init(struct rte_eth_dev *dev)
> +{
> + struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +
> + PMD_INIT_FUNC_TRACE();
> + dev->dev_ops = ð_igc_ops;
> +
> + /*
> + * for secondary processes, we don't initialize any further as primary
> + * has already done this work. Only check we don't need a different
> + * RX function.
> + */
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> + return 0;
> +
> + rte_eth_copy_pci_info(dev, pci_dev);
This shouldn't be required, since it is done by
'rte_eth_dev_pci_generic_probe()' just before this funtion
('eth_igc_dev_init()') called.
> +
> + dev->data->mac_addrs = rte_zmalloc("igc",
> + RTE_ETHER_ADDR_LEN, 0);
> + if (dev->data->mac_addrs == NULL) {
> + PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to "
> + "store MAC addresses", RTE_ETHER_ADDR_LEN);
> + return -ENOMEM;
> + }
> +
> + /* Pass the information to the rte_eth_dev_close() that it should also
> + * release the private port resources.
> + */
> + dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
> +
> + PMD_INIT_LOG(DEBUG, "port_id %d vendorID=0x%x deviceID=0x%x",
> + dev->data->port_id, pci_dev->id.vendor_id,
> + pci_dev->id.device_id);
> +
> + return 0;
> +}
> +
> +static int
> +eth_igc_dev_uninit(__rte_unused struct rte_eth_dev *eth_dev)
> +{
> + PMD_INIT_FUNC_TRACE();
> +
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> + return -EPERM;
It shouldn't return error for secondary. 'rte_eth_dev_release_port()' has
already process type in it, so returning '0' should work better which will cause
some process specific variables cleared.
<...>
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation
> + */
> +
> +#include "igc_logs.h"
> +#include "rte_common.h"
'rte_common.h' should be included with '<>', #include <rte_common.h>
> +
> +/* declared as extern in igc_logs.h */
> +int igc_logtype_init = -1;
> +int igc_logtype_driver = -1;
I guess no need to set initial values for these, by default '0' will work fine
for below logic.
<...>
> @@ -0,0 +1,3 @@
> +DPDK_20.0.1 {
This release it become "DPDK_20.0.2", although it doesn't matter for the PMD at
all, good to be consistent.
> + local: *;
> +};
> diff --git a/drivers/net/meson.build b/drivers/net/meson.build
> index b0ea8fe..7d0ae3b 100644
> --- a/drivers/net/meson.build
> +++ b/drivers/net/meson.build
> @@ -49,6 +49,7 @@ drivers = ['af_packet',
> 'vhost',
> 'virtio',
> 'vmxnet3',
> + 'igc',
Can you please add it alphabetically sorted?
