On 10/26/2023 3:59 AM, Karol Kolacinski wrote: > From: Jacob Keller <[email protected]> > > The ice driver currently discards any outstanding timestamps that are > happening very near to a .adjtime or .settime callback. This was > originally added by commit b1a582e64bf2 ("ice: introduce > ice_ptp_reset_cached_phctime function") and later extended by commit > d40fd6009332 ("ice: handle flushing stale Tx timestamps in > ice_ptp_tx_tstamp"). > > The original motivation for discarding timestamps was that extending an > old timestamp using the new cached value of PHC was a problem, as it > could produce incorrect results. The change did not describe what such > "incorrect results" were. > > There are no such incorrect results. Extending the 32 bit timestamp with > the new time value just means that the timestamp is reported in terms of > the newly updated and adjusted system clock. This won't produce > incorrect results or problematic timestamps to applications. Either the > timestamp will be extended with the value of the PHC just prior to the > time adjustment (if the timestamp completes prior to the adjust > callback), or it will be extended using the new PHC value after the > adjustment. In either case, the resulting extended timestamp value makes > sense. > > The timestamp extension logic is very similar to the logic found in > timecounter_cyc2time, the primary difference being that the ice hardware > maintains the full 64 bits of nanoseconds in the MAC rather than being > maintained purely by software as in the timecounter case. > > Indeed, I couldn't find an example of a driver using > timecounter_cyc2time which does discard timestamps that occur nearby > a time adjustment. The ice driver behavior of discarding such timestamps > just results in failure to deliver a Tx timestamp to userspace, > resulting in applications such as ptp4l to timeout and enter a fault > state. Reporting the extended timestamp based on the updated PHC value > isn't producing "garbage" results, and doesn't lead to incorrect > behavior. > > Remove the unnecessary stale timestamp logic. > > Signed-off-by: Jacob Keller <[email protected]> > Signed-off-by: Karol Kolacinski <[email protected]> > Reviewed-by: Przemek Kitszel <[email protected]> > ---
This doesn't compile when applied on top of dev-queue: > ../drivers/net/ethernet/intel/ice/ice_ptp.c: In function > ‘ice_ptp_rebuild_owner’: > ../drivers/net/ethernet/intel/ice/ice_ptp.c:2509:2: error: implicit > declaration of function ‘ice_ptp_flush_all_tx_tracker’; did you mean > ‘ice_ptp_flush_tx_tracker’? [-Werror=implicit-function-declaration] > ice_ptp_flush_all_tx_tracker(pf); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ice_ptp_flush_tx_tracker Thanks, Jake _______________________________________________ Intel-wired-lan mailing list [email protected] https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
