> -----Original Message-----
> From: Keller, Jacob E <[email protected]> 
> Sent: Friday, August 21, 2026 5:13 PM
> To: Intel Wired LAN <[email protected]>
> Cc: [email protected]; Machnikowski, Maciej 
> <[email protected]>; Nguyen, Anthony L 
> <[email protected]>; Korba, Przemyslaw <[email protected]>; 
> Nitka, Grzegorz <[email protected]>; Oros, Petr <[email protected]>; 
> Nowlin, Alexander <[email protected]>; Bross, Kevin 
> <[email protected]>; Cavatur, Ranjit <[email protected]>; Keller, 
> Jacob E <[email protected]>
> Subject: [PATCH iwl-net 12/12] ice: don't clear in_use until HW clears ready 
> bitmap
> 
> During a link down transition, the E825 PHY has a small window where it does 
> not properly respond to reading the PHY timestamp registers. When this 
> occurs, the PHY does not automatically clear the ready bitmap or the valid 
> bit for the timestamp. This begins happening slightly before a link 
> transition even before the firmware has notified the driver of the state 
> change.
> 
> The driver happily completes the timestamp, releasing the in_use bit. This 
> allows another request to reuse the bit potentially reporting an invalid 
> stale timestamp. Additionally, with the ready bit still set high the > driver 
> continues to re-trigger the IRQ and check for timestamps in a tight loop, 
> wasting CPU cycles.
> 
> To fix this, re-read the PHY timestamp memory status after each read of a PHY 
> index. Double check if the hardware cleared the index properly. If it hasn't, 
> mark the timestamp index as stale and skip processing it.
> 
> Stale timestamps are already ignored by the ice_any_port_has_timestamps() 
> function. However, the ice_ptp_tx_tstamps_pending() function also checks the 
> ready bitmap. Instead, modify it to only check the software tracker.
> Additionally, stop re-triggering the interrupt from the IRQ if the timestamp 
> tracker is calibrating or has the link marked as down. Continue to check the 
> hardware ready bitmap from the watchdog to catch cases of unexpected 
> timestamps.
> 
> With these changes, the timestamp processing no longer triggers a repeated 
> spamming of the IRQ during link down events where timestamps get stuck as the 
> PHY transitions to link down. Once link is restored, the PHY will be reset 
> and the stuck timestamps are cleared.
> 
> Measuring CPU utilization of the miscellaneous IRQ thread function during 
> timestamp storms near a link reset shows that this prevents the spikes caused 
> by the "stuck" ready bit. Without this fix, the CPU handling the IRQ becomes 
> slammed due to the IRQ re-triggering logic.
> 
> Measuring latency using the ice Tx timestamp traces does show that this fix 
> comes at a latency cost. Latency is measured using the ice Tx timestamp 
> traces for the request to completion time. I measured a couple of different 
> workloads both before and after this fix:
> 
>  * ptp4l using a profile with ~16 SYNC messages per second
> 
>     before: 159.40 microseconds mean, stdev 45.28
>      after: 182.07 microseconds mean,  stdev 43.43
> 
>  * a C program generating 16 timestamp requests every 10 milliseconds on
>    two different ports:
> 
>     before: 604.35 microseconds mean, stdev 345.32
>      after: 990.13 microseconds mean, stdev 625.64
> 
> In the normal work flows this comes with about a 20 microsecond penalty on 
> the average, and the standard deviation remains approximately the same. For 
> heavy workloads with many more timestamps than expected for typical 
> applications this comes at a significant cost. This is because we handle all 
> timestamps in a single thread. If there are many concurrent timestamps being 
> requested at once, any which use the later slots on ports later in the port 
> list will take much longer to be processed once the interrupt is fired. Since 
> each timestamp now requires an additional PHY register access, this cost is 
> much higher in the case where the device is under unusually heavy load.
> 
> However, *correctness* is more important than speed here. Additionally, we 
> still remain well below the default limit of 10 milliseconds that ptp4l will 
> wait before complaining about missing timestamps.
> 
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
> Signed-off-by: Jacob Keller <[email protected]>
> ---
>  drivers/net/ethernet/intel/ice/ice_ptp.c | 54 
> +++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 28 deletions(-)

Tested-by: Alexander Nowlin <[email protected]>

Reply via email to