Merge branch 'nfp-ethtool-and-related-improvements'

Simon Horman says:

====================
nfp: ethtool and related improvements

Dirk van der Merwe says:

This patch series throws a couple of loosely related items into a single
series.

Patch 1: Clang compilation fix reported by
  Matthias Kaehlcke <mka@chromium.org>

Patch 2: Driver can now do MAC reinit on load when there has been a
  media override set in the NSP.

Patch 3: Refactor the nfp_app_reprs_set API.

Patch 4: Similar to vNICs, representors must be able to deal with media
  override changes in the NSP.

Patch 5: Since representors can now handle media overrides, we can
  allocate the get/set link ndo's to them.

Patch 6 & 7: Add support for FEC mode modification.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>

drivers/net/ethernet/netronome/nfp/flower/main.c

@@ -142,8 +142,8 @@ nfp_flower_spawn_vnic_reprs(struct nfp_app *app,
 {
 	u8 nfp_pcie = nfp_cppcore_pcie_unit(app->pf->cpp);
 	struct nfp_flower_priv *priv = app->priv;
-	struct nfp_reprs *reprs, *old_reprs;
 	enum nfp_port_type port_type;
+	struct nfp_reprs *reprs;
 	const u8 queue = 0;
 	int i, err;
 
@@ -194,11 +194,7 @@ nfp_flower_spawn_vnic_reprs(struct nfp_app *app,
 			 reprs->reprs[i]->name);
 	}
 
-	old_reprs = nfp_app_reprs_set(app, repr_type, reprs);
-	if (IS_ERR(old_reprs)) {
-		err = PTR_ERR(old_reprs);
-		goto err_reprs_clean;
-	}
+	nfp_app_reprs_set(app, repr_type, reprs);
 
 	return 0;
 err_reprs_clean:
@@ -222,8 +218,8 @@ static int
 nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv)
 {
 	struct nfp_eth_table *eth_tbl = app->pf->eth_tbl;
-	struct nfp_reprs *reprs, *old_reprs;
 	struct sk_buff *ctrl_skb;
+	struct nfp_reprs *reprs;
 	unsigned int i;
 	int err;
 
@@ -280,11 +276,7 @@ nfp_flower_spawn_phy_reprs(struct nfp_app *app, struct nfp_flower_priv *priv)
 			 phys_port, reprs->reprs[phys_port]->name);
 	}
 
-	old_reprs = nfp_app_reprs_set(app, NFP_REPR_TYPE_PHYS_PORT, reprs);
-	if (IS_ERR(old_reprs)) {
-		err = PTR_ERR(old_reprs);
-		goto err_reprs_clean;
-	}
+	nfp_app_reprs_set(app, NFP_REPR_TYPE_PHYS_PORT, reprs);
 
 	/* The MAC_REPR control message should be sent after the MAC
 	 * representors are registered using nfp_app_reprs_set().  This is

drivers/net/ethernet/netronome/nfp/nfp_app.c

@@ -106,14 +106,8 @@ nfp_app_reprs_set(struct nfp_app *app, enum nfp_repr_type type,
 
 	old = rcu_dereference_protected(app->reprs[type],
 					lockdep_is_held(&app->pf->lock));
-	if (reprs && old) {
-		old = ERR_PTR(-EBUSY);
-		goto exit_unlock;
-	}
-
 	rcu_assign_pointer(app->reprs[type], reprs);
 
-exit_unlock:
 	return old;
 }
 

drivers/net/ethernet/netronome/nfp/nfp_main.c

@@ -346,6 +346,32 @@ exit_release_fw:
 	return err < 0 ? err : 1;
 }
 
+static void
+nfp_nsp_init_ports(struct pci_dev *pdev, struct nfp_pf *pf,
+		   struct nfp_nsp *nsp)
+{
+	bool needs_reinit = false;
+	int i;
+
+	pf->eth_tbl = __nfp_eth_read_ports(pf->cpp, nsp);
+	if (!pf->eth_tbl)
+		return;
+
+	if (!nfp_nsp_has_mac_reinit(nsp))
+		return;
+
+	for (i = 0; i < pf->eth_tbl->count; i++)
+		needs_reinit |= pf->eth_tbl->ports[i].override_changed;
+	if (!needs_reinit)
+		return;
+
+	kfree(pf->eth_tbl);
+	if (nfp_nsp_mac_reinit(nsp))
+		dev_warn(&pdev->dev, "MAC reinit failed\n");
+
+	pf->eth_tbl = __nfp_eth_read_ports(pf->cpp, nsp);
+}
+
 static int nfp_nsp_init(struct pci_dev *pdev, struct nfp_pf *pf)
 {
 	struct nfp_nsp *nsp;
@@ -366,7 +392,7 @@ static int nfp_nsp_init(struct pci_dev *pdev, struct nfp_pf *pf)
 	if (err < 0)
 		goto exit_close_nsp;
 
-	pf->eth_tbl = __nfp_eth_read_ports(pf->cpp, nsp);
+	nfp_nsp_init_ports(pdev, pf, nsp);
 
 	pf->nspi = __nfp_nsp_identify(nsp);
 	if (pf->nspi)

drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c

@@ -244,6 +244,30 @@ nfp_app_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
 	nfp_get_drvinfo(app, app->pdev, "*", drvinfo);
 }
 
+static void
+nfp_net_set_fec_link_mode(struct nfp_eth_table_port *eth_port,
+			  struct ethtool_link_ksettings *c)
+{
+	unsigned int modes;
+
+	ethtool_link_ksettings_add_link_mode(c, supported, FEC_NONE);
+	if (!nfp_eth_can_support_fec(eth_port)) {
+		ethtool_link_ksettings_add_link_mode(c, advertising, FEC_NONE);
+		return;
+	}
+
+	modes = nfp_eth_supported_fec_modes(eth_port);
+	if (modes & NFP_FEC_BASER) {
+		ethtool_link_ksettings_add_link_mode(c, supported, FEC_BASER);
+		ethtool_link_ksettings_add_link_mode(c, advertising, FEC_BASER);
+	}
+
+	if (modes & NFP_FEC_REED_SOLOMON) {
+		ethtool_link_ksettings_add_link_mode(c, supported, FEC_RS);
+		ethtool_link_ksettings_add_link_mode(c, advertising, FEC_RS);
+	}
+}
+
 /**
  * nfp_net_get_link_ksettings - Get Link Speed settings
  * @netdev:	network interface device structure
@@ -278,9 +302,11 @@ nfp_net_get_link_ksettings(struct net_device *netdev,
 
 	port = nfp_port_from_netdev(netdev);
 	eth_port = nfp_port_get_eth_port(port);
-	if (eth_port)
+	if (eth_port) {
 		cmd->base.autoneg = eth_port->aneg != NFP_ANEG_DISABLED ?
 			AUTONEG_ENABLE : AUTONEG_DISABLE;
+		nfp_net_set_fec_link_mode(eth_port, cmd);
+	}
 
 	if (!netif_carrier_ok(netdev))
 		return 0;
@@ -328,7 +354,7 @@ nfp_net_set_link_ksettings(struct net_device *netdev,
 		return -EOPNOTSUPP;
 
 	if (netif_running(netdev)) {
-		netdev_warn(netdev, "Changing settings not allowed on an active interface. It may cause the port to be disabled until reboot.\n");
+		netdev_warn(netdev, "Changing settings not allowed on an active interface. It may cause the port to be disabled until driver reload.\n");
 		return -EBUSY;
 	}
 
@@ -686,6 +712,91 @@ static int nfp_port_get_sset_count(struct net_device *netdev, int sset)
 	}
 }
 
+static int nfp_port_fec_ethtool_to_nsp(u32 fec)
+{
+	switch (fec) {
+	case ETHTOOL_FEC_AUTO:
+		return NFP_FEC_AUTO_BIT;
+	case ETHTOOL_FEC_OFF:
+		return NFP_FEC_DISABLED_BIT;
+	case ETHTOOL_FEC_RS:
+		return NFP_FEC_REED_SOLOMON_BIT;
+	case ETHTOOL_FEC_BASER:
+		return NFP_FEC_BASER_BIT;
+	default:
+		/* NSP only supports a single mode at a time */
+		return -EOPNOTSUPP;
+	}
+}
+
+static u32 nfp_port_fec_nsp_to_ethtool(u32 fec)
+{
+	u32 result = 0;
+
+	if (fec & NFP_FEC_AUTO)
+		result |= ETHTOOL_FEC_AUTO;
+	if (fec & NFP_FEC_BASER)
+		result |= ETHTOOL_FEC_BASER;
+	if (fec & NFP_FEC_REED_SOLOMON)
+		result |= ETHTOOL_FEC_RS;
+	if (fec & NFP_FEC_DISABLED)
+		result |= ETHTOOL_FEC_OFF;
+
+	return result ?: ETHTOOL_FEC_NONE;
+}
+
+static int
+nfp_port_get_fecparam(struct net_device *netdev,
+		      struct ethtool_fecparam *param)
+{
+	struct nfp_eth_table_port *eth_port;
+	struct nfp_port *port;
+
+	param->active_fec = ETHTOOL_FEC_NONE_BIT;
+	param->fec = ETHTOOL_FEC_NONE_BIT;
+
+	port = nfp_port_from_netdev(netdev);
+	eth_port = nfp_port_get_eth_port(port);
+	if (!eth_port)
+		return -EOPNOTSUPP;
+
+	if (!nfp_eth_can_support_fec(eth_port))
+		return 0;
+
+	param->fec = nfp_port_fec_nsp_to_ethtool(eth_port->fec_modes_supported);
+	param->active_fec = nfp_port_fec_nsp_to_ethtool(eth_port->fec);
+
+	return 0;
+}
+
+static int
+nfp_port_set_fecparam(struct net_device *netdev,
+		      struct ethtool_fecparam *param)
+{
+	struct nfp_eth_table_port *eth_port;
+	struct nfp_port *port;
+	int err, fec;
+
+	port = nfp_port_from_netdev(netdev);
+	eth_port = nfp_port_get_eth_port(port);
+	if (!eth_port)
+		return -EOPNOTSUPP;
+
+	if (!nfp_eth_can_support_fec(eth_port))
+		return -EOPNOTSUPP;
+
+	fec = nfp_port_fec_ethtool_to_nsp(param->fec);
+	if (fec < 0)
+		return fec;
+
+	err = nfp_eth_set_fec(port->app->cpp, eth_port->index, fec);
+	if (!err)
+		/* Only refresh if we did something */
+		nfp_net_refresh_port_table(port);
+
+	return err < 0 ? err : 0;
+}
+
 /* RX network flow classification (RSS, filters, etc)
  */
 static u32 ethtool_flow_to_nfp_flag(u32 flow_type)
@@ -1144,6 +1255,8 @@ static const struct ethtool_ops nfp_net_ethtool_ops = {
 	.set_channels		= nfp_net_set_channels,
 	.get_link_ksettings	= nfp_net_get_link_ksettings,
 	.set_link_ksettings	= nfp_net_set_link_ksettings,
+	.get_fecparam		= nfp_port_get_fecparam,
+	.set_fecparam		= nfp_port_set_fecparam,
 };
 
 const struct ethtool_ops nfp_port_ethtool_ops = {
@@ -1155,6 +1268,10 @@ const struct ethtool_ops nfp_port_ethtool_ops = {
 	.set_dump		= nfp_app_set_dump,
 	.get_dump_flag		= nfp_app_get_dump_flag,
 	.get_dump_data		= nfp_app_get_dump_data,
+	.get_link_ksettings	= nfp_net_get_link_ksettings,
+	.set_link_ksettings	= nfp_net_set_link_ksettings,
+	.get_fecparam		= nfp_port_get_fecparam,
+	.set_fecparam		= nfp_port_set_fecparam,
 };
 
 void nfp_net_set_ethtool_ops(struct net_device *netdev)

drivers/net/ethernet/netronome/nfp/nfp_net_main.c

@@ -597,7 +597,7 @@ nfp_net_eth_port_update(struct nfp_cpp *cpp, struct nfp_port *port,
 		return -EIO;
 	}
 	if (eth_port->override_changed) {
-		nfp_warn(cpp, "Port #%d config changed, unregistering. Reboot required before port will be operational again.\n", port->eth_id);
+		nfp_warn(cpp, "Port #%d config changed, unregistering. Driver reload required before port will be operational again.\n", port->eth_id);
 		port->type = NFP_PORT_INVALID;
 	}
 
@@ -611,6 +611,7 @@ int nfp_net_refresh_port_table_sync(struct nfp_pf *pf)
 	struct nfp_eth_table *eth_table;
 	struct nfp_net *nn, *next;
 	struct nfp_port *port;
+	int err;
 
 	lockdep_assert_held(&pf->lock);
 
@@ -640,6 +641,11 @@ int nfp_net_refresh_port_table_sync(struct nfp_pf *pf)
 
 	kfree(eth_table);
 
+	/* Resync repr state. This may cause reprs to be removed. */
+	err = nfp_reprs_resync_phys_ports(pf->app);
+	if (err)
+		return err;
+
 	/* Shoot off the ports which became invalid */
 	list_for_each_entry_safe(nn, next, &pf->vnics, vnic_list) {
 		if (!nn->port || nn->port->type != NFP_PORT_INVALID)

drivers/net/ethernet/netronome/nfp/nfp_net_repr.c

@@ -390,3 +390,50 @@ struct nfp_reprs *nfp_reprs_alloc(unsigned int num_reprs)
 
 	return reprs;
 }
+
+int nfp_reprs_resync_phys_ports(struct nfp_app *app)
+{
+	struct nfp_reprs *reprs, *old_reprs;
+	struct nfp_repr *repr;
+	int i;
+
+	old_reprs =
+		rcu_dereference_protected(app->reprs[NFP_REPR_TYPE_PHYS_PORT],
+					  lockdep_is_held(&app->pf->lock));
+	if (!old_reprs)
+		return 0;
+
+	reprs = nfp_reprs_alloc(old_reprs->num_reprs);
+	if (!reprs)
+		return -ENOMEM;
+
+	for (i = 0; i < old_reprs->num_reprs; i++) {
+		if (!old_reprs->reprs[i])
+			continue;
+
+		repr = netdev_priv(old_reprs->reprs[i]);
+		if (repr->port->type == NFP_PORT_INVALID)
+			continue;
+
+		reprs->reprs[i] = old_reprs->reprs[i];
+	}
+
+	old_reprs = nfp_app_reprs_set(app, NFP_REPR_TYPE_PHYS_PORT, reprs);
+	synchronize_rcu();
+
+	/* Now we free up removed representors */
+	for (i = 0; i < old_reprs->num_reprs; i++) {
+		if (!old_reprs->reprs[i])
+			continue;
+
+		repr = netdev_priv(old_reprs->reprs[i]);
+		if (repr->port->type != NFP_PORT_INVALID)
+			continue;
+
+		nfp_app_repr_stop(app, repr);
+		nfp_repr_clean(repr);
+	}
+
+	kfree(old_reprs);
+	return 0;
+}

drivers/net/ethernet/netronome/nfp/nfp_net_repr.h

@@ -124,5 +124,6 @@ void
 nfp_reprs_clean_and_free_by_type(struct nfp_app *app,
 				 enum nfp_repr_type type);
 struct nfp_reprs *nfp_reprs_alloc(unsigned int num_reprs);
+int nfp_reprs_resync_phys_ports(struct nfp_app *app);
 
 #endif /* NFP_NET_REPR_H */

drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c

@@ -477,6 +477,11 @@ int nfp_nsp_device_soft_reset(struct nfp_nsp *state)
 	return nfp_nsp_command(state, SPCODE_SOFT_RESET, 0, 0, 0);
 }
 
+int nfp_nsp_mac_reinit(struct nfp_nsp *state)
+{
+	return nfp_nsp_command(state, SPCODE_MAC_INIT, 0, 0, 0);
+}
+
 int nfp_nsp_load_fw(struct nfp_nsp *state, const struct firmware *fw)
 {
 	return nfp_nsp_command_buf(state, SPCODE_FW_LOAD, fw->size, fw->data,

drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h

@@ -48,6 +48,12 @@ u16 nfp_nsp_get_abi_ver_minor(struct nfp_nsp *state);
 int nfp_nsp_wait(struct nfp_nsp *state);
 int nfp_nsp_device_soft_reset(struct nfp_nsp *state);
 int nfp_nsp_load_fw(struct nfp_nsp *state, const struct firmware *fw);
+int nfp_nsp_mac_reinit(struct nfp_nsp *state);
+
+static inline bool nfp_nsp_has_mac_reinit(struct nfp_nsp *state)
+{
+	return nfp_nsp_get_abi_ver_minor(state) > 20;
+}
 
 enum nfp_eth_interface {
 	NFP_INTERFACE_NONE	= 0,
@@ -73,6 +79,18 @@ enum nfp_eth_aneg {
 	NFP_ANEG_DISABLED,
 };
 
+enum nfp_eth_fec {
+	NFP_FEC_AUTO_BIT = 0,
+	NFP_FEC_BASER_BIT,
+	NFP_FEC_REED_SOLOMON_BIT,
+	NFP_FEC_DISABLED_BIT,
+};
+
+#define NFP_FEC_AUTO		BIT(NFP_FEC_AUTO_BIT)
+#define NFP_FEC_BASER		BIT(NFP_FEC_BASER_BIT)
+#define NFP_FEC_REED_SOLOMON	BIT(NFP_FEC_REED_SOLOMON_BIT)
+#define NFP_FEC_DISABLED	BIT(NFP_FEC_DISABLED_BIT)
+
 /**
  * struct nfp_eth_table - ETH table information
  * @count:	number of table entries
@@ -87,6 +105,7 @@ enum nfp_eth_aneg {
  * @speed:	interface speed (in Mbps)
  * @interface:	interface (module) plugged in
  * @media:	media type of the @interface
+ * @fec:	forward error correction mode
  * @aneg:	auto negotiation mode
  * @mac_addr:	interface MAC address
  * @label_port:	port id
@@ -99,6 +118,7 @@ enum nfp_eth_aneg {
  * @port_type:	one of %PORT_* defines for ethtool
  * @port_lanes:	total number of lanes on the port (sum of lanes of all subports)
  * @is_split:	is interface part of a split port
+ * @fec_modes_supported:	bitmap of FEC modes supported
  */
 struct nfp_eth_table {
 	unsigned int count;
@@ -114,6 +134,7 @@ struct nfp_eth_table {
 		unsigned int interface;
 		enum nfp_eth_media media;
 
+		enum nfp_eth_fec fec;
 		enum nfp_eth_aneg aneg;
 
 		u8 mac_addr[ETH_ALEN];
@@ -133,6 +154,8 @@ struct nfp_eth_table {
 		unsigned int port_lanes;
 
 		bool is_split;
+
+		unsigned int fec_modes_supported;
 	} ports[0];
 };
 
@@ -143,6 +166,19 @@ __nfp_eth_read_ports(struct nfp_cpp *cpp, struct nfp_nsp *nsp);
 int nfp_eth_set_mod_enable(struct nfp_cpp *cpp, unsigned int idx, bool enable);
 int nfp_eth_set_configured(struct nfp_cpp *cpp, unsigned int idx,
 			   bool configed);
+int
+nfp_eth_set_fec(struct nfp_cpp *cpp, unsigned int idx, enum nfp_eth_fec mode);
+
+static inline bool nfp_eth_can_support_fec(struct nfp_eth_table_port *eth_port)
+{
+	return !!eth_port->fec_modes_supported;
+}
+
+static inline unsigned int
+nfp_eth_supported_fec_modes(struct nfp_eth_table_port *eth_port)
+{
+	return eth_port->fec_modes_supported;
+}
 
 struct nfp_nsp *nfp_eth_config_start(struct nfp_cpp *cpp, unsigned int idx);
 int nfp_eth_config_commit_end(struct nfp_nsp *nsp);

drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c

@@ -55,6 +55,8 @@
 #define NSP_ETH_PORT_INDEX		GENMASK_ULL(15, 8)
 #define NSP_ETH_PORT_LABEL		GENMASK_ULL(53, 48)
 #define NSP_ETH_PORT_PHYLABEL		GENMASK_ULL(59, 54)
+#define NSP_ETH_PORT_FEC_SUPP_BASER	BIT_ULL(60)
+#define NSP_ETH_PORT_FEC_SUPP_RS	BIT_ULL(61)
 
 #define NSP_ETH_PORT_LANES_MASK		cpu_to_le64(NSP_ETH_PORT_LANES)
 
@@ -67,6 +69,7 @@
 #define NSP_ETH_STATE_MEDIA		GENMASK_ULL(21, 20)
 #define NSP_ETH_STATE_OVRD_CHNG		BIT_ULL(22)
 #define NSP_ETH_STATE_ANEG		GENMASK_ULL(25, 23)
+#define NSP_ETH_STATE_FEC		GENMASK_ULL(27, 26)
 
 #define NSP_ETH_CTRL_CONFIGURED		BIT_ULL(0)
 #define NSP_ETH_CTRL_ENABLED		BIT_ULL(1)
@@ -75,6 +78,7 @@
 #define NSP_ETH_CTRL_SET_RATE		BIT_ULL(4)
 #define NSP_ETH_CTRL_SET_LANES		BIT_ULL(5)
 #define NSP_ETH_CTRL_SET_ANEG		BIT_ULL(6)
+#define NSP_ETH_CTRL_SET_FEC		BIT_ULL(7)
 
 enum nfp_eth_raw {
 	NSP_ETH_RAW_PORT = 0,
@@ -152,6 +156,7 @@ nfp_eth_port_translate(struct nfp_nsp *nsp, const union eth_table_entry *src,
 		       unsigned int index, struct nfp_eth_table_port *dst)
 {
 	unsigned int rate;
+	unsigned int fec;
 	u64 port, state;
 
 	port = le64_to_cpu(src->port);
@@ -183,6 +188,18 @@ nfp_eth_port_translate(struct nfp_nsp *nsp, const union eth_table_entry *src,
 
 	dst->override_changed = FIELD_GET(NSP_ETH_STATE_OVRD_CHNG, state);
 	dst->aneg = FIELD_GET(NSP_ETH_STATE_ANEG, state);
+
+	if (nfp_nsp_get_abi_ver_minor(nsp) < 22)
+		return;
+
+	fec = FIELD_GET(NSP_ETH_PORT_FEC_SUPP_BASER, port);
+	dst->fec_modes_supported |= fec << NFP_FEC_BASER_BIT;
+	fec = FIELD_GET(NSP_ETH_PORT_FEC_SUPP_RS, port);
+	dst->fec_modes_supported |= fec << NFP_FEC_REED_SOLOMON_BIT;
+	if (dst->fec_modes_supported)
+		dst->fec_modes_supported |= NFP_FEC_AUTO | NFP_FEC_DISABLED;
+
+	dst->fec = 1 << FIELD_GET(NSP_ETH_STATE_FEC, state);
 }
 
 static void
@@ -469,10 +486,10 @@ int nfp_eth_set_configured(struct nfp_cpp *cpp, unsigned int idx, bool configed)
 	return nfp_eth_config_commit_end(nsp);
 }
 
-/* Force inline, FIELD_* macroes require masks to be compilation-time known */
-static __always_inline int
+static int
 nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
-		       const u64 mask, unsigned int val, const u64 ctrl_bit)
+		       const u64 mask, const unsigned int shift,
+		       unsigned int val, const u64 ctrl_bit)
 {
 	union eth_table_entry *entries = nfp_nsp_config_entries(nsp);
 	unsigned int idx = nfp_nsp_config_idx(nsp);
@@ -489,11 +506,11 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
 
 	/* Check if we are already in requested state */
 	reg = le64_to_cpu(entries[idx].raw[raw_idx]);
-	if (val == FIELD_GET(mask, reg))
+	if (val == (reg & mask) >> shift)
 		return 0;
 
 	reg &= ~mask;
-	reg |= FIELD_PREP(mask, val);
+	reg |= (val << shift) & mask;
 	entries[idx].raw[raw_idx] = cpu_to_le64(reg);
 
 	entries[idx].control |= cpu_to_le64(ctrl_bit);
@@ -503,6 +520,13 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
 	return 0;
 }
 
+#define NFP_ETH_SET_BIT_CONFIG(nsp, raw_idx, mask, val, ctrl_bit)	\
+	({								\
+		__BF_FIELD_CHECK(mask, 0ULL, val, "NFP_ETH_SET_BIT_CONFIG: "); \
+		nfp_eth_set_bit_config(nsp, raw_idx, mask, __bf_shf(mask), \
+				       val, ctrl_bit);			\
+	})
+
 /**
  * __nfp_eth_set_aneg() - set PHY autonegotiation control bit
  * @nsp:	NFP NSP handle returned from nfp_eth_config_start()
@@ -515,11 +539,58 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx,
  */
 int __nfp_eth_set_aneg(struct nfp_nsp *nsp, enum nfp_eth_aneg mode)
 {
-	return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_STATE,
+	return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE,
 				      NSP_ETH_STATE_ANEG, mode,
 				      NSP_ETH_CTRL_SET_ANEG);
 }
 
+/**
+ * __nfp_eth_set_fec() - set PHY forward error correction control bit
+ * @nsp:	NFP NSP handle returned from nfp_eth_config_start()
+ * @mode:	Desired fec mode
+ *
+ * Set the PHY module forward error correction mode.
+ * Will write to hwinfo overrides in the flash (persistent config).
+ *
+ * Return: 0 or -ERRNO.
+ */
+static int __nfp_eth_set_fec(struct nfp_nsp *nsp, enum nfp_eth_fec mode)
+{
+	return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE,
+				      NSP_ETH_STATE_FEC, mode,
+				      NSP_ETH_CTRL_SET_FEC);
+}
+
+/**
+ * nfp_eth_set_fec() - set PHY forward error correction control mode
+ * @cpp:	NFP CPP handle
+ * @idx:	NFP chip-wide port index
+ * @mode:	Desired fec mode
+ *
+ * Return:
+ * 0 - configuration successful;
+ * 1 - no changes were needed;
+ * -ERRNO - configuration failed.
+ */
+int
+nfp_eth_set_fec(struct nfp_cpp *cpp, unsigned int idx, enum nfp_eth_fec mode)
+{
+	struct nfp_nsp *nsp;
+	int err;
+
+	nsp = nfp_eth_config_start(cpp, idx);
+	if (IS_ERR(nsp))
+		return PTR_ERR(nsp);
+
+	err = __nfp_eth_set_fec(nsp, mode);
+	if (err) {
+		nfp_eth_config_cleanup_end(nsp);
+		return err;
+	}
+
+	return nfp_eth_config_commit_end(nsp);
+}
+
 /**
  * __nfp_eth_set_speed() - set interface speed/rate
  * @nsp:	NFP NSP handle returned from nfp_eth_config_start()
@@ -544,7 +615,7 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int speed)
 		return -EINVAL;
 	}
 
-	return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_STATE,
+	return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE,
 				      NSP_ETH_STATE_RATE, rate,
 				      NSP_ETH_CTRL_SET_RATE);
 }
@@ -561,6 +632,6 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int speed)
  */
 int __nfp_eth_set_split(struct nfp_nsp *nsp, unsigned int lanes)
 {
-	return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
+	return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES,
 				      lanes, NSP_ETH_CTRL_SET_LANES);
 }