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

[Jakub Kicinski <kuba@kernel.org>]

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.