Message-ID: <aTFuJUiLMnHrnpW5@shell.armlinux.org.uk>
Date: Thu, 4 Dec 2025 11:19:01 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Rohan G Thomas <rohan.g.thomas@...era.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Richard Cochran <richardcochran@...il.com>,
	Jose Abreu <Jose.Abreu@...opsys.com>,
	Fugang Duan <fugang.duan@....com>,
	Kurt Kanzenbach <kurt@...utronix.de>, netdev@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] net: stmmac: Fix E2E delay mechanism

On Thu, Dec 04, 2025 at 10:58:40AM +0100, Paolo Abeni wrote:
> On 11/29/25 4:07 AM, Rohan G Thomas wrote:
> > For E2E delay mechanism, "received DELAY_REQ without timestamp" error
> > messages show up for dwmac v3.70+ and dwxgmac IPs.
> > 
> > This issue affects socfpga platforms, Agilex7 (dwmac 3.70) and
> > Agilex5 (dwxgmac). According to the databook, to enable timestamping
> > for all events, the SNAPTYPSEL bits in the MAC_Timestamp_Control
> > register must be set to 2'b01, and the TSEVNTENA bit must be cleared
> > to 0'b0.
> > 
> > Commit 3cb958027cb8 ("net: stmmac: Fix E2E delay mechanism") already
> > addresses this problem for all dwmacs above version v4.10. However,
> > same holds true for v3.70 and above, as well as for dwxgmac. Updates
> > the check accordingly.
> > 
> > Fixes: 14f347334bf2 ("net: stmmac: Correctly take timestamp for PTPv2")
> > Fixes: f2fb6b6275eb ("net: stmmac: enable timestamp snapshot for required PTP packets in dwmac v5.10a")
> > Fixes: 3cb958027cb8 ("net: stmmac: Fix E2E delay mechanism")
> > Signed-off-by: Rohan G Thomas <rohan.g.thomas@...era.com>
> > ---
> > v1 -> v2:
> >    - Rebased patch to net tree
> >    - Replace core_type with has_xgmac
> >    - Nit changes in the commit message
> >    - Link: https://lore.kernel.org/all/20251125-ext-ptp-fix-v1-1-83f9f069cb36@altera.com/
> 
> Given there is some uncertain WRT the exact oldest version to be used,
> it would be great to have some 3rd party testing/feedback on this. Let's
> wait a little more.

As I said, in the v3.74 documentation, it is stated that the SNAPTYPSEL
functions changed between v3.50 and v3.60, so I think it would be better
to propose a patch to test for < v3.6.

Alternatively, if someone has the pre-v3.6 databook to check what the
SNAPTYPSEL definition is and compare it with the v3.6+ definition, that
would also be a good thing to do.

>From the 3.74:

SNAPTYPSEL
00		?
01		?
10		Sync, Delay_Req
11		Sync, PDelay_Req, PDelay_Resp

TSEVNTENA
0		All messages except Announce, Management and Signalling
1		Sync, Delay_Req, PDelay_Req, PDelay_Resp

No table is provided, so it's difficult to know what all the bit
combinations do for v3.74.

>From STM32MP151 documentation (v4.2 according to GMAC4_VERSION
register):

SNAPTYPSEL	TSMSTRENA	TSEVNTENA
00		x		0		Sync, Delay_Req
00		0		1		Delay_Req
00		1		1		Sync
01		x		0		Sync, PDelay_Req, PDelay_Resp
01		0		1		Sync, Delay_Req, PDelay_Req,
						PDelay_Resp
01		1		1		Sync, PDelay_Req, PDelay_Resp
10		x		x		Sync, Delay_Req
11		x		x		Sync, PDelay_Req, PDelay_Resp

For iMX8MP (v5.1) and STM32MP23/25xx (v5.3) documentatiion:

SNAPTYPSEL	TSMSTRENA	TSEVNTENA
00		x		0		Sync, Follow_Up, Delay_Req,
						Delay_Resp
00		0		1		Sync
00		1		1		Delay_Req
01		x		0		Sync, Follow_Up, Delay_Req,
						Delay_Resp, PDelay_Req,
						PDelay_Resp
01		0		1		Sync, PDelay_Req, PDelay_Resp
01		1		1		Delay_Req, PDelay_Req,
						PDelay_Resp
10		x		x		Sync, Delay_Req
11		x		x		PDelay_Req, PDelay_Resp

Differences:
00 x 0 - adds Follow_Up
00 X 1 - TSMSTRENA bit inverted
01 x 0 - adds Follow_Up, Delay_Req, Delay_Resp
01 0 1 - removes Delay_Req
01 1 1 - removes Sync, adds Delay_Req
11 x x - removes Sync

So, it looks like there's another difference between v4.2 and v5.1.

If the STM32MP151 (v4.2) documentation is correct, then from what I see
in the driver, if HWTSTAMP_FILTER_PTP_V1_L4_SYNC is requested, we set
SNAPTYPSEL=00 TSMSTRENA=0 TSEVNTENA=1, which semects Delay_Req messages
only, but on iMX8MP this selects Sync messages.

HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ is the opposite (due to the
inversion of TSMSTRENA) for SNAPTYPSEL=00.

For HWTSTAMP_FILTER_PTP_V2_EVENT, we currently set SNAPTYPSEL=01
TSMSTRENA=0 and TSEVNTENA=1 for cores < v4.1:
- For STM32MP151 (v4.2) we get Sync, PDelay_Req, PDelay_Resp but
  _not_ Delay_Req. Seems broken.
- For iMX8MP (v5.1) and STM32MP23/25xx (v5.3), we get
  Sync, Follow_Up, Delay_Req, Delay_Resp, PDelay_Req, PDelay_Resp

Basically, the conclusion I am coming to is that Synopsys's idea
of "lets tell the hardware what _kind_ of PTP clock we want to be,
whether we're master, etc" is subject to multiple revisions in
terms of which messages each mode selects, and it would have been
_far_ simpler and easier to understand had they just provided a
16-bit bitfield of message types to accept.

So, I'm wary about this change - I think there's more "mess"
here than just that single version check in
HWTSTAMP_FILTER_PTP_V2_EVENT, I think it's a lot more complicated.
I'm not sure what the best solution is right now, because I don't
have the full information, but it looks to me like the current
approach does not result in the expected configuration for each
of the dwmac core versions, and there are multiple issues here.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Powered by blists - more mailing lists