[PATCH net-next] nfp: ethtool: support reporting link modes

[PATCH net-next] nfp: ethtool: support reporting link modes

[Simon Horman]

From: Yu Xiao <yu.xiao@corigine.com>

Add support for reporting link modes,
including `Supported link modes` and `Advertised link modes`,
via ethtool $DEV.

A new command `SPCODE_READ_MEDIA` is added to read info from
management firmware. Also, the mapping table `nfp_eth_media_table`
associates the link modes between NFP and kernel. Both of them
help to support this ability.

Signed-off-by: Yu Xiao <yu.xiao@corigine.com>
Reviewed-by: Louis Peens <louis.peens@corigine.com>
Signed-off-by: Simon Horman <simon.horman@corigine.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.h |  1 +
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 56 ++++++++++++++++++
 .../ethernet/netronome/nfp/nfpcore/nfp_nsp.c  | 17 ++++++
 .../ethernet/netronome/nfp/nfpcore/nfp_nsp.h  | 59 +++++++++++++++++++
 .../netronome/nfp/nfpcore/nfp_nsp_eth.c       | 27 +++++++++
 5 files changed, 160 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h b/drivers/net/ethernet/netronome/nfp/nfp_main.h
index 5a18af672e6b..14a751bfe1fe 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
@@ -27,6 +27,7 @@ struct nfp_hwinfo;
 struct nfp_mip;
 struct nfp_net;
 struct nfp_nsp_identify;
+struct nfp_eth_media_buf;
 struct nfp_port;
 struct nfp_rtsym;
 struct nfp_rtsym_table;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 0058ba2b3505..be434b01b724 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -293,6 +293,59 @@ nfp_net_set_fec_link_mode(struct nfp_eth_table_port *eth_port,
 	}
 }
 
+static const u8 nfp_eth_media_table[] = {
+	[NFP_MEDIA_1000BASE_CX] = ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+	[NFP_MEDIA_1000BASE_KX] = ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
+	[NFP_MEDIA_10GBASE_KX4] = ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+	[NFP_MEDIA_10GBASE_KR] = ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
+	[NFP_MEDIA_10GBASE_CX4] = ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
+	[NFP_MEDIA_10GBASE_CR] = ETHTOOL_LINK_MODE_10000baseCR_Full_BIT,
+	[NFP_MEDIA_10GBASE_SR] = ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
+	[NFP_MEDIA_10GBASE_ER] = ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
+	[NFP_MEDIA_25GBASE_KR] = ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
+	[NFP_MEDIA_25GBASE_KR_S] = ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
+	[NFP_MEDIA_25GBASE_CR] = ETHTOOL_LINK_MODE_25000baseCR_Full_BIT,
+	[NFP_MEDIA_25GBASE_CR_S] = ETHTOOL_LINK_MODE_25000baseCR_Full_BIT,
+	[NFP_MEDIA_25GBASE_SR] = ETHTOOL_LINK_MODE_25000baseSR_Full_BIT,
+	[NFP_MEDIA_40GBASE_CR4] = ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT,
+	[NFP_MEDIA_40GBASE_KR4] = ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
+	[NFP_MEDIA_40GBASE_SR4] = ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT,
+	[NFP_MEDIA_40GBASE_LR4] = ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT,
+	[NFP_MEDIA_50GBASE_KR] = ETHTOOL_LINK_MODE_50000baseKR_Full_BIT,
+	[NFP_MEDIA_50GBASE_SR] = ETHTOOL_LINK_MODE_50000baseSR_Full_BIT,
+	[NFP_MEDIA_50GBASE_CR] = ETHTOOL_LINK_MODE_50000baseCR_Full_BIT,
+	[NFP_MEDIA_50GBASE_LR] = ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
+	[NFP_MEDIA_50GBASE_ER] = ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
+	[NFP_MEDIA_50GBASE_FR] = ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
+	[NFP_MEDIA_100GBASE_KR4] = ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
+	[NFP_MEDIA_100GBASE_SR4] = ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT,
+	[NFP_MEDIA_100GBASE_CR4] = ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT,
+	[NFP_MEDIA_100GBASE_KP4] = ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
+	[NFP_MEDIA_100GBASE_CR10] = ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT,
+};
+
+static void nfp_add_media_link_mode(struct nfp_port *port,
+				    struct nfp_eth_table_port *eth_port,
+				    struct ethtool_link_ksettings *cmd)
+{
+	struct nfp_eth_media_buf ethm = {.eth_index = eth_port->eth_index};
+	struct nfp_cpp *cpp = port->app->cpp;
+	u8 i;
+
+	if (nfp_eth_read_media(cpp, &ethm))
+		return;
+
+	for (i = 0; i < NFP_MEDIA_LINK_MODES_NUMBER; ++i) {
+		if (test_bit(i, ethm.supported_modes))
+			__set_bit(nfp_eth_media_table[i],
+				  cmd->link_modes.supported);
+
+		if (test_bit(i, ethm.advertised_modes))
+			__set_bit(nfp_eth_media_table[i],
+				  cmd->link_modes.advertising);
+	}
+}
+
 /**
  * nfp_net_get_link_ksettings - Get Link Speed settings
  * @netdev:	network interface device structure
@@ -311,6 +364,8 @@ nfp_net_get_link_ksettings(struct net_device *netdev,
 	u16 sts;
 
 	/* Init to unknowns */
+	ethtool_link_ksettings_zero_link_mode(cmd, supported);
+	ethtool_link_ksettings_zero_link_mode(cmd, advertising);
 	ethtool_link_ksettings_add_link_mode(cmd, supported, FIBRE);
 	cmd->base.port = PORT_OTHER;
 	cmd->base.speed = SPEED_UNKNOWN;
@@ -321,6 +376,7 @@ nfp_net_get_link_ksettings(struct net_device *netdev,
 	if (eth_port) {
 		ethtool_link_ksettings_add_link_mode(cmd, supported, Pause);
 		ethtool_link_ksettings_add_link_mode(cmd, advertising, Pause);
+		nfp_add_media_link_mode(port, eth_port, cmd);
 		if (eth_port->supp_aneg) {
 			ethtool_link_ksettings_add_link_mode(cmd, supported, Autoneg);
 			if (eth_port->aneg == NFP_ANEG_AUTO) {
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index 730fea214b8a..7136bc48530b 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -100,6 +100,7 @@ enum nfp_nsp_cmd {
 	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 either the supported or advertised media for a port */
 };
 
 struct nfp_nsp_dma_buf {
@@ -1100,4 +1101,20 @@ int nfp_nsp_read_module_eeprom(struct nfp_nsp *state, int eth_index,
 	kfree(buf);
 
 	return ret;
+};
+
+int nfp_nsp_read_media(struct nfp_nsp *state, void *buf, unsigned int size)
+{
+	struct nfp_nsp_command_buf_arg media = {
+		{
+			.code		= SPCODE_READ_MEDIA,
+			.option		= size,
+		},
+		.in_buf		= buf,
+		.in_size	= size,
+		.out_buf	= buf,
+		.out_size	= size,
+	};
+
+	return nfp_nsp_command_buf(state, &media);
 }
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
index 992d72ac98d3..c5630fe88a66 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
@@ -65,6 +65,11 @@ 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,
@@ -97,6 +102,47 @@ enum nfp_eth_fec {
 	NFP_FEC_DISABLED_BIT,
 };
 
+/* link modes about RJ45 haven't been used, so there's no mapping to them */
+enum nfp_ethtool_link_mode_list {
+	NFP_MEDIA_W0_RJ45_10M,
+	NFP_MEDIA_W0_RJ45_10M_HD,
+	NFP_MEDIA_W0_RJ45_100M,
+	NFP_MEDIA_W0_RJ45_100M_HD,
+	NFP_MEDIA_W0_RJ45_1G,
+	NFP_MEDIA_W0_RJ45_2P5G,
+	NFP_MEDIA_W0_RJ45_5G,
+	NFP_MEDIA_W0_RJ45_10G,
+	NFP_MEDIA_1000BASE_CX,
+	NFP_MEDIA_1000BASE_KX,
+	NFP_MEDIA_10GBASE_KX4,
+	NFP_MEDIA_10GBASE_KR,
+	NFP_MEDIA_10GBASE_CX4,
+	NFP_MEDIA_10GBASE_CR,
+	NFP_MEDIA_10GBASE_SR,
+	NFP_MEDIA_10GBASE_ER,
+	NFP_MEDIA_25GBASE_KR,
+	NFP_MEDIA_25GBASE_KR_S,
+	NFP_MEDIA_25GBASE_CR,
+	NFP_MEDIA_25GBASE_CR_S,
+	NFP_MEDIA_25GBASE_SR,
+	NFP_MEDIA_40GBASE_CR4,
+	NFP_MEDIA_40GBASE_KR4,
+	NFP_MEDIA_40GBASE_SR4,
+	NFP_MEDIA_40GBASE_LR4,
+	NFP_MEDIA_50GBASE_KR,
+	NFP_MEDIA_50GBASE_SR,
+	NFP_MEDIA_50GBASE_CR,
+	NFP_MEDIA_50GBASE_LR,
+	NFP_MEDIA_50GBASE_ER,
+	NFP_MEDIA_50GBASE_FR,
+	NFP_MEDIA_100GBASE_KR4,
+	NFP_MEDIA_100GBASE_SR4,
+	NFP_MEDIA_100GBASE_CR4,
+	NFP_MEDIA_100GBASE_KP4,
+	NFP_MEDIA_100GBASE_CR10,
+	NFP_MEDIA_LINK_MODES_NUMBER
+};
+
 #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)
@@ -256,6 +302,19 @@ enum nfp_nsp_sensor_id {
 int nfp_hwmon_read_sensor(struct nfp_cpp *cpp, enum nfp_nsp_sensor_id id,
 			  long *val);
 
+/* The maximum number of link modes can be added */
+#define NFP_NSP_MAX_MODE_SIZE	128
+
+struct nfp_eth_media_buf {
+	u8 eth_index;
+	u8 reserved[7];
+	DECLARE_BITMAP(supported_modes, NFP_NSP_MAX_MODE_SIZE);
+	DECLARE_BITMAP(advertised_modes, NFP_NSP_MAX_MODE_SIZE);
+};
+
+int nfp_nsp_read_media(struct nfp_nsp *state, void *buf, unsigned int size);
+int nfp_eth_read_media(struct nfp_cpp *cpp, struct nfp_eth_media_buf *ethm);
+
 #define NFP_NSP_VERSION_BUFSZ	1024 /* reasonable size, not in the ABI */
 
 enum nfp_nsp_versions {
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
index bb64efec4c46..0996fefd6cd9 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
@@ -647,3 +647,30 @@ 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,
 				      lanes, NSP_ETH_CTRL_SET_LANES);
 }
+
+int nfp_eth_read_media(struct nfp_cpp *cpp, struct nfp_eth_media_buf *ethm)
+{
+	struct nfp_nsp *nsp;
+	int err;
+
+	nsp = nfp_nsp_open(cpp);
+	if (IS_ERR(nsp)) {
+		err = PTR_ERR(nsp);
+		nfp_err(cpp, "Failed to access the NSP: %d\n", err);
+		return err;
+	}
+
+	if (!nfp_nsp_has_read_media(nsp)) {
+		nfp_warn(cpp, "Reading media link modes not supported. Please update flash\n");
+		err = -EOPNOTSUPP;
+		goto exit_close_nsp;
+	}
+
+	err = nfp_nsp_read_media(nsp, ethm, sizeof(*ethm));
+	if (err)
+		nfp_err(cpp, "Reading link modes failed %d\n", err);
+
+exit_close_nsp:
+	nfp_nsp_close(nsp);
+	return err;
+}
-- 
2.30.2

Re: [PATCH net-next] nfp: ethtool: support reporting link modes

[Andrew Lunn]

On Mon, Nov 21, 2022 at 12:20:45PM +0100, Simon Horman wrote:
> From: Yu Xiao <yu.xiao@corigine.com>
> 
> Add support for reporting link modes,
> including `Supported link modes` and `Advertised link modes`,
> via ethtool $DEV.

Does the firmware support returning the link partners advertised link
modes? Knowing what the other end is advertising is often the key to
figuring out why a link has ended up on a specific link mode.

	 Andrew

Re: [PATCH net-next] nfp: ethtool: support reporting link modes

[Yinjun Zhang]

On Tue, 22 Nov 2022 01:51:43 +0100, Andrew Lunn wrote:
> On Mon, Nov 21, 2022 at 12:20:45PM +0100, Simon Horman wrote:
> > From: Yu Xiao <yu.xiao@corigine.com>
> > 
> > Add support for reporting link modes,
> > including `Supported link modes` and `Advertised link modes`,
> > via ethtool $DEV.
> 
> Does the firmware support returning the link partners advertised link
> modes? Knowing what the other end is advertising is often the key to
> figuring out why a link has ended up on a specific link mode.

I'm afraid it's not supported yet. Anyway thanks for the info, we may
consider to schedule it sometime.

Re: [PATCH net-next] nfp: ethtool: support reporting link modes

[Jakub Kicinski]

On Mon, 21 Nov 2022 12:20:45 +0100 Simon Horman wrote:
> From: Yu Xiao <yu.xiao@corigine.com>
> 
> Add support for reporting link modes,
> including `Supported link modes` and `Advertised link modes`,
> via ethtool $DEV.
> 
> A new command `SPCODE_READ_MEDIA` is added to read info from
> management firmware. Also, the mapping table `nfp_eth_media_table`
> associates the link modes between NFP and kernel. Both of them
> help to support this ability.

> +static void nfp_add_media_link_mode(struct nfp_port *port,
> +				    struct nfp_eth_table_port *eth_port,
> +				    struct ethtool_link_ksettings *cmd)
> +{
> +	struct nfp_eth_media_buf ethm = {.eth_index = eth_port->eth_index};

nit: spaces, comma at the end

> +	struct nfp_cpp *cpp = port->app->cpp;
> +	u8 i;

nit: int or uint, don't pointlessly limit the size of iterators

> +struct nfp_eth_media_buf {
> +	u8 eth_index;
> +	u8 reserved[7];
> +	DECLARE_BITMAP(supported_modes, NFP_NSP_MAX_MODE_SIZE);
> +	DECLARE_BITMAP(advertised_modes, NFP_NSP_MAX_MODE_SIZE);

You can't use DECLATE_BITMAP() in FW APIs. unsigned long will have
different layout depending on word side and endian.

> +int nfp_eth_read_media(struct nfp_cpp *cpp, struct nfp_eth_media_buf *ethm)
> +{
> +	struct nfp_nsp *nsp;
> +	int err;

nit: you use your ret for variables which are returned both 
     on the success and failure paths.

Re: [PATCH net-next] nfp: ethtool: support reporting link modes

[Alexander Lobakin]

From: Simon Horman <simon.horman@corigine.com>
Date: Mon, 21 Nov 2022 12:20:45 +0100

> From: Yu Xiao <yu.xiao@corigine.com>
> 
> Add support for reporting link modes,
> including `Supported link modes` and `Advertised link modes`,
> via ethtool $DEV.
> 
> A new command `SPCODE_READ_MEDIA` is added to read info from
> management firmware. Also, the mapping table `nfp_eth_media_table`
> associates the link modes between NFP and kernel. Both of them
> help to support this ability.
> 
> Signed-off-by: Yu Xiao <yu.xiao@corigine.com>
> Reviewed-by: Louis Peens <louis.peens@corigine.com>
> Signed-off-by: Simon Horman <simon.horman@corigine.com>
> ---
>  drivers/net/ethernet/netronome/nfp/nfp_main.h |  1 +
>  .../ethernet/netronome/nfp/nfp_net_ethtool.c  | 56 ++++++++++++++++++
>  .../ethernet/netronome/nfp/nfpcore/nfp_nsp.c  | 17 ++++++
>  .../ethernet/netronome/nfp/nfpcore/nfp_nsp.h  | 59 +++++++++++++++++++
>  .../netronome/nfp/nfpcore/nfp_nsp_eth.c       | 27 +++++++++
>  5 files changed, 160 insertions(+)

[...]

> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
> index 0058ba2b3505..be434b01b724 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
> @@ -293,6 +293,59 @@ nfp_net_set_fec_link_mode(struct nfp_eth_table_port *eth_port,
>  	}
>  }
>  
> +static const u8 nfp_eth_media_table[] = {

I wouldn't use u8 for a bit number, 256 bits is not difficult to
overflow. u16 would be safe here.

> +	[NFP_MEDIA_1000BASE_CX] = ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
> +	[NFP_MEDIA_1000BASE_KX] = ETHTOOL_LINK_MODE_1000baseKX_Full_BIT,
> +	[NFP_MEDIA_10GBASE_KX4] = ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
> +	[NFP_MEDIA_10GBASE_KR] = ETHTOOL_LINK_MODE_10000baseKR_Full_BIT,
> +	[NFP_MEDIA_10GBASE_CX4] = ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT,
> +	[NFP_MEDIA_10GBASE_CR] = ETHTOOL_LINK_MODE_10000baseCR_Full_BIT,
> +	[NFP_MEDIA_10GBASE_SR] = ETHTOOL_LINK_MODE_10000baseSR_Full_BIT,
> +	[NFP_MEDIA_10GBASE_ER] = ETHTOOL_LINK_MODE_10000baseER_Full_BIT,
> +	[NFP_MEDIA_25GBASE_KR] = ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
> +	[NFP_MEDIA_25GBASE_KR_S] = ETHTOOL_LINK_MODE_25000baseKR_Full_BIT,
> +	[NFP_MEDIA_25GBASE_CR] = ETHTOOL_LINK_MODE_25000baseCR_Full_BIT,
> +	[NFP_MEDIA_25GBASE_CR_S] = ETHTOOL_LINK_MODE_25000baseCR_Full_BIT,
> +	[NFP_MEDIA_25GBASE_SR] = ETHTOOL_LINK_MODE_25000baseSR_Full_BIT,
> +	[NFP_MEDIA_40GBASE_CR4] = ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT,
> +	[NFP_MEDIA_40GBASE_KR4] = ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT,
> +	[NFP_MEDIA_40GBASE_SR4] = ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT,
> +	[NFP_MEDIA_40GBASE_LR4] = ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT,
> +	[NFP_MEDIA_50GBASE_KR] = ETHTOOL_LINK_MODE_50000baseKR_Full_BIT,
> +	[NFP_MEDIA_50GBASE_SR] = ETHTOOL_LINK_MODE_50000baseSR_Full_BIT,
> +	[NFP_MEDIA_50GBASE_CR] = ETHTOOL_LINK_MODE_50000baseCR_Full_BIT,
> +	[NFP_MEDIA_50GBASE_LR] = ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
> +	[NFP_MEDIA_50GBASE_ER] = ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
> +	[NFP_MEDIA_50GBASE_FR] = ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT,
> +	[NFP_MEDIA_100GBASE_KR4] = ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
> +	[NFP_MEDIA_100GBASE_SR4] = ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT,
> +	[NFP_MEDIA_100GBASE_CR4] = ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT,
> +	[NFP_MEDIA_100GBASE_KP4] = ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT,
> +	[NFP_MEDIA_100GBASE_CR10] = ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT,

Wouldn't it look fancier with one tab alignment? :>

> +};
> +
> +static void nfp_add_media_link_mode(struct nfp_port *port,
> +				    struct nfp_eth_table_port *eth_port,
> +				    struct ethtool_link_ksettings *cmd)
> +{
> +	struct nfp_eth_media_buf ethm = {.eth_index = eth_port->eth_index};

I'm not sure it's okay to do one-line struct initialization. Up to
you I guess, but looks a bit confusing to me, in comparison to the
initialization two functions below.

> +	struct nfp_cpp *cpp = port->app->cpp;
> +	u8 i;

Same here. And there's no point in using types shorter than u32 on
the stack. Just go with u32. And also,

> +
> +	if (nfp_eth_read_media(cpp, &ethm))
> +		return;
> +
> +	for (i = 0; i < NFP_MEDIA_LINK_MODES_NUMBER; ++i) {

Since we're on -std=gnu11 for a bunch of releases already, all new
loops are expected to go with the iterator declarations inside them.
E.g.

	for (u32 i = 0; i < ...

> +		if (test_bit(i, ethm.supported_modes))
> +			__set_bit(nfp_eth_media_table[i],
> +				  cmd->link_modes.supported);
> +
> +		if (test_bit(i, ethm.advertised_modes))
> +			__set_bit(nfp_eth_media_table[i],
> +				  cmd->link_modes.advertising);
> +	}
> +}
> +
>  /**
>   * nfp_net_get_link_ksettings - Get Link Speed settings
>   * @netdev:	network interface device structure
> @@ -311,6 +364,8 @@ nfp_net_get_link_ksettings(struct net_device *netdev,
>  	u16 sts;
>  
>  	/* Init to unknowns */
> +	ethtool_link_ksettings_zero_link_mode(cmd, supported);
> +	ethtool_link_ksettings_zero_link_mode(cmd, advertising);
>  	ethtool_link_ksettings_add_link_mode(cmd, supported, FIBRE);
>  	cmd->base.port = PORT_OTHER;
>  	cmd->base.speed = SPEED_UNKNOWN;
> @@ -321,6 +376,7 @@ nfp_net_get_link_ksettings(struct net_device *netdev,
>  	if (eth_port) {
>  		ethtool_link_ksettings_add_link_mode(cmd, supported, Pause);
>  		ethtool_link_ksettings_add_link_mode(cmd, advertising, Pause);
> +		nfp_add_media_link_mode(port, eth_port, cmd);
>  		if (eth_port->supp_aneg) {
>  			ethtool_link_ksettings_add_link_mode(cmd, supported, Autoneg);
>  			if (eth_port->aneg == NFP_ANEG_AUTO) {
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
> index 730fea214b8a..7136bc48530b 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
> @@ -100,6 +100,7 @@ enum nfp_nsp_cmd {
>  	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 either the supported or advertised media for a port */
>  };
>  
>  struct nfp_nsp_dma_buf {
> @@ -1100,4 +1101,20 @@ int nfp_nsp_read_module_eeprom(struct nfp_nsp *state, int eth_index,
>  	kfree(buf);
>  
>  	return ret;
> +};
> +
> +int nfp_nsp_read_media(struct nfp_nsp *state, void *buf, unsigned int size)
> +{
> +	struct nfp_nsp_command_buf_arg media = {
> +		{
> +			.code		= SPCODE_READ_MEDIA,
> +			.option		= size,
> +		},
> +		.in_buf		= buf,
> +		.in_size	= size,
> +		.out_buf	= buf,
> +		.out_size	= size,
> +	};
> +
> +	return nfp_nsp_command_buf(state, &media);
>  }
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
> index 992d72ac98d3..c5630fe88a66 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
> @@ -65,6 +65,11 @@ 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,
> @@ -97,6 +102,47 @@ enum nfp_eth_fec {
>  	NFP_FEC_DISABLED_BIT,
>  };
>  
> +/* link modes about RJ45 haven't been used, so there's no mapping to them */
> +enum nfp_ethtool_link_mode_list {
> +	NFP_MEDIA_W0_RJ45_10M,
> +	NFP_MEDIA_W0_RJ45_10M_HD,
> +	NFP_MEDIA_W0_RJ45_100M,
> +	NFP_MEDIA_W0_RJ45_100M_HD,
> +	NFP_MEDIA_W0_RJ45_1G,
> +	NFP_MEDIA_W0_RJ45_2P5G,
> +	NFP_MEDIA_W0_RJ45_5G,
> +	NFP_MEDIA_W0_RJ45_10G,
> +	NFP_MEDIA_1000BASE_CX,
> +	NFP_MEDIA_1000BASE_KX,
> +	NFP_MEDIA_10GBASE_KX4,
> +	NFP_MEDIA_10GBASE_KR,
> +	NFP_MEDIA_10GBASE_CX4,
> +	NFP_MEDIA_10GBASE_CR,
> +	NFP_MEDIA_10GBASE_SR,
> +	NFP_MEDIA_10GBASE_ER,
> +	NFP_MEDIA_25GBASE_KR,
> +	NFP_MEDIA_25GBASE_KR_S,
> +	NFP_MEDIA_25GBASE_CR,
> +	NFP_MEDIA_25GBASE_CR_S,
> +	NFP_MEDIA_25GBASE_SR,
> +	NFP_MEDIA_40GBASE_CR4,
> +	NFP_MEDIA_40GBASE_KR4,
> +	NFP_MEDIA_40GBASE_SR4,
> +	NFP_MEDIA_40GBASE_LR4,
> +	NFP_MEDIA_50GBASE_KR,
> +	NFP_MEDIA_50GBASE_SR,
> +	NFP_MEDIA_50GBASE_CR,
> +	NFP_MEDIA_50GBASE_LR,
> +	NFP_MEDIA_50GBASE_ER,
> +	NFP_MEDIA_50GBASE_FR,
> +	NFP_MEDIA_100GBASE_KR4,
> +	NFP_MEDIA_100GBASE_SR4,
> +	NFP_MEDIA_100GBASE_CR4,
> +	NFP_MEDIA_100GBASE_KP4,
> +	NFP_MEDIA_100GBASE_CR10,
> +	NFP_MEDIA_LINK_MODES_NUMBER
> +};
> +
>  #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)
> @@ -256,6 +302,19 @@ enum nfp_nsp_sensor_id {
>  int nfp_hwmon_read_sensor(struct nfp_cpp *cpp, enum nfp_nsp_sensor_id id,
>  			  long *val);
>  
> +/* The maximum number of link modes can be added */
> +#define NFP_NSP_MAX_MODE_SIZE	128
> +
> +struct nfp_eth_media_buf {
> +	u8 eth_index;
> +	u8 reserved[7];
> +	DECLARE_BITMAP(supported_modes, NFP_NSP_MAX_MODE_SIZE);
> +	DECLARE_BITMAP(advertised_modes, NFP_NSP_MAX_MODE_SIZE);

Jakub said that already, but yeah, object shared with the hardware
must have explicit fixed size and endianness, bitmaps are pure Linux
API and have nothing of these two.
I think here you'd want to use __le64.

> +};
> +
> +int nfp_nsp_read_media(struct nfp_nsp *state, void *buf, unsigned int size);
> +int nfp_eth_read_media(struct nfp_cpp *cpp, struct nfp_eth_media_buf *ethm);
> +
>  #define NFP_NSP_VERSION_BUFSZ	1024 /* reasonable size, not in the ABI */
>  
>  enum nfp_nsp_versions {
> diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> index bb64efec4c46..0996fefd6cd9 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
> @@ -647,3 +647,30 @@ 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,
>  				      lanes, NSP_ETH_CTRL_SET_LANES);
>  }
> +
> +int nfp_eth_read_media(struct nfp_cpp *cpp, struct nfp_eth_media_buf *ethm)
> +{
> +	struct nfp_nsp *nsp;
> +	int err;
> +
> +	nsp = nfp_nsp_open(cpp);
> +	if (IS_ERR(nsp)) {
> +		err = PTR_ERR(nsp);
> +		nfp_err(cpp, "Failed to access the NSP: %d\n", err);
> +		return err;

		nfp_err(cpp, "Failed to access the NSP: -%pe\n, nsp);
		return PTR_ERR(nsp);

%pe prints the name if SYMBOLIC_ERRNAME is enabled, and the error
code in dec otherwise.

> +	}
> +
> +	if (!nfp_nsp_has_read_media(nsp)) {
> +		nfp_warn(cpp, "Reading media link modes not supported. Please update flash\n");
> +		err = -EOPNOTSUPP;
> +		goto exit_close_nsp;
> +	}
> +
> +	err = nfp_nsp_read_media(nsp, ethm, sizeof(*ethm));
> +	if (err)
> +		nfp_err(cpp, "Reading link modes failed %d\n", err);

Same here. Also, you don't have a ':' in between 'failed' and the
code format literal.

> +
> +exit_close_nsp:
> +	nfp_nsp_close(nsp);
> +	return err;
> +}
> -- 
> 2.30.2

Thanks,
Olek