* [PATCH] net: stmmac: program the FSM timeout timer
@ 2026-11-11 14:33 Zxyan Zhu
2026-11-11 14:37 ` netdev-bot+sinfo
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Zxyan Zhu @ 2026-11-11 14:33 UTC (permalink / raw)
To: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni
Cc: netdev, mcoquelin.stm32, alexandre.torgue, linux-stm32,
linux-arm-kernel, linux-kernel, rmk+kernel, Jose.Abreu, Zxyan Zhu
stmmac enables the FSM timeout checker (TMOUTEN) but never programs the
MAC_FSM_ACT_Timer register. Per the databook the checker only fires once
NTMRMD is programmed with a non-zero duration; NTMRMD == 0 disables the
timer, so with TMOUTEN set but the timer left at reset a stuck FSM is
never reported and the advertised timeout protection is ineffective.
Program MAC_FSM_ACT_Timer right after TMOUTEN is enabled, deriving the
TMR field (CSR clocks per 1us tic) from the CSR clock rate and defaulting
to a 1s timeout. A platform can override the default through the new
ntmrmd field in stmmac_safety_feature_cfg.
The issue was hit in testing on a downstream platform, where a stuck FSM
was not reported through the safety interrupt despite TMOUTEN being set.
Fixes: 8bf993a5877e ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core")
Signed-off-by: Zxyan Zhu <zxyan0222@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 20 ++++++++++++++++++-
drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 6 +++++-
.../net/ethernet/stmicro/stmmac/dwxgmac2.h | 3 +++
.../ethernet/stmicro/stmmac/dwxgmac2_core.c | 20 ++++++++++++++++++-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 3 ++-
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++++-
include/linux/stmmac.h | 17 ++++++++++++++++
7 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
index f1bb981cab7c..4ab7a3df98c6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
@@ -2,6 +2,7 @@
// Copyright (c) 2017 Synopsys, Inc. and/or its affiliates.
// stmmac Support for 5.xx Ethernet QoS cores
+#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/iopoll.h>
#include "common.h"
@@ -184,7 +185,8 @@ static void dwmac5_handle_dma_err(struct net_device *ndev,
}
int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
- struct stmmac_safety_feature_cfg *safety_feat_cfg)
+ struct stmmac_safety_feature_cfg *safety_feat_cfg,
+ unsigned int csr_clk_mhz)
{
struct stmmac_safety_feature_cfg all_safety_feats = {
.tsoee = 1,
@@ -245,6 +247,22 @@ int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
value |= TMOUTEN; /* FSM Timeout Feature */
writel(value, ioaddr + MAC_FSM_CONTROL);
+ /* Program the FSM active timer so the timeout checker actually fires.
+ * TMR is the number of CSR clocks per 1us tic, i.e. csr_clk in MHz.
+ * Default to a 1s timeout when the platform did not select one.
+ */
+ if (safety_feat_cfg->tmouten && csr_clk_mhz) {
+ u32 ntmrmd = safety_feat_cfg->ntmrmd;
+
+ ntmrmd = (ntmrmd >= STMMAC_NTMRMD_1US &&
+ ntmrmd <= STMMAC_NTMRMD_64SEC) ?
+ ntmrmd : STMMAC_NTMRMD_1SEC;
+
+ writel(FIELD_PREP(MAC_FSM_ACT_TIMER_NTMRMD, ntmrmd) |
+ FIELD_PREP(MAC_FSM_ACT_TIMER_TMR, csr_clk_mhz),
+ ioaddr + MAC_FSM_ACT_TIMER);
+ }
+
/* 4. Enable Data Parity Protection */
value = readl(ioaddr + MTL_DPP_CONTROL);
if (safety_feat_cfg->edpp)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
index 00b151b3b688..734e7630f28d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
@@ -8,8 +8,11 @@
#define MAC_DPP_FSM_INT_STATUS 0x00000140
#define MAC_AXI_SLV_DPE_ADDR_STATUS 0x00000144
#define MAC_FSM_CONTROL 0x00000148
+#define MAC_FSM_ACT_TIMER 0x0000014C
#define PRTYEN BIT(1)
#define TMOUTEN BIT(0)
+#define MAC_FSM_ACT_TIMER_TMR GENMASK(9, 0)
+#define MAC_FSM_ACT_TIMER_NTMRMD GENMASK(19, 16)
#define MAC_PPS_CONTROL 0x00000b70
#define PPS_MAXIDX(x) ((((x) + 1) * 8) - 1)
@@ -82,7 +85,8 @@
#define GMAC_INT_FPE_EN BIT(17)
int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
- struct stmmac_safety_feature_cfg *safety_cfg);
+ struct stmmac_safety_feature_cfg *safety_cfg,
+ unsigned int csr_clk_mhz);
int dwmac5_safety_feat_irq_status(struct net_device *ndev,
void __iomem *ioaddr, unsigned int asp,
struct stmmac_safety_stats *stats);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index f8ab347f7b5b..1fd501728b18 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -169,8 +169,11 @@
#define XGMAC_HWFEAT_PCSEL GENMASK(1, 0)
#define XGMAC_MAC_DPP_FSM_INT_STATUS 0x00000150
#define XGMAC_MAC_FSM_CONTROL 0x00000158
+#define XGMAC_MAC_FSM_ACT_TIMER 0x0000015C
#define XGMAC_PRTYEN BIT(1)
#define XGMAC_TMOUTEN BIT(0)
+#define XGMAC_MAC_FSM_ACT_TIMER_TMR GENMASK(9, 0)
+#define XGMAC_MAC_FSM_ACT_TIMER_NTMRMD GENMASK(19, 16)
#define XGMAC_MDIO_ADDR 0x00000200
#define XGMAC_MDIO_DATA 0x00000204
#define XGMAC_MDIO_C22P 0x00000220
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index 1a88cbaed70c..76c850ea13a5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -4,6 +4,7 @@
* stmmac XGMAC support.
*/
+#include <linux/bitfield.h>
#include <linux/bitrev.h>
#include <linux/crc32.h>
#include <linux/iopoll.h>
@@ -835,7 +836,8 @@ static void dwxgmac3_handle_dma_err(struct net_device *ndev,
static int
dwxgmac3_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
- struct stmmac_safety_feature_cfg *safety_cfg)
+ struct stmmac_safety_feature_cfg *safety_cfg,
+ unsigned int csr_clk_mhz)
{
u32 value;
@@ -871,6 +873,22 @@ dwxgmac3_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
value |= XGMAC_TMOUTEN; /* FSM Timeout Feature */
writel(value, ioaddr + XGMAC_MAC_FSM_CONTROL);
+ /* Program the FSM active timer so the timeout checker actually fires.
+ * TMR is the number of CSR clocks per 1us tic, i.e. csr_clk in MHz.
+ * Default to a 1s timeout when the platform did not select one.
+ */
+ if (safety_cfg && safety_cfg->tmouten && csr_clk_mhz) {
+ u32 ntmrmd = safety_cfg->ntmrmd;
+
+ ntmrmd = (ntmrmd >= STMMAC_NTMRMD_1US &&
+ ntmrmd <= STMMAC_NTMRMD_64SEC) ?
+ ntmrmd : STMMAC_NTMRMD_1SEC;
+
+ writel(FIELD_PREP(XGMAC_MAC_FSM_ACT_TIMER_NTMRMD, ntmrmd) |
+ FIELD_PREP(XGMAC_MAC_FSM_ACT_TIMER_TMR, csr_clk_mhz),
+ ioaddr + XGMAC_MAC_FSM_ACT_TIMER);
+ }
+
/* 5. Enable Data Path Parity Protection */
value = readl(ioaddr + XGMAC_MTL_DPP_CONTROL);
/* already enabled by default, explicit enable it again */
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 8ae95e91a36a..395655b01ea8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -378,7 +378,8 @@ struct stmmac_ops {
bool srgmi_ral);
/* Safety Features */
int (*safety_feat_config)(void __iomem *ioaddr, unsigned int asp,
- struct stmmac_safety_feature_cfg *safety_cfg);
+ struct stmmac_safety_feature_cfg *safety_cfg,
+ unsigned int csr_clk_mhz);
int (*safety_feat_irq_status)(struct net_device *ndev,
void __iomem *ioaddr, unsigned int asp,
struct stmmac_safety_stats *stats);
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f8dceafdda2d..52f63f624d67 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3639,10 +3639,16 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)
{
+ unsigned int csr_clk_mhz = 0;
+
if (priv->dma_cap.asp) {
netdev_info(priv->dev, "Enabling Safety Features\n");
+ if (priv->plat->stmmac_clk)
+ csr_clk_mhz = clk_get_rate(priv->plat->stmmac_clk) /
+ 1000000;
stmmac_safety_feat_config(priv, priv->ioaddr, priv->dma_cap.asp,
- priv->plat->safety_feat_cfg);
+ priv->plat->safety_feat_cfg,
+ csr_clk_mhz);
} else {
netdev_info(priv->dev, "No Safety Features support found\n");
}
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 00be2df63d22..cb470a153bf8 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -168,8 +168,25 @@ struct stmmac_safety_feature_cfg {
u32 edpp;
u32 prtyen;
u32 tmouten;
+ /* FSM normal-mode timeout value (MAC_FSM_ACT_Timer.NTMRMD). The TMR
+ * field (CSR clocks per 1us) is derived from the CSR clock by stmmac.
+ * 0 selects the default 1s timeout.
+ */
+ u32 ntmrmd;
};
+/* MAC_FSM_ACT_Timer.NTMRMD timeout durations */
+#define STMMAC_NTMRMD_1US 0x1
+#define STMMAC_NTMRMD_4MS 0x2
+#define STMMAC_NTMRMD_16MS 0x3
+#define STMMAC_NTMRMD_64MS 0x4
+#define STMMAC_NTMRMD_256MS 0x5
+#define STMMAC_NTMRMD_1SEC 0x6
+#define STMMAC_NTMRMD_4SEC 0x7
+#define STMMAC_NTMRMD_16SEC 0x8
+#define STMMAC_NTMRMD_32SEC 0x9
+#define STMMAC_NTMRMD_64SEC 0xa
+
/* Addresses that may be customized by a platform */
struct dwmac4_addrs {
u32 dma_chan;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] net: stmmac: program the FSM timeout timer
2026-11-11 14:33 [PATCH] net: stmmac: program the FSM timeout timer Zxyan Zhu
@ 2026-11-11 14:37 ` netdev-bot+sinfo
2026-11-11 16:19 ` Maxime Chevallier
2026-11-15 14:52 ` netdev-bot+sashiko
2 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sinfo @ 2026-11-11 14:37 UTC (permalink / raw)
To: Zxyan Zhu
Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
netdev, mcoquelin.stm32, alexandre.torgue, linux-stm32,
linux-arm-kernel, linux-kernel, rmk+kernel, Jose.Abreu
Hi!
This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:
- What hardware the change was tested on. For driver fixes please
mention the device (and if relevant firmware version) used for
testing, or say that the change was not tested on real hardware.
Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: stmmac: program the FSM timeout timer
2026-11-11 14:33 [PATCH] net: stmmac: program the FSM timeout timer Zxyan Zhu
2026-11-11 14:37 ` netdev-bot+sinfo
@ 2026-11-11 16:19 ` Maxime Chevallier
2026-11-12 2:05 ` zhu xin
2026-11-15 14:52 ` netdev-bot+sashiko
2 siblings, 1 reply; 5+ messages in thread
From: Maxime Chevallier @ 2026-11-11 16:19 UTC (permalink / raw)
To: Zxyan Zhu, andrew+netdev, davem, edumazet, kuba, pabeni
Cc: netdev, mcoquelin.stm32, alexandre.torgue, linux-stm32,
linux-arm-kernel, linux-kernel, rmk+kernel, Jose.Abreu
Hi,
On 11/11/26 15:33, Zxyan Zhu wrote:
> stmmac enables the FSM timeout checker (TMOUTEN) but never programs the
> MAC_FSM_ACT_Timer register. Per the databook the checker only fires once
> NTMRMD is programmed with a non-zero duration; NTMRMD == 0 disables the
> timer, so with TMOUTEN set but the timer left at reset a stuck FSM is
> never reported and the advertised timeout protection is ineffective.
>
> Program MAC_FSM_ACT_Timer right after TMOUTEN is enabled, deriving the
> TMR field (CSR clocks per 1us tic) from the CSR clock rate and defaulting
> to a 1s timeout. A platform can override the default through the new
> ntmrmd field in stmmac_safety_feature_cfg.
>
> The issue was hit in testing on a downstream platform, where a stuck FSM
> was not reported through the safety interrupt despite TMOUTEN being set.
>
> Fixes: 8bf993a5877e ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
> Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core")
> Signed-off-by: Zxyan Zhu <zxyan0222@gmail.com>
IMO this should go to net-next, this seems to just never have worked.
Out of curiosity, did you test on dwmac4 or on dwxgmac ?
Maxime
> ---
> drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 20 ++++++++++++++++++-
> drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 6 +++++-
> .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 3 +++
> .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 20 ++++++++++++++++++-
> drivers/net/ethernet/stmicro/stmmac/hwif.h | 3 ++-
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++++-
> include/linux/stmmac.h | 17 ++++++++++++++++
> 7 files changed, 72 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
> index f1bb981cab7c..4ab7a3df98c6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
> @@ -2,6 +2,7 @@
> // Copyright (c) 2017 Synopsys, Inc. and/or its affiliates.
> // stmmac Support for 5.xx Ethernet QoS cores
>
> +#include <linux/bitfield.h>
> #include <linux/bitops.h>
> #include <linux/iopoll.h>
> #include "common.h"
> @@ -184,7 +185,8 @@ static void dwmac5_handle_dma_err(struct net_device *ndev,
> }
>
> int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> - struct stmmac_safety_feature_cfg *safety_feat_cfg)
> + struct stmmac_safety_feature_cfg *safety_feat_cfg,
> + unsigned int csr_clk_mhz)
> {
> struct stmmac_safety_feature_cfg all_safety_feats = {
> .tsoee = 1,
> @@ -245,6 +247,22 @@ int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> value |= TMOUTEN; /* FSM Timeout Feature */
> writel(value, ioaddr + MAC_FSM_CONTROL);
>
> + /* Program the FSM active timer so the timeout checker actually fires.
> + * TMR is the number of CSR clocks per 1us tic, i.e. csr_clk in MHz.
> + * Default to a 1s timeout when the platform did not select one.
> + */
> + if (safety_feat_cfg->tmouten && csr_clk_mhz) {
> + u32 ntmrmd = safety_feat_cfg->ntmrmd;
> +
> + ntmrmd = (ntmrmd >= STMMAC_NTMRMD_1US &&
> + ntmrmd <= STMMAC_NTMRMD_64SEC) ?
> + ntmrmd : STMMAC_NTMRMD_1SEC;
> +
> + writel(FIELD_PREP(MAC_FSM_ACT_TIMER_NTMRMD, ntmrmd) |
> + FIELD_PREP(MAC_FSM_ACT_TIMER_TMR, csr_clk_mhz),
> + ioaddr + MAC_FSM_ACT_TIMER);
> + }
> +
> /* 4. Enable Data Parity Protection */
> value = readl(ioaddr + MTL_DPP_CONTROL);
> if (safety_feat_cfg->edpp)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
> index 00b151b3b688..734e7630f28d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
> @@ -8,8 +8,11 @@
> #define MAC_DPP_FSM_INT_STATUS 0x00000140
> #define MAC_AXI_SLV_DPE_ADDR_STATUS 0x00000144
> #define MAC_FSM_CONTROL 0x00000148
> +#define MAC_FSM_ACT_TIMER 0x0000014C
> #define PRTYEN BIT(1)
> #define TMOUTEN BIT(0)
> +#define MAC_FSM_ACT_TIMER_TMR GENMASK(9, 0)
> +#define MAC_FSM_ACT_TIMER_NTMRMD GENMASK(19, 16)
>
> #define MAC_PPS_CONTROL 0x00000b70
> #define PPS_MAXIDX(x) ((((x) + 1) * 8) - 1)
> @@ -82,7 +85,8 @@
> #define GMAC_INT_FPE_EN BIT(17)
>
> int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> - struct stmmac_safety_feature_cfg *safety_cfg);
> + struct stmmac_safety_feature_cfg *safety_cfg,
> + unsigned int csr_clk_mhz);
> int dwmac5_safety_feat_irq_status(struct net_device *ndev,
> void __iomem *ioaddr, unsigned int asp,
> struct stmmac_safety_stats *stats);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> index f8ab347f7b5b..1fd501728b18 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> @@ -169,8 +169,11 @@
> #define XGMAC_HWFEAT_PCSEL GENMASK(1, 0)
> #define XGMAC_MAC_DPP_FSM_INT_STATUS 0x00000150
> #define XGMAC_MAC_FSM_CONTROL 0x00000158
> +#define XGMAC_MAC_FSM_ACT_TIMER 0x0000015C
> #define XGMAC_PRTYEN BIT(1)
> #define XGMAC_TMOUTEN BIT(0)
> +#define XGMAC_MAC_FSM_ACT_TIMER_TMR GENMASK(9, 0)
> +#define XGMAC_MAC_FSM_ACT_TIMER_NTMRMD GENMASK(19, 16)
> #define XGMAC_MDIO_ADDR 0x00000200
> #define XGMAC_MDIO_DATA 0x00000204
> #define XGMAC_MDIO_C22P 0x00000220
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> index 1a88cbaed70c..76c850ea13a5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> @@ -4,6 +4,7 @@
> * stmmac XGMAC support.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/bitrev.h>
> #include <linux/crc32.h>
> #include <linux/iopoll.h>
> @@ -835,7 +836,8 @@ static void dwxgmac3_handle_dma_err(struct net_device *ndev,
>
> static int
> dwxgmac3_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> - struct stmmac_safety_feature_cfg *safety_cfg)
> + struct stmmac_safety_feature_cfg *safety_cfg,
> + unsigned int csr_clk_mhz)
> {
> u32 value;
>
> @@ -871,6 +873,22 @@ dwxgmac3_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> value |= XGMAC_TMOUTEN; /* FSM Timeout Feature */
> writel(value, ioaddr + XGMAC_MAC_FSM_CONTROL);
>
> + /* Program the FSM active timer so the timeout checker actually fires.
> + * TMR is the number of CSR clocks per 1us tic, i.e. csr_clk in MHz.
> + * Default to a 1s timeout when the platform did not select one.
> + */
> + if (safety_cfg && safety_cfg->tmouten && csr_clk_mhz) {
> + u32 ntmrmd = safety_cfg->ntmrmd;
> +
> + ntmrmd = (ntmrmd >= STMMAC_NTMRMD_1US &&
> + ntmrmd <= STMMAC_NTMRMD_64SEC) ?
> + ntmrmd : STMMAC_NTMRMD_1SEC;
> +
> + writel(FIELD_PREP(XGMAC_MAC_FSM_ACT_TIMER_NTMRMD, ntmrmd) |
> + FIELD_PREP(XGMAC_MAC_FSM_ACT_TIMER_TMR, csr_clk_mhz),
> + ioaddr + XGMAC_MAC_FSM_ACT_TIMER);
> + }
> +
> /* 5. Enable Data Path Parity Protection */
> value = readl(ioaddr + XGMAC_MTL_DPP_CONTROL);
> /* already enabled by default, explicit enable it again */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> index 8ae95e91a36a..395655b01ea8 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> @@ -378,7 +378,8 @@ struct stmmac_ops {
> bool srgmi_ral);
> /* Safety Features */
> int (*safety_feat_config)(void __iomem *ioaddr, unsigned int asp,
> - struct stmmac_safety_feature_cfg *safety_cfg);
> + struct stmmac_safety_feature_cfg *safety_cfg,
> + unsigned int csr_clk_mhz);
> int (*safety_feat_irq_status)(struct net_device *ndev,
> void __iomem *ioaddr, unsigned int asp,
> struct stmmac_safety_stats *stats);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index f8dceafdda2d..52f63f624d67 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3639,10 +3639,16 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
>
> static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)
> {
> + unsigned int csr_clk_mhz = 0;
> +
> if (priv->dma_cap.asp) {
> netdev_info(priv->dev, "Enabling Safety Features\n");
> + if (priv->plat->stmmac_clk)
> + csr_clk_mhz = clk_get_rate(priv->plat->stmmac_clk) /
> + 1000000;
> stmmac_safety_feat_config(priv, priv->ioaddr, priv->dma_cap.asp,
> - priv->plat->safety_feat_cfg);
> + priv->plat->safety_feat_cfg,
> + csr_clk_mhz);
> } else {
> netdev_info(priv->dev, "No Safety Features support found\n");
> }
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index 00be2df63d22..cb470a153bf8 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -168,8 +168,25 @@ struct stmmac_safety_feature_cfg {
> u32 edpp;
> u32 prtyen;
> u32 tmouten;
> + /* FSM normal-mode timeout value (MAC_FSM_ACT_Timer.NTMRMD). The TMR
> + * field (CSR clocks per 1us) is derived from the CSR clock by stmmac.
> + * 0 selects the default 1s timeout.
> + */
> + u32 ntmrmd;
> };
>
> +/* MAC_FSM_ACT_Timer.NTMRMD timeout durations */
> +#define STMMAC_NTMRMD_1US 0x1
> +#define STMMAC_NTMRMD_4MS 0x2
> +#define STMMAC_NTMRMD_16MS 0x3
> +#define STMMAC_NTMRMD_64MS 0x4
> +#define STMMAC_NTMRMD_256MS 0x5
> +#define STMMAC_NTMRMD_1SEC 0x6
> +#define STMMAC_NTMRMD_4SEC 0x7
> +#define STMMAC_NTMRMD_16SEC 0x8
> +#define STMMAC_NTMRMD_32SEC 0x9
> +#define STMMAC_NTMRMD_64SEC 0xa
> +
> /* Addresses that may be customized by a platform */
> struct dwmac4_addrs {
> u32 dma_chan;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: stmmac: program the FSM timeout timer
2026-11-11 16:19 ` Maxime Chevallier
@ 2026-11-12 2:05 ` zhu xin
0 siblings, 0 replies; 5+ messages in thread
From: zhu xin @ 2026-11-12 2:05 UTC (permalink / raw)
To: Maxime Chevallier
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
mcoquelin.stm32, alexandre.torgue, linux-stm32, linux-arm-kernel,
linux-kernel, rmk+kernel, Jose.Abreu
Hi Maxime,
On Thu, Nov 12, 2026 at 12:19 AM Maxime Chevallier
<maxime.chevallier@bootlin.com> wrote:
>
> Hi,
>
> On 11/11/26 15:33, Zxyan Zhu wrote:
> > stmmac enables the FSM timeout checker (TMOUTEN) but never programs the
> > MAC_FSM_ACT_Timer register. Per the databook the checker only fires once
> > NTMRMD is programmed with a non-zero duration; NTMRMD == 0 disables the
> > timer, so with TMOUTEN set but the timer left at reset a stuck FSM is
> > never reported and the advertised timeout protection is ineffective.
> >
> > Program MAC_FSM_ACT_Timer right after TMOUTEN is enabled, deriving the
> > TMR field (CSR clocks per 1us tic) from the CSR clock rate and defaulting
> > to a 1s timeout. A platform can override the default through the new
> > ntmrmd field in stmmac_safety_feature_cfg.
> >
> > The issue was hit in testing on a downstream platform, where a stuck FSM
> > was not reported through the safety interrupt despite TMOUTEN being set.
> >
> > Fixes: 8bf993a5877e ("net: stmmac: Add support for DWMAC5 and implement Safety Features")
> > Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core")
> > Signed-off-by: Zxyan Zhu <zxyan0222@gmail.com>
>
> IMO this should go to net-next, this seems to just never have worked.
OK, I'll resend this as a net-next patch.
>
> Out of curiosity, did you test on dwmac4 or on dwxgmac ?
Both. The timer programming was tested on dwmac4 and on dwxgmac.
>
> Maxime
>
> > ---
> > drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 20 ++++++++++++++++++-
> > drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 6 +++++-
> > .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 3 +++
> > .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 20 ++++++++++++++++++-
> > drivers/net/ethernet/stmicro/stmmac/hwif.h | 3 ++-
> > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++++-
> > include/linux/stmmac.h | 17 ++++++++++++++++
> > 7 files changed, 72 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
> > index f1bb981cab7c..4ab7a3df98c6 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
> > @@ -2,6 +2,7 @@
> > // Copyright (c) 2017 Synopsys, Inc. and/or its affiliates.
> > // stmmac Support for 5.xx Ethernet QoS cores
> >
> > +#include <linux/bitfield.h>
> > #include <linux/bitops.h>
> > #include <linux/iopoll.h>
> > #include "common.h"
> > @@ -184,7 +185,8 @@ static void dwmac5_handle_dma_err(struct net_device *ndev,
> > }
> >
> > int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> > - struct stmmac_safety_feature_cfg *safety_feat_cfg)
> > + struct stmmac_safety_feature_cfg *safety_feat_cfg,
> > + unsigned int csr_clk_mhz)
> > {
> > struct stmmac_safety_feature_cfg all_safety_feats = {
> > .tsoee = 1,
> > @@ -245,6 +247,22 @@ int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> > value |= TMOUTEN; /* FSM Timeout Feature */
> > writel(value, ioaddr + MAC_FSM_CONTROL);
> >
> > + /* Program the FSM active timer so the timeout checker actually fires.
> > + * TMR is the number of CSR clocks per 1us tic, i.e. csr_clk in MHz.
> > + * Default to a 1s timeout when the platform did not select one.
> > + */
> > + if (safety_feat_cfg->tmouten && csr_clk_mhz) {
> > + u32 ntmrmd = safety_feat_cfg->ntmrmd;
> > +
> > + ntmrmd = (ntmrmd >= STMMAC_NTMRMD_1US &&
> > + ntmrmd <= STMMAC_NTMRMD_64SEC) ?
> > + ntmrmd : STMMAC_NTMRMD_1SEC;
> > +
> > + writel(FIELD_PREP(MAC_FSM_ACT_TIMER_NTMRMD, ntmrmd) |
> > + FIELD_PREP(MAC_FSM_ACT_TIMER_TMR, csr_clk_mhz),
> > + ioaddr + MAC_FSM_ACT_TIMER);
> > + }
> > +
> > /* 4. Enable Data Parity Protection */
> > value = readl(ioaddr + MTL_DPP_CONTROL);
> > if (safety_feat_cfg->edpp)
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
> > index 00b151b3b688..734e7630f28d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h
> > @@ -8,8 +8,11 @@
> > #define MAC_DPP_FSM_INT_STATUS 0x00000140
> > #define MAC_AXI_SLV_DPE_ADDR_STATUS 0x00000144
> > #define MAC_FSM_CONTROL 0x00000148
> > +#define MAC_FSM_ACT_TIMER 0x0000014C
> > #define PRTYEN BIT(1)
> > #define TMOUTEN BIT(0)
> > +#define MAC_FSM_ACT_TIMER_TMR GENMASK(9, 0)
> > +#define MAC_FSM_ACT_TIMER_NTMRMD GENMASK(19, 16)
> >
> > #define MAC_PPS_CONTROL 0x00000b70
> > #define PPS_MAXIDX(x) ((((x) + 1) * 8) - 1)
> > @@ -82,7 +85,8 @@
> > #define GMAC_INT_FPE_EN BIT(17)
> >
> > int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> > - struct stmmac_safety_feature_cfg *safety_cfg);
> > + struct stmmac_safety_feature_cfg *safety_cfg,
> > + unsigned int csr_clk_mhz);
> > int dwmac5_safety_feat_irq_status(struct net_device *ndev,
> > void __iomem *ioaddr, unsigned int asp,
> > struct stmmac_safety_stats *stats);
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> > index f8ab347f7b5b..1fd501728b18 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
> > @@ -169,8 +169,11 @@
> > #define XGMAC_HWFEAT_PCSEL GENMASK(1, 0)
> > #define XGMAC_MAC_DPP_FSM_INT_STATUS 0x00000150
> > #define XGMAC_MAC_FSM_CONTROL 0x00000158
> > +#define XGMAC_MAC_FSM_ACT_TIMER 0x0000015C
> > #define XGMAC_PRTYEN BIT(1)
> > #define XGMAC_TMOUTEN BIT(0)
> > +#define XGMAC_MAC_FSM_ACT_TIMER_TMR GENMASK(9, 0)
> > +#define XGMAC_MAC_FSM_ACT_TIMER_NTMRMD GENMASK(19, 16)
> > #define XGMAC_MDIO_ADDR 0x00000200
> > #define XGMAC_MDIO_DATA 0x00000204
> > #define XGMAC_MDIO_C22P 0x00000220
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> > index 1a88cbaed70c..76c850ea13a5 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> > @@ -4,6 +4,7 @@
> > * stmmac XGMAC support.
> > */
> >
> > +#include <linux/bitfield.h>
> > #include <linux/bitrev.h>
> > #include <linux/crc32.h>
> > #include <linux/iopoll.h>
> > @@ -835,7 +836,8 @@ static void dwxgmac3_handle_dma_err(struct net_device *ndev,
> >
> > static int
> > dwxgmac3_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> > - struct stmmac_safety_feature_cfg *safety_cfg)
> > + struct stmmac_safety_feature_cfg *safety_cfg,
> > + unsigned int csr_clk_mhz)
> > {
> > u32 value;
> >
> > @@ -871,6 +873,22 @@ dwxgmac3_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> > value |= XGMAC_TMOUTEN; /* FSM Timeout Feature */
> > writel(value, ioaddr + XGMAC_MAC_FSM_CONTROL);
> >
> > + /* Program the FSM active timer so the timeout checker actually fires.
> > + * TMR is the number of CSR clocks per 1us tic, i.e. csr_clk in MHz.
> > + * Default to a 1s timeout when the platform did not select one.
> > + */
> > + if (safety_cfg && safety_cfg->tmouten && csr_clk_mhz) {
> > + u32 ntmrmd = safety_cfg->ntmrmd;
> > +
> > + ntmrmd = (ntmrmd >= STMMAC_NTMRMD_1US &&
> > + ntmrmd <= STMMAC_NTMRMD_64SEC) ?
> > + ntmrmd : STMMAC_NTMRMD_1SEC;
> > +
> > + writel(FIELD_PREP(XGMAC_MAC_FSM_ACT_TIMER_NTMRMD, ntmrmd) |
> > + FIELD_PREP(XGMAC_MAC_FSM_ACT_TIMER_TMR, csr_clk_mhz),
> > + ioaddr + XGMAC_MAC_FSM_ACT_TIMER);
> > + }
> > +
> > /* 5. Enable Data Path Parity Protection */
> > value = readl(ioaddr + XGMAC_MTL_DPP_CONTROL);
> > /* already enabled by default, explicit enable it again */
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > index 8ae95e91a36a..395655b01ea8 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
> > @@ -378,7 +378,8 @@ struct stmmac_ops {
> > bool srgmi_ral);
> > /* Safety Features */
> > int (*safety_feat_config)(void __iomem *ioaddr, unsigned int asp,
> > - struct stmmac_safety_feature_cfg *safety_cfg);
> > + struct stmmac_safety_feature_cfg *safety_cfg,
> > + unsigned int csr_clk_mhz);
> > int (*safety_feat_irq_status)(struct net_device *ndev,
> > void __iomem *ioaddr, unsigned int asp,
> > struct stmmac_safety_stats *stats);
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index f8dceafdda2d..52f63f624d67 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -3639,10 +3639,16 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
> >
> > static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)
> > {
> > + unsigned int csr_clk_mhz = 0;
> > +
> > if (priv->dma_cap.asp) {
> > netdev_info(priv->dev, "Enabling Safety Features\n");
> > + if (priv->plat->stmmac_clk)
> > + csr_clk_mhz = clk_get_rate(priv->plat->stmmac_clk) /
> > + 1000000;
> > stmmac_safety_feat_config(priv, priv->ioaddr, priv->dma_cap.asp,
> > - priv->plat->safety_feat_cfg);
> > + priv->plat->safety_feat_cfg,
> > + csr_clk_mhz);
> > } else {
> > netdev_info(priv->dev, "No Safety Features support found\n");
> > }
> > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> > index 00be2df63d22..cb470a153bf8 100644
> > --- a/include/linux/stmmac.h
> > +++ b/include/linux/stmmac.h
> > @@ -168,8 +168,25 @@ struct stmmac_safety_feature_cfg {
> > u32 edpp;
> > u32 prtyen;
> > u32 tmouten;
> > + /* FSM normal-mode timeout value (MAC_FSM_ACT_Timer.NTMRMD). The TMR
> > + * field (CSR clocks per 1us) is derived from the CSR clock by stmmac.
> > + * 0 selects the default 1s timeout.
> > + */
> > + u32 ntmrmd;
> > };
> >
> > +/* MAC_FSM_ACT_Timer.NTMRMD timeout durations */
> > +#define STMMAC_NTMRMD_1US 0x1
> > +#define STMMAC_NTMRMD_4MS 0x2
> > +#define STMMAC_NTMRMD_16MS 0x3
> > +#define STMMAC_NTMRMD_64MS 0x4
> > +#define STMMAC_NTMRMD_256MS 0x5
> > +#define STMMAC_NTMRMD_1SEC 0x6
> > +#define STMMAC_NTMRMD_4SEC 0x7
> > +#define STMMAC_NTMRMD_16SEC 0x8
> > +#define STMMAC_NTMRMD_32SEC 0x9
> > +#define STMMAC_NTMRMD_64SEC 0xa
> > +
> > /* Addresses that may be customized by a platform */
> > struct dwmac4_addrs {
> > u32 dma_chan;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net: stmmac: program the FSM timeout timer
2026-11-11 14:33 [PATCH] net: stmmac: program the FSM timeout timer Zxyan Zhu
2026-11-11 14:37 ` netdev-bot+sinfo
2026-11-11 16:19 ` Maxime Chevallier
@ 2026-11-15 14:52 ` netdev-bot+sashiko
2 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-11-15 14:52 UTC (permalink / raw)
To: zxyan0222
Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
netdev, mcoquelin.stm32, alexandre.torgue, linux-stm32,
linux-arm-kernel, linux-kernel, rmk+kernel, Jose.Abreu
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 2 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] On every in-tree XGMAC platform, the XGMAC half of this fix
never runs.
- [High] The new code takes the CSR clock rate only from
priv->plat->stmmac_clk (stmmac_safety_feat_configuration(),…
- [Medium] dwmac5_safety_feat_config() (dwmac5.c:261-263) and
dwxgmac3_safety_feat_config() (dwxgmac2_core.c, around lines 887-889)…
--
net: stmmac: program the FSM timeout timer
This commit programs MAC_FSM_ACT_Timer right after TMOUTEN is enabled, in
both dwmac5_safety_feat_config() and dwxgmac3_safety_feat_config(). TMR is
derived from the CSR clock rate and NTMRMD defaults to a 1s timeout. A new
ntmrmd field in stmmac_safety_feature_cfg lets a platform override that
default, and csr_clk_mhz is passed through the safety_feat_config callback.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
> index f1bb981cab7cd..4ab7a3df98c6d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c
[ ... ]
> @@ -245,6 +247,22 @@ int dwmac5_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> value |= TMOUTEN; /* FSM Timeout Feature */
> writel(value, ioaddr + MAC_FSM_CONTROL);
>
> + /* Program the FSM active timer so the timeout checker actually fires.
> + * TMR is the number of CSR clocks per 1us tic, i.e. csr_clk in MHz.
> + * Default to a 1s timeout when the platform did not select one.
> + */
> + if (safety_feat_cfg->tmouten && csr_clk_mhz) {
> + u32 ntmrmd = safety_feat_cfg->ntmrmd;
> +
> + ntmrmd = (ntmrmd >= STMMAC_NTMRMD_1US &&
> + ntmrmd <= STMMAC_NTMRMD_64SEC) ?
> + ntmrmd : STMMAC_NTMRMD_1SEC;
> +
> + writel(FIELD_PREP(MAC_FSM_ACT_TIMER_NTMRMD, ntmrmd) |
> + FIELD_PREP(MAC_FSM_ACT_TIMER_TMR, csr_clk_mhz),
> + ioaddr + MAC_FSM_ACT_TIMER);
[Severity: Medium]
This writel() replaces the whole MAC_FSM_ACT_Timer register with only the
NTMRMD and TMR fields set. Every other bit becomes zero. Is that intended?
In the Synopsys EQoS/XGMAC layout, this register also has a large-mode
timeout field (LTMRMD, bits 23:20) for FSMs that need longer timeouts. The
patch doesn't define or program it.
The commit message says a zero mode field disables the timer. If that also
applies to LTMRMD, would the large-mode FSMs stay unmonitored after this
write? And if reset or boot firmware had set LTMRMD, would this write turn
that monitoring off?
Should this be a read-modify-write, or should LTMRMD be programmed too? The
same whole-register write is in dwxgmac3_safety_feat_config().
> + }
> +
[ ... ]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> index 1a88cbaed70ca..76c850ea13a5c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
[ ... ]
> @@ -871,6 +873,22 @@ dwxgmac3_safety_feat_config(void __iomem *ioaddr, unsigned int asp,
> value |= XGMAC_TMOUTEN; /* FSM Timeout Feature */
> writel(value, ioaddr + XGMAC_MAC_FSM_CONTROL);
>
> + /* Program the FSM active timer so the timeout checker actually fires.
> + * TMR is the number of CSR clocks per 1us tic, i.e. csr_clk in MHz.
> + * Default to a 1s timeout when the platform did not select one.
> + */
> + if (safety_cfg && safety_cfg->tmouten && csr_clk_mhz) {
[Severity: High]
Can this block run on any in-tree XGMAC platform?
When safety_cfg is NULL, dwmac5_safety_feat_config() substitutes a default
config, but dwxgmac3_safety_feat_config() does not:
dwmac5_safety_feat_config() {
...
if (!safety_feat_cfg)
safety_feat_cfg = &all_safety_feats;
...
}
stmmac_safety_feat_configuration() passes priv->plat->safety_feat_cfg
through unchanged. The only in-tree platforms that allocate it appear to be
dwmac-intel.c, dwmac-mediatek.c and stmmac_pci.c, and all of them are
GMAC/GMAC4 cores.
The XGMAC users leave it NULL: dwmac-socfpga.c, dwmac-tegra.c and generic
"snps,dwxgmac" DT through stmmac_platform.c. Both dwxgmac210_ops and
dwxlgmac2_ops use this function.
On those platforms, XGMAC_TMOUTEN is still set unconditionally above, but
the timer write is skipped. The hardware stays in the state the commit
message describes as the bug, even though the patch carries:
Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core")
This also contradicts the new comment "Default to a 1s timeout when the
platform did not select one".
There is a related mismatch when a cfg is passed with tmouten = 0: TMOUTEN
is still set, but the timer is not programmed.
Should TMOUTEN and the timer write be gated on the same condition? And
should a NULL cfg be handled the same way dwmac5 handles it?
[ ... ]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index f8dceafdda2d2..52f63f624d67c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3639,10 +3639,16 @@ static void stmmac_mtl_configuration(struct stmmac_priv *priv)
>
> static void stmmac_safety_feat_configuration(struct stmmac_priv *priv)
> {
> + unsigned int csr_clk_mhz = 0;
> +
> if (priv->dma_cap.asp) {
> netdev_info(priv->dev, "Enabling Safety Features\n");
> + if (priv->plat->stmmac_clk)
> + csr_clk_mhz = clk_get_rate(priv->plat->stmmac_clk) /
> + 1000000;
[Severity: High]
What happens when priv->plat->stmmac_clk is NULL here? csr_clk_mhz stays 0.
By the time dwmac5_safety_feat_config() reaches the timer block it has
already set TMOUTEN, and it then skips the MAC_FSM_ACT_Timer write without
any message:
if (safety_feat_cfg->tmouten && csr_clk_mhz) {
Both in-tree platforms that explicitly set safety_feat_cfg->tmouten = 1
appear to have no stmmac_clk.
In stmmac_pci.c, stmmac_pci_probe() sets tmouten = 1 for snps_gmac5. The
only CSR clock information is plat->clk_csr = STMMAC_CSR_250_300M in
snps_gmac5_default_data(), and nothing in stmmac_pci.c assigns stmmac_clk.
In dwmac-mediatek.c, mediatek_dwmac_common_data() sets tmouten = 1. The
mediatek binding and dtsi files have no "stmmaceth" clock-name. As a result,
devm_clk_get() fails in stmmac_probe_config_dt(), which prints "Cannot get
CSR clock" and sets plat->stmmac_clk = NULL.
On both platforms the timer is never programmed and nothing is logged. That
doesn't match the new comment on stmmac_safety_feature_cfg.ntmrmd, which
says TMR "is derived from the CSR clock by stmmac" and that "0 selects the
default 1s timeout".
Could this fall back to plat->clk_csr, use a different clock source, or at
least warn when the timer can't be programmed?
> stmmac_safety_feat_config(priv, priv->ioaddr, priv->dma_cap.asp,
> - priv->plat->safety_feat_cfg);
> + priv->plat->safety_feat_cfg,
> + csr_clk_mhz);
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111143322.2234321-1-zxyan0222%40gmail.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-15 14:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-11 14:33 [PATCH] net: stmmac: program the FSM timeout timer Zxyan Zhu
2026-11-11 14:37 ` netdev-bot+sinfo
2026-11-11 16:19 ` Maxime Chevallier
2026-11-12 2:05 ` zhu xin
2026-11-15 14:52 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox