On 9/18/2025 1:47 PM, Vadim Fedorenko wrote: > On 18/09/2025 19:38, Chwee-Lin Choong wrote: >> 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. >> >> 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. >> >> Fixes: c789ad7cbebc ("igc: Work around HW bug causing missing timestamps") >> Suggested-by: Avi Shalev >> Signed-off-by: Song Yoong Siang >> Signed-off-by: Chwee-Lin Choong >> --- >> drivers/net/ethernet/intel/igc/igc_ptp.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> > > [...] > >> * timestamp was captured, we can read the "high" >> * register again. >> */ > > This comment begins with 'read the "high" register (to latch a new > timestamp)' ... > >> - u32 txstmpl_old, txstmpl_new; >> + u32 txstmpl_new; >> >> - txstmpl_old = rd32(IGC_TXSTMPL); >> rd32(IGC_TXSTMPH); >> txstmpl_new = rd32(IGC_TXSTMPL); > > and a couple of lines later in this function you have > > regval = txstmpl_new; > regval |= (u64)rd32(IGC_TXSTMPH) << 32; > > According to the comment above, the value in the register will be > latched after reading IGC_TXSTMPH. As there will be no read of "low" > part of the register, it will stay latched with old value until the > next call to the same function. Could it be the reason of unit hangs? > > It looks like the value of previous read of IGC_TXSTMPH should be stored > and used to construct new timestamp, right? > I wouldn't trust the comment, but instead double check the data sheets. Unfortunately, I don't seem to have a copy of the igc hardware data sheet handy :( Thanks, Jake