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

["G Thomas, Rohan" <rohan.g.thomas@altera.com>]

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@altera.com>
>>>>
>>>> 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@altera.com>
> 
> [...]
> 
>>>> +	/* 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