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

On Mon, Aug 17, 2026 at 11:11 AM Nazle Asmade, Muhammad Nazim Amirul
<[email protected]> wrote:
>
> On 13/8/2026 2:50 pm, zhu xin wrote:
> > [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Hi Maxime,
> >
> > On Wed, Aug 12, 2026 at 10:08 PM Maxime Chevallier
> > <[email protected]> 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