DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Sachin Saxena (OSS)" <sachin.saxena@oss.nxp.com>
To: Apeksha Gupta <apeksha.gupta@nxp.com>,
	ferruh.yigit@intel.com, dev@dpdk.org
Cc: hemant.agrawal@nxp.com
Subject: Re: [dpdk-dev] [PATCH 2/4] drivers/net/enetfec: UIO support added
Date: Sun, 4 Jul 2021 09:57:50 +0530
Message-ID: <b54a36b5-fcdc-441a-27b6-54a83e04ce29@oss.nxp.com> (raw)
In-Reply-To: <20210430043424.19752-3-apeksha.gupta@nxp.com>



On 30-Apr-21 10:04 AM, Apeksha Gupta wrote:
> Implemented the fec-uio driver in kernel. enetfec PMD uses
> UIO interface to interact with kernel for PHY initialisation

enetfec PMD uses UIO interface to interact with "fec-uio" driver implemented in kernel for PHY initialization...

> and for mapping the allocated memory of register & BD from
> kernel to DPDK which gives access to non-cacheble memory for BD.
>
> Signed-off-by: Sachin Saxena <sachin.saxena@nxp.com>
> Signed-off-by: Apeksha Gupta <apeksha.gupta@nxp.com>
> ---
>   drivers/net/enetfec/enet_ethdev.c | 204 ++++++++++++++++++++++++++++++
>   drivers/net/enetfec/enet_regs.h   | 179 ++++++++++++++++++++++++++
>   drivers/net/enetfec/enet_uio.c    | 192 ++++++++++++++++++++++++++++
>   drivers/net/enetfec/enet_uio.h    |  54 ++++++++
>   drivers/net/enetfec/meson.build   |   3 +-
>   5 files changed, 631 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/net/enetfec/enet_regs.h
>   create mode 100644 drivers/net/enetfec/enet_uio.c
>   create mode 100644 drivers/net/enetfec/enet_uio.h
>
> diff --git a/drivers/net/enetfec/enet_ethdev.c b/drivers/net/enetfec/enet_ethdev.c
> index 5fd2dbc2d..5f4f2cf9e 100644
> --- a/drivers/net/enetfec/enet_ethdev.c
> +++ b/drivers/net/enetfec/enet_ethdev.c
> @@ -11,18 +11,195 @@
>   #include <rte_bus_vdev.h>
>   #include <rte_dev.h>
>   #include <rte_ether.h>
> +#include <rte_io.h>
>   #include "enet_pmd_logs.h"
>   #include "enet_ethdev.h"
> +#include "enet_regs.h"
> +#include "enet_uio.h"
>   
>   #define ENETFEC_NAME_PMD        net_enetfec
>   #define ENET_VDEV_GEM_ID_ARG    "intf"
>   #define ENET_CDEV_INVALID_FD    -1
>   
> +#define BIT(nr)			(1 << (nr))
> +/* FEC receive acceleration */
> +#define ENET_RACC_IPDIS		(1 << 1)
> +#define ENET_RACC_PRODIS	(1 << 2)

Please consider Andrew's suggestions here.
> +#define ENET_RACC_SHIFT16	BIT(7)
> +#define ENET_RACC_OPTIONS	(ENET_RACC_IPDIS | ENET_RACC_PRODIS)
> +
> +/* Transmitter timeout */
> +#define TX_TIMEOUT (2 * HZ)
> +
> +#define ENET_PAUSE_FLAG_AUTONEG		0x1
> +#define ENET_PAUSE_FLAG_ENABLE		0x2
> +#define ENET_WOL_HAS_MAGIC_PACKET	(0x1 << 0)
> +#define ENET_WOL_FLAG_ENABLE		(0x1 << 1)
> +#define ENET_WOL_FLAG_SLEEP_ON		(0x1 << 2)

ENET_WOL_* are unused. please remove the unused code.
> +
> +/* Pause frame feild and FIFO threshold */
> +#define ENET_ENET_FCE		(1 << 5)

ENET_ENET_*, repeating ENET  don't look reasonable.
> +#define ENET_ENET_RSEM_V	0x84
> +#define ENET_ENET_RSFL_V	16
> +#define ENET_ENET_RAEM_V	0x8
> +#define ENET_ENET_RAFL_V	0x8
> +#define ENET_ENET_OPD_V		0xFFF0

Unused.

> +#define ENET_MDIO_PM_TIMEOUT	100 /* ms */
> +
>   int enetfec_logtype_pmd;
>   
> +/*
> + * This function is called to start or restart the FEC during a link
FEC -> ENETFEC
> + * change, transmit timeout or to reconfigure the FEC. The network
> + * packet processing for this device must be stopped before this call.
> + */
> +static void
> +enetfec_restart(struct rte_eth_dev *dev)
> +{
> +	struct enetfec_private *fep = dev->data->dev_private;
> +	uint32_t temp_mac[2];
> +	uint32_t rcntl = OPT_FRAME_SIZE | 0x04;
> +	uint32_t ecntl = ENET_ETHEREN; /* ETHEREN */
> +	/* TODO get eth addr from eth dev */
Remove TODO. We may use this addr as default Mac address for device for now.
> +	struct rte_ether_addr addr = {
> +		.addr_bytes = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6} };
> +	uint32_t val;
> +
> +	/*
> +	 * enet-mac reset will reset mac address registers too,
> +	 * so need to reconfigure it.
> +	 */
> +	memcpy(&temp_mac, addr.addr_bytes, ETH_ALEN);
> +	rte_write32(rte_cpu_to_be_32(temp_mac[0]),
> +		fep->hw_baseaddr_v + ENET_PALR);
> +	rte_write32(rte_cpu_to_be_32(temp_mac[1]),
> +		fep->hw_baseaddr_v + ENET_PAUR);
> +
> +	/* Clear any outstanding interrupt. */
> +	writel(0xffffffff, fep->hw_baseaddr_v + ENET_EIR);
> +
> +	/* Enable MII mode */
> +	if (fep->full_duplex == FULL_DUPLEX) {
> +		/* FD enable */
> +		rte_write32(0x04, fep->hw_baseaddr_v + ENET_TCR);
> +	} else {
> +		/* No Rcv on Xmit */
> +		rcntl |= 0x02;
> +		rte_write32(0x0, fep->hw_baseaddr_v + ENET_TCR);
> +	}
> +
> +	if (fep->quirks & QUIRK_RACC) {
> +		val = rte_read32(fep->hw_baseaddr_v + ENET_RACC);
> +		/* align IP header */
> +		val |= ENET_RACC_SHIFT16;
> +		if (fep->flag_csum & RX_FLAG_CSUM_EN)
> +			/* set RX checksum */
> +			val |= ENET_RACC_OPTIONS;
> +		else
> +			val &= ~ENET_RACC_OPTIONS;
> +		rte_write32(val, fep->hw_baseaddr_v + ENET_RACC);
> +		rte_write32(PKT_MAX_BUF_SIZE,
> +			fep->hw_baseaddr_v + ENET_FRAME_TRL);
> +	}
> +
> +	/*
> +	 * The phy interface and speed need to get configured
> +	 * differently on enet-mac.
> +	 */
> +	if (fep->quirks & QUIRK_HAS_ENET_MAC) {
> +		/* Enable flow control and length check */
> +		rcntl |= 0x40000000 | 0x00000020;
> +
> +		/* RGMII, RMII or MII */
> +		rcntl |= (1 << 6);
> +		ecntl |= (1 << 5);
> +	}
> +
> +	/* enable pause frame*/
> +	if ((fep->flag_pause & ENET_PAUSE_FLAG_ENABLE) ||
> +		((fep->flag_pause & ENET_PAUSE_FLAG_AUTONEG)
> +		/*&& ndev->phydev && ndev->phydev->pause*/)) {
> +		rcntl |= ENET_ENET_FCE;
> +
> +		/* set FIFO threshold parameter to reduce overrun */
> +		rte_write32(ENET_ENET_RSEM_V,
> +				fep->hw_baseaddr_v + ENET_R_FIFO_SEM);
> +		rte_write32(ENET_ENET_RSFL_V,
> +				fep->hw_baseaddr_v + ENET_R_FIFO_SFL);
> +		rte_write32(ENET_ENET_RAEM_V,
> +				fep->hw_baseaddr_v + ENET_R_FIFO_AEM);
> +		rte_write32(ENET_ENET_RAFL_V,
> +				fep->hw_baseaddr_v + ENET_R_FIFO_AFL);
> +
> +		/* OPD */
> +		rte_write32(ENET_ENET_OPD_V, fep->hw_baseaddr_v + ENET_OPD);
> +	} else {
> +		rcntl &= ~ENET_ENET_FCE;
> +	}
> +
> +	rte_write32(rcntl, fep->hw_baseaddr_v + ENET_RCR);
> +
> +	rte_write32(0, fep->hw_baseaddr_v + ENET_IAUR);
> +	rte_write32(0, fep->hw_baseaddr_v + ENET_IALR);
> +
> +	if (fep->quirks & QUIRK_HAS_ENET_MAC) {
> +		/* enable ENET endian swap */
> +		ecntl |= (1 << 8);
> +		/* enable ENET store and forward mode */
> +		rte_write32(1 << 8, fep->hw_baseaddr_v + ENET_TFWR);
> +	}
> +
> +	if (fep->bufdesc_ex)
> +		ecntl |= (1 << 4);
> +
> +	if (fep->quirks & QUIRK_SUPPORT_DELAYED_CLKS &&
> +		fep->rgmii_txc_delay)
> +		ecntl |= ENET_TXC_DLY;
> +	if (fep->quirks & QUIRK_SUPPORT_DELAYED_CLKS &&
> +		fep->rgmii_rxc_delay)
> +		ecntl |= ENET_RXC_DLY;
> +
> +	/* Enable the MIB statistic event counters */
> +	rte_write32(0 << 31, fep->hw_baseaddr_v + ENET_MIBC);
> +
> +	ecntl |= 0x70000000;
> +	/* And last, enable the transmit and receive processing */
> +	rte_write32(ecntl, fep->hw_baseaddr_v + ENET_ECR);
> +	rte_delay_us(10);
> +}
> +
> +static int
> +enetfec_eth_open(struct rte_eth_dev *dev)
> +{
> +	enetfec_restart(dev);
> +
> +	return 0;
> +}
> +
> +static const struct eth_dev_ops ops = {
> +	.dev_start = enetfec_eth_open,

enetfec_eth_open -> enetfec_eth_start ?

Also, enetfec_eth_stop() is missing in this patch.

> +};
> +
>   static int
>   enetfec_eth_init(struct rte_eth_dev *dev)
>   {
> +	struct enetfec_private *fep = dev->data->dev_private;
> +	struct rte_eth_conf *eth_conf = &fep->dev->data->dev_conf;
> +	uint64_t rx_offloads = eth_conf->rxmode.offloads;
> +
> +	fep->full_duplex = FULL_DUPLEX;
> +
> +	dev->dev_ops = &ops;
> +	if (fep->quirks & QUIRK_VLAN)
> +		/* enable hw VLAN support */
> +		rx_offloads |= DEV_RX_OFFLOAD_VLAN;
> +
> +	if (fep->quirks & QUIRK_CSUM) {
> +		/* enable hw accelerator */
> +		rx_offloads |= DEV_RX_OFFLOAD_CHECKSUM;
> +		fep->flag_csum |= RX_FLAG_CSUM_EN;
> +	}
These changes and updating features list in patch 4/4, should be handled 
in separate individual patches to add NIC features.
> +
>   	rte_eth_dev_probing_finish(dev);
>   	return 0;
>   }
> @@ -34,6 +211,8 @@ pmd_enetfec_probe(struct rte_vdev_device *vdev)
>   	struct enetfec_private *fep;
>   	const char *name;
>   	int rc = -1;
> +	int i;
> +	unsigned int bdsize;
>   
>   	name = rte_vdev_device_name(vdev);
>   	if (name == NULL)
> @@ -47,6 +226,31 @@ pmd_enetfec_probe(struct rte_vdev_device *vdev)
>   	/* setup board info structure */
>   	fep = dev->data->dev_private;
>   	fep->dev = dev;
> +
> +	fep->max_rx_queues = ENET_MAX_Q;
> +	fep->max_tx_queues = ENET_MAX_Q;
> +	fep->quirks = QUIRK_HAS_ENET_MAC | QUIRK_GBIT | QUIRK_BUFDESC_EX
> +		| QUIRK_CSUM | QUIRK_VLAN | QUIRK_ERR007885
> +		| QUIRK_RACC | QUIRK_COALESCE | QUIRK_EEE;
Same as above.
> +
> +	config_enetfec_uio(fep);
return type should be checked.
> +
> +	/* Get the BD size for distributing among six queues */
> +	bdsize = (fep->bd_size) / 6;
Use MACRO for number of queues instead of hard coding. Also, should 
configure only 1 queue
as a part of this patch as multi-queue support is upcoming feature.

> +
> +	for (i = 0; i < fep->max_tx_queues; i++) {
> +		fep->dma_baseaddr_t[i] = fep->bd_addr_v;
> +		fep->bd_addr_p_t[i] = fep->bd_addr_p;
> +		fep->bd_addr_v = fep->bd_addr_v + bdsize;
> +		fep->bd_addr_p = fep->bd_addr_p + bdsize;
> +	}
> +	for (i = 0; i < fep->max_rx_queues; i++) {
> +		fep->dma_baseaddr_r[i] = fep->bd_addr_v;
> +		fep->bd_addr_p_r[i] = fep->bd_addr_p;
> +		fep->bd_addr_v = fep->bd_addr_v + bdsize;
> +		fep->bd_addr_p = fep->bd_addr_p + bdsize;
> +	}
> +
>   	rc = enetfec_eth_init(dev);
>   	if (rc)
>   		goto failed_init;
> diff --git a/drivers/net/enetfec/enet_regs.h b/drivers/net/enetfec/enet_regs.h
> new file mode 100644
> index 000000000..d037aafae
> --- /dev/null
> +++ b/drivers/net/enetfec/enet_regs.h
> @@ -0,0 +1,179 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2021 NXP
> + */
Need a 1 line gap.
> +#ifndef __ENET_REGS_H
use ENETFEC at all places.
> +#define __ENET_REGS_H
> +
> +/* Ethernet receive use control and status of buffer descriptor
> + */
> +#define RX_BD_TR	((ushort)0x0001) /* Truncated */
> +#define RX_BD_OV	((ushort)0x0002) /* Over-run */
> +#define RX_BD_CR	((ushort)0x0004) /* CRC or Frame error */
> +#define RX_BD_SH	((ushort)0x0008) /* Reserved */
> +#define RX_BD_NO	((ushort)0x0010) /* Rcvd non-octet aligned frame */
> +#define RX_BD_LG	((ushort)0x0020) /* Rcvd frame length voilation */
> +#define RX_BD_MC	((ushort)0x0040) /* Rcvd Multicast */
> +#define RX_BD_BC	((ushort)0x0080) /* Rcvd Broadcast */
> +#define RX_BD_MISS	((ushort)0x0100) /* Miss: promisc mode frame */
> +#define RX_BD_FIRST	((ushort)0x0400) /* Reserved */
> +#define RX_BD_LAST	((ushort)0x0800) /* Buffer is the last in the frame */
> +#define RX_BD_INTR	((ushort)0x1000) /* Software specified field */
> +/*  0 The next BD in consecutive location
> + *  1 The next BD in ENETn_RDSR.
> + */
> +#define RX_BD_WRAP	((ushort)0x2000)
> +#define RX_BD_EMPTY	((ushort)0x8000) /* BD is empty */
> +#define RX_BD_STATS	((ushort)0x013f) /* All buffer descriptor status bits */
> +
> +/* Ethernet receive use control and status of enhanced buffer descriptor */
> +#define BD_ENET_RX_VLAN	0x00000004
> +
> +/* Ethernet transmit use control and status of buffer descriptor.
> + */
> +#define TX_BD_CSL	((ushort)0x0001)
> +#define TX_BD_UN	((ushort)0x0002)
> +#define TX_BD_RCMASK	((ushort)0x003c)
> +#define TX_BD_RL	((ushort)0x0040)
> +#define TX_BD_LC	((ushort)0x0080)
> +#define TX_BD_HB	((ushort)0x0100)
> +#define TX_BD_DEF	((ushort)0x0200)
> +#define TX_BD_TC	((ushort)0x0400) /* Transmit CRC */
> +#define TX_BD_LAST	((ushort)0x0800) /* Last in frame */
> +#define TX_BD_INTR	((ushort)0x1000)
> +#define TX_BD_WRAP	((ushort)0x2000)
> +#define TX_BD_PAD	((ushort)0x4000)
> +#define TX_BD_READY	((ushort)0x8000) /* Data is ready */
> +
> +#define TX_BD_STATS	((ushort)0x0fff) /* All buffer descriptor status bits */
> +
> +/* Ethernet transmit use control and status of enhanced buffer descriptor */
> +#define TX_BD_IINS		0x08000000
> +#define TX_BD_PINS		0x10000000
> +#define TX_BD_TS		0x20000000
> +#define TX_BD_INT		0x40000000
> +
> +#define ENET_RD_START(X)	(((X) == 1) ? ENET_RD_START_1 : \
> +				(((X) == 2) ? \
> +					ENET_RD_START_2 : ENET_RD_START_0))
> +#define ENET_TD_START(X)	(((X) == 1) ? ENET_TD_START_1 : \
> +				(((X) == 2) ? \
> +					ENET_TD_START_2 : ENET_TD_START_0))
> +#define ENET_MRB_SIZE(X)	(((X) == 1) ? ENET_MRB_SIZE_1 : \
> +				(((X) == 2) ? \
> +					ENET_MRB_SIZE_2 : ENET_MRB_SIZE_0))
> +
> +#define ENET_DMACFG(X)	(((X) == 2) ? ENET_DMA2CFG : ENET_DMA1CFG)
unused.
> +
> +#define ENABLE_DMA_CLASS	(1 << 16)
> +#define ENET_RCM(X)	(((X) == 2) ? ENET_RCM2 : ENET_RCM1)
> +#define SLOPE_IDLE_MASK		0xffff
> +#define SLOPE_IDLE_1		0x200 /* BW_fraction: 0.5 */
> +#define SLOPE_IDLE_2		0x200 /* BW_fraction: 0.5 */
> +#define SLOPE_IDLE(X)		(((X) == 1) ?				\
> +				(SLOPE_IDLE_1 & SLOPE_IDLE_MASK) :	\
> +				(SLOPE_IDLE_2 & SLOPE_IDLE_MASK))
> +#define RCM_MATCHEN		(0x1 << 16)
> +#define CFG_RCMR_CMP(v, n)	(((v) & 0x7) <<  ((n) << 2))
> +#define RCMR_CMP1		(CFG_RCMR_CMP(0, 0) | CFG_RCMR_CMP(1, 1) | \
> +				CFG_RCMR_CMP(2, 2) | CFG_RCMR_CMP(3, 3))
> +#define RCMR_CMP2		(CFG_RCMR_CMP(4, 0) | CFG_RCMR_CMP(5, 1) | \
> +				CFG_RCMR_CMP(6, 2) | CFG_RCMR_CMP(7, 3))
> +#define RCM_CMP(X)		(((X) == 1) ? RCMR_CMP1 : RCMR_CMP2)
> +#define BD_TX_FTYPE(X)		(((X) & 0xf) << 20)
> +
All above Macros appears unused as of now. Please check and remove.

> +#define RX_BD_INT		0x00800000
> +#define RX_BD_PTP		((ushort)0x0400)
PTP is supported?
> +#define RX_BD_ICE		0x00000020
> +#define RX_BD_PCR		0x00000010
> +#define RX_FLAG_CSUM_EN		(RX_BD_ICE | RX_BD_PCR)
> +#define RX_FLAG_CSUM_ERR	(RX_BD_ICE | RX_BD_PCR)
> +#define ENET_MII		((uint)0x00800000)      /*MII_interrupt*/
> +
> +#define ENET_ETHEREN		((uint)0x00000002)
> +#define ENET_TXC_DLY		((uint)0x00010000)
> +#define ENET_RXC_DLY		((uint)0x00020000)
> +
> +/* ENET MAC is in controller */
> +#define QUIRK_HAS_ENET_MAC	(1 << 0)
> +/* gasket is used in controller */
> +#define QUIRK_GASKET		(1 << 2)
> +/* GBIT supported in controller */
> +#define QUIRK_GBIT		(1 << 3)
> +/* Controller has extended descriptor buffer */
> +#define QUIRK_BUFDESC_EX	(1 << 4)
> +/* Controller support hardware checksum */
> +#define QUIRK_CSUM		(1 << 5)
> +/* Controller support hardware vlan*/
> +#define QUIRK_VLAN		(1 << 6)
> +/* ENET IP hardware AVB
> + * i.MX8MM ENET IP supports the AVB (Audio Video Bridging) feature.
> + */
> +#define QUIRK_AVB		(1 << 8)
> +#define QUIRK_ERR007885		(1 << 9)
> +/* RACC register supported by controller */
> +#define QUIRK_RACC		(1 << 12)
> +/* interrupt coalesc supported by controller*/
> +#define QUIRK_COALESCE		(1 << 13)
> +/* To support IEEE 802.3az EEE std, new feature is added by i.MX8MQ ENET IP
> + * version.
> + */
> +#define QUIRK_EEE		(1 << 17)
> +/* i.MX8QM ENET IP version added the feature to generate the delayed TXC or
> + * RXC. For its implementation, ENET uses synchronized clocks (250MHz) for
> + * generating delay of 2ns.
> + */
> +#define QUIRK_SUPPORT_DELAYED_CLKS	(1 << 18)
> +
> +#define ENET_EIR	0x004 /* Interrupt event register */
> +#define ENET_EIMR	0x008 /* Interrupt mask register */
> +#define ENET_RDAR_0	0x010 /* Receive descriptor active register ring0 */
> +#define ENET_TDAR_0	0x014 /* Transmit descriptor active register ring0 */
> +#define ENET_ECR	0x024 /* Ethernet control register */
> +#define ENET_MSCR	0x044 /* MII speed control register */
> +#define ENET_MIBC	0x064 /* MIB control and status register */
> +#define ENET_RCR	0x084 /* Receive control register */
> +#define ENET_TCR	0x0c4 /* Transmit Control register */
> +#define ENET_PALR	0x0e4 /* MAC address low 32 bits */
> +#define ENET_PAUR	0x0e8 /* MAC address high 16 bits */
> +#define ENET_OPD	0x0ec /* Opcode/Pause duration register */
> +#define ENET_IAUR	0x118 /* hash table 32 bits high */
> +#define ENET_IALR	0x11c /* hash table 32 bits low */
> +#define ENET_GAUR	0x120 /* grp hash table 32 bits high */
> +#define ENET_GALR	0x124 /* grp hash table 32 bits low */
> +#define ENET_TFWR	0x144 /* transmit FIFO water_mark */
> +#define ENET_RD_START_1	0x160 /* Receive descriptor ring1 start register */
> +#define ENET_TD_START_1	0x164 /* Transmit descriptor ring1 start register */
> +#define ENET_MRB_SIZE_1	0x168 /* Maximum receive buffer size register ring1 */
> +#define ENET_RD_START_2	0x16c /* Receive descriptor ring2 start register */
> +#define ENET_TD_START_2	0x170 /* Transmit descriptor ring2 start register */
> +#define ENET_MRB_SIZE_2	0x174 /* Maximum receive buffer size register ring2 */
> +#define ENET_RD_START_0	0x180 /* Receive descriptor ring0 start reg */
> +#define ENET_TD_START_0	0x184 /* Transmit buffer descriptor ring0 start reg */
> +#define ENET_MRB_SIZE_0	0x188 /* Maximum receive buffer size register ring0*/
> +#define ENET_R_FIFO_SFL	0x190 /* Rx FIFO full threshold */
> +#define ENET_R_FIFO_SEM	0x194 /* Rx FIFO empty threshold */
> +#define ENET_R_FIFO_AEM	0x198 /* Rx FIFO almost empty threshold */
> +#define ENET_R_FIFO_AFL	0x19c /* Rx FIFO almost full threshold */
> +#define ENET_FRAME_TRL	0x1b0 /* Frame truncation length */
> +#define ENET_RACC	0x1c4 /* Receive Accelerator function configuration*/
> +#define ENET_RCM1	0x1c8 /* Receive classification match register ring1 */
> +#define ENET_RCM2	0x1cc /* Receive classification match register ring2 */
> +#define ENET_DMA1CFG	0x1d8 /* DMA class based configuration ring1 */
> +#define ENET_DMA2CFG	0x1dc /* DMA class based Configuration ring2 */
> +#define ENET_RDAR_1	0x1e0 /* Rx descriptor active register ring1 */
> +#define ENET_TDAR_1	0x1e4 /* Tx descriptor active register ring1 */
> +#define ENET_RDAR_2	0x1e8 /* Rx descriptor active register ring2 */
> +#define ENET_TDAR_2	0x1ec /* Tx descriptor active register ring2 */
> +#define ENET_MII_GSK_CFGR	0x300 /* MII_GSK Configuration register */
> +#define ENET_MII_GSK_ENR		0x308 /* MII_GSK Enable register*/
> +
> +#define BM_MII_GSK_CFGR_MII		0x00
> +#define BM_MII_GSK_CFGR_RMII		0x01
> +#define BM_MII_GSK_CFGR_FRCONT_10M	0x40
We may remove all MII_* defines as will not use same. Please check.

> +
> +/* full duplex or half duplex */
> +#define HALF_DUPLEX             0x00
> +#define FULL_DUPLEX             0x01
> +#define UNKNOWN_DUPLEX          0xff


Duplicate Macros definition.  Also defined in "enet_ethdev.h"
> +
> +#endif /*__ENET_REGS_H */
> diff --git a/drivers/net/enetfec/enet_uio.c b/drivers/net/enetfec/enet_uio.c
> new file mode 100644
> index 000000000..b64dc522e
> --- /dev/null
> +++ b/drivers/net/enetfec/enet_uio.c
> @@ -0,0 +1,192 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2021 NXP
> + */
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <dirent.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +
> +#include <rte_common.h>
> +#include <rte_malloc.h>
> +#include "enet_pmd_logs.h"
> +#include "enet_uio.h"
> +
> +static struct uio_job enetfec_uio_job;
> +int count;
> +
> +/** @brief Reads first line from a file.
> + * Composes file name as: root/subdir/filename
> + *
> + * @param [in]  root     Root path
> + * @param [in]  subdir   Subdirectory name
> + * @param [in]  filename File name
> + * @param [out] line     The first line read from file.
> + *
> + * @retval 0 for succes
> + * @retval other value for error
> + */
> +static int
> +file_read_first_line(const char root[], const char subdir[],
> +			const char filename[], char *line)
> +{
> +	char absolute_file_name[FEC_UIO_MAX_ATTR_FILE_NAME];
> +	int fd = 0, ret = 0;
> +
> +	/*compose the file name: root/subdir/filename */
> +	memset(absolute_file_name, 0, sizeof(absolute_file_name));
> +	snprintf(absolute_file_name, FEC_UIO_MAX_ATTR_FILE_NAME,
> +		"%s/%s/%s", root, subdir, filename);
> +
> +	fd = open(absolute_file_name, O_RDONLY);
> +	if (fd <= 0)
> +		ENET_PMD_ERR("Error opening file %s", absolute_file_name);
> +
> +	/* read UIO device name from first line in file */
> +	ret = read(fd, line, FEC_UIO_MAX_DEVICE_FILE_NAME_LENGTH);
> +	close(fd);
> +
> +	/* NULL-ify string */
> +	line[FEC_UIO_MAX_DEVICE_FILE_NAME_LENGTH - 1] = '\0';

We must NULL-ify after actual number of bytes read.
line[ret] ='\0';

Also, set this after checking ret value.
> +
> +	if (ret <= 0) {
> +		ENET_PMD_ERR("Error reading from file %s", absolute_file_name);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/** @brief Maps rx-tx bd range assigned for a bd ring.
> + *
> + * @param [in] uio_device_fd    UIO device file descriptor
> + * @param [in] uio_device_id    UIO device id
> + * @param [in] uio_map_id       UIO allows maximum 5 different mapping for
> +				each device. Maps start with id 0.
> + * @param [out] map_size        Map size.
> + * @param [out] map_addr	Map physical address
> + * @retval  NULL if failed to map registers
> + * @retval  Virtual address for mapped register address range
> + */
> +static void *
> +uio_map_mem(int uio_device_fd, int uio_device_id,
> +		int uio_map_id, int *map_size, uint64_t *map_addr)
> +{
> +	void *mapped_address = NULL;
> +	unsigned int uio_map_size = 0;
> +	unsigned int uio_map_p_addr = 0;
> +	char uio_sys_root[FEC_UIO_MAX_ATTR_FILE_NAME];
> +	char uio_sys_map_subdir[FEC_UIO_MAX_ATTR_FILE_NAME];
> +	char uio_map_size_str[32];
must be

uio_map_size_str[FEC_UIO_MAX_DEVICE_FILE_NAME_LENGTH + 1];

> +	char uio_map_p_addr_str[64];
> +	int ret = 0;
> +
> +	/* compose the file name: root/subdir/filename */
> +	memset(uio_sys_root, 0, sizeof(uio_sys_root));
> +	memset(uio_sys_map_subdir, 0, sizeof(uio_sys_map_subdir));
> +	memset(uio_map_size_str, 0, sizeof(uio_map_size_str));
> +	memset(uio_map_p_addr_str, 0, sizeof(uio_map_p_addr_str));
> +
> +	/* Compose string: /sys/class/uio/uioX */
> +	snprintf(uio_sys_root, sizeof(uio_sys_root), "%s/%s%d",
> +			FEC_UIO_DEVICE_SYS_ATTR_PATH, "uio", uio_device_id);
> +	/* Compose string: maps/mapY */
> +	snprintf(uio_sys_map_subdir, sizeof(uio_sys_map_subdir), "%s%d",
> +			FEC_UIO_DEVICE_SYS_MAP_ATTR, uio_map_id);
> +
> +	/* Read first (and only) line from file
> +	 * /sys/class/uio/uioX/maps/mapY/size
> +	 */
> +	ret = file_read_first_line(uio_sys_root, uio_sys_map_subdir,
> +				"size", uio_map_size_str);
> +	if (ret)
check with 0. Please handle it every where.
> +		ENET_PMD_ERR("file_read_first_line() failed");
return NULL on error.
> +
> +	ret = file_read_first_line(uio_sys_root, uio_sys_map_subdir,
> +				"addr", uio_map_p_addr_str);
the file_read_first_line() will read only 
FEC_UIO_MAX_DEVICE_FILE_NAME_LENGTH bytes. If that is sufficient then why

uio_map_p_addr_str[] has length of 64?

> +	if (ret)
> +		ENET_PMD_ERR("file_read_first_line() failed");
return NULL on error.
> +
> +	/* Read mapping size and physical address expressed in hexa(base 16) */
> +	uio_map_size = strtol(uio_map_size_str, NULL, 16);
> +	uio_map_p_addr = strtol(uio_map_p_addr_str, NULL, 16);
> +
> +	if (uio_map_id == 0) {
> +		/* Map the register address in user space when map_id is 0 */
> +		mapped_address = mmap(0 /*dynamically choose virtual address */,
> +				uio_map_size, PROT_READ | PROT_WRITE,
> +				MAP_SHARED, uio_device_fd, 0);
> +	} else {
> +		/* Map the BD memory in user space */
> +		mapped_address = mmap(NULL, uio_map_size,
> +				PROT_READ | PROT_WRITE,
> +				MAP_SHARED, uio_device_fd, (1 * MAP_PAGE_SIZE));
> +	}
> +
> +	if (mapped_address == MAP_FAILED) {
> +		ENET_PMD_ERR("Failed to map! errno = %d uio job fd = %d,"
> +			"uio device id = %d, uio map id = %d", errno,
> +			uio_device_fd, uio_device_id, uio_map_id);
> +		return 0;
return NULL on error.
> +	}
> +
> +	/* Save the map size to use it later on for munmap-ing */
> +	*map_size = uio_map_size;
> +	*map_addr = uio_map_p_addr;
> +	ENET_PMD_INFO("UIO dev[%d] mapped region [id =%d] size 0x%x at %p",
> +		uio_device_id, uio_map_id, uio_map_size, mapped_address);
> +
> +	return mapped_address;
> +}
> +
> +int
> +config_enetfec_uio(struct enetfec_private *fep)
> +{
> +	char uio_device_file_name[32];
> +	struct uio_job *uio_job = NULL;
> +
> +	/* Mapping is done only one time */
> +	if (count) {

Suggestion: may be we can use any self explanatory flag name like, "mapped".
> +		printf("Mapping already done, can't map again!\n");
> +		return 0;
> +	}
> +
> +	uio_job = &enetfec_uio_job;
> +
> +	/* Find UIO device created by ENETFEC-UIO kernel driver */
> +	memset(uio_device_file_name, 0, sizeof(uio_device_file_name));
> +	snprintf(uio_device_file_name, sizeof(uio_device_file_name), "%s%d",
> +			FEC_UIO_DEVICE_FILE_NAME, uio_job->uio_minor_number);
> +
> +	/* Open device file */
> +	uio_job->uio_fd = open(uio_device_file_name, O_RDWR);
> +	if (uio_job->uio_fd < 0) {
> +		printf("US_UIO: Open Failed\n");
> +		exit(1);
> +	}
> +
> +	ENET_PMD_INFO("US_UIO: Open device(%s) file with uio_fd = %d",
> +			uio_device_file_name, uio_job->uio_fd);
> +
> +	fep->hw_baseaddr_v = uio_map_mem(uio_job->uio_fd,
> +		uio_job->uio_minor_number, FEC_UIO_REG_MAP_ID,
> +		&uio_job->map_size, &uio_job->map_addr);
Check for NULL return.
> +	fep->hw_baseaddr_p = uio_job->map_addr;
> +	fep->reg_size = uio_job->map_size;
> +
> +	fep->bd_addr_v = uio_map_mem(uio_job->uio_fd,
Check for NULL return.
> +		uio_job->uio_minor_number, FEC_UIO_BD_MAP_ID,
> +		&uio_job->map_size, &uio_job->map_addr);
> +	fep->bd_addr_p = uio_job->map_addr;
> +	fep->bd_size = uio_job->map_size;
> +
> +	count++;
> +
> +	return 0;
> +}
> diff --git a/drivers/net/enetfec/enet_uio.h b/drivers/net/enetfec/enet_uio.h
> new file mode 100644
> index 000000000..b220cae9d
> --- /dev/null
> +++ b/drivers/net/enetfec/enet_uio.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2021 NXP
> + */
> +
> +#include "enet_ethdev.h"
> +
> +/* Prefix path to sysfs directory where UIO device attributes are exported.
> + * Path for UIO device X is /sys/class/uio/uioX
> + */
> +#define FEC_UIO_DEVICE_SYS_ATTR_PATH	"/sys/class/uio"
> +
> +/* Subfolder in sysfs where mapping attributes are exported
> + * for each UIO device. Path for mapping Y for device X is:
> + * /sys/class/uio/uioX/maps/mapY
> + */
> +#define FEC_UIO_DEVICE_SYS_MAP_ATTR	"maps/map"
> +
> +/* Name of UIO device file prefix. Each UIO device will have a device file
> + * /dev/uioX, where X is the minor device number.
> + */
> +#define FEC_UIO_DEVICE_FILE_NAME	"/dev/uio"
> +
> +/* Maximum length for the name of an UIO device file.
> + * Device file name format is: /dev/uioX.
> + */
> +#define FEC_UIO_MAX_DEVICE_FILE_NAME_LENGTH	30
> +
> +/* Maximum length for the name of an attribute file for an UIO device.
> + * Attribute files are exported in sysfs and have the name formatted as:
> + * /sys/class/uio/uioX/<attribute_file_name>
> + */
> +#define FEC_UIO_MAX_ATTR_FILE_NAME	100
> +
> +/* The id for the mapping used to export ENETFEC registers and BD memory to
> + * user space through UIO device.
> + */
> +#define FEC_UIO_REG_MAP_ID		0
> +#define FEC_UIO_BD_MAP_ID		1
> +
> +#define MAP_PAGE_SIZE			4096
> +
> +struct uio_job {
> +	uint32_t fec_id;
> +	int uio_fd;
> +	void *bd_start_addr;
> +	void *register_base_addr;
> +	int map_size;
> +	uint64_t map_addr;
> +	int uio_minor_number;
> +};
> +
> +int config_enetfec_uio(struct enetfec_private *fep);
> +void enetfec_uio_init(void);
> +void enetfec_cleanup(void);
> diff --git a/drivers/net/enetfec/meson.build b/drivers/net/enetfec/meson.build
> index 252bf8330..05183bd44 100644
> --- a/drivers/net/enetfec/meson.build
> +++ b/drivers/net/enetfec/meson.build
> @@ -8,7 +8,8 @@ endif
>   
>   deps += ['common_dpaax']
>   
> -sources = files('enet_ethdev.c')
> +sources = files('enet_ethdev.c',
> +		'enet_uio.c')
>   
>   if cc.has_argument('-Wno-pointer-arith')
>   	cflags += '-Wno-pointer-arith'


  parent reply	other threads:[~2021-07-04  4:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-30  4:34 [dpdk-dev] [PATCH 0/4] drivers/net: add NXP ENETFEC driver Apeksha Gupta
2021-04-30  4:34 ` [dpdk-dev] [PATCH 1/4] drivers/net/enetfec: Introduce " Apeksha Gupta
2021-06-08 13:10   ` Andrew Rybchenko
2021-07-02 13:55     ` David Marchand
2021-07-04  2:57   ` Sachin Saxena (OSS)
2021-04-30  4:34 ` [dpdk-dev] [PATCH 2/4] drivers/net/enetfec: UIO support added Apeksha Gupta
2021-06-08 13:21   ` Andrew Rybchenko
2021-07-04  4:27   ` Sachin Saxena (OSS) [this message]
2021-04-30  4:34 ` [dpdk-dev] [PATCH 3/4] drivers/net/enetfec: queue configuration Apeksha Gupta
2021-06-08 13:38   ` Andrew Rybchenko
2021-07-04  6:46   ` Sachin Saxena (OSS)
2021-04-30  4:34 ` [dpdk-dev] [PATCH 4/4] drivers/net/enetfec: add enqueue and dequeue support Apeksha Gupta
2021-06-08 13:42   ` Andrew Rybchenko
2021-06-21  9:14     ` [dpdk-dev] [EXT] " Apeksha Gupta
2021-07-05  8:48   ` [dpdk-dev] " Sachin Saxena (OSS)
2021-05-04 15:40 ` [dpdk-dev] [PATCH 0/4] drivers/net: add NXP ENETFEC driver Ferruh Yigit
2021-07-04  2:55 ` Sachin Saxena (OSS)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b54a36b5-fcdc-441a-27b6-54a83e04ce29@oss.nxp.com \
    --to=sachin.saxena@oss.nxp.com \
    --cc=apeksha.gupta@nxp.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hemant.agrawal@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git