> stmmac_dl_ts_coarse_set() runs under devl_lock only while > stmmac_hwtstamp_set() runs under RTNL. They read/write > systime_flags/tsfupdt_coarse and run stmmac_config_hw_tstamping(). > > stmmac_dl_ts_coarse_set() snapshots priv->systime_flags, programs > PTP_TCR, PTP_SSIR and PTP_TAR, and only then publishes tsfupdt_coarse > and systime_flags. A concurrent stmmac_hwtstamp_set() can read the > stale tsfupdt_coarse, build fine-mode flags, program PTP_TCR in fine > mode and set hwts_rx_en. > > Take priv->lock in both stmmac_dl_ts_coarse_set() and > stmmac_hwtstamp_set() so the timestamping configuration is updated > under a single lock. > > Fixes: 4347287b8627 ("net: stmmac: propagate PTP addend and system time programming errors") I will repost with proper Fixes tag in v2. Regards, Lorenzo > Signed-off-by: Lorenzo Bianconi > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 43 ++++++++++++++--------- > 1 file changed, 27 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 9741f97fa37a..a6793054a179 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -685,14 +685,7 @@ static int stmmac_hwtstamp_set(struct net_device *dev, > u32 snap_type_sel = 0; > u32 ts_master_en = 0; > u32 ts_event_en = 0; > - > - if (!(priv->dma_cap.time_stamp || priv->adv_ts)) { > - NL_SET_ERR_MSG_MOD(extack, "No support for HW time stamping"); > - priv->hwts_tx_en = 0; > - priv->hwts_rx_en = 0; > - > - return -EOPNOTSUPP; > - } > + int ret = 0; > > if (!netif_running(dev)) { > NL_SET_ERR_MSG_MOD(extack, > @@ -700,13 +693,23 @@ static int stmmac_hwtstamp_set(struct net_device *dev, > return -ENODEV; > } > > - netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n", > - __func__, config->flags, config->tx_type, config->rx_filter); > - > if (config->tx_type != HWTSTAMP_TX_OFF && > config->tx_type != HWTSTAMP_TX_ON) > return -ERANGE; > > + netdev_dbg(priv->dev, "%s config flags:0x%x, tx_type:0x%x, rx_filter:0x%x\n", > + __func__, config->flags, config->tx_type, config->rx_filter); > + > + mutex_lock(&priv->lock); > + > + if (!(priv->dma_cap.time_stamp || priv->adv_ts)) { > + NL_SET_ERR_MSG_MOD(extack, "No support for HW time stamping"); > + priv->hwts_tx_en = 0; > + priv->hwts_rx_en = 0; > + ret = -EOPNOTSUPP; > + goto unlock; > + } > + > if (priv->adv_ts) { > switch (config->rx_filter) { > case HWTSTAMP_FILTER_NONE: > @@ -829,7 +832,8 @@ static int stmmac_hwtstamp_set(struct net_device *dev, > break; > > default: > - return -ERANGE; > + ret = -ERANGE; > + goto unlock; > } > } else { > switch (config->rx_filter) { > @@ -859,8 +863,10 @@ static int stmmac_hwtstamp_set(struct net_device *dev, > stmmac_config_hw_tstamping(priv, priv->ptpaddr, priv->systime_flags); > > priv->tstamp_config = *config; > +unlock: > + mutex_unlock(&priv->lock); > > - return 0; > + return ret; > } > > /** > @@ -7753,9 +7759,12 @@ static int stmmac_dl_ts_coarse_set(struct devlink *dl, u32 id, > { > struct stmmac_devlink_priv *dl_priv = devlink_priv(dl); > struct stmmac_priv *priv = dl_priv->stmmac_priv; > - u32 systime_flags = priv->systime_flags; > + u32 systime_flags; > int ret; > > + mutex_lock(&priv->lock); > + > + systime_flags = priv->systime_flags; > if (ctx->val.vbool) > systime_flags &= ~PTP_TCR_TSCFUPDT; > else > @@ -7768,13 +7777,15 @@ static int stmmac_dl_ts_coarse_set(struct devlink *dl, u32 id, > if (ret) { > NL_SET_ERR_MSG_MOD(extack, > "failed to reconfigure PTP adjustment"); > - return ret; > + goto unlock; > } > > priv->tsfupdt_coarse = ctx->val.vbool; > priv->systime_flags = systime_flags; > +unlock: > + mutex_unlock(&priv->lock); > > - return 0; > + return ret; > } > > static int stmmac_dl_ts_coarse_get(struct devlink *dl, u32 id, > > --- > base-commit: 232d49dd4b40a666283de9e722899f088ed581b2 > change-id: 20261002-stmmac-hwtstamp-missing-lock-b6a7979c643c > > Best regards, > -- > Lorenzo Bianconi >