[v2,19/27] net/nfp: refact the nsp module

Message ID 20230830021457.2064750-20-chaoyong.he@corigine.com (mailing list archive)
State Superseded
Delegated to: Ferruh Yigit
Headers
Series refact the nfpcore module |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chaoyong He Aug. 30, 2023, 2:14 a.m. UTC
  Move the definition of data structure into the implement file.
Also sync the logic from kernel driver and remove the unneeded header
file include statements.

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c           |   2 +-
 drivers/net/nfp/nfpcore/nfp_nsp.c      | 390 +++++++++++++++++++------
 drivers/net/nfp/nfpcore/nfp_nsp.h      | 140 ++++-----
 drivers/net/nfp/nfpcore/nfp_nsp_cmds.c |   4 -
 drivers/net/nfp/nfpcore/nfp_nsp_eth.c  |  79 ++---
 5 files changed, 398 insertions(+), 217 deletions(-)
  

Patch

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 2e43055fd5..9243191de3 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -661,7 +661,7 @@  nfp_net_init(struct rte_eth_dev *eth_dev)
 static int
 nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)
 {
-	struct nfp_cpp *cpp = nsp->cpp;
+	struct nfp_cpp *cpp = nfp_nsp_cpp(nsp);
 	void *fw_buf;
 	char fw_name[125];
 	char serial[40];
diff --git a/drivers/net/nfp/nfpcore/nfp_nsp.c b/drivers/net/nfp/nfpcore/nfp_nsp.c
index 8e65064b10..75d13cb84f 100644
--- a/drivers/net/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/nfp/nfpcore/nfp_nsp.c
@@ -3,20 +3,127 @@ 
  * All rights reserved.
  */
 
-#define NFP_SUBSYS "nfp_nsp"
-
-#include <stdio.h>
-#include <time.h>
+#include "nfp_nsp.h"
 
 #include <rte_common.h>
 
-#include "nfp_cpp.h"
 #include "nfp_logs.h"
-#include "nfp_nsp.h"
 #include "nfp_platform.h"
 #include "nfp_resource.h"
 
-int
+/* Offsets relative to the CSR base */
+#define NSP_STATUS              0x00
+#define   NSP_STATUS_MAGIC      GENMASK_ULL(63, 48)
+#define   NSP_STATUS_MAJOR      GENMASK_ULL(47, 44)
+#define   NSP_STATUS_MINOR      GENMASK_ULL(43, 32)
+#define   NSP_STATUS_CODE       GENMASK_ULL(31, 16)
+#define   NSP_STATUS_RESULT     GENMASK_ULL(15, 8)
+#define   NSP_STATUS_BUSY       RTE_BIT64(0)
+
+#define NSP_COMMAND             0x08
+#define   NSP_COMMAND_OPTION    GENMASK_ULL(63, 32)
+#define   NSP_COMMAND_CODE      GENMASK_ULL(31, 16)
+#define   NSP_COMMAND_DMA_BUF   RTE_BIT64(1)
+#define   NSP_COMMAND_START     RTE_BIT64(0)
+
+/* CPP address to retrieve the data from */
+#define NSP_BUFFER              0x10
+#define   NSP_BUFFER_CPP        GENMASK_ULL(63, 40)
+#define   NSP_BUFFER_ADDRESS    GENMASK_ULL(39, 0)
+
+#define NSP_DFLT_BUFFER         0x18
+#define   NSP_DFLT_BUFFER_CPP          GENMASK_ULL(63, 40)
+#define   NSP_DFLT_BUFFER_ADDRESS      GENMASK_ULL(39, 0)
+
+#define NSP_DFLT_BUFFER_CONFIG  0x20
+#define   NSP_DFLT_BUFFER_SIZE_4KB     GENMASK_ULL(15, 8)
+#define   NSP_DFLT_BUFFER_SIZE_MB      GENMASK_ULL(7, 0)
+
+#define NSP_MAGIC               0xab10
+#define NSP_MAJOR               0
+#define NSP_MINOR               8
+
+#define NSP_CODE_MAJOR          GENMASK_ULL(15, 12)
+#define NSP_CODE_MINOR          GENMASK_ULL(11, 0)
+
+#define NFP_FW_LOAD_RET_MAJOR   GENMASK_ULL(15, 8)
+#define NFP_FW_LOAD_RET_MINOR   GENMASK_ULL(23, 16)
+
+enum nfp_nsp_cmd {
+	SPCODE_NOOP             = 0, /* No operation */
+	SPCODE_SOFT_RESET       = 1, /* Soft reset the NFP */
+	SPCODE_FW_DEFAULT       = 2, /* Load default (UNDI) FW */
+	SPCODE_PHY_INIT         = 3, /* Initialize the PHY */
+	SPCODE_MAC_INIT         = 4, /* Initialize the MAC */
+	SPCODE_PHY_RXADAPT      = 5, /* Re-run PHY RX Adaptation */
+	SPCODE_FW_LOAD          = 6, /* Load fw from buffer, len in option */
+	SPCODE_ETH_RESCAN       = 7, /* Rescan ETHs, write ETH_TABLE to buf */
+	SPCODE_ETH_CONTROL      = 8, /* Update media config from buffer */
+	SPCODE_NSP_WRITE_FLASH  = 11, /* Load and flash image from buffer */
+	SPCODE_NSP_SENSORS      = 12, /* Read NSP sensor(s) */
+	SPCODE_NSP_IDENTIFY     = 13, /* Read NSP version */
+	SPCODE_FW_STORED        = 16, /* If no FW loaded, load flash app FW */
+	SPCODE_HWINFO_LOOKUP    = 17, /* Lookup HWinfo with overwrites etc. */
+	SPCODE_HWINFO_SET       = 18, /* Set HWinfo entry */
+	SPCODE_FW_LOADED        = 19, /* Is application firmware loaded */
+	SPCODE_VERSIONS         = 21, /* Report FW versions */
+	SPCODE_READ_SFF_EEPROM  = 22, /* Read module EEPROM */
+	SPCODE_READ_MEDIA       = 23, /* Get the supported/advertised media for a port */
+};
+
+static const struct {
+	uint32_t code;
+	const char *msg;
+} nsp_errors[] = {
+	{ 6010, "could not map to phy for port" },
+	{ 6011, "not an allowed rate/lanes for port" },
+	{ 6012, "not an allowed rate/lanes for port" },
+	{ 6013, "high/low error, change other port first" },
+	{ 6014, "config not found in flash" },
+};
+
+struct nfp_nsp {
+	struct nfp_cpp *cpp;
+	struct nfp_resource *res;
+	struct {
+		uint16_t major;
+		uint16_t minor;
+	} ver;
+
+	/** Eth table config state */
+	bool modified;
+	uint32_t idx;
+	void *entries;
+};
+
+/* NFP command argument structure */
+struct nfp_nsp_command_arg {
+	uint16_t code;         /**< NFP SP Command Code */
+	bool dma;              /**< @buf points to a host buffer, not NSP buffer */
+	bool error_quiet;      /**< Don't print command error/warning */
+	uint32_t timeout_sec;  /**< Timeout value to wait for completion in seconds */
+	uint32_t option;       /**< NSP Command Argument */
+	uint64_t buf;          /**< NSP Buffer Address */
+	/** Callback for interpreting option if error occurred */
+	void (*error_cb)(struct nfp_nsp *state, uint32_t ret_val);
+};
+
+/* NFP command with buffer argument structure */
+struct nfp_nsp_command_buf_arg {
+	struct nfp_nsp_command_arg arg;  /**< NFP command argument structure */
+	const void *in_buf;              /**< Buffer with data for input */
+	void *out_buf;                   /**< Buffer for output data */
+	uint32_t in_size;                /**< Size of @in_buf */
+	uint32_t out_size;               /**< Size of @out_buf */
+};
+
+struct nfp_cpp *
+nfp_nsp_cpp(struct nfp_nsp *state)
+{
+	return state->cpp;
+}
+
+bool
 nfp_nsp_config_modified(struct nfp_nsp *state)
 {
 	return state->modified;
@@ -24,7 +131,7 @@  nfp_nsp_config_modified(struct nfp_nsp *state)
 
 void
 nfp_nsp_config_set_modified(struct nfp_nsp *state,
-		int modified)
+		bool modified)
 {
 	state->modified = modified;
 }
@@ -66,7 +173,7 @@  nfp_nsp_print_extended_error(uint32_t ret_val)
 		return;
 
 	for (i = 0; i < RTE_DIM(nsp_errors); i++)
-		if (ret_val == (uint32_t)nsp_errors[i].code)
+		if (ret_val == nsp_errors[i].code)
 			PMD_DRV_LOG(ERR, "err msg: %s", nsp_errors[i].msg);
 }
 
@@ -222,11 +329,8 @@  nfp_nsp_wait_reg(struct nfp_cpp *cpp,
  *   - -ETIMEDOUT if the NSP took longer than @timeout_sec seconds to complete
  */
 static int
-nfp_nsp_command(struct nfp_nsp *state,
-		uint16_t code,
-		uint32_t option,
-		uint32_t buff_cpp,
-		uint64_t buff_addr)
+nfp_nsp_command_real(struct nfp_nsp *state,
+		const struct nfp_nsp_command_arg *arg)
 {
 	int err;
 	uint64_t reg;
@@ -250,22 +354,14 @@  nfp_nsp_command(struct nfp_nsp *state,
 		return err;
 	}
 
-	if (!FIELD_FIT(NSP_BUFFER_CPP, buff_cpp >> 8) ||
-			!FIELD_FIT(NSP_BUFFER_ADDRESS, buff_addr)) {
-		PMD_DRV_LOG(ERR, "Host buffer out of reach %08x %" PRIx64,
-				buff_cpp, buff_addr);
-		return -EINVAL;
-	}
-
-	err = nfp_cpp_writeq(cpp, nsp_cpp, nsp_buffer,
-			FIELD_PREP(NSP_BUFFER_CPP, buff_cpp >> 8) |
-			FIELD_PREP(NSP_BUFFER_ADDRESS, buff_addr));
+	err = nfp_cpp_writeq(cpp, nsp_cpp, nsp_buffer, arg->buf);
 	if (err < 0)
 		return err;
 
 	err = nfp_cpp_writeq(cpp, nsp_cpp, nsp_command,
-			FIELD_PREP(NSP_COMMAND_OPTION, option) |
-			FIELD_PREP(NSP_COMMAND_CODE, code) |
+			FIELD_PREP(NSP_COMMAND_OPTION, arg->option) |
+			FIELD_PREP(NSP_COMMAND_CODE, arg->code) |
+			FIELD_PREP(NSP_COMMAND_DMA_BUF, arg->dma) |
 			FIELD_PREP(NSP_COMMAND_START, 1));
 	if (err < 0)
 		return err;
@@ -275,7 +371,7 @@  nfp_nsp_command(struct nfp_nsp *state,
 			NSP_COMMAND_START, 0);
 	if (err != 0) {
 		PMD_DRV_LOG(ERR, "Error %d waiting for code %#04x to start",
-				err, code);
+				err, arg->code);
 		return err;
 	}
 
@@ -284,7 +380,7 @@  nfp_nsp_command(struct nfp_nsp *state,
 			NSP_STATUS_BUSY, 0);
 	if (err != 0) {
 		PMD_DRV_LOG(ERR, "Error %d waiting for code %#04x to complete",
-				err, code);
+				err, arg->code);
 		return err;
 	}
 
@@ -296,84 +392,85 @@  nfp_nsp_command(struct nfp_nsp *state,
 
 	err = FIELD_GET(NSP_STATUS_RESULT, reg);
 	if (err != 0) {
-		PMD_DRV_LOG(ERR, "Result (error) code set: %d (%d) command: %d",
-				-err, (int)ret_val, code);
-		nfp_nsp_print_extended_error(ret_val);
+		if (!arg->error_quiet)
+			PMD_DRV_LOG(WARNING, "Result (error) code set: %d (%d) command: %d",
+					-err, (int)ret_val, arg->code);
+
+		if (arg->error_cb != 0)
+			arg->error_cb(state, ret_val);
+		else
+			nfp_nsp_print_extended_error(ret_val);
+
 		return -err;
 	}
 
 	return ret_val;
 }
 
-#define SZ_1M 0x00100000
+static int
+nfp_nsp_command(struct nfp_nsp *state,
+		uint16_t code)
+{
+	const struct nfp_nsp_command_arg arg = {
+		.code = code,
+	};
+
+	return nfp_nsp_command_real(state, &arg);
+}
 
 static int
-nfp_nsp_command_buf(struct nfp_nsp *nsp,
-		uint16_t code, uint32_t option,
-		const void *in_buf,
-		unsigned int in_size,
-		void *out_buf,
-		unsigned int out_size)
+nfp_nsp_command_buf_def(struct nfp_nsp *nsp,
+		struct nfp_nsp_command_buf_arg *arg)
 {
 	int err;
 	int ret;
 	uint64_t reg;
-	size_t max_size;
 	uint32_t cpp_id;
 	uint64_t cpp_buf;
 	struct nfp_cpp *cpp = nsp->cpp;
 
-	if (nsp->ver.minor < 13) {
-		PMD_DRV_LOG(ERR, "NSP: Code 0x%04x with buffer not supported ABI %hu.%hu)",
-				code, nsp->ver.major, nsp->ver.minor);
-		return -EOPNOTSUPP;
-	}
-
-	err = nfp_cpp_readq(cpp, nfp_resource_cpp_id(nsp->res),
-			nfp_resource_address(nsp->res) + NSP_DFLT_BUFFER_CONFIG,
-			&reg);
-	if (err < 0)
-		return err;
-
-	max_size = RTE_MAX(in_size, out_size);
-	if (FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M < max_size) {
-		PMD_DRV_LOG(ERR, "NSP: default buffer too small for command 0x%04x (%llu < %lu)",
-				code, FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M, max_size);
-		return -EINVAL;
-	}
-
 	err = nfp_cpp_readq(cpp, nfp_resource_cpp_id(nsp->res),
 			nfp_resource_address(nsp->res) + NSP_DFLT_BUFFER,
 			&reg);
 	if (err < 0)
 		return err;
 
-	cpp_id = FIELD_GET(NSP_BUFFER_CPP, reg) << 8;
-	cpp_buf = FIELD_GET(NSP_BUFFER_ADDRESS, reg);
+	cpp_id = FIELD_GET(NSP_DFLT_BUFFER_CPP, reg) << 8;
+	cpp_buf = FIELD_GET(NSP_DFLT_BUFFER_ADDRESS, reg);
 
-	if (in_buf != NULL && in_size > 0) {
-		err = nfp_cpp_write(cpp, cpp_id, cpp_buf, in_buf, in_size);
+	if (arg->in_buf != NULL && arg->in_size > 0) {
+		err = nfp_cpp_write(cpp, cpp_id, cpp_buf,
+				arg->in_buf, arg->in_size);
 		if (err < 0)
 			return err;
 	}
 
 	/* Zero out remaining part of the buffer */
-	if (out_buf != NULL && out_size > 0 && out_size > in_size) {
-		memset(out_buf, 0, out_size - in_size);
-		err = nfp_cpp_write(cpp, cpp_id, cpp_buf + in_size, out_buf,
-				out_size - in_size);
+	if (arg->out_buf != NULL && arg->out_size > arg->in_size) {
+		err = nfp_cpp_write(cpp, cpp_id, cpp_buf + arg->in_size,
+				arg->out_buf, arg->out_size - arg->in_size);
 		if (err < 0)
 			return err;
 	}
 
-	ret = nfp_nsp_command(nsp, code, option, cpp_id, cpp_buf);
+	if (!FIELD_FIT(NSP_BUFFER_CPP, cpp_id >> 8) ||
+			!FIELD_FIT(NSP_BUFFER_ADDRESS, cpp_buf)) {
+		PMD_DRV_LOG(ERR, "Buffer out of reach %#08x %#016lx",
+				cpp_id, cpp_buf);
+		return -EINVAL;
+	}
+
+	arg->arg.buf = FIELD_PREP(NSP_BUFFER_CPP, cpp_id >> 8) |
+			FIELD_PREP(NSP_BUFFER_ADDRESS, cpp_buf);
+	ret = nfp_nsp_command_real(nsp, &arg->arg);
 	if (ret < 0) {
 		PMD_DRV_LOG(ERR, "NSP command failed");
 		return ret;
 	}
 
-	if (out_buf != NULL && out_size > 0) {
-		err = nfp_cpp_read(cpp, cpp_id, cpp_buf, out_buf, out_size);
+	if (arg->out_buf != NULL && arg->out_size > 0) {
+		err = nfp_cpp_read(cpp, cpp_id, cpp_buf,
+				arg->out_buf, arg->out_size);
 		if (err < 0)
 			return err;
 	}
@@ -381,6 +478,43 @@  nfp_nsp_command_buf(struct nfp_nsp *nsp,
 	return ret;
 }
 
+#define SZ_1M 0x00100000
+#define SZ_4K 0x00001000
+
+static int
+nfp_nsp_command_buf(struct nfp_nsp *nsp,
+		struct nfp_nsp_command_buf_arg *arg)
+{
+	int err;
+	uint64_t reg;
+	uint32_t size;
+	uint32_t max_size;
+	struct nfp_cpp *cpp = nsp->cpp;
+
+	if (nsp->ver.minor < 13) {
+		PMD_DRV_LOG(ERR, "NSP: Code %#04x with buffer not supported ABI %hu.%hu)",
+				arg->arg.code, nsp->ver.major, nsp->ver.minor);
+		return -EOPNOTSUPP;
+	}
+
+	err = nfp_cpp_readq(cpp, nfp_resource_cpp_id(nsp->res),
+			nfp_resource_address(nsp->res) + NSP_DFLT_BUFFER_CONFIG,
+			&reg);
+	if (err < 0)
+		return err;
+
+	max_size = RTE_MAX(arg->in_size, arg->out_size);
+	size = FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M +
+			FIELD_GET(NSP_DFLT_BUFFER_SIZE_4KB, reg) * SZ_4K;
+	if (size < max_size) {
+		PMD_DRV_LOG(ERR, "NSP: default buffer too small for command %#04x (%u < %u)",
+				arg->arg.code, size, max_size);
+		return -EINVAL;
+	}
+
+	return nfp_nsp_command_buf_def(nsp, arg);
+}
+
 int
 nfp_nsp_wait(struct nfp_nsp *state)
 {
@@ -392,7 +526,7 @@  nfp_nsp_wait(struct nfp_nsp *state)
 	wait.tv_nsec = 25000000;    /* 25ms */
 
 	for (;;) {
-		err = nfp_nsp_command(state, SPCODE_NOOP, 0, 0, 0);
+		err = nfp_nsp_command(state, SPCODE_NOOP);
 		if (err != -EAGAIN)
 			break;
 
@@ -413,13 +547,57 @@  nfp_nsp_wait(struct nfp_nsp *state)
 int
 nfp_nsp_device_soft_reset(struct nfp_nsp *state)
 {
-	return nfp_nsp_command(state, SPCODE_SOFT_RESET, 0, 0, 0);
+	return nfp_nsp_command(state, SPCODE_SOFT_RESET);
 }
 
 int
 nfp_nsp_mac_reinit(struct nfp_nsp *state)
 {
-	return nfp_nsp_command(state, SPCODE_MAC_INIT, 0, 0, 0);
+	return nfp_nsp_command(state, SPCODE_MAC_INIT);
+}
+
+static void
+nfp_nsp_load_fw_extended_msg(struct nfp_nsp *state,
+		uint32_t ret_val)
+{
+	uint32_t minor;
+	uint32_t major;
+	static const char * const major_msg[] = {
+		/* 0 */ "Firmware from driver loaded",
+		/* 1 */ "Firmware from flash loaded",
+		/* 2 */ "Firmware loading failure",
+	};
+	static const char * const minor_msg[] = {
+		/*  0 */ "",
+		/*  1 */ "no named partition on flash",
+		/*  2 */ "error reading from flash",
+		/*  3 */ "can not deflate",
+		/*  4 */ "not a trusted file",
+		/*  5 */ "can not parse FW file",
+		/*  6 */ "MIP not found in FW file",
+		/*  7 */ "null firmware name in MIP",
+		/*  8 */ "FW version none",
+		/*  9 */ "FW build number none",
+		/* 10 */ "no FW selection policy HWInfo key found",
+		/* 11 */ "static FW selection policy",
+		/* 12 */ "FW version has precedence",
+		/* 13 */ "different FW application load requested",
+		/* 14 */ "development build",
+	};
+
+	major = FIELD_GET(NFP_FW_LOAD_RET_MAJOR, ret_val);
+	minor = FIELD_GET(NFP_FW_LOAD_RET_MINOR, ret_val);
+
+	if (!nfp_nsp_has_stored_fw_load(state))
+		return;
+
+	if (major >= RTE_DIM(major_msg))
+		PMD_DRV_LOG(INFO, "FW loading status: %x", ret_val);
+	else if (minor >= RTE_DIM(minor_msg))
+		PMD_DRV_LOG(INFO, "%s, reason code: %d", major_msg[major], minor);
+	else
+		PMD_DRV_LOG(INFO, "%s%c %s", major_msg[major],
+				minor != 0 ? ',' : '.', minor_msg[minor]);
 }
 
 int
@@ -427,8 +605,24 @@  nfp_nsp_load_fw(struct nfp_nsp *state,
 		void *buf,
 		size_t size)
 {
-	return nfp_nsp_command_buf(state, SPCODE_FW_LOAD, size, buf, size,
-			NULL, 0);
+	int ret;
+	struct nfp_nsp_command_buf_arg load_fw = {
+		{
+			.code     = SPCODE_FW_LOAD,
+			.option   = size,
+			.error_cb = nfp_nsp_load_fw_extended_msg,
+		},
+		.in_buf  = buf,
+		.in_size = size,
+	};
+
+	ret = nfp_nsp_command_buf(state, &load_fw);
+	if (ret < 0)
+		return ret;
+
+	nfp_nsp_load_fw_extended_msg(state, ret);
+
+	return 0;
 }
 
 int
@@ -436,8 +630,16 @@  nfp_nsp_read_eth_table(struct nfp_nsp *state,
 		void *buf,
 		size_t size)
 {
-	return nfp_nsp_command_buf(state, SPCODE_ETH_RESCAN, size, NULL, 0,
-			buf, size);
+	struct nfp_nsp_command_buf_arg eth_rescan = {
+		{
+			.code   = SPCODE_ETH_RESCAN,
+			.option = size,
+		},
+		.out_buf  = buf,
+		.out_size = size,
+	};
+
+	return nfp_nsp_command_buf(state, &eth_rescan);
 }
 
 int
@@ -445,8 +647,16 @@  nfp_nsp_write_eth_table(struct nfp_nsp *state,
 		const void *buf,
 		size_t size)
 {
-	return nfp_nsp_command_buf(state, SPCODE_ETH_CONTROL, size, buf, size,
-			NULL, 0);
+	struct nfp_nsp_command_buf_arg eth_ctrl = {
+		{
+			.code   = SPCODE_ETH_CONTROL,
+			.option = size,
+		},
+		.in_buf  = buf,
+		.in_size = size,
+	};
+
+	return nfp_nsp_command_buf(state, &eth_ctrl);
 }
 
 int
@@ -454,8 +664,16 @@  nfp_nsp_read_identify(struct nfp_nsp *state,
 		void *buf,
 		size_t size)
 {
-	return nfp_nsp_command_buf(state, SPCODE_NSP_IDENTIFY, size, NULL, 0,
-			buf, size);
+	struct nfp_nsp_command_buf_arg identify = {
+		{
+			.code   = SPCODE_NSP_IDENTIFY,
+			.option = size,
+		},
+		.out_buf  = buf,
+		.out_size = size,
+	};
+
+	return nfp_nsp_command_buf(state, &identify);
 }
 
 int
@@ -464,6 +682,14 @@  nfp_nsp_read_sensors(struct nfp_nsp *state,
 		void *buf,
 		size_t size)
 {
-	return nfp_nsp_command_buf(state, SPCODE_NSP_SENSORS, sensor_mask, NULL,
-			0, buf, size);
+	struct nfp_nsp_command_buf_arg sensors = {
+		{
+			.code   = SPCODE_NSP_SENSORS,
+			.option = sensor_mask,
+		},
+		.out_buf  = buf,
+		.out_size = size,
+	};
+
+	return nfp_nsp_command_buf(state, &sensors);
 }
diff --git a/drivers/net/nfp/nfpcore/nfp_nsp.h b/drivers/net/nfp/nfpcore/nfp_nsp.h
index 14986a9130..fe52dffeb7 100644
--- a/drivers/net/nfp/nfpcore/nfp_nsp.h
+++ b/drivers/net/nfp/nfpcore/nfp_nsp.h
@@ -7,78 +7,8 @@ 
 #define __NSP_NSP_H__
 
 #include "nfp_cpp.h"
-#include "nfp_nsp.h"
-
-/* Offsets relative to the CSR base */
-#define NSP_STATUS              0x00
-#define   NSP_STATUS_MAGIC      GENMASK_ULL(63, 48)
-#define   NSP_STATUS_MAJOR      GENMASK_ULL(47, 44)
-#define   NSP_STATUS_MINOR      GENMASK_ULL(43, 32)
-#define   NSP_STATUS_CODE       GENMASK_ULL(31, 16)
-#define   NSP_STATUS_RESULT     GENMASK_ULL(15, 8)
-#define   NSP_STATUS_BUSY       RTE_BIT64(0)
-
-#define NSP_COMMAND             0x08
-#define   NSP_COMMAND_OPTION    GENMASK_ULL(63, 32)
-#define   NSP_COMMAND_CODE      GENMASK_ULL(31, 16)
-#define   NSP_COMMAND_START     RTE_BIT64(0)
-
-/* CPP address to retrieve the data from */
-#define NSP_BUFFER              0x10
-#define   NSP_BUFFER_CPP        GENMASK_ULL(63, 40)
-#define   NSP_BUFFER_PCIE       GENMASK_ULL(39, 38)
-#define   NSP_BUFFER_ADDRESS    GENMASK_ULL(37, 0)
-
-#define NSP_DFLT_BUFFER         0x18
-
-#define NSP_DFLT_BUFFER_CONFIG 0x20
-#define   NSP_DFLT_BUFFER_SIZE_MB    GENMASK_ULL(7, 0)
-
-#define NSP_MAGIC               0xab10
-#define NSP_MAJOR               0
-#define NSP_MINOR               8
-
-#define NSP_CODE_MAJOR          GENMASK(15, 12)
-#define NSP_CODE_MINOR          GENMASK(11, 0)
-
-enum nfp_nsp_cmd {
-	SPCODE_NOOP             = 0, /* No operation */
-	SPCODE_SOFT_RESET       = 1, /* Soft reset the NFP */
-	SPCODE_FW_DEFAULT       = 2, /* Load default (UNDI) FW */
-	SPCODE_PHY_INIT         = 3, /* Initialize the PHY */
-	SPCODE_MAC_INIT         = 4, /* Initialize the MAC */
-	SPCODE_PHY_RXADAPT      = 5, /* Re-run PHY RX Adaptation */
-	SPCODE_FW_LOAD          = 6, /* Load fw from buffer, len in option */
-	SPCODE_ETH_RESCAN       = 7, /* Rescan ETHs, write ETH_TABLE to buf */
-	SPCODE_ETH_CONTROL      = 8, /* Update media config from buffer */
-	SPCODE_NSP_SENSORS      = 12, /* Read NSP sensor(s) */
-	SPCODE_NSP_IDENTIFY     = 13, /* Read NSP version */
-};
-
-static const struct {
-	int code;
-	const char *msg;
-} nsp_errors[] = {
-	{ 6010, "could not map to phy for port" },
-	{ 6011, "not an allowed rate/lanes for port" },
-	{ 6012, "not an allowed rate/lanes for port" },
-	{ 6013, "high/low error, change other port first" },
-	{ 6014, "config not found in flash" },
-};
 
-struct nfp_nsp {
-	struct nfp_cpp *cpp;
-	struct nfp_resource *res;
-	struct {
-		uint16_t major;
-		uint16_t minor;
-	} ver;
-
-	/* Eth table config state */
-	int modified;
-	unsigned int idx;
-	void *entries;
-};
+struct nfp_nsp;
 
 struct nfp_nsp *nfp_nsp_open(struct nfp_cpp *cpp);
 void nfp_nsp_close(struct nfp_nsp *state);
@@ -92,18 +22,61 @@  int nfp_nsp_read_identify(struct nfp_nsp *state, void *buf, size_t size);
 int nfp_nsp_read_sensors(struct nfp_nsp *state, uint32_t sensor_mask,
 		void *buf, size_t size);
 
-static inline int
+static inline bool
 nfp_nsp_has_mac_reinit(struct nfp_nsp *state)
 {
 	return nfp_nsp_get_abi_ver_minor(state) > 20;
 }
 
+static inline bool
+nfp_nsp_has_stored_fw_load(struct nfp_nsp *state)
+{
+	return nfp_nsp_get_abi_ver_minor(state) > 23;
+}
+
+static inline bool
+nfp_nsp_has_hwinfo_lookup(struct nfp_nsp *state)
+{
+	return nfp_nsp_get_abi_ver_minor(state) > 24;
+}
+
+static inline bool
+nfp_nsp_has_hwinfo_set(struct nfp_nsp *state)
+{
+	return nfp_nsp_get_abi_ver_minor(state) > 25;
+}
+
+static inline bool
+nfp_nsp_has_fw_loaded(struct nfp_nsp *state)
+{
+	return nfp_nsp_get_abi_ver_minor(state) > 25;
+}
+
+static inline bool
+nfp_nsp_has_versions(struct nfp_nsp *state)
+{
+	return nfp_nsp_get_abi_ver_minor(state) > 27;
+}
+
+static inline bool
+nfp_nsp_has_read_module_eeprom(struct nfp_nsp *state)
+{
+	return nfp_nsp_get_abi_ver_minor(state) > 28;
+}
+
+static inline bool
+nfp_nsp_has_read_media(struct nfp_nsp *state)
+{
+	return nfp_nsp_get_abi_ver_minor(state) > 33;
+}
+
 enum nfp_eth_interface {
 	NFP_INTERFACE_NONE      = 0,
 	NFP_INTERFACE_SFP       = 1,
 	NFP_INTERFACE_SFPP      = 10,
 	NFP_INTERFACE_SFP28     = 28,
 	NFP_INTERFACE_QSFP      = 40,
+	NFP_INTERFACE_RJ45      = 45,
 	NFP_INTERFACE_CXP       = 100,
 	NFP_INTERFACE_QSFP28    = 112,
 };
@@ -151,6 +124,7 @@  struct nfp_eth_table {
 		enum nfp_eth_media media; /**< Media type of the @interface */
 
 		enum nfp_eth_fec fec;     /**< Forward Error Correction mode */
+		enum nfp_eth_fec act_fec; /**< Active Forward Error Correction mode */
 		enum nfp_eth_aneg aneg;   /**< Auto negotiation mode */
 
 		struct rte_ether_addr mac_addr;  /**< Interface MAC address */
@@ -159,17 +133,18 @@  struct nfp_eth_table {
 		/** Id of interface within port (for split ports) */
 		uint8_t label_subport;
 
-		int enabled;     /**< Enable port */
-		int tx_enabled;  /**< Enable TX */
-		int rx_enabled;  /**< Enable RX */
+		bool enabled;     /**< Enable port */
+		bool tx_enabled;  /**< Enable TX */
+		bool rx_enabled;  /**< Enable RX */
+		bool supp_aneg;   /**< Support auto negotiation */
 
-		int override_changed;  /**< Media reconfig pending */
+		bool override_changed;  /**< Media reconfig pending */
 
 		uint8_t port_type;    /**< One of %PORT_* */
 		/** Sum of lanes of all subports of this port */
 		uint32_t port_lanes;
 
-		int is_split;   /**< Split port */
+		bool is_split;   /**< Split port */
 
 		uint32_t fec_modes_supported;  /**< Bitmap of FEC modes supported */
 	} ports[]; /**< Table of ports */
@@ -177,8 +152,8 @@  struct nfp_eth_table {
 
 struct nfp_eth_table *nfp_eth_read_ports(struct nfp_cpp *cpp);
 
-int nfp_eth_set_mod_enable(struct nfp_cpp *cpp, uint32_t idx, int enable);
-int nfp_eth_set_configured(struct nfp_cpp *cpp, uint32_t idx, int configed);
+int nfp_eth_set_mod_enable(struct nfp_cpp *cpp, uint32_t idx, bool enable);
+int nfp_eth_set_configured(struct nfp_cpp *cpp, uint32_t idx, bool configured);
 int nfp_eth_set_fec(struct nfp_cpp *cpp, uint32_t idx, enum nfp_eth_fec mode);
 
 int nfp_nsp_read_eth_table(struct nfp_nsp *state, void *buf, size_t size);
@@ -187,12 +162,13 @@  int nfp_nsp_write_eth_table(struct nfp_nsp *state, const void *buf,
 void nfp_nsp_config_set_state(struct nfp_nsp *state, void *entries,
 		uint32_t idx);
 void nfp_nsp_config_clear_state(struct nfp_nsp *state);
-void nfp_nsp_config_set_modified(struct nfp_nsp *state, int modified);
+void nfp_nsp_config_set_modified(struct nfp_nsp *state, bool modified);
 void *nfp_nsp_config_entries(struct nfp_nsp *state);
-int nfp_nsp_config_modified(struct nfp_nsp *state);
+struct nfp_cpp *nfp_nsp_cpp(struct nfp_nsp *state);
+bool nfp_nsp_config_modified(struct nfp_nsp *state);
 uint32_t nfp_nsp_config_idx(struct nfp_nsp *state);
 
-static inline int
+static inline bool
 nfp_eth_can_support_fec(struct nfp_eth_table_port *eth_port)
 {
 	return eth_port->fec_modes_supported != 0;
diff --git a/drivers/net/nfp/nfpcore/nfp_nsp_cmds.c b/drivers/net/nfp/nfpcore/nfp_nsp_cmds.c
index 429f639fa2..86956f4330 100644
--- a/drivers/net/nfp/nfpcore/nfp_nsp_cmds.c
+++ b/drivers/net/nfp/nfpcore/nfp_nsp_cmds.c
@@ -3,12 +3,8 @@ 
  * All rights reserved.
  */
 
-#include <stdio.h>
-#include <rte_byteorder.h>
-#include "nfp_cpp.h"
 #include "nfp_logs.h"
 #include "nfp_nsp.h"
-#include "nfp_nffw.h"
 
 struct nsp_identify {
 	uint8_t version[40];
diff --git a/drivers/net/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/nfp/nfpcore/nfp_nsp_eth.c
index 355d907f4d..996fd4b44a 100644
--- a/drivers/net/nfp/nfpcore/nfp_nsp_eth.c
+++ b/drivers/net/nfp/nfpcore/nfp_nsp_eth.c
@@ -3,10 +3,6 @@ 
  * All rights reserved.
  */
 
-#include <stdio.h>
-#include <rte_common.h>
-#include <rte_byteorder.h>
-#include "nfp_cpp.h"
 #include "nfp_logs.h"
 #include "nfp_nsp.h"
 #include "nfp_platform.h"
@@ -21,6 +17,7 @@ 
 #define NSP_ETH_PORT_PHYLABEL           GENMASK_ULL(59, 54)
 #define NSP_ETH_PORT_FEC_SUPP_BASER     RTE_BIT64(60)
 #define NSP_ETH_PORT_FEC_SUPP_RS        RTE_BIT64(61)
+#define NSP_ETH_PORT_SUPP_ANEG          RTE_BIT64(63)
 
 #define NSP_ETH_PORT_LANES_MASK         rte_cpu_to_le_64(NSP_ETH_PORT_LANES)
 
@@ -34,6 +31,7 @@ 
 #define NSP_ETH_STATE_OVRD_CHNG         RTE_BIT64(22)
 #define NSP_ETH_STATE_ANEG              GENMASK_ULL(25, 23)
 #define NSP_ETH_STATE_FEC               GENMASK_ULL(27, 26)
+#define NSP_ETH_STATE_ACT_FEC           GENMASK_ULL(29, 28)
 
 #define NSP_ETH_CTRL_CONFIGURED         RTE_BIT64(0)
 #define NSP_ETH_CTRL_ENABLED            RTE_BIT64(1)
@@ -54,26 +52,12 @@ 
 #define PORT_NONE               0xef
 #define PORT_OTHER              0xff
 
-#define SPEED_10                10
-#define SPEED_100               100
-#define SPEED_1000              1000
-#define SPEED_2500              2500
-#define SPEED_5000              5000
-#define SPEED_10000             10000
-#define SPEED_14000             14000
-#define SPEED_20000             20000
-#define SPEED_25000             25000
-#define SPEED_40000             40000
-#define SPEED_50000             50000
-#define SPEED_56000             56000
-#define SPEED_100000            100000
-
 enum nfp_eth_raw {
 	NSP_ETH_RAW_PORT = 0,
 	NSP_ETH_RAW_STATE,
 	NSP_ETH_RAW_MAC,
 	NSP_ETH_RAW_CONTROL,
-	NSP_ETH_NUM_RAW
+	NSP_ETH_NUM_RAW,
 };
 
 enum nfp_eth_rate {
@@ -100,12 +84,12 @@  static const struct {
 	enum nfp_eth_rate rate;
 	uint32_t speed;
 } nsp_eth_rate_tbl[] = {
-	{ RATE_INVALID, 0, },
-	{ RATE_10M,     SPEED_10, },
-	{ RATE_100M,    SPEED_100, },
-	{ RATE_1G,      SPEED_1000, },
-	{ RATE_10G,     SPEED_10000, },
-	{ RATE_25G,     SPEED_25000, },
+	{ RATE_INVALID, RTE_ETH_SPEED_NUM_NONE, },
+	{ RATE_10M,     RTE_ETH_SPEED_NUM_10M, },
+	{ RATE_100M,    RTE_ETH_SPEED_NUM_100M, },
+	{ RATE_1G,      RTE_ETH_SPEED_NUM_1G, },
+	{ RATE_10G,     RTE_ETH_SPEED_NUM_10G, },
+	{ RATE_25G,     RTE_ETH_SPEED_NUM_25G, },
 };
 
 static uint32_t
@@ -192,7 +176,14 @@  nfp_eth_port_translate(struct nfp_nsp *nsp,
 	if (dst->fec_modes_supported != 0)
 		dst->fec_modes_supported |= NFP_FEC_AUTO | NFP_FEC_DISABLED;
 
-	dst->fec = 1 << FIELD_GET(NSP_ETH_STATE_FEC, state);
+	dst->fec = FIELD_GET(NSP_ETH_STATE_FEC, state);
+	dst->act_fec = dst->fec;
+
+	if (nfp_nsp_get_abi_ver_minor(nsp) < 33)
+		return;
+
+	dst->act_fec = FIELD_GET(NSP_ETH_STATE_ACT_FEC, state);
+	dst->supp_aneg = FIELD_GET(NSP_ETH_PORT_SUPP_ANEG, port);
 }
 
 static void
@@ -221,7 +212,7 @@  nfp_eth_calc_port_geometry(struct nfp_eth_table *table)
 						table->ports[i].label_port,
 						table->ports[i].label_subport);
 
-			table->ports[i].is_split = 1;
+			table->ports[i].is_split = true;
 		}
 	}
 }
@@ -232,6 +223,9 @@  nfp_eth_calc_port_type(struct nfp_eth_table_port *entry)
 	if (entry->interface == NFP_INTERFACE_NONE) {
 		entry->port_type = PORT_NONE;
 		return;
+	} else if (entry->interface == NFP_INTERFACE_RJ45) {
+		entry->port_type = PORT_TP;
+		return;
 	}
 
 	if (entry->media == NFP_MEDIA_FIBRE)
@@ -250,7 +244,6 @@  nfp_eth_read_ports_real(struct nfp_nsp *nsp)
 	uint32_t table_sz;
 	struct nfp_eth_table *table;
 	union eth_table_entry *entries;
-	const struct rte_ether_addr *mac;
 
 	entries = rte_zmalloc(NULL, NSP_ETH_TABLE_SIZE, 0);
 	if (entries == NULL)
@@ -262,16 +255,9 @@  nfp_eth_read_ports_real(struct nfp_nsp *nsp)
 		goto err;
 	}
 
-	/*
-	 * The NFP3800 NIC support 8 ports, but only 2 ports are valid,
-	 * the rest 6 ports mac are all 0, ensure we don't use these port
-	 */
-	for (i = 0; i < NSP_ETH_MAX_COUNT; i++) {
-		mac = (const struct rte_ether_addr *)entries[i].mac_addr;
-		if ((entries[i].port & NSP_ETH_PORT_LANES_MASK) != 0 &&
-				!rte_is_zero_ether_addr(mac))
+	for (i = 0; i < NSP_ETH_MAX_COUNT; i++)
+		if ((entries[i].port & NSP_ETH_PORT_LANES_MASK) != 0)
 			cnt++;
-	}
 
 	/*
 	 * Some versions of flash will give us 0 instead of port count. For
@@ -291,11 +277,8 @@  nfp_eth_read_ports_real(struct nfp_nsp *nsp)
 
 	table->count = cnt;
 	for (i = 0, j = 0; i < NSP_ETH_MAX_COUNT; i++) {
-		mac = (const struct rte_ether_addr *)entries[i].mac_addr;
-		if ((entries[i].port & NSP_ETH_PORT_LANES_MASK) != 0 &&
-				!rte_is_zero_ether_addr(mac))
-			nfp_eth_port_translate(nsp, &entries[i], i,
-					&table->ports[j++]);
+		if ((entries[i].port & NSP_ETH_PORT_LANES_MASK) != 0)
+			nfp_eth_port_translate(nsp, &entries[i], i, &table->ports[j++]);
 	}
 
 	nfp_eth_calc_port_geometry(table);
@@ -436,7 +419,7 @@  nfp_eth_config_commit_end(struct nfp_nsp *nsp)
 int
 nfp_eth_set_mod_enable(struct nfp_cpp *cpp,
 		uint32_t idx,
-		int enable)
+		bool enable)
 {
 	uint64_t reg;
 	struct nfp_nsp *nsp;
@@ -444,7 +427,7 @@  nfp_eth_set_mod_enable(struct nfp_cpp *cpp,
 
 	nsp = nfp_eth_config_start(cpp, idx);
 	if (nsp == NULL)
-		return -1;
+		return -EIO;
 
 	entries = nfp_nsp_config_entries(nsp);
 
@@ -456,7 +439,7 @@  nfp_eth_set_mod_enable(struct nfp_cpp *cpp,
 		reg |= FIELD_PREP(NSP_ETH_CTRL_ENABLED, enable);
 		entries[idx].control = rte_cpu_to_le_64(reg);
 
-		nfp_nsp_config_set_modified(nsp, 1);
+		nfp_nsp_config_set_modified(nsp, true);
 	}
 
 	return nfp_eth_config_commit_end(nsp);
@@ -480,7 +463,7 @@  nfp_eth_set_mod_enable(struct nfp_cpp *cpp,
 int
 nfp_eth_set_configured(struct nfp_cpp *cpp,
 		uint32_t idx,
-		int configured)
+		bool configured)
 {
 	uint64_t reg;
 	struct nfp_nsp *nsp;
@@ -509,7 +492,7 @@  nfp_eth_set_configured(struct nfp_cpp *cpp,
 		reg |= FIELD_PREP(NSP_ETH_CTRL_CONFIGURED, configured);
 		entries[idx].control = rte_cpu_to_le_64(reg);
 
-		nfp_nsp_config_set_modified(nsp, 1);
+		nfp_nsp_config_set_modified(nsp, true);
 	}
 
 	return nfp_eth_config_commit_end(nsp);
@@ -547,7 +530,7 @@  nfp_eth_set_bit_config(struct nfp_nsp *nsp,
 
 	entries[idx].control |= rte_cpu_to_le_64(ctrl_bit);
 
-	nfp_nsp_config_set_modified(nsp, 1);
+	nfp_nsp_config_set_modified(nsp, true);
 
 	return 0;
 }