[PATCH net] net: stmmac: re-apply taprio offload in __stmmac_open()

[PATCH net] net: stmmac: re-apply taprio offload in __stmmac_open()

[Lorenzo Bianconi]

The core soft reset issued in stmmac_init_dma_engine() clears the
MTL_EST registers, but nothing re-applies the taprio offload after it:
priv->est->enable stays true while the hardware EST block is left
disabled. The TX/XDP paths then keep dropping frames larger than
priv->est->max_sdu[] and taprio is reported as offloaded, although the
EST block is not programmed.

Re-apply the taprio offload in __stmmac_open() after PTP is up. The
base time is recomputed from the reserved base time and the current PTP
time, since the timestamp counter has been re-initialized and the
previously programmed base time is stale.

Fixes: b60189e0392f ("net: stmmac: Integrate EST with TAPRIO scheduler API")
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_est.c  | 28 +++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_est.h  |  2 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  7 ++++++
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c  | 26 ++-------------------
 4 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_est.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_est.c
index afc516059b89..594f61c07dbb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_est.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_est.c
@@ -80,6 +80,34 @@ static int est_configure(struct stmmac_priv *priv, struct stmmac_est *cfg,
 	return 0;
 }
 
+void stmmac_est_reconfigure(struct stmmac_priv *priv)
+{
+	struct timespec64 current_time, time;
+	ktime_t current_time_ns, basetime;
+	u64 cycle_time;
+
+	mutex_lock(&priv->est_lock);
+
+	priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, &current_time);
+	current_time_ns = timespec64_to_ktime(current_time);
+
+	time.tv_nsec = priv->est->btr_reserve[0];
+	time.tv_sec = priv->est->btr_reserve[1];
+	basetime = timespec64_to_ktime(time);
+
+	cycle_time = (u64)priv->est->ctr[1] * NSEC_PER_SEC + priv->est->ctr[0];
+
+	time = stmmac_calc_tas_basetime(basetime, current_time_ns, cycle_time);
+	priv->est->btr[0] = (u32)time.tv_nsec;
+	priv->est->btr[1] = (u32)time.tv_sec;
+
+	if (stmmac_est_configure(priv, priv, priv->est,
+				 priv->plat->clk_ptp_rate))
+		netdev_err(priv->dev, "failed to re-configure EST\n");
+
+	mutex_unlock(&priv->est_lock);
+}
+
 static void est_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.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_est.h
index f70221c9c84a..fcbee673f2e7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_est.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_est.h
@@ -65,3 +65,5 @@
 #define EST_GCL_DATA			0x00000034
 
 extern const struct stmmac_est_ops dwmac510_est_ops;
+
+void stmmac_est_reconfigure(struct stmmac_priv *priv);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index efa35cfecc4f..d71d430519d3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -48,6 +48,7 @@
 #include "stmmac_ptp.h"
 #include "stmmac_fpe.h"
 #include "stmmac.h"
+#include "stmmac_est.h"
 #include "stmmac_pcs.h"
 #include "stmmac_xdp.h"
 #include <linux/reset.h>
@@ -4158,6 +4159,12 @@ static int __stmmac_open(struct net_device *dev,
 
 	stmmac_setup_ptp(priv);
 
+	/* The core soft reset in stmmac_hw_setup() clears the MTL_EST
+	 * registers, so re-apply the taprio offload after PTP is up.
+	 */
+	if (priv->est && priv->est->enable)
+		stmmac_est_reconfigure(priv);
+
 	stmmac_init_coalesce(priv);
 
 	phylink_start(priv->phylink);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 960249960004..3682c9d3f3ca 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -8,6 +8,7 @@
   Author: Rayagond Kokatanur <rayagond@vayavyalabs.com>
 *******************************************************************************/
 #include "stmmac.h"
+#include "stmmac_est.h"
 #include "stmmac_ptp.h"
 
 #define PTP_SAFE_TIME_OFFSET_NS	500000
@@ -55,7 +56,6 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
 	u32 quotient, reminder;
 	int neg_adj = 0;
 	bool xmac, est_rst = false;
-	int ret;
 
 	xmac = dwmac_is_xmac(priv->plat->core_type);
 
@@ -84,30 +84,8 @@ static int stmmac_adjust_time(struct ptp_clock_info *ptp, s64 delta)
 
 	/* Calculate new basetime and re-configured EST after PTP time adjust. */
 	if (est_rst) {
-		struct timespec64 current_time, time;
-		ktime_t current_time_ns, basetime;
-		u64 cycle_time;
-
-		mutex_lock(&priv->est_lock);
-		priv->ptp_clock_ops.gettime64(&priv->ptp_clock_ops, &current_time);
-		current_time_ns = timespec64_to_ktime(current_time);
-		time.tv_nsec = priv->est->btr_reserve[0];
-		time.tv_sec = priv->est->btr_reserve[1];
-		basetime = timespec64_to_ktime(time);
-		cycle_time = (u64)priv->est->ctr[1] * NSEC_PER_SEC +
-			     priv->est->ctr[0];
-		time = stmmac_calc_tas_basetime(basetime,
-						current_time_ns,
-						cycle_time);
-
-		priv->est->btr[0] = (u32)time.tv_nsec;
-		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);
-		mutex_unlock(&priv->est_lock);
-		if (ret)
-			netdev_err(priv->dev, "failed to configure EST\n");
+		stmmac_est_reconfigure(priv);
 	}
 
 	return 0;

---
base-commit: 2db9bfa3e27bdea15e05ea70b56bad3d21e570ec
change-id: 20260824-stmmac-est-reapply-after-open-181d70a15eb6

Best regards,
-- 
Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

Re: [PATCH net] net: stmmac: re-apply taprio offload in __stmmac_open()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]

> Hello Lorenzo,

Hi Maxime,

> 
> On 8/25/26 11:24, Lorenzo Bianconi wrote:
> > The core soft reset issued in stmmac_init_dma_engine() clears the
> > MTL_EST registers, but nothing re-applies the taprio offload after it:
> > priv->est->enable stays true while the hardware EST block is left
> > disabled. The TX/XDP paths then keep dropping frames larger than
> > priv->est->max_sdu[] and taprio is reported as offloaded, although the
> > EST block is not programmed.
> > 
> > Re-apply the taprio offload in __stmmac_open() after PTP is up. The
> > base time is recomputed from the reserved base time and the current PTP
> > time, since the timestamp counter has been re-initialized and the
> > previously programmed base time is stale.
> 
> I agree with idea, but the way I see that, this new reconfigure method shares
> some code with stmmac_tc.c's tc_taprio_configure() step (the base time computation).
> 
> Can you also take a pass on that, so that we have a single helper (your new
> stmmac_est_reconfigure) that does the base time computation and the EST setup, that
> would be used in the .ndo_open, clock adjust and tc_taprio configuration ?

Do you mean introducing a single helper (e.g stmmac_setup_est()) where we have a
tc_taprio_qopt_offload pointer in the routine signature to distinguish between
the 'configure' and 'reconfigure' cases? E.g:

void stmmac_setup_est(struct stmmac_priv *priv,
		      struct tc_taprio_qopt_offload *qopt)
{
	...
	if (qopt) {
		/* configure from tc path */
	} else {
		/* reconfigure from __stmmac_open() path */
	}
	...
}

Regards,
Lorenzo

> 
> Maybe rename it stmmac_setup_est() to keep consistency with the stmmac_setup_ptp()
> that comes before, this would also open the door for an equivalent
> stmmac_cleanup_est() if we ever need it.
> 
> Maxime
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]