Subject: Re: [net-next PATCH v5 01/10] octeontx2-pf: Refactoring RVU driver

From: Simon Horman <[email-masked]>
Date: Tue, 18 Jun 2024 08:40:58 +0100

On Tue, Jun 11, 2024 at 09:52:04PM +0530, Geetha sowjanya wrote:
> Refactoring and export list of shared functions such that
> they can be used by both RVU NIC and representor driver.
> 
> Signed-off-by: Geetha sowjanya <[email-masked]>

...

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c

...

> @@ -2949,6 +2952,7 @@ static int nix_tx_vtag_alloc(struct rvu *rvu, int blkaddr,
>  	mutex_unlock(&vlan->rsrc_lock);
>  
>  	regval = size ? vtag : vtag << 32;
> +	regval |= (vtag & ~GENMASK_ULL(47, 0)) << 48;

Hi Geetha,

I'm a little confused by the line above.

vtag is a 64 bit value.
It is masked, leaving the upper 16 bits intact,
and the lower 48 bits as zeros.
It is then left-shifted 48 bits.
By my reasoning the result is always 0.

e.g.
  0x123456789abcdef1 & ~GENMASK_ULL(47, 0) => 0x1234000000000000
  0x1234000000000000 << 48 => 0

Also, I suspect that FIELD_PREP could be used to good effect here.
(And, as an aside, elsewhere in this file/driver.)

>  	rvu_write64(rvu, blkaddr,
>  		    NIX_AF_TX_VTAG_DEFX_DATA(index), regval);
> @@ -4619,6 +4623,7 @@ static void nix_link_config(struct rvu *rvu, int blkaddr,
>  	rvu_get_lbk_link_max_frs(rvu, &lbk_max_frs);
>  	rvu_get_lmac_link_max_frs(rvu, &lmac_max_frs);
>  
> +	rvu_write64(rvu, blkaddr, NIX_AF_SDP_LINK_CREDIT, SDP_LINK_CREDIT);
>  	/* Set default min/max packet lengths allowed on NIX Rx links.
>  	 *
>  	 * With HW reset minlen value of 60byte, HW will treat ARP pkts
> @@ -4630,14 +4635,14 @@ static void nix_link_config(struct rvu *rvu, int blkaddr,
>  				((u64)lmac_max_frs << 16) | NIC_HW_MIN_FRS);
>  	}
>  
> -	for (link = hw->cgx_links; link < hw->lbk_links; link++) {
> +	for (link = hw->cgx_links; link < hw->cgx_links + hw->lbk_links; link++) {
>  		rvu_write64(rvu, blkaddr, NIX_AF_RX_LINKX_CFG(link),
>  			    ((u64)lbk_max_frs << 16) | NIC_HW_MIN_FRS);
>  	}
>  	if (hw->sdp_links) {
>  		link = hw->cgx_links + hw->lbk_links;
>  		rvu_write64(rvu, blkaddr, NIX_AF_RX_LINKX_CFG(link),
> -			    SDP_HW_MAX_FRS << 16 | NIC_HW_MIN_FRS);
> +			    SDP_HW_MAX_FRS << 16 | SDP_HW_MIN_FRS);
>  	}
>  
>  	/* Get MCS external bypass status for CN10K-B */

...