Re: [PATCH] net/nfp: write link speed to control BAR

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 2/21/2023 6:29 AM, Chaoyong He wrote:
> From: James Hershaw <james.hershaw@corigine.com>
> 
> Due to changes in the firmware for NFPs, firmware will no longer write
> the link speed of a port to the control BAR. In line with the behaviour
> of the kernel NFP driver, this is now handled by the PMD by reading the
> value provided by the NSP in the nfp_eth_table struct within the pf_dev
> of the port and subsequently writing this value to the control BAR.
> 

Don't you need some kind of FW version check to figure out if
'NFP_NET_CFG_STS_NSP_LINK_RATE' needs to be updated by driver or not?

How do you manage driver <-> FW dependency?


> Signed-off-by: James Hershaw <james.hershaw@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> ---
>  drivers/net/nfp/nfp_common.c | 90 ++++++++++++++++++++++--------------
>  drivers/net/nfp/nfp_ctrl.h   |  9 ++++
>  2 files changed, 65 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c
> index 5922bfea8e..006ea58008 100644
> --- a/drivers/net/nfp/nfp_common.c
> +++ b/drivers/net/nfp/nfp_common.c
> @@ -52,6 +52,53 @@
>  #include <sys/ioctl.h>
>  #include <errno.h>
>  
> +static const uint32_t nfp_net_link_speed_nfp2rte[] = {
> +	[NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = RTE_ETH_SPEED_NUM_NONE,
> +	[NFP_NET_CFG_STS_LINK_RATE_UNKNOWN]     = RTE_ETH_SPEED_NUM_NONE,
> +	[NFP_NET_CFG_STS_LINK_RATE_1G]          = RTE_ETH_SPEED_NUM_1G,
> +	[NFP_NET_CFG_STS_LINK_RATE_10G]         = RTE_ETH_SPEED_NUM_10G,
> +	[NFP_NET_CFG_STS_LINK_RATE_25G]         = RTE_ETH_SPEED_NUM_25G,
> +	[NFP_NET_CFG_STS_LINK_RATE_40G]         = RTE_ETH_SPEED_NUM_40G,
> +	[NFP_NET_CFG_STS_LINK_RATE_50G]         = RTE_ETH_SPEED_NUM_50G,
> +	[NFP_NET_CFG_STS_LINK_RATE_100G]        = RTE_ETH_SPEED_NUM_100G,
> +};
> +
> +static uint32_t
> +nfp_net_link_speed_rte2nfp(uint32_t speed)
> +{
> +	uint32_t i;
> +
> +	for (i = 0; i < RTE_DIM(nfp_net_link_speed_nfp2rte); i++) {
> +		if (speed == nfp_net_link_speed_nfp2rte[i])
> +			return i;
> +	}
> +
> +	return NFP_NET_CFG_STS_LINK_RATE_UNKNOWN;
> +}
> +
> +static void
> +nfp_net_notify_port_speed(struct rte_eth_dev *dev)
> +{
> +	struct nfp_net_hw *hw;
> +	struct nfp_eth_table *eth_table;
> +	uint32_t nn_link_status;
> +
> +	hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	eth_table = hw->pf_dev->nfp_eth_table;
> +
> +	nn_link_status = nn_cfg_readl(hw, NFP_NET_CFG_STS);
> +	nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) &
> +			NFP_NET_CFG_STS_LINK_RATE_MASK;
> +
> +	if ((nn_link_status & NFP_NET_CFG_STS_LINK) == 0) {
> +		nn_cfg_writel(hw, NFP_NET_CFG_STS_NSP_LINK_RATE, NFP_NET_CFG_STS_LINK_RATE_UNKNOWN);
> +		return;
> +	}
> +
> +	nn_cfg_writel(hw, NFP_NET_CFG_STS_NSP_LINK_RATE,
> +		      nfp_net_link_speed_rte2nfp(eth_table->ports[hw->idx].speed));

PF driver writes link speed to 'NFP_NET_CFG_STS_NSP_LINK_RATE' register,
but 'nfp_net_link_update()' still gets it from 'NFP_NET_CFG_STS'
register (via 'nfp_net_link_speed_nfp2rte[nn_link_status]').

Shouldn't 'nfp_net_link_update()' needs to be updated to read speed from
'NFP_NET_CFG_STS_NSP_LINK_RATE' register?