Re: [PATCH net v4 2/3] net: stmmac: pass the desired EST enable state to est_configure()

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi,

On 10/26/26 17:55, Lorenzo Bianconi wrote:
> Pass the desired EST enable state explicitly to est_configure() instead
> of having it derive from cfg->enable. This decouples the hardware
> programming state from the priv->est->enable flag, which records whether
> the taprio offload is attached.
> 
> No functional change intended: the callers keep toggling
> priv->est->enable around the EST programming, as before.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

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

Maxime

> ---
>  drivers/net/ethernet/stmicro/stmmac/hwif.h       | 2 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_est.c | 6 +++---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 4 ++--
>  drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c  | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 50b590d67d87..b28509555d81 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -620,7 +620,7 @@ struct stmmac_mmc_ops {
>  
>  struct stmmac_est_ops {
>  	int (*configure)(struct stmmac_priv *priv, struct stmmac_est *cfg,
> -			 unsigned int ptp_rate);
> +			 unsigned int ptp_rate, bool enable);
>  	void (*irq_status)(struct stmmac_priv *priv, struct net_device *dev,
>  			   struct stmmac_extra_stats *x, u32 txqcnt);
>  };
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_est.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_est.c
> index afc516059b89..f15d4d046aa7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_est.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_est.c
> @@ -26,7 +26,7 @@ static int est_write(void __iomem *est_addr, u32 reg, u32 val, bool gcl)
>  }
>  
>  static int est_configure(struct stmmac_priv *priv, struct stmmac_est *cfg,
> -			 unsigned int ptp_rate)
> +			 unsigned int ptp_rate, bool enable)
>  {
>  	void __iomem *est_addr = priv->estaddr;
>  	int i, ret = 0;
> @@ -62,7 +62,7 @@ static int est_configure(struct stmmac_priv *priv, struct stmmac_est *cfg,
>  		ctrl |= ((NSEC_PER_SEC / ptp_rate) * EST_GMAC5_PTOV_MUL) <<
>  			 EST_GMAC5_PTOV_SHIFT;
>  	}
> -	if (cfg->enable)
> +	if (enable)
>  		ctrl |= EST_EEST | EST_SSWL | EST_DFBS;
>  	else
>  		ctrl &= ~EST_EEST;
> @@ -70,7 +70,7 @@ static int est_configure(struct stmmac_priv *priv, struct stmmac_est *cfg,
>  	writel(ctrl, est_addr + EST_CONTROL);
>  
>  	/* Configure EST interrupt */
> -	if (cfg->enable)
> +	if (enable)
>  		ctrl = EST_IECGCE | EST_IEHS | EST_IEHF | EST_IEBE | EST_IECC;
>  	else
>  		ctrl = 0;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index e54a3c887826..9a32af00d7d2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -74,7 +74,7 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
>  		mutex_lock(&priv->est_lock);
>  		priv->est->enable = false;
>  		stmmac_est_configure(priv, priv, priv->est,
> -				     priv->plat->clk_ptp_rate);
> +				     priv->plat->clk_ptp_rate, false);
>  		mutex_unlock(&priv->est_lock);
>  	}
>  
> @@ -105,7 +105,7 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
>  		priv->est->btr[1] = (u32)time.tv_sec;
>  		priv->est->enable = true;
>  		ret = stmmac_est_configure(priv, priv, priv->est,
> -					   priv->plat->clk_ptp_rate);
> +					   priv->plat->clk_ptp_rate, true);
>  		mutex_unlock(&priv->est_lock);
>  		if (ret)
>  			netdev_err(priv->dev, "failed to configure EST\n");
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> index aea235a69105..ac926c60f9f9 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
> @@ -1090,7 +1090,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
>  	tc_taprio_map_maxsdu_txq(priv, qopt);
>  
>  	ret = stmmac_est_configure(priv, priv, priv->est,
> -				   priv->plat->clk_ptp_rate);
> +				   priv->plat->clk_ptp_rate, true);
>  	mutex_unlock(&priv->est_lock);
>  	if (ret) {
>  		netdev_err(priv->dev, "failed to configure EST\n");
> @@ -1109,7 +1109,7 @@ static int tc_taprio_configure(struct stmmac_priv *priv,
>  		mutex_lock(&priv->est_lock);
>  		priv->est->enable = false;
>  		stmmac_est_configure(priv, priv, priv->est,
> -				     priv->plat->clk_ptp_rate);
> +				     priv->plat->clk_ptp_rate, false);
>  		/* Reset taprio status */
>  		for (i = 0; i < priv->plat->tx_queues_to_use; i++) {
>  			priv->xstats.max_sdu_txq_drop[i] = 0;
>