[PATCH net] net: stmmac: re-base EST after PTP settime64

[PATCH net] net: stmmac: re-base EST after PTP settime64

[Lorenzo Bianconi]

stmmac_set_time() initializes the PTP clock through
stmmac_init_systime() (TSINIT) while holding only ptp_lock.  It neither
takes est_lock nor disables and re-bases EST around the step, unlike
stmmac_adjust_time(), which performs TSUPDT through
stmmac_adjust_systime() under est_lock.

Both operations change the PHC system time, so both invalidate the
absolute EST base time.  With settime64, a concurrent
tc_taprio_configure() can read the pre-step clock through gettimex64()
and program EST with a base time that is already stale once TSINIT
lands.  Separately, a taprio offload installed before the clock_settime()
is never re-based and keeps running with its old base time until taprio
is reconfigured.

Take est_lock for the whole disable/step/reprogram sequence, as
stmmac_adjust_time() does: disable EST before initializing the clock and
call __stmmac_setup_est() afterwards so the base time is recomputed from
the reserved taprio base time and the new clock.

This keeps the established est_lock -> ptp_lock ordering:
stmmac_init_systime() only uses readl_poll_timeout_atomic() under
ptp_lock, and __stmmac_setup_est() takes ptp_lock through gettimex64().

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_ptp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 31a66e855d6b..6370bdc6c58b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -129,10 +129,19 @@ static int stmmac_set_time(struct ptp_clock_info *ptp,
 	    container_of(ptp, struct stmmac_priv, ptp_clock_ops);
 	unsigned long flags;
 
+	mutex_lock(&priv->est_lock);
+	if (priv->est && priv->est->enable)
+		stmmac_est_configure(priv, priv, priv->est,
+				     priv->plat->clk_ptp_rate, false);
+
 	write_lock_irqsave(&priv->ptp_lock, flags);
 	stmmac_init_systime(priv, priv->ptpaddr, ts->tv_sec, ts->tv_nsec);
 	write_unlock_irqrestore(&priv->ptp_lock, flags);
 
+	if (priv->est && priv->est->enable)
+		__stmmac_setup_est(priv);
+	mutex_unlock(&priv->est_lock);
+
 	return 0;
 }
 

---
base-commit: 22b7cb932c6a0f7566d41072be6956cb6c8af4e1
change-id: 20261111-stmmac_set_time-est_lock-f74f4ce678b7

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

Re: [PATCH net] net: stmmac: re-base EST after PTP settime64

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: stmmac: re-base EST after PTP settime64

[Lorenzo Bianconi]

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

> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.

the issue has been reported by sashiko.

> 
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by LLM and not proven at runtime). If it was triggered
>    please include the symptoms, like the stack trace or kernel messages.

this is a real issue reported by sashiko.

> 
>  - What hardware the change was tested on. For driver fixes please
>    mention the device (and if relevant firmware version) used for
>    testing, or say that the change was not tested on real hardware.

I tested it on a Qualcomm rb3-gen2 board.

Regards,
Lorenzo

> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

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