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

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

[Zxyan Zhu]

DWXGMAC2 uses XGMAC_TIMESTAMP_STATUS at offset 0xd20, while the
generic stmmac PTP handler reads the dwmac4 offset GMAC_TIMESTAMP_STATUS
(0xb20).  Before this change, the DWXGMAC2 and DWXLGMAC2 hwif entries
used &stmmac_ptp, whose timestamp_interrupt callback read the wrong
register and whose config_hw_tstamping callback never enabled the
XGMAC timestamp interrupt (XGMAC_TSIE was not in XGMAC_INT_DEFAULT_EN).
As a result, auxiliary snapshot events were never reported on XGMAC
platforms.

Add a dedicated DWXGMAC2 timestamp interrupt handler that:
- reads XGMAC_TIMESTAMP_STATUS before checking
  STMMAC_FLAG_EXT_SNAPSHOT_EN, so that the timestamp interrupt status is
  cleared even when auxiliary snapshots are disabled
- derives the pending auxiliary snapshot count from the persistent
  ATSNS field instead of the transient AUXTSTRIG status bit
- generates the corresponding PTP_CLOCK_EXTTS events

Also enable XGMAC_TSIE in XGMAC_INT_DEFAULT_EN and hook the new
handler into the DWXGMAC2 and DWXLGMAC2 hwif entries.

Signed-off-by: Zxyan Zhu <zxyan0222@gmail.com>
---
v1: https://lore.kernel.org/netdev/20260806-dwxgmac2-timestamp-irq-v1-1-c051c79c9d90@gmail.com/
v2:
- reads XGMAC_TIMESTAMP_STATUS before checking
  STMMAC_FLAG_EXT_SNAPSHOT_EN, so that the timestamp interrupt status is
  cleared even when auxiliary snapshots are disabled.
- Drop the unnecessary (u64) cast in dwxgmac2_get_mac_tx_timestamp().
- Reword the changelog to describe the real problem: the generic handler
  read the wrong register offset (0xb20 vs 0xd20) and the interrupt was
  never enabled, so auxiliary snapshot reporting never worked on XGMAC.
- Update the comment to clarify that TXTSC is cleared by
  XGMAC_TXTIMESTAMP_SEC, not by XGMAC_TIMESTAMP_STATUS.
---
 .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |  2 +-
 .../ethernet/stmicro/stmmac/dwxgmac2_core.c   | 43 +++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/hwif.c    |  4 +-
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |  1 +
 .../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 12 ++++++
 .../net/ethernet/stmicro/stmmac/stmmac_ptp.h  |  1 +
 6 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 61b6d45a02f5..76e2860a9517 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -87,7 +87,7 @@
 #define XGMAC_TSIE			BIT(12)
 #define XGMAC_LPIIE			BIT(5)
 #define XGMAC_PMTIE			BIT(4)
-#define XGMAC_INT_DEFAULT_EN		(XGMAC_LPIIE | XGMAC_PMTIE)
+#define XGMAC_INT_DEFAULT_EN		(XGMAC_LPIIE | XGMAC_PMTIE | XGMAC_TSIE)
 #define XGMAC_Qx_TX_FLOW_CTRL(x)	(0x00000070 + (x) * 4)
 #define XGMAC_PT			GENMASK(31, 16)
 #define XGMAC_TFE			BIT(1)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index f02b434bbd50..b849cebf9b29 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -1154,6 +1154,49 @@ static int dwxgmac2_get_mac_tx_timestamp(struct mac_device_info *hw, u64 *ts)
 	return 0;
 }
 
+void dwxgmac2_timestamp_interrupt(struct stmmac_priv *priv)
+{
+	u32 ts_status, pending_snapshots, acr_value, channel;
+	struct ptp_clock_event event;
+	unsigned long flags;
+	u64 ptp_time;
+	int i;
+
+	/* Read XGMAC_TIMESTAMP_STATUS to get the AUX snapshot
+	 * count.  This read also clears the TSIS bit in
+	 * XGMAC_INT_STATUS.
+	 * TX timestamp polling may have already cleared TSIS
+	 * and AUXTSTRIG, so rely on ATSNS instead.
+	 * TXTSC is cleared by XGMAC_TXTIMESTAMP_SEC, not by
+	 * this register, so there is no conflict.
+	 */
+	ts_status = readl(priv->ioaddr + XGMAC_TIMESTAMP_STATUS);
+
+	if (!(priv->plat->flags & STMMAC_FLAG_EXT_SNAPSHOT_EN))
+		return;
+
+	pending_snapshots = FIELD_GET(XGMAC_TIMESTAMP_ATSNS_MASK, ts_status);
+	if (!pending_snapshots)
+		return;
+
+	acr_value = readl(priv->ptpaddr + PTP_ACR);
+	channel = FIELD_GET(PTP_ACR_MASK, acr_value);
+	if (!channel)
+		return;
+	channel = ilog2(channel);
+
+	for (i = 0; i < pending_snapshots; i++) {
+		read_lock_irqsave(&priv->ptp_lock, flags);
+		stmmac_get_ptptime(priv, priv->ptpaddr, &ptp_time);
+		read_unlock_irqrestore(&priv->ptp_lock, flags);
+
+		event.type = PTP_CLOCK_EXTTS;
+		event.index = channel;
+		event.timestamp = ptp_time;
+		ptp_clock_event(priv->ptp_clock, &event);
+	}
+}
+
 static int dwxgmac2_flex_pps_config(void __iomem *ioaddr, int index,
 				    struct stmmac_pps_cfg *cfg, bool enable,
 				    u32 sub_second_inc, u32 systime_flags)
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
index 511b0fd5e834..9718582b8480 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
@@ -258,7 +258,7 @@ static const struct stmmac_hwif_entry {
 		.dma = &dwxgmac210_dma_ops,
 		.mac = &dwxgmac210_ops,
 		.vlan = &dwxgmac210_vlan_ops,
-		.hwtimestamp = &stmmac_ptp,
+		.hwtimestamp = &dwxgmac2_ptp,
 		.ptp = &stmmac_ptp_clock_ops,
 		.mode = NULL,
 		.tc = &dwmac510_tc_ops,
@@ -280,7 +280,7 @@ static const struct stmmac_hwif_entry {
 		.dma = &dwxgmac210_dma_ops,
 		.mac = &dwxlgmac2_ops,
 		.vlan = &dwxlgmac2_vlan_ops,
-		.hwtimestamp = &stmmac_ptp,
+		.hwtimestamp = &dwxgmac2_ptp,
 		.ptp = &stmmac_ptp_clock_ops,
 		.mode = NULL,
 		.tc = &dwmac510_tc_ops,
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index e6317b94fff7..818ab3daa91c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -671,6 +671,7 @@ extern const struct stmmac_desc_ops ndesc_ops;
 
 extern const struct stmmac_hwtimestamp stmmac_ptp;
 extern const struct stmmac_hwtimestamp dwmac1000_ptp;
+extern const struct stmmac_hwtimestamp dwxgmac2_ptp;
 
 extern const struct stmmac_mode_ops ring_mode_ops;
 extern const struct stmmac_mode_ops chain_mode_ops;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
index b9a985fa772c..9d7d24259abd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
@@ -277,3 +277,15 @@ const struct stmmac_hwtimestamp dwmac1000_ptp = {
 	.get_ptptime = dwmac1000_get_ptptime,
 	.timestamp_interrupt = dwmac1000_timestamp_interrupt,
 };
+
+const struct stmmac_hwtimestamp dwxgmac2_ptp = {
+	.config_hw_tstamping = config_hw_tstamping,
+	.init_systime = init_systime,
+	.config_sub_second_increment = config_sub_second_increment,
+	.config_addend = config_addend,
+	.adjust_systime = adjust_systime,
+	.get_systime = get_systime,
+	.get_ptptime = get_ptptime,
+	.timestamp_interrupt = dwxgmac2_timestamp_interrupt,
+	.hwtstamp_correct_latency = hwtstamp_correct_latency,
+};
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
index 3fe0e3a80e80..dade09614163 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
@@ -103,6 +103,7 @@ int dwmac1000_ptp_enable(struct ptp_clock_info *ptp,
 
 void dwmac1000_get_ptptime(void __iomem *ptpaddr, u64 *ptp_time);
 void dwmac1000_timestamp_interrupt(struct stmmac_priv *priv);
+void dwxgmac2_timestamp_interrupt(struct stmmac_priv *priv);
 
 extern const struct ptp_clock_info stmmac_ptp_clock_ops;
 extern const struct ptp_clock_info dwmac1000_ptp_clock_ops;
-- 
2.34.1

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

[Maxime Chevallier]

Hi,

On 8/10/26 12:02, Zxyan Zhu wrote:
> DWXGMAC2 uses XGMAC_TIMESTAMP_STATUS at offset 0xd20, while the
> generic stmmac PTP handler reads the dwmac4 offset GMAC_TIMESTAMP_STATUS
> (0xb20).  Before this change, the DWXGMAC2 and DWXLGMAC2 hwif entries
> used &stmmac_ptp, whose timestamp_interrupt callback read the wrong
> register and whose config_hw_tstamping callback never enabled the
> XGMAC timestamp interrupt (XGMAC_TSIE was not in XGMAC_INT_DEFAULT_EN).
> As a result, auxiliary snapshot events were never reported on XGMAC
> platforms.
> 
> Add a dedicated DWXGMAC2 timestamp interrupt handler that:
> - reads XGMAC_TIMESTAMP_STATUS before checking
>   STMMAC_FLAG_EXT_SNAPSHOT_EN, so that the timestamp interrupt status is
>   cleared even when auxiliary snapshots are disabled
> - derives the pending auxiliary snapshot count from the persistent
>   ATSNS field instead of the transient AUXTSTRIG status bit
> - generates the corresponding PTP_CLOCK_EXTTS events
> 
> Also enable XGMAC_TSIE in XGMAC_INT_DEFAULT_EN and hook the new
> handler into the DWXGMAC2 and DWXLGMAC2 hwif entries.
> 
> Signed-off-by: Zxyan Zhu <zxyan0222@gmail.com>

If there's an interrupt storm, then this should probably be a fix for -net
with a Fixes tag.

Maxime

> ---
> v1: https://lore.kernel.org/netdev/20260806-dwxgmac2-timestamp-irq-v1-1-c051c79c9d90@gmail.com/
> v2:
> - reads XGMAC_TIMESTAMP_STATUS before checking
>   STMMAC_FLAG_EXT_SNAPSHOT_EN, so that the timestamp interrupt status is
>   cleared even when auxiliary snapshots are disabled.
> - Drop the unnecessary (u64) cast in dwxgmac2_get_mac_tx_timestamp().
> - Reword the changelog to describe the real problem: the generic handler
>   read the wrong register offset (0xb20 vs 0xd20) and the interrupt was
>   never enabled, so auxiliary snapshot reporting never worked on XGMAC.
> - Update the comment to clarify that TXTSC is cleared by
>   XGMAC_TXTIMESTAMP_SEC, not by XGMAC_TIMESTAMP_STATUS.
> ---
>  .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |  2 +-
>  .../ethernet/stmicro/stmmac/dwxgmac2_core.c   | 43 +++++++++++++++++++
>  drivers/net/ethernet/stmicro/stmmac/hwif.c    |  4 +-
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  1 +
>  .../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 12 ++++++
>  .../net/ethernet/stmicro/stmmac/stmmac_ptp.h  |  1 +
>  6 files changed, 60 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> index 61b6d45a02f5..76e2860a9517 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> @@ -87,7 +87,7 @@
>  #define XGMAC_TSIE			BIT(12)
>  #define XGMAC_LPIIE			BIT(5)
>  #define XGMAC_PMTIE			BIT(4)
> -#define XGMAC_INT_DEFAULT_EN		(XGMAC_LPIIE | XGMAC_PMTIE)
> +#define XGMAC_INT_DEFAULT_EN		(XGMAC_LPIIE | XGMAC_PMTIE | XGMAC_TSIE)
>  #define XGMAC_Qx_TX_FLOW_CTRL(x)	(0x00000070 + (x) * 4)
>  #define XGMAC_PT			GENMASK(31, 16)
>  #define XGMAC_TFE			BIT(1)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> index f02b434bbd50..b849cebf9b29 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> @@ -1154,6 +1154,49 @@ static int dwxgmac2_get_mac_tx_timestamp(struct mac_device_info *hw, u64 *ts)
>  	return 0;
>  }
>  
> +void dwxgmac2_timestamp_interrupt(struct stmmac_priv *priv)
> +{
> +	u32 ts_status, pending_snapshots, acr_value, channel;
> +	struct ptp_clock_event event;
> +	unsigned long flags;
> +	u64 ptp_time;
> +	int i;
> +
> +	/* Read XGMAC_TIMESTAMP_STATUS to get the AUX snapshot
> +	 * count.  This read also clears the TSIS bit in
> +	 * XGMAC_INT_STATUS.
> +	 * TX timestamp polling may have already cleared TSIS
> +	 * and AUXTSTRIG, so rely on ATSNS instead.
> +	 * TXTSC is cleared by XGMAC_TXTIMESTAMP_SEC, not by
> +	 * this register, so there is no conflict.
> +	 */
> +	ts_status = readl(priv->ioaddr + XGMAC_TIMESTAMP_STATUS);
> +
> +	if (!(priv->plat->flags & STMMAC_FLAG_EXT_SNAPSHOT_EN))
> +		return;
> +
> +	pending_snapshots = FIELD_GET(XGMAC_TIMESTAMP_ATSNS_MASK, ts_status);
> +	if (!pending_snapshots)
> +		return;
> +
> +	acr_value = readl(priv->ptpaddr + PTP_ACR);
> +	channel = FIELD_GET(PTP_ACR_MASK, acr_value);
> +	if (!channel)
> +		return;
> +	channel = ilog2(channel);
> +
> +	for (i = 0; i < pending_snapshots; i++) {
> +		read_lock_irqsave(&priv->ptp_lock, flags);
> +		stmmac_get_ptptime(priv, priv->ptpaddr, &ptp_time);
> +		read_unlock_irqrestore(&priv->ptp_lock, flags);
> +
> +		event.type = PTP_CLOCK_EXTTS;
> +		event.index = channel;
> +		event.timestamp = ptp_time;
> +		ptp_clock_event(priv->ptp_clock, &event);
> +	}
> +}
> +
>  static int dwxgmac2_flex_pps_config(void __iomem *ioaddr, int index,
>  				    struct stmmac_pps_cfg *cfg, bool enable,
>  				    u32 sub_second_inc, u32 systime_flags)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> index 511b0fd5e834..9718582b8480 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> @@ -258,7 +258,7 @@ static const struct stmmac_hwif_entry {
>  		.dma = &dwxgmac210_dma_ops,
>  		.mac = &dwxgmac210_ops,
>  		.vlan = &dwxgmac210_vlan_ops,
> -		.hwtimestamp = &stmmac_ptp,
> +		.hwtimestamp = &dwxgmac2_ptp,
>  		.ptp = &stmmac_ptp_clock_ops,
>  		.mode = NULL,
>  		.tc = &dwmac510_tc_ops,
> @@ -280,7 +280,7 @@ static const struct stmmac_hwif_entry {
>  		.dma = &dwxgmac210_dma_ops,
>  		.mac = &dwxlgmac2_ops,
>  		.vlan = &dwxlgmac2_vlan_ops,
> -		.hwtimestamp = &stmmac_ptp,
> +		.hwtimestamp = &dwxgmac2_ptp,
>  		.ptp = &stmmac_ptp_clock_ops,
>  		.mode = NULL,
>  		.tc = &dwmac510_tc_ops,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index e6317b94fff7..818ab3daa91c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -671,6 +671,7 @@ extern const struct stmmac_desc_ops ndesc_ops;
>  
>  extern const struct stmmac_hwtimestamp stmmac_ptp;
>  extern const struct stmmac_hwtimestamp dwmac1000_ptp;
> +extern const struct stmmac_hwtimestamp dwxgmac2_ptp;
>  
>  extern const struct stmmac_mode_ops ring_mode_ops;
>  extern const struct stmmac_mode_ops chain_mode_ops;
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> index b9a985fa772c..9d7d24259abd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
> @@ -277,3 +277,15 @@ const struct stmmac_hwtimestamp dwmac1000_ptp = {
>  	.get_ptptime = dwmac1000_get_ptptime,
>  	.timestamp_interrupt = dwmac1000_timestamp_interrupt,
>  };
> +
> +const struct stmmac_hwtimestamp dwxgmac2_ptp = {
> +	.config_hw_tstamping = config_hw_tstamping,
> +	.init_systime = init_systime,
> +	.config_sub_second_increment = config_sub_second_increment,
> +	.config_addend = config_addend,
> +	.adjust_systime = adjust_systime,
> +	.get_systime = get_systime,
> +	.get_ptptime = get_ptptime,
> +	.timestamp_interrupt = dwxgmac2_timestamp_interrupt,
> +	.hwtstamp_correct_latency = hwtstamp_correct_latency,
> +};
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
> index 3fe0e3a80e80..dade09614163 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
> @@ -103,6 +103,7 @@ int dwmac1000_ptp_enable(struct ptp_clock_info *ptp,
>  
>  void dwmac1000_get_ptptime(void __iomem *ptpaddr, u64 *ptp_time);
>  void dwmac1000_timestamp_interrupt(struct stmmac_priv *priv);
> +void dwxgmac2_timestamp_interrupt(struct stmmac_priv *priv);
>  
>  extern const struct ptp_clock_info stmmac_ptp_clock_ops;
>  extern const struct ptp_clock_info dwmac1000_ptp_clock_ops;

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

[Zxyan Zhu]

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.

So I'd like to keep this on net-next as a new feature rather than
retarget it to net.

Thanks,
Zxyan

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

[Maxime Chevallier]

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

I don't have any xgmac boards to verify sashiko's claim though.

> 
> Thanks,
> Zxyan
> 

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

[zhu xin]

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.

Thanks,
Zxyan

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

[Nazle Asmade, Muhammad Nazim Amirul]

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