[v2,02/13] net/ppfe: introduce ppfe net poll mode driver

Message ID 20190828110849.14759-3-g.singh@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series introduces ppfe network PMD |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Gagandeep Singh Aug. 28, 2019, 11:08 a.m. UTC
  ppfe (programmable packet forwarding engine)
is a network poll mode driver for NXP SoC
ls1012a.

This patch introduces the framework of ppfe
driver with basic functions of initialisation
and teardown.

Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
Acked-by: Nipun Gupta <nipun.gupta@nxp.com>
---
 MAINTAINERS                               |   6 +
 config/common_base                        |   5 +
 config/common_linux                       |   5 +
 doc/guides/nics/features/ppfe.ini         |   8 +
 drivers/net/Makefile                      |   1 +
 drivers/net/meson.build                   |   1 +
 drivers/net/ppfe/Makefile                 |  28 ++
 drivers/net/ppfe/meson.build              |  11 +
 drivers/net/ppfe/pfe_eth.h                |  77 +++++
 drivers/net/ppfe/pfe_mod.h                |  51 ++++
 drivers/net/ppfe/ppfe_ethdev.c            | 329 ++++++++++++++++++++++
 drivers/net/ppfe/rte_pmd_ppfe_version.map |   4 +
 mk/rte.app.mk                             |   1 +
 13 files changed, 527 insertions(+)
 create mode 100644 doc/guides/nics/features/ppfe.ini
 create mode 100644 drivers/net/ppfe/Makefile
 create mode 100644 drivers/net/ppfe/meson.build
 create mode 100644 drivers/net/ppfe/pfe_eth.h
 create mode 100644 drivers/net/ppfe/pfe_mod.h
 create mode 100644 drivers/net/ppfe/ppfe_ethdev.c
 create mode 100644 drivers/net/ppfe/rte_pmd_ppfe_version.map
  

Comments

Ferruh Yigit Sept. 26, 2019, 4:53 p.m. UTC | #1
On 8/28/2019 12:08 PM, Gagandeep Singh wrote:
> ppfe (programmable packet forwarding engine)
> is a network poll mode driver for NXP SoC
> ls1012a.
> 
> This patch introduces the framework of ppfe
> driver with basic functions of initialisation
> and teardown.
> 
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> Acked-by: Nipun Gupta <nipun.gupta@nxp.com>
<...>

> @@ -0,0 +1,8 @@
> +;
> +; Supported features of the 'ppfe' network poll mode driver.
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +[Features]
> +Linux VFIO           = Y
> +ARMv8                = Y

Only arm? Should this be reflected to config file? like enabling it only in
'common_armv8a_linux'.


<...>

> +struct pfe_eth {
> +	struct pfe_eth_priv_s *eth_priv[3];

Why hardcoded 3? Can it use PFE_CDEV_ETH_COUNT (if they are for same thing) ?

<...>

> +
> +/* for link status and IOCTL support using pfe character device
> + * XXX: Should be kept in sync with Kernel module

How can we know if it is in sync with kernel module, is there any check in the code?

> + */
> +
> +/* Extracted from ls1012a_pfe_platform_data, there are 3 interfaces which are
> + * supported by PFE driver. Should be updated if number of eth devices are
> + * changed.
> + */
> +#define PFE_CDEV_ETH_COUNT 3
> +
> +#define PFE_CDEV_PATH		"/dev/pfe_us_cdev"

Can you please add a comment why this char device is used?

<...>

> +struct pfe *g_pfe;

Any reason to not make this static?

> +unsigned int pfe_svr = SVR_LS1012A_REV1;

There are some checks for this in hal layer, should this be a runtime option
instead of hardcoded in the .c file?

<...>

> +/* pfe_eth_exit
> + */
> +static void
> +pfe_eth_exit(struct rte_eth_dev *dev, struct pfe *pfe)
> +{
> +	/* Close the device file for link status */
> +	pfe_eth_close_cdev(dev->data->dev_private);

You may want to do stop before close.

> +
> +	rte_eth_dev_release_port(dev);
> +	pfe->nb_devs--;
> +}
> +
> +/* pfe_eth_init_one

Comment is wrong, and not sure commenting the function name is that useful

> + */
> +static int pfe_eth_init(struct rte_vdev_device *vdev, struct pfe *pfe, int id)

Please apply same syntax to function prototypes.

> +{
> +	struct rte_eth_dev_data *data = NULL;
> +	struct rte_eth_dev *eth_dev = NULL;
> +	struct pfe_eth_priv_s *priv = NULL;
> +	int err;
> +
> +	if (id >= pfe->max_intf)
> +		return -EINVAL;

Can this happen at this stage?

> +
> +	data = rte_zmalloc(NULL, sizeof(*data), 64);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	eth_dev = rte_eth_vdev_allocate(vdev, sizeof(*priv));
> +	if (eth_dev == NULL) {
> +		rte_free(data);
> +		return -ENOMEM;
> +	}
> +
> +	priv = eth_dev->data->dev_private;
> +	rte_memcpy(data, eth_dev->data, sizeof(*data));

Why you allocate and copy to 'data', as far as I can see you are not using it at
all J Unless I am missing something.

<...>

> +
> +	/* For link status, open the PFE CDEV; Error from this function
> +	 * is silently ignored; In case of error, the link status will not
> +	 * be available.
> +	 */
> +	pfe_eth_open_cdev(priv);
> +	rte_eth_dev_probing_finish(eth_dev);
> +
> +	return 0;
> +err0:
> +	rte_free(data);

Need to release the 'eth_dev', rte_eth_dev_release_port()

> +	return err;
> +}
> +
> +/* Parse integer from integer argument */
> +static int
> +parse_integer_arg(const char *key __rte_unused,
> +		const char *value, void *extra_args)
> +{
> +	int *i = (int *)extra_args;
> +
> +	*i = atoi(value);
> +	if (*i < 0)

If you are trying to detect the error, atoi is not working that way, 'strtol'
can be better option for that.

> +		return -1;
> +
> +	return 0;
> +}
> +
> +static int
> +pfe_parse_vdev_init_params(struct pfe_vdev_init_params *params,
> +				struct rte_vdev_device *dev)
> +{
> +	struct rte_kvargs *kvlist = NULL;
> +	int ret = 0;
> +
> +	static const char * const pfe_vdev_valid_params[] = {
> +		PPFE_VDEV_GEM_ID_ARG,
> +		NULL
> +	};
> +
> +	const char *input_args = rte_vdev_device_args(dev);
> +	if (params == NULL)
> +		return -EINVAL;

This is a static function and only used once, not sure about verifying the input
param, although it won't hurt.

> +
> +
> +	if (input_args) {

Not sure if 'input_args' can bu NULL at all, but anyway, what about retun
immediately if it is NULL, it can reduce the indentation.


Also if the PPFE_VDEV_GEM_ID_ARG is optional and you want to check if user
provided it or not, you should check the value of the 'input_args' instead of
its address I think.

> +		kvlist = rte_kvargs_parse(input_args, pfe_vdev_valid_params);
> +		if (kvlist == NULL)
> +			return -1;
> +
> +		ret = rte_kvargs_process(kvlist,
> +					PPFE_VDEV_GEM_ID_ARG,
> +					&parse_integer_arg,
> +					&params->gem_id);
> +		if (ret < 0)
> +			goto free_kvlist;
> +	}
> +
> +free_kvlist:
> +	rte_kvargs_free(kvlist);
> +	return ret;
> +}
> +
> +static int
> +pmd_pfe_probe(struct rte_vdev_device *vdev)
> +{
> +	const u32 *prop;
> +	const struct device_node *np;
> +	const char *name;
> +	const uint32_t *addr;
> +	uint64_t cbus_addr, ddr_size, cbus_size;
> +	int rc = -1, fd = -1, gem_id;
> +	unsigned int interface_count = 0;
> +	size_t size = 0;
> +	struct pfe_vdev_init_params init_params = {
> +		-1

I would prefer designated initializes to make it more clear, up to you.

> +	};
> +
> +	name = rte_vdev_device_name(vdev);
> +	rc = pfe_parse_vdev_init_params(&init_params, vdev);
> +	if (rc < 0)
> +		return -EINVAL;
> +
> +	if (g_pfe) {
> +		if (g_pfe->nb_devs >= g_pfe->max_intf)
> +			return -EINVAL;
> +
> +		goto eth_init;
> +	}
> +
> +	g_pfe = rte_zmalloc(NULL, sizeof(*g_pfe), 64);
If 64 is for cache line, may be better to use macro for it.

> +	if (g_pfe == NULL)
> +		return  -EINVAL;

Is there a value of dynamically allocate the g_pfe, it is already global, so why
not define it statically? May be secondary process can have benefit from
allocating in shared memory but you are not using shared process as far as I can
see.

> +
> +	/* Load the device-tree driver */
> +	rc = of_init();
> +	if (rc)
> +		goto err;
> +
> +	np = of_find_compatible_node(NULL, NULL, "fsl,pfe");
> +	if (!np) {
> +		rc = -EINVAL;
> +		goto err;
> +	}
> +
> +	addr = of_get_address(np, 0, &cbus_size, NULL);
> +	if (!addr)
> +		goto err;
> +
> +	cbus_addr = of_translate_address(np, addr);
> +	if (!cbus_addr)
> +		goto err;
> +
> +
> +	addr = of_get_address(np, 1, &ddr_size, NULL);
> +	if (!addr)
> +		goto err;
> +
> +	g_pfe->ddr_phys_baseaddr = of_translate_address(np, addr);
> +	if (!g_pfe->ddr_phys_baseaddr)
> +		goto err;
> +
> +	g_pfe->ddr_size = ddr_size;
> +
> +	fd = open("/dev/mem", O_RDWR);
> +	g_pfe->cbus_baseaddr = mmap(NULL, cbus_size, PROT_READ | PROT_WRITE,
> +					MAP_SHARED, fd, cbus_addr);

Above reads device information from a device tree file and creates a new mapping
for its address space. I wonder if device can be probbed as we do the pcie
devices, do you have to depend on the 'of', can't the driver registered for a
pysical device?

<...>

> +	rc = pfe_eth_init(vdev, g_pfe, gem_id);
> +	if (rc < 0)
> +		goto err_eth;
> +	else
> +		g_pfe->nb_devs++;
> +
> +	return 0;
> +
> +err_eth:
> +err_prop:

Should 'munmap' here? and close fd?

<...>

> +RTE_PMD_REGISTER_VDEV(PFE_PMD, pmd_pfe_drv);
> +RTE_PMD_REGISTER_ALIAS(PFE_PMD, eth_pfe);

Please drop the alias, that is for compatibility for old pmds.

> +RTE_PMD_REGISTER_PARAM_STRING(PFE_PMD, "intf=<int> ");

Better to use 'PPFE_VDEV_GEM_ID_ARG' macro here to prevent typos etc...
  
Gagandeep Singh Oct. 1, 2019, 7:05 a.m. UTC | #2
Hi Ferruh,

I have incorporated most of your comments in V3 of this series. I'll send the series by today.
For those, which will not be included in this version, I have added some comments, please see inline.

> <...>
> 
> > +
> > +/* for link status and IOCTL support using pfe character device
> > + * XXX: Should be kept in sync with Kernel module
> 
> How can we know if it is in sync with kernel module, is there any check in the
> code?
There is no check for this in the code, but it is mandatory to load "pfe.ko"  kernel
Module to run PFFE driver, I have added this requirement in document under prerequisites section.
 
<...>
> 
> > +unsigned int pfe_svr = SVR_LS1012A_REV1;
> 
> There are some checks for this in hal layer, should this be a runtime option
> instead of hardcoded in the .c file?
I agree with you, It should be a runtime option. Is it ok, if I send a separate patch
for this? It may take some time as I have to look into the manual to find possible ways
to get svr value from the HW. I'll add a comment here for this.

<...>
> > +	g_pfe->ddr_size = ddr_size;
> > +
> > +	fd = open("/dev/mem", O_RDWR);
> > +	g_pfe->cbus_baseaddr = mmap(NULL, cbus_size, PROT_READ |
> PROT_WRITE,
> > +					MAP_SHARED, fd, cbus_addr);
> 
> Above reads device information from a device tree file and creates a new
> mapping
> for its address space. I wonder if device can be probbed as we do the pcie
> devices, do you have to depend on the 'of', can't the driver registered for a
> pysical device?
> 
These eth devices are actual Virtual interfaces created by Client driver (a part
of the ppfe driver).  Actually, HW provides a single interface (Host interface), which
is the only device accessible by the userspace driver for enqueue and dequeue and ppfe driver create
two virtual interfaces out of this single host interface. Also, this device is not on pcie bus.

Thanks,
Gagan
  

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 410026086..7996dcdd8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -781,6 +781,12 @@  F: drivers/net/enetc/
 F: doc/guides/nics/enetc.rst
 F: doc/guides/nics/features/enetc.ini
 
+NXP ppfe
+M: Gagandeep Singh <g.singh@nxp.com>
+M: Akhil Goyal <akhil.goyal@nxp.com>
+F: drivers/net/ppfe/
+F: doc/guides/nics/features/ppfe.ini
+
 QLogic bnx2x
 M: Rasesh Mody <rmody@marvell.com>
 M: Shahed Shaikh <shshaikh@marvell.com>
diff --git a/config/common_base b/config/common_base
index 8ef75c203..1776a8fa8 100644
--- a/config/common_base
+++ b/config/common_base
@@ -224,6 +224,11 @@  CONFIG_RTE_LIBRTE_CXGBE_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_CXGBE_DEBUG_RX=n
 CONFIG_RTE_LIBRTE_CXGBE_TPUT=y
 
+#
+# Compile burst-oriented NXP PPFE PMD driver
+#
+CONFIG_RTE_LIBRTE_PPFE_PMD=n
+
 # NXP DPAA Bus
 CONFIG_RTE_LIBRTE_DPAA_BUS=n
 CONFIG_RTE_LIBRTE_DPAA_MEMPOOL=n
diff --git a/config/common_linux b/config/common_linux
index 6e252553a..2ba70ba95 100644
--- a/config/common_linux
+++ b/config/common_linux
@@ -59,6 +59,11 @@  CONFIG_RTE_LIBRTE_PMD_DPAA2_QDMA_RAWDEV=y
 #
 CONFIG_RTE_LIBRTE_ENETC_PMD=y
 
+#
+# NXP PPFE PMD Driver
+#
+CONFIG_RTE_LIBRTE_PPFE_PMD=y
+
 #
 # HINIC PMD driver
 #
diff --git a/doc/guides/nics/features/ppfe.ini b/doc/guides/nics/features/ppfe.ini
new file mode 100644
index 000000000..4bcac779c
--- /dev/null
+++ b/doc/guides/nics/features/ppfe.ini
@@ -0,0 +1,8 @@ 
+;
+; Supported features of the 'ppfe' network poll mode driver.
+;
+; Refer to default.ini for the full list of available PMD features.
+;
+[Features]
+Linux VFIO           = Y
+ARMv8                = Y
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 5767fdf65..b59cdbb83 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -49,6 +49,7 @@  DIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += null
 DIRS-$(CONFIG_RTE_LIBRTE_OCTEONTX_PMD) += octeontx
 DIRS-$(CONFIG_RTE_LIBRTE_OCTEONTX2_PMD) += octeontx2
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += pcap
+DIRS-$(CONFIG_RTE_LIBRTE_PPFE_PMD) += ppfe
 DIRS-$(CONFIG_RTE_LIBRTE_QEDE_PMD) += qede
 DIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) += ring
 DIRS-$(CONFIG_RTE_LIBRTE_SFC_EFX_PMD) += sfc
diff --git a/drivers/net/meson.build b/drivers/net/meson.build
index 513f19b33..a3be718a6 100644
--- a/drivers/net/meson.build
+++ b/drivers/net/meson.build
@@ -37,6 +37,7 @@  drivers = ['af_packet',
 	'octeontx',
 	'octeontx2',
 	'pcap',
+	'ppfe',
 	'ring',
 	'sfc',
 	'softnic',
diff --git a/drivers/net/ppfe/Makefile b/drivers/net/ppfe/Makefile
new file mode 100644
index 000000000..b9e894ffd
--- /dev/null
+++ b/drivers/net/ppfe/Makefile
@@ -0,0 +1,28 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2019 NXP
+#
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+LIB = librte_pmd_ppfe.a
+
+CFLAGS += -O3 $(WERROR_FLAGS)
+CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa/include/
+CFLAGS += -I$(RTE_SDK)/drivers/common/dpaax
+
+EXPORT_MAP := rte_pmd_ppfe_version.map
+LIBABIVER := 1
+
+# Interfaces with DPDK
+SRCS-$(CONFIG_RTE_LIBRTE_PPFE_PMD) += ppfe_ethdev.c
+
+LDLIBS += -lrte_bus_vdev
+LDLIBS += -lrte_bus_dpaa
+LDLIBS += -lrte_common_dpaax
+LDLIBS += -lrte_eal
+LDLIBS += -lrte_ethdev -lrte_kvargs
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/ppfe/meson.build b/drivers/net/ppfe/meson.build
new file mode 100644
index 000000000..8ca2cb87d
--- /dev/null
+++ b/drivers/net/ppfe/meson.build
@@ -0,0 +1,11 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2019 NXP
+
+if host_machine.system() != 'linux'
+	build = false
+endif
+deps += ['bus_dpaa']
+
+sources = files('ppfe_ethdev.c')
+
+includes += include_directories('../../bus/dpaa/include/')
diff --git a/drivers/net/ppfe/pfe_eth.h b/drivers/net/ppfe/pfe_eth.h
new file mode 100644
index 000000000..a8ce3e314
--- /dev/null
+++ b/drivers/net/ppfe/pfe_eth.h
@@ -0,0 +1,77 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2019 NXP
+ */
+
+#ifndef _PFE_ETH_H_
+#define _PFE_ETH_H_
+
+#include <compat.h>
+#include <rte_ethdev.h>
+#include <rte_ethdev_vdev.h>
+
+#define ETH_ALEN 6
+#define GEMAC_NO_PHY            BIT(0)
+
+#define PFE_SOC_ID_FILE	"/sys/devices/soc0/soc_id"
+extern unsigned int pfe_svr;
+#define SVR_LS1012A_REV2	0x87040020
+#define SVR_LS1012A_REV1	0x87040010
+
+struct ls1012a_eth_platform_data {
+	/* device specific information */
+	u32 device_flags;
+	char name[16];
+
+	/* board specific information */
+	u32 mii_config;
+	u32 phy_flags;
+	u32 gem_id;
+	u32 bus_id;
+	u32 phy_id;
+	u32 mdio_muxval;
+	u8 mac_addr[ETH_ALEN];
+};
+
+struct ls1012a_mdio_platform_data {
+	int enabled;
+	int irq[32];
+	u32 phy_mask;
+	int mdc_div;
+};
+
+struct ls1012a_pfe_platform_data {
+	struct ls1012a_eth_platform_data ls1012a_eth_pdata[3];
+	struct ls1012a_mdio_platform_data ls1012a_mdio_pdata[3];
+};
+
+#define EMAC_TXQ_CNT	16
+#define EMAC_TXQ_DEPTH	(HIF_TX_DESC_NT)
+
+#define JUMBO_FRAME_SIZE	10258
+#define EMAC_RXQ_CNT	1
+#define EMAC_RXQ_DEPTH	HIF_RX_DESC_NT
+
+struct  pfe_eth_priv_s {
+	struct pfe		*pfe;
+	int			low_tmu_q;
+	int			high_tmu_q;
+	struct rte_eth_dev	*ndev;
+	struct rte_eth_stats	stats;
+	int			id;
+	int			promisc;
+	int			link_fd;
+
+	spinlock_t		lock; /* protect member variables */
+	void			*EMAC_baseaddr;
+	/* This points to the EMAC base from where we access PHY */
+	void			*PHY_baseaddr;
+	void			*GPI_baseaddr;
+
+	struct ls1012a_eth_platform_data *einfo;
+};
+
+struct pfe_eth {
+	struct pfe_eth_priv_s *eth_priv[3];
+};
+
+#endif /* _PFE_ETH_H_ */
diff --git a/drivers/net/ppfe/pfe_mod.h b/drivers/net/ppfe/pfe_mod.h
new file mode 100644
index 000000000..61db998e5
--- /dev/null
+++ b/drivers/net/ppfe/pfe_mod.h
@@ -0,0 +1,51 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2019 NXP
+ */
+
+#ifndef _PFE_MOD_H_
+#define _PFE_MOD_H_
+
+struct pfe;
+
+#include "pfe_eth.h"
+
+#define PHYID_MAX_VAL 32
+
+struct pfe {
+	uint64_t ddr_phys_baseaddr;
+	void *ddr_baseaddr;
+	uint64_t ddr_size;
+	void *cbus_baseaddr;
+	struct pfe_eth eth;
+	int mdio_muxval[PHYID_MAX_VAL];
+	uint8_t nb_devs;
+	uint8_t max_intf;
+	int cdev_fd;
+};
+
+/* for link status and IOCTL support using pfe character device
+ * XXX: Should be kept in sync with Kernel module
+ */
+
+/* Extracted from ls1012a_pfe_platform_data, there are 3 interfaces which are
+ * supported by PFE driver. Should be updated if number of eth devices are
+ * changed.
+ */
+#define PFE_CDEV_ETH_COUNT 3
+
+#define PFE_CDEV_PATH		"/dev/pfe_us_cdev"
+#define PFE_CDEV_INVALID_FD	-1
+
+/* used when 'read' call is issued, returning PFE_CDEV_ETH_COUNT number of
+ * pfe_shared_info as array.
+ */
+struct pfe_shared_info {
+	uint32_t phy_id; /* Link phy ID */
+	uint8_t state;  /* Has either 0 or 1 */
+};
+
+/* IOCTL Commands */
+#define PFE_CDEV_ETH0_STATE_GET		_IOR('R', 0, int)
+#define PFE_CDEV_ETH1_STATE_GET		_IOR('R', 1, int)
+#define PFE_CDEV_HIF_INTR_EN		_IOWR('R', 2, int)
+#endif /* _PFE_MOD_H */
diff --git a/drivers/net/ppfe/ppfe_ethdev.c b/drivers/net/ppfe/ppfe_ethdev.c
new file mode 100644
index 000000000..f31a11b93
--- /dev/null
+++ b/drivers/net/ppfe/ppfe_ethdev.c
@@ -0,0 +1,329 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2019 NXP
+ */
+
+#include <rte_kvargs.h>
+#include <rte_ethdev_vdev.h>
+#include <rte_bus_vdev.h>
+#include <of.h>
+
+#include "pfe_mod.h"
+
+#define PPFE_MAX_MACS 1 /*we can support upto 4 MACs per IF*/
+#define PPFE_VDEV_GEM_ID_ARG	("intf")
+
+struct pfe_vdev_init_params {
+	int8_t	gem_id;
+};
+struct pfe *g_pfe;
+unsigned int pfe_svr = SVR_LS1012A_REV1;
+
+static void
+pfe_soc_version_get(void)
+{
+	FILE *svr_file = NULL;
+	unsigned int svr_ver = 0;
+
+	svr_file = fopen(PFE_SOC_ID_FILE, "r");
+	if (!svr_file)
+		return; /* Not supported on this infra */
+
+	if (fscanf(svr_file, "svr:%x", &svr_ver) > 0)
+		pfe_svr = svr_ver;
+	else
+		printf("Unable to read SoC device");
+
+	fclose(svr_file);
+}
+
+static int
+pfe_eth_open_cdev(struct pfe_eth_priv_s *priv)
+{
+	int pfe_cdev_fd;
+
+	if (priv == NULL)
+		return -1;
+
+	pfe_cdev_fd = open(PFE_CDEV_PATH, O_RDONLY);
+	if (pfe_cdev_fd < 0) {
+		priv->link_fd = PFE_CDEV_INVALID_FD;
+		return -1;
+	}
+
+	priv->link_fd = pfe_cdev_fd;
+
+	return 0;
+}
+
+static void
+pfe_eth_close_cdev(struct pfe_eth_priv_s *priv)
+{
+	if (priv == NULL)
+		return;
+
+	if (priv->link_fd != PFE_CDEV_INVALID_FD) {
+		close(priv->link_fd);
+		priv->link_fd = PFE_CDEV_INVALID_FD;
+	}
+}
+
+/* pfe_eth_exit
+ */
+static void
+pfe_eth_exit(struct rte_eth_dev *dev, struct pfe *pfe)
+{
+	/* Close the device file for link status */
+	pfe_eth_close_cdev(dev->data->dev_private);
+
+	rte_eth_dev_release_port(dev);
+	pfe->nb_devs--;
+}
+
+/* pfe_eth_init_one
+ */
+static int pfe_eth_init(struct rte_vdev_device *vdev, struct pfe *pfe, int id)
+{
+	struct rte_eth_dev_data *data = NULL;
+	struct rte_eth_dev *eth_dev = NULL;
+	struct pfe_eth_priv_s *priv = NULL;
+	int err;
+
+	if (id >= pfe->max_intf)
+		return -EINVAL;
+
+	data = rte_zmalloc(NULL, sizeof(*data), 64);
+	if (data == NULL)
+		return -ENOMEM;
+
+	eth_dev = rte_eth_vdev_allocate(vdev, sizeof(*priv));
+	if (eth_dev == NULL) {
+		rte_free(data);
+		return -ENOMEM;
+	}
+
+	priv = eth_dev->data->dev_private;
+	rte_memcpy(data, eth_dev->data, sizeof(*data));
+
+	priv->ndev = eth_dev;
+	priv->pfe = pfe;
+
+	pfe->eth.eth_priv[id] = priv;
+
+#define HIF_GEMAC_TMUQ_BASE	6
+	priv->low_tmu_q = HIF_GEMAC_TMUQ_BASE + (id * 2);
+	priv->high_tmu_q = priv->low_tmu_q + 1;
+
+	rte_spinlock_init(&priv->lock);
+
+	/* Copy the station address into the dev structure, */
+	eth_dev->data->mac_addrs = rte_zmalloc("mac_addr",
+			ETHER_ADDR_LEN * PPFE_MAX_MACS, 0);
+	if (eth_dev->data->mac_addrs == NULL) {
+		err = -ENOMEM;
+		goto err0;
+	}
+
+	eth_dev->data->mtu = 1500;
+
+	eth_dev->data->nb_rx_queues = 1;
+	eth_dev->data->nb_tx_queues = 1;
+
+	/* For link status, open the PFE CDEV; Error from this function
+	 * is silently ignored; In case of error, the link status will not
+	 * be available.
+	 */
+	pfe_eth_open_cdev(priv);
+	rte_eth_dev_probing_finish(eth_dev);
+
+	return 0;
+err0:
+	rte_free(data);
+	return err;
+}
+
+/* Parse integer from integer argument */
+static int
+parse_integer_arg(const char *key __rte_unused,
+		const char *value, void *extra_args)
+{
+	int *i = (int *)extra_args;
+
+	*i = atoi(value);
+	if (*i < 0)
+		return -1;
+
+	return 0;
+}
+
+static int
+pfe_parse_vdev_init_params(struct pfe_vdev_init_params *params,
+				struct rte_vdev_device *dev)
+{
+	struct rte_kvargs *kvlist = NULL;
+	int ret = 0;
+
+	static const char * const pfe_vdev_valid_params[] = {
+		PPFE_VDEV_GEM_ID_ARG,
+		NULL
+	};
+
+	const char *input_args = rte_vdev_device_args(dev);
+	if (params == NULL)
+		return -EINVAL;
+
+
+	if (input_args) {
+		kvlist = rte_kvargs_parse(input_args, pfe_vdev_valid_params);
+		if (kvlist == NULL)
+			return -1;
+
+		ret = rte_kvargs_process(kvlist,
+					PPFE_VDEV_GEM_ID_ARG,
+					&parse_integer_arg,
+					&params->gem_id);
+		if (ret < 0)
+			goto free_kvlist;
+	}
+
+free_kvlist:
+	rte_kvargs_free(kvlist);
+	return ret;
+}
+
+static int
+pmd_pfe_probe(struct rte_vdev_device *vdev)
+{
+	const u32 *prop;
+	const struct device_node *np;
+	const char *name;
+	const uint32_t *addr;
+	uint64_t cbus_addr, ddr_size, cbus_size;
+	int rc = -1, fd = -1, gem_id;
+	unsigned int interface_count = 0;
+	size_t size = 0;
+	struct pfe_vdev_init_params init_params = {
+		-1
+	};
+
+	name = rte_vdev_device_name(vdev);
+	rc = pfe_parse_vdev_init_params(&init_params, vdev);
+	if (rc < 0)
+		return -EINVAL;
+
+	if (g_pfe) {
+		if (g_pfe->nb_devs >= g_pfe->max_intf)
+			return -EINVAL;
+
+		goto eth_init;
+	}
+
+	g_pfe = rte_zmalloc(NULL, sizeof(*g_pfe), 64);
+	if (g_pfe == NULL)
+		return  -EINVAL;
+
+	/* Load the device-tree driver */
+	rc = of_init();
+	if (rc)
+		goto err;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,pfe");
+	if (!np) {
+		rc = -EINVAL;
+		goto err;
+	}
+
+	addr = of_get_address(np, 0, &cbus_size, NULL);
+	if (!addr)
+		goto err;
+
+	cbus_addr = of_translate_address(np, addr);
+	if (!cbus_addr)
+		goto err;
+
+
+	addr = of_get_address(np, 1, &ddr_size, NULL);
+	if (!addr)
+		goto err;
+
+	g_pfe->ddr_phys_baseaddr = of_translate_address(np, addr);
+	if (!g_pfe->ddr_phys_baseaddr)
+		goto err;
+
+	g_pfe->ddr_size = ddr_size;
+
+	fd = open("/dev/mem", O_RDWR);
+	g_pfe->cbus_baseaddr = mmap(NULL, cbus_size, PROT_READ | PROT_WRITE,
+					MAP_SHARED, fd, cbus_addr);
+	if (g_pfe->cbus_baseaddr == MAP_FAILED) {
+		rc = -EINVAL;
+		goto err;
+	}
+
+	/* Read interface count */
+	prop = of_get_property(np, "fsl,pfe-num-interfaces", &size);
+	if (!prop) {
+		rc = -ENXIO;
+		goto err_prop;
+	}
+
+	interface_count = rte_be_to_cpu_32((unsigned int)*prop);
+	if (interface_count <= 0) {
+		rc = -ENXIO;
+		goto err_prop;
+	}
+	g_pfe->max_intf  = interface_count;
+	pfe_soc_version_get();
+eth_init:
+	if (init_params.gem_id < 0)
+		gem_id = g_pfe->nb_devs;
+	else
+		gem_id = init_params.gem_id;
+
+	RTE_LOG(INFO, PMD, "Init pmd_pfe for %s gem-id %d(given =%d)\n",
+		name, gem_id, init_params.gem_id);
+
+	rc = pfe_eth_init(vdev, g_pfe, gem_id);
+	if (rc < 0)
+		goto err_eth;
+	else
+		g_pfe->nb_devs++;
+
+	return 0;
+
+err_eth:
+err_prop:
+err:
+	rte_free(g_pfe);
+	return rc;
+}
+
+static int
+pmd_pfe_remove(struct rte_vdev_device *vdev)
+{
+	const char *name;
+	struct rte_eth_dev *eth_dev = NULL;
+
+	name = rte_vdev_device_name(vdev);
+	if (name == NULL)
+		return -EINVAL;
+
+	if (!g_pfe)
+		return 0;
+
+	eth_dev = rte_eth_dev_allocated(name);
+	if (eth_dev == NULL)
+		return -ENODEV;
+
+	pfe_eth_exit(eth_dev, g_pfe);
+
+	return 0;
+}
+
+static struct rte_vdev_driver pmd_pfe_drv = {
+	.probe = pmd_pfe_probe,
+	.remove = pmd_pfe_remove,
+};
+
+RTE_PMD_REGISTER_VDEV(PFE_PMD, pmd_pfe_drv);
+RTE_PMD_REGISTER_ALIAS(PFE_PMD, eth_pfe);
+RTE_PMD_REGISTER_PARAM_STRING(PFE_PMD, "intf=<int> ");
diff --git a/drivers/net/ppfe/rte_pmd_ppfe_version.map b/drivers/net/ppfe/rte_pmd_ppfe_version.map
new file mode 100644
index 000000000..b7b7c9168
--- /dev/null
+++ b/drivers/net/ppfe/rte_pmd_ppfe_version.map
@@ -0,0 +1,4 @@ 
+DPDK_19.11 {
+
+	local: *;
+};
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index ba5c39e01..4df36a53c 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -161,6 +161,7 @@  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)       += -lrte_pmd_bond
 _LDLIBS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD)      += -lrte_pmd_cxgbe
 ifeq ($(CONFIG_RTE_LIBRTE_DPAA_BUS),y)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA_PMD)       += -lrte_pmd_dpaa
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PPFE_PMD)       += -lrte_pmd_ppfe
 endif
 ifeq ($(CONFIG_RTE_EAL_VFIO)$(CONFIG_RTE_LIBRTE_FSLMC_BUS),yy)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD)      += -lrte_pmd_dpaa2