Re: [PATCH net-next 3/3] net: stmmac: fpe: Move fprq configuration to stmmac_fpe_configure_pmac()

[Maxime Chevallier]

Hi,

On 7/29/26 03:52, muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx wrote:
> From: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx>
> 
> The Frame Preemption Residue Queue (fprq) controls which Rx queue
> receives residual preemptive mPackets from the pmac. This is a pmac
> concern, not a TX concern, so move its configuration from
> stmmac_fpe_configure_tx() to stmmac_fpe_configure_pmac() where it
> logically belongs.
> 
> Use u32_replace_bits() instead of the open-coded mask/shift sequence.
> 
> Signed-off-by: Rohan G Thomas <rohan.g.thomas@xxxxxxxxxx>
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@xxxxxxxxxx>
> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_fpe.c  | 20 +++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
> index 3436e962a238..27e500edc8ac 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_fpe.c
> @@ -47,20 +47,13 @@ static void stmmac_fpe_configure_tx(struct ethtool_mmsv *mmsv, bool tx_enable)
>  	struct stmmac_fpe_cfg *cfg = container_of(mmsv, struct stmmac_fpe_cfg, mmsv);
>  	struct stmmac_priv *priv = container_of(cfg, struct stmmac_priv, fpe_cfg);
>  	const struct stmmac_fpe_reg *reg = cfg->reg;
> -	u32 num_rxq = priv->plat->rx_queues_to_use;
>  	void __iomem *ioaddr = priv->ioaddr;
> -	u32 value;
>  
> -	if (tx_enable) {
> +	if (tx_enable)
>  		cfg->fpe_csr = STMMAC_MAC_FPE_CTRL_STS_EFPE;
> -		value = readl(ioaddr + reg->rxq_ctrl1_reg);
> -		value &= ~reg->fprq_mask;
> -		/* Keep this SHIFT, FIELD_PREP() expects a constant mask :-/ */
> -		value |= (num_rxq - 1) << __ffs(reg->fprq_mask);
> -		writel(value, ioaddr + reg->rxq_ctrl1_reg);
> -	} else {
> +	else
>  		cfg->fpe_csr = 0;
> -	}
> +
>  	writel(cfg->fpe_csr, ioaddr + reg->mac_fpe_reg);
>  }
>  
> @@ -68,6 +61,7 @@ static void stmmac_fpe_configure_pmac(struct ethtool_mmsv *mmsv, bool pmac_enabl
>  {
>  	struct stmmac_fpe_cfg *cfg = container_of(mmsv, struct stmmac_fpe_cfg, mmsv);
>  	struct stmmac_priv *priv = container_of(cfg, struct stmmac_priv, fpe_cfg);
> +	u32 num_rxq = priv->plat->rx_queues_to_use;
>  	const struct stmmac_fpe_reg *reg = cfg->reg;
>  	void __iomem *ioaddr = priv->ioaddr;
>  	unsigned long flags;
> @@ -83,6 +77,12 @@ static void stmmac_fpe_configure_pmac(struct ethtool_mmsv *mmsv, bool pmac_enabl
>  
>  			value |= reg->int_en_bit;
>  		}
> +		/* Frame Preemption Residue Queue is the Rx Queue to which
> +		 * residual preemptive mPackets must be forwarded from the pmac.
> +		 */
> +		writel(u32_replace_bits(readl(ioaddr + reg->rxq_ctrl1_reg),
> +					num_rxq - 1, reg->fprq_mask),
> +		       ioaddr + reg->rxq_ctrl1_reg);
>  	} else {
>  		value &= ~reg->int_en_bit;
>  	}

Not a bit fan of the writel(u32_replace(readl(...))) but I don't feel strong
enough about that to warrant a resend 

Reviewed-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>

Maxime