> -----Original Message----- > From: Jacob Keller <[email protected]> > Sent: Friday, September 25, 2026 4:57 PM > To: Intel Wired LAN <[email protected]>; Machnikowski, Maciej > <[email protected]>; Keller, Jacob E <[email protected]>; > Korba, Przemyslaw <[email protected]>; Nguyen, Anthony L > <[email protected]>; Nitka, Grzegorz <[email protected]>; > Kubalewski, Arkadiusz <[email protected]> > Cc: Keller, Jacob E <[email protected]>; Machnikowski, Maciej > <[email protected]> > Subject: [PATCH iwl-net v3 08/15] ice: E825: stop clearing > PHY_REG_TX_OFFSET_READY > > The ice_stop_phy_timer_eth56g() function is called by the driver for E825 > devices to ensure that the PHY timer has been stopped. The equivalent > function for older E822 devices performed many steps. However, on E825 it > only clears the PHY_REG_TX_OFFSET_READY and PHY_REG_RX_OFFSET_READY bits to > indicate to HW that it should no longer treat the PHY offset as valid. > > When PHY_REG_TX_OFFSET_READY is cleared, the hardware still captures Tx > timestamps, but it no longer sets the valid bit for these timestamps. This > sounds reasonable at first glance. However, this results in the internal > outstanding timestamp counter becoming out of sync. > > When capturing a timestamp, hardware increments its internal counter and sets > the associated "ready" bit in the timestamp memory status. Then it compares > the timestamp count to the threshold to determine if it should trigger an > interrupt to the MAC. > > Upon reading the timestamp hardware is supposed to decrement the counter, > clear the valid bit, and clear the associated bit from the memory status > register. However, it only performs these steps *if* the valid bit is set. > > Since the valid bit is not set while PHY_REG_TX_OFFSET_READY is clear, the > timestamp counter is not decremented and the memory status is not cleared. > This leaves the counter out-of-sync until a PHY soft reset. > > According to the hardware engineers, the PHY_REG_TX_OFFSET_READY bit has no > other effects. It only controls whether hardware captures timestamps with the > valid bit set or not. Since capturing timestamps with the valid bit clear is > problematic, they recommend simply not clearing PHY_REG_TX_OFFSET_READY. > > Note that the PHY_REG_RX_OFFSET_READY performs a similar task. However, > clearing it is fine as there is no associated timestamp counter on the Rx > side. Receive timestamps are simply inserted into the descriptor. Clearing > this register clears the valid bit for timestamps until we complete > calibration and re-enable the register. > > Notice that the soft_reset parameter of ice_stop_phy_timer_eth56g() is > totally unused. It is a relic from a copy-paste of > ice_stop_phy_timer_e82x() that is unnecessary, so remove it. > > Now that we do not clear the PHY_REG_TX_OFFSET_READY, new timestamp requests > could happen while the PHY is calibrating. To avoid this, set the > tx.calibrating field of the Tx timestamp tracker when stopping the timer and > clear it when finishing the restart. This ensures that any new requests will > be rejected until the PHY timer calibration has completed. Also call > ice_ptp_mark_tx_tracker_stale() to prevent reporting any previous outstanding > timestamps to the stack. > > Unlike E822 devices, set the calibrating flag when "stopping" the PHY and > clear it immediately after the start procedure. The E825 device does not > perform vernier calibration and thus does not need to wait for hardware to > mark the offsets as valid. > > In the event that the PHY timer start procedure fails, the device is in an > unknown state and timestamps will not behave properly. As such, and similar > to E822 devices, the calibrating field is not cleared on failure. > This > leaves timestamp requests disabled until the next link restart. > > Failure in the start flow is unexpected and it is unclear precisely what > state the hardware is left in. Attempting to add a complex retry mechanism > for a rare event is not worthwhile. The port restart procedure can already be > re-initiated by triggering a link reset (i.e. via ethtool). > > Instead update the dev_err message at the end of ice_ptp_port_phy_restart. > Clearly indicate that timestamping is disabled, and add a note that a link > toggle might recover the device. For the invalid MAC type path returning > -ENODEV, skip this message and log a dev_dbg that we failed with an unknown > MAC type instead. > > Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products") > Suggested-by: Maciej Machnikowski <[email protected]> > Signed-off-by: Jacob Keller <[email protected]> > --- > drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 2 +- > drivers/net/ethernet/intel/ice/ice_ptp.c | 36 > +++++++++++++++++++++++++---- > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 25 ++++++++++---------- > 3 files changed, 46 insertions(+), 17 deletions(-)
Tested-by: Alexander Nowlin <[email protected]>
