[dpdk-dev] [PATCH 2/3] net/enetc: add ENETC PMD with basic operations

Shreyansh Jain shreyansh.jain at nxp.com
Wed Sep 19 14:15:41 CEST 2018


On Thursday 06 September 2018 11:24 AM, Gagandeep Singh wrote:
> This patch introduces the enetc PMD with basic
> initialisation functions includes probe, teardown,
> hardware intialisation
> 
> Signed-off-by: Gagandeep Singh <g.singh at nxp.com>
> ---
>   MAINTAINERS                                 |   1 +
>   config/common_base                          |   5 +
>   config/common_linuxapp                      |   5 +
>   doc/guides/nics/enetc.rst                   |   1 +
>   doc/guides/nics/features/enetc.ini          |   2 +
>   drivers/net/Makefile                        |   1 +
>   drivers/net/enetc/Makefile                  |  24 ++
>   drivers/net/enetc/base/enetc_hw.h           | 220 ++++++++++++++++
>   drivers/net/enetc/enetc.h                   | 111 ++++++++
>   drivers/net/enetc/enetc_ethdev.c            | 269 ++++++++++++++++++++
>   drivers/net/enetc/enetc_logs.h              |  40 +++
>   drivers/net/enetc/meson.build               |  10 +
>   drivers/net/enetc/rte_pmd_enetc_version.map |   4 +
>   drivers/net/meson.build                     |   1 +
>   mk/rte.app.mk                               |   1 +
>   15 files changed, 695 insertions(+)
>   create mode 100644 drivers/net/enetc/Makefile
>   create mode 100644 drivers/net/enetc/base/enetc_hw.h
>   create mode 100644 drivers/net/enetc/enetc.h
>   create mode 100644 drivers/net/enetc/enetc_ethdev.c
>   create mode 100644 drivers/net/enetc/enetc_logs.h
>   create mode 100644 drivers/net/enetc/meson.build
>   create mode 100644 drivers/net/enetc/rte_pmd_enetc_version.map
> 

[...]

> diff --git a/drivers/net/enetc/base/enetc_hw.h b/drivers/net/enetc/base/enetc_hw.h
> new file mode 100644
> index 000000000..806b26a2c
> --- /dev/null
> +++ b/drivers/net/enetc/base/enetc_hw.h
> @@ -0,0 +1,220 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2018 NXP
> + */
> +

[...]

> +
> +/* PCI device info */
> +struct enetc_hw {
> +	/* SI registers, used by all PCI functions */
> +	void *reg;
> +	/* Port registers, PF only */
> +	void *port;
> +	/* IP global registers, PF only */
> +	void *global;
> +};

A trivial one: Some structures have comments, but some don't.
Even the one above has comments *before* the variable. There is a no 
fixed standard, but it is expected that comments would be uniform across 
the file. If you see in file enetc/enetc.h, you would observe some 
comments *after* the variable.

Can you make them uniform?

> +
> +struct enetc_eth_mac_info {
> +	uint8_t addr[ETH_ADDR_LEN];
> +	uint8_t perm_addr[ETH_ADDR_LEN];
> +	bool get_link_status;
> +};
> +
> +struct enetc_eth_hw {
> +	struct net_device *ndev;
> +	struct enetc_hw hw;
> +	uint16_t device_id;
> +	uint16_t vendor_id;
> +	uint8_t revision_id;
> +	struct enetc_eth_mac_info mac;
> +};
> +
> +/* Transmit Descriptor */
> +struct enetc_tx_desc {
> +	uint64_t addr;
> +	uint16_t frm_len;
> +	uint16_t buf_len;
> +	uint32_t flags_errors;
> +};
> +
> +/* TX Buffer Descriptors (BD) */
> +struct enetc_tx_bd {
> +	uint64_t addr;
> +	uint16_t buf_len;
> +	uint16_t frm_len;
> +	uint16_t err_csum;
> +	uint16_t flags;
> +};
> +
> +/* RX buffer descriptor */
> +union enetc_rx_bd {
> +	struct {
> +		uint64_t addr;
> +		uint8_t reserved[8];
> +	} w;
> +	struct {
> +		uint16_t inet_csum;
> +		uint16_t parse_summary;
> +		uint32_t rss_hash;
> +		uint16_t buf_len;
> +		uint16_t vlan_opt;
> +		union {
> +			struct {
> +				uint16_t flags;
> +				uint16_t error;
> +			};
> +			uint32_t lstatus;
> +		};
> +	} r;
> +};

[...]

> +#endif /* _ENETC_H_ */
> diff --git a/drivers/net/enetc/enetc_ethdev.c b/drivers/net/enetc/enetc_ethdev.c
> new file mode 100644
> index 000000000..06438835d
> --- /dev/null
> +++ b/drivers/net/enetc/enetc_ethdev.c
> @@ -0,0 +1,269 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2018 NXP
> + */
> +
> +#include <stdbool.h>
> +#include <rte_ethdev_pci.h>
> +
> +#include "enetc_logs.h"
> +#include "enetc.h"
> +
> +int enetc_logtype_pmd;
> +
> +/* Functions Prototypes */
> +static int enetc_dev_configure(struct rte_eth_dev *dev);
> +static int enetc_dev_start(struct rte_eth_dev *dev);
> +static void enetc_dev_stop(struct rte_eth_dev *dev);
> +static void enetc_dev_close(struct rte_eth_dev *dev);
> +static void enetc_dev_infos_get(struct rte_eth_dev *dev,
> +			struct rte_eth_dev_info *dev_info);
> +static int enetc_link_update(struct rte_eth_dev *dev, int wait_to_complete);
> +static int enetc_hardware_init(struct enetc_eth_hw *hw);
> +
> +/*
> + * The set of PCI devices this driver supports
> + */
> +static const struct rte_pci_id pci_id_enetc_map[] = {
> +	{ RTE_PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID) },
> +	{ RTE_PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_VF) },
> +	{ .vendor_id = 0, /* sentinel */ },
> +};
> +
> +/* Features supported by this driver */
> +static const struct eth_dev_ops enetc_ops = {
> +	.dev_configure        = enetc_dev_configure,
> +	.dev_start            = enetc_dev_start,
> +	.dev_stop             = enetc_dev_stop,
> +	.dev_close            = enetc_dev_close,
> +	.link_update          = enetc_link_update,
> +	.dev_infos_get        = enetc_dev_infos_get,
> +};
> +
> +/**
> + * Initialisation of the enetc device
> + *
> + * @param eth_dev
> + *   - Pointer to the structure rte_eth_dev
> + *
> + * @return
> + *   - On success, zero.
> + *   - On failure, negative value.
> + */
> +static int
> +enetc_dev_init(struct rte_eth_dev *eth_dev)
> +{
> +	int error = 0, i;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
> +	struct enetc_eth_hw *hw =
> +		ENETC_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
> +	struct enetc_eth_adapter *adapter =
> +		ENETC_DEV_PRIVATE(eth_dev->data->dev_private);
> +
> +	PMD_INIT_FUNC_TRACE();
> +	eth_dev->dev_ops = &enetc_ops;
> +	eth_dev->rx_pkt_burst = NULL;
> +	eth_dev->tx_pkt_burst = NULL;
> +
> +	rte_eth_copy_pci_info(eth_dev, pci_dev);
> +
> +	/* Retrieving and storing the HW base address of device */
> +	hw->hw.reg = (void *)pci_dev->mem_resource[0].addr;
> +
> +	adapter->tx_bd_count = MAX_BD_COUNT;
> +	adapter->rx_bd_count = MAX_BD_COUNT;
> +
> +	adapter->num_rx_rings = MAX_RX_RINGS;
> +	adapter->num_tx_rings = MAX_TX_RINGS;
> +
> +	for (i = 0; i < adapter->num_rx_rings; i++) {
> +		adapter->rx_ring[i] = rte_zmalloc(NULL,
> +						sizeof(struct enetc_bdr), 0);
> +		if (!adapter->rx_ring[i]) {
> +			ENETC_PMD_ERR("Failed to allocate RX ring memory");
> +			while (--i >= 0)
> +				rte_free(adapter->rx_ring[i]);
> +			error = -ENOMEM;
> +			goto err_late;
> +		}
> +		adapter->rx_ring[i]->bd_count = adapter->rx_bd_count;
> +	}
> +
> +	for (i = 0; i < adapter->num_tx_rings; i++) {
> +		adapter->tx_ring[i] = rte_zmalloc(NULL,
> +						sizeof(struct enetc_bdr), 0);
> +		if (!adapter->tx_ring[i]) {
> +			ENETC_PMD_ERR("Failed to allocate TX ring memory");
> +			while (--i >= 0)
> +				rte_free(adapter->tx_ring[i]);
> +			error = -ENOMEM;
> +			goto err_second;
> +		}
> +		adapter->tx_ring[i]->bd_count = adapter->tx_bd_count;
> +	}
> +
> +	error = enetc_hardware_init(hw);
> +	if (error != 0) {
> +		ENETC_PMD_ERR("Hardware initialization failed");
> +		goto err_first;
> +	}
> +
> +	/* Allocate memory for storing MAC addresses */
> +	eth_dev->data->mac_addrs = rte_zmalloc("enetc_eth",
> +		ETHER_ADDR_LEN, 0);
> +	if (!eth_dev->data->mac_addrs) {
> +		ENETC_PMD_ERR("Failed to allocate %d bytes needed to "
> +						"store MAC addresses",
> +				ETHER_ADDR_LEN * 1);

Formatting of the above log is not correct. It should be in format:

FUNCTION_NAME(Some argument,
               argument on new line after just below the first,
               ...);

> +		error = -ENOMEM;
> +		goto err_first;
> +	}
> +
> +	/* Copy the permanent MAC address */
> +	ether_addr_copy((struct ether_addr *)hw->mac.addr,
> +						&eth_dev->data->mac_addrs[0]);

Alignment of the second argument should be below the first one.

> +
> +	ENETC_PMD_DEBUG("port_id %d vendorID=0x%x deviceID=0x%x",
> +			eth_dev->data->port_id, pci_dev->id.vendor_id,
> +			pci_dev->id.device_id);
> +	return 0;
> +
> +err_first:
> +	for (i = 0; i < adapter->num_tx_rings; i++)
> +		rte_free(adapter->tx_ring[i]);
> +err_second:
> +	for (i = 0; i < adapter->num_rx_rings; i++)
> +		rte_free(adapter->rx_ring[i]);
> +err_late:
> +	return error;
> +}
> +
> +static int
> +enetc_dev_uninit(struct rte_eth_dev *eth_dev)
> +{

Some functions have FUNC_TRACE, some don't. Maybe you should make it 
uniform.

> +	return 0;
> +}
> +
> +static int
> +enetc_dev_configure(struct rte_eth_dev *dev)
> +{
> +	PMD_INIT_FUNC_TRACE();
> +	return 0;
> +}
> +
> +static int
> +enetc_dev_start(struct rte_eth_dev *dev)
> +{
> +	PMD_INIT_FUNC_TRACE();
> +	return 0;
> +}
> +
> +static void
> +enetc_dev_stop(struct rte_eth_dev *dev)
> +{
> +}
> +
> +static void
> +enetc_dev_close(struct rte_eth_dev *dev)
> +{
> +}
> +
> +/* return 0 means link status changed, -1 means not changed */
> +static int
> +enetc_link_update(struct rte_eth_dev *dev, int wait_to_complete)
> +{
> +	struct enetc_eth_hw *hw =
> +		ENETC_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct rte_eth_link link;
> +
> +	hw->mac.get_link_status = 1;
> +
> +	memset(&link, 0, sizeof(link));
> +	rte_eth_linkstatus_get(dev, &link);
> +
> +	link.link_duplex = ETH_LINK_FULL_DUPLEX;
> +	link.link_status = ETH_LINK_UP;
> +	rte_eth_linkstatus_set(dev, &link);
> +
> +	return 0;
> +}
> +
> +static int
> +enetc_hardware_init(struct enetc_eth_hw *hw)
> +{
> +	uint32_t psipmr = 0;
> +
> +	/* Calculating and storing the base HW addresses */
> +	hw->hw.port = hw->hw.reg + ENETC_PORT_BASE;
> +	hw->hw.global = hw->hw.reg + ENETC_GLOBAL_BASE;
> +
> +	/* Enabling Station Interface */
> +	ENETC_REG_WRITE(ENETC_GET_HW_ADDR(hw->hw.reg, ENETC_SIMR),
> +								ENETC_SIMR_EN);

Second argument on new line should start below first.

> +
> +	/* Setting to accept broadcast packets for each inetrface */
> +	psipmr |= ENETC_PSIPMR_SET_UP(0) | ENETC_PSIPMR_SET_MP(0) |
> +						ENETC_PSIPMR_SET_VLAN_MP(0);
> +	psipmr |= ENETC_PSIPMR_SET_UP(1) | ENETC_PSIPMR_SET_MP(1) |
> +						ENETC_PSIPMR_SET_VLAN_MP(1);
> +	psipmr |= ENETC_PSIPMR_SET_UP(2) | ENETC_PSIPMR_SET_MP(2) |
> +						ENETC_PSIPMR_SET_VLAN_MP(2);
> +
> +	ENETC_REG_WRITE(ENETC_GET_HW_ADDR(hw->hw.port, ENETC_PSIPMR),
> +			psipmr);
> +
> +	ENETC_REG_WRITE(ENETC_GET_HW_ADDR(hw->hw.port, ENETC_PM0_CMD_CFG),
> +			ENETC_PM0_TX_EN | ENETC_PM0_RX_EN);
> +
> +	/* Enable port */
> +	ENETC_REG_WRITE(ENETC_GET_HW_ADDR(hw->hw.port, ENETC_PMR),
> +							ENETC_PMR_EN);
> +
> +	/* Enabling broadcast address */
> +	ENETC_REG_WRITE(ENETC_GET_HW_ADDR(hw->hw.port, ENETC_PSIPMAR0(0)),
> +								0xFFFFFFFF);
> +	ENETC_REG_WRITE(ENETC_GET_HW_ADDR(hw->hw.port, ENETC_PSIPMAR1(0)),
> +								0xFFFF << 16);
> +
> +	return 0;
> +}

[...]

> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index de33883be..154ae3b2c 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -135,6 +135,7 @@ endif
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)      += -lrte_pmd_e1000
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_ENA_PMD)        += -lrte_pmd_ena
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD)       += -lrte_pmd_enic
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_ENETC_PMD)      += -lrte_pmd_enetc
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD)      += -lrte_pmd_fm10k
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_FAILSAFE)   += -lrte_pmd_failsafe
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD)       += -lrte_pmd_i40e
> 

32 bit compilation for this is failing with errors like this:

/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/enetc_rxtx.c: In 
function ‘enetc_xmit_pkts’:
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/enetc_rxtx.c:73:3: 
warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
    (uint64_t)rte_cpu_to_le_64(tx_swbd->buffer_addr->buf_addr +
    ^
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/enetc_rxtx.c: In 
function ‘enetc_refill_rx_ring’:
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/enetc_rxtx.c:98:18: 
warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
    rxbd->w.addr = (uint64_t)rx_swbd->buffer_addr->buf_addr +
                   ^
In file included from 
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/enetc_rxtx.c:13:0:
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/enetc_rxtx.c: In 
function ‘enetc_setup_txbdr’:
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/enetc_rxtx.c:293:18: 
warning: cast from pointer to integer of different size 
[-Wpointer-to-int-cast]
     lower_32_bits((uint64_t)tx_ring->bd_base));
                   ^
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/base/enetc_hw.h:108:45: 
note: in definition of macro ‘enetc_wr_reg’
  #define enetc_wr_reg(reg, val) rte_write32((val), (reg))
                                              ^~~
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/base/enetc_hw.h:121:5: 
note: in expansion of macro ‘enetc_wr’
      enetc_wr(hw, ENETC_BDR(t, n, off), val)
      ^~~~~~~~
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/base/enetc_hw.h:126:5: 
note: in expansion of macro ‘enetc_bdr_wr’
      enetc_bdr_wr(hw, TX, n, off, val)
      ^~~~~~~~~~~~
/home/shreyansh/build/DPDK/05_dpdk/drivers/net/enetc/enetc_rxtx.c:292:2: 
note: in expansion of macro ‘enetc_txbdr_wr’


More information about the dev mailing list