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 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