Re: [PATCH net-next v2] net: stmmac: dwxgmac2: add XGMAC timestamp interrupt support

["Nazle Asmade, Muhammad Nazim Amirul" <muhammad.nazim.amirul.nazle.asmade@altera.com>]

On 18/8/2026 11:37 am, zhu xin wrote:
> [You don't often get email from zxyan0222@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Mon, Aug 17, 2026 at 11:11 AM Nazle Asmade, Muhammad Nazim Amirul
> <muhammad.nazim.amirul.nazle.asmade@altera.com> wrote:
>>
>> On 13/8/2026 2:50 pm, zhu xin wrote:
>>> [You don't often get email from zxyan0222@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> Hi Maxime,
>>>
>>> On Wed, Aug 12, 2026 at 10:08 PM Maxime Chevallier
>>> <maxime.chevallier@bootlin.com> wrote:
>>>>
>>>> +Nazim
>>>>
>>>> On 8/12/26 14:59, Zxyan Zhu wrote:
>>>>> Hi Maxime,
>>>>>
>>>>> On 8/12/26 5:10 PM, Maxime Chevallier wrote:
>>>>>> If there's an interrupt storm, then this should probably be a fix for -net
>>>>>> with a Fixes tag.
>>>>>
>>>>> There is no interrupt storm. XGMAC_TSIE was never enabled before this
>>>>> patch (XGMAC_INT_DEFAULT_EN was LPIIE | PMTIE only), and
>>>>> dwxgmac2_host_irq_status() masks the status with the enable mask, so
>>>>> XGMAC_INT_TSIS could never fire. The actual symptom is that
>>>>> auxiliary-snapshot extts events were never reported -- a feature that
>>>>> was never wired up, not a live interrupt storm.
>>>>
>>>> Ah indeed, quite the opposite then.
>>>>
>>>>>
>>>>> So I'd like to keep this on net-next as a new feature rather than
>>>>> retarget it to net.
>>>>
>>>> Yes makes sense indeed.
>>>>
>>>> Can you take a look at the sashiko-reported issues, it spots that this
>>>> could interfere with the timestamping on agilex5 (another XGMAC platform) :
>>>>
>>>> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260810100221.9166-1-zxyan0222%40gmail.com
>>>>
>>>> Maybe there's some stuff that could be made more generic by moving them out
>>>> of the socfpga code and putting it in the more generic timestamping code ?
>>>
>>> Thanks for the pointer. I looked at the sashiko v2 review and the
>>> Agilex5 concern is real. On Agilex5, smtg_crosststamp() polls
>>> XGMAC_INT_STATUS for TSIS in process context as its only completion
>>> signal. The patch enables XGMAC_TSIE in XGMAC_INT_DEFAULT_EN, so the
>>> new hardirq handler now runs on every timestamp event and clears TSIS
>>> by reading XGMAC_TIMESTAMP_STATUS -- that read can win against the
>>> poll loop, and getcrosststamp() then times out with "Wait for time
>>> sync operation timeout".
>>>
>>> On moving socfpga bits to the generic timestamping code: the
>>> crosststamp path splits into a generic half (pick ATSENx, set ATSFC,
>>> poll TSIS, read ATSNS, pop ATNR/ATSR) and an Agilex5-specific half
>>> (GPO0 rising-edge trigger + SMTG MDIO system counter). Only the
>>> generic half is worth lifting, and the platform-specific trigger
>>> belongs in plat->crosststamp. That's a useful cleanup but may be
>>> orthogonal to this race.
>>>
>>> For the race itself I'd like your view on the fix below. The simplest
>>> option I found is to mask XGMAC_TSIE around the cross-timestamp so the
>>> hardirq handler cannot steal TSIS while smtg_crosststamp() owns the
>>> snapshot FIFO:
>>>
>>>     static int smtg_crosststamp(ktime_t *device,
>>>                                 struct system_counterval_t *system,
>>>                                 void *ctx)
>>>     {
>>>         ...
>>>         if (priv->plat->flags & STMMAC_FLAG_EXT_SNAPSHOT_EN)
>>>             return -EBUSY;
>>>
>>>         /* mask TSIE so the hardirq dwxgmac2_timestamp_interrupt() can't
>>>          * clear TSIS via reading XGMAC_TIMESTAMP_STATUS while we poll it
>>>          */
>>>         stmmac_mac_irq_modify(priv, XGMAC_TSIE, 0);
>>>
>>>         ... arm ATSENx, ATSFC, toggle GPO0 ...
>>>
>>>         ret = readl_poll_timeout(ioaddr + XGMAC_INT_STATUS, v,
>>>                                  (v & XGMAC_INT_TSIS), 100, 10000);
>>>         if (ret) {
>>>             ...
>>>             stmmac_mac_irq_modify(priv, 0, XGMAC_TSIE);
>>>             return ret;
>>>         }
>>>
>>>         ... read ATSNS, pop FIFO, get_smtgtime() ...
>>>
>>>         stmmac_mac_irq_modify(priv, 0, XGMAC_TSIE);
>>>         return 0;
>>>     }
>>>
>>>     /* every return path, including the -EINVAL default of the
>>>      * int_snapshot_num switch, restores XGMAC_TSIE */
>>>
>>>     static void dwxgmac2_timestamp_interrupt(struct stmmac_priv *priv)
>>>     {
>>>         u32 ts_status = readl(priv->ioaddr + XGMAC_TIMESTAMP_STATUS);
>>>
>>>         if (!(priv->plat->flags & STMMAC_FLAG_EXT_SNAPSHOT_EN))
>>>             return;
>>>         ...
>>>     }
>>>
>>> This keeps the extts handler unchanged and just makes "who owns TSIS"
>>> explicit on the crosststamp side.
>>>
>>> An alternative I considered:
>>>     - making socfpga set STMMAC_FLAG_INT_SNAPSHOT_EN and wait on
>>>       tstamp_busy_wait like intel: cleaner long-term, but a bigger
>>>       rewrite than fixing the regression warrants here.
>>>
>>> One worry: I'd like to confirm masking TSIE for the ~poll window is
>>> acceptable on this platform -- extts snapshots taken during the window
>>> still latch TSIS (just don't raise an interrupt) and are reported after
>>> TSIE is restored, so they are delayed rather than lost. Does that match
>>> your expectations?
>>>
>>>>
>>>> I don't have any xgmac boards to verify sashiko's claim though.
>>>>
>>>
>>> I don't have Agilex5 hardware to validate the crosststamp path either,
>>> so I'm relying on the code analysis above. If this approach looks
>>> reasonable I'll send it as a v3 with this as a second patch.
>> Hi Zxyan,
>>
>> Let me know what kind of test needed, I can help to verify on my end
>>
>> BR,
>> Nazim> Thanks,
>>> Zxyan
>>
>
> Hi Nazim,
>
> That would be great, thanks. The concern is that patch enables
> XGMAC_TSIE in XGMAC_INT_DEFAULT_EN, so the new hardirq timestamp handler
> runs on every XGMAC timestamp event and clears TSIS by reading
> XGMAC_TIMESTAMP_STATUS.  On Agilex5, smtg_crosststamp() polls
> XGMAC_INT_STATUS for TSIS in process context as its only completion
> signal, so the hardirq read can win the race and getcrosststamp() times
> out with "Wait for time sync operation timeout".
>
> If you are able to verify on your Agilex5 board using the v2 patch
> (which enables XGMAC_TSIE but has no socfpga fix yet), I would like to
> confirm the following two points, to check whether the Agilex5
> cross-timestamp regression that sashiko flagged actually reproduces:
>
> 1. Does smtg_crosststamp() time out -- that is, does the error "Wait
>     for time sync operation timeout" appear in dmesg?
> 2. Is the cross-timestamp obtained correctly?
>
> Anything else you'd normally run for the crosststamp path is welcome --
> you know the board far better than I do.
>
> If the issue does exist, I will send v3 with the fix.  The specific
> approach still needs input from both Maxime and you; the options I see
> are:
>
>    1. Isolate the interrupt using stmmac_mac_irq_modify within
>       smtg_crosststamp -- the minimal fix.
>    2. Making socfpga set STMMAC_FLAG_INT_SNAPSHOT_EN and wait on
>       tstamp_busy_wait like intel -- cleaner long-term, but a bigger
>       rewrite.
>
> Best regards,
> Zxyan Zhu
Hi Zxyan,

I tested your proposed fix (masking XGMAC_TSIE around the
smtg_crosststamp() poll window) on my Agilex5 board. The cross-timestamp
path works correctly.

No "Wait for time sync operation timeout" error in dmesg. PTP clock
registers and operates normally.

hence,
Tested-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>