Re: [PATCH net-next v3 2/2] net: stmmac: dwmac-socfpga: mask XGMAC_TSIE during cross-timestamp

Hi, Andrew

On Tue, Aug 18, 2026 at 11:07 PM Andrew Lunn <[email protected]> wrote:
>
> On Tue, Aug 18, 2026 at 09:27:22PM +0800, Zxyan Zhu wrote:
> > The Agilex5 smtg_crosststamp() handler arms an internal auxiliary
> > snapshot, toggles GPO0 and then polls XGMAC_INT_STATUS for TSIS in
> > process context to learn that the snapshot is ready.
> >
> > Once XGMAC_TSIE is unmasked (done by a companion change that enables it
> > in XGMAC_INT_DEFAULT_EN), the DWXGMAC2 timestamp interrupt handler runs
> > from hardirq on every timestamp event and clears TSIS by reading
> > XGMAC_TIMESTAMP_STATUS.  That read can win the race against the poll
> > loop, which then times out and makes PTP_SYS_OFFSET_PRECISE fail with
> > "Wait for time sync operation timeout".
> >
> > Mask XGMAC_TSIE around the snapshot trigger and FIFO read so the hardirq
> > handler cannot clear TSIS while smtg_crosststamp() owns it, and restore
> > it on every return path.
>
> Did you look at the other system which implement crosststamp?
> dwmac-intel.c? Does it need similar changes?

Yes, I looked at dwmac-intel.c.  It does not need the same change.

intel_crosststamp() sets STMMAC_FLAG_INT_SNAPSHOT_EN and waits on
tstamp_busy_wait for the interrupt:

  priv->plat->flags |= STMMAC_FLAG_INT_SNAPSHOT_EN;
  ...
  wait_event_interruptible_timeout(priv->tstamp_busy_wait,
                                   stmmac_cross_ts_isr(priv), HZ / 100);

and the generic timestamp_interrupt() early-returns when that flag is
set:

  if (priv->plat->flags & STMMAC_FLAG_INT_SNAPSHOT_EN) {
          wake_up(&priv->tstamp_busy_wait);
          return;
  }

so the Intel handler never reads GMAC_TIMESTAMP_STATUS and does not
clear TSIS.  Intel is interrupt-driven, so it depends on TSIE being
enabled; masking it would break the wake-up.

socfpga is the odd one: it does not set INT_SNAPSHOT_EN, polls
XGMAC_INT_STATUS for TSIS in process context, and sits on
DWMAC_CORE_XGMAC -- the core type this series newly enables TSIE on.
So only socfpga needs the mask.

That said, the cleaner long-term fix is to align socfpga with the
Intel approach: set INT_SNAPSHOT_EN and wait_event instead of polling.
The trade-off:

  - Masking TSIE (this series): minimal, keeps the poll loop, but a
    local workaround for the XGMAC hardirq-vs-poll race only.
  - Intel pattern: no per-platform masking, similar across XGMAC and
    GMAC4; but a bigger rewrite -- stmmac_cross_ts_isr() reads
    GMAC_INT_STATUS (wrong offset for XGMAC), needing an XGMAC variant,
    and the poll becomes wait_event.

I went with the mask as the minimal regression fix, and can do the
Intel-style conversion as a separate follow-up if preferred.

Best regards,
Zxyan Zhu

>
>         Andrew