Re: [PATCH net-next 4/4] net: stmmac: socfpga: Add hardware supported cross-timestamp

[G Thomas, Rohan]

Hi Maxime,

On 10/29/2025 8:36 PM, Maxime Chevallier wrote:
> Hi Rohan,
>
> On 29/10/2025 15:41, G Thomas, Rohan wrote:
> > Hi Maxime,
> >
> > On 10/29/2025 3:20 PM, Maxime Chevallier wrote:
> > > Hi Rohan,
> > >
> > > On 29/10/2025 09:06, Rohan G Thomas via B4 Relay wrote:
> > > > From: Rohan G Thomas <rohan.g.thomas@xxxxxxxxxx>
> > > >
> > > > Cross timestamping is supported on Agilex5 platform with Synchronized
> > > > Multidrop Timestamp Gathering(SMTG) IP. The hardware cross-timestamp
> > > > result is made available the applications through the ioctl call
> > > > PTP_SYS_OFFSET_PRECISE, which inturn calls stmmac_getcrosststamp().
> > > >
> > > > Device time is stored in the MAC Auxiliary register. The 64-bit System
> > > > time (ARM_ARCH_COUNTER) is stored in SMTG IP. SMTG IP is an MDIO device
> > > > with 0xC - 0xF MDIO register space holds 64-bit system time.
> > > >
> > > > This commit is similar to following commit for Intel platforms:
> > > > Commit 341f67e424e5 ("net: stmmac: Add hardware supported cross-timestamp")
> > > >
> > > > Signed-off-by: Rohan G Thomas <rohan.g.thomas@xxxxxxxxxx>
>
> [...]
>
> > > > +	/* Time sync done Indication - Interrupt method */
> > > > +	if (!wait_event_interruptible_timeout(priv->tstamp_busy_wait,
> > > > +					      dwxgmac_cross_ts_isr(priv),
> > > > +					      HZ / 100)) {
> > > > +		priv->plat->flags &= ~STMMAC_FLAG_INT_SNAPSHOT_EN;
> > > > +		return -ETIMEDOUT;
> > >
> > > Don't you need to set priv->plat->flags |= STMMAC_FLAG_INT_SNAPSHOT_EN first?
> > > Otherwise, timestamp_interrupt() in stmmac_hwtstamp() won't call wake_up()
> > > on the wait_queue.
> >
> > Thanks for pointing this out. My intention was to use the polling
> > method, but I accidentally left behind some code from experimenting with
> > the interrupt method. While reverting those changes, I missed updating
> > this part of the code. Will fix this in the next revision. Sorry for the
> > error. Currently not seeing any timeout issues with polling method on
> > XGMAC IP. Also some spurios interrupts causing stall when using
> > the interrupt method in XGMAC.
>
> So, if you use the polling method, this will likely bring this code
> even closer to what's implemented in the intel dwmac wrapper. Is this
> the same IP ?

AFAIK, this is an Altera specific IP. Altera was part of Intel and may
be the SMTG IP implementation was inspired by ART timer on x86 platforms
:). But for later platforms this may diverge.

Also, latest intel dwmac wrapper is using interrupt method.

> To me it looks like the only difference will be a few
> register offsets (XGMAC vs GMAC), some clock id and the mdio accesses,
> maybe it could be worth considering re-using what's been done on the
> Intel side and avoid duplication...
>
> That could be all moved to stmmac_ptp for instance, using some flag
> in the plat data to indicate if cross timestamping is supported, and
> use the core type (xgmac, gmac, etc.) for the offsets ?

Since SMTG or ART is a vendor specific IP, I'm not sure it is good to
move this to the stmmac_ptp. Also, not sure other ways (other than mdio
acess, gpo toggling) to implement hw cross timestamping. Open to
suggestions.

> Of course with the risk of regressing dwmac-intel.c :(
>
> Maxime

Best Regards,
Rohan