Dear Chwee-Lin,
Thank you for your patch. Am 18.09.25 um 20:38 schrieb Chwee-Lin Choong:
The current HW bug workaround checks the TXTT_0 ready bit first, then reads LOW -> HIGH -> LOW from register 0 to detect if a timestamp was captured. This sequence has a race: if a new timestamp is latched after reading the TXTT mask but before the first LOW read, both old and new timestamp match, causing the driver to drop a valid timestamp.
Reading the TXTT mask is `rd32(IGC_TSYNCTXCTL)`, correct?
Fix by reading the LOW register first, then the TXTT mask, so a newly latched timestamp will always be detected. This fix also prevents TX unit hangs observed under heavy timestamping load.
The unit shouldn’t hang, even if valid timestamps are dropped? Do you have a reproducer?
Fixes: c789ad7cbebc ("igc: Work around HW bug causing missing timestamps") Suggested-by: Avi Shalev <[email protected]> Signed-off-by: Song Yoong Siang <[email protected]> Signed-off-by: Chwee-Lin Choong <[email protected]> --- drivers/net/ethernet/intel/igc/igc_ptp.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c index b7b46d863bee..930486b02fc1 100644 --- a/drivers/net/ethernet/intel/igc/igc_ptp.c +++ b/drivers/net/ethernet/intel/igc/igc_ptp.c @@ -774,10 +774,17 @@ static void igc_ptp_tx_reg_to_stamp(struct igc_adapter *adapter, static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter) { struct igc_hw *hw = &adapter->hw; + u32 txstmpl_old; u64 regval; u32 mask; int i;+ /* Read the "low" register 0 first to establish a baseline value.+ * This avoids a race where a new timestamp could be latched + * after checking the TXTT mask. + */ + txstmpl_old = rd32(IGC_TXSTMPL); + mask = rd32(IGC_TSYNCTXCTL) & IGC_TSYNCTXCTL_TXTT_ANY; if (mask & IGC_TSYNCTXCTL_TXTT_0) { regval = rd32(IGC_TXSTMPL); @@ -801,9 +808,8 @@ static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter) * timestamp was captured, we can read the "high" * register again. */ - u32 txstmpl_old, txstmpl_new; + u32 txstmpl_new;- txstmpl_old = rd32(IGC_TXSTMPL);rd32(IGC_TXSTMPH); txstmpl_new = rd32(IGC_TXSTMPL);
Kind regards, Paul
