On 8/21/2026 5:13 PM, Jacob Keller wrote:
> To ensure that the E825 PHY timer begins in a clean state, initiate a PHY
> soft reset prior to programming the PHY. This ensures that we clear any
> outstanding Tx timestamp memory, and ensures that the PHY internal state
> has been completely reset.
>
> Fixes: 7cab44f1c35f ("ice: Introduce ETH56G PHY model for E825C products")
> Signed-off-by: Jacob Keller <[email protected]>
> Reviewed-by: Maciek Machnikowski <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index b7d217ac31f3..10e973876608 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -2119,9 +2119,11 @@ int ice_stop_phy_timer_eth56g(struct ice_hw *hw, u8
> port, bool soft_reset)
> * @hw: pointer to the HW struct
> * @port: the PHY port to start
> *
> - * Start the clock of a PHY port. This must be done as part of the flow to
> - * re-calibrate Tx and Rx timestamping offsets whenever the clock time is
> - * initialized or when link speed changes.
> + * Perform a PHY soft reset and then start the clock for the PHY port.
> + *
> + * This must be done as part of the flow to re-calibrate Tx and Rx
> + * timestamping offsets whenever the clock time is initialized or when link
> + * speed changes.
> *
> * Return:
> * * %0 - success
> @@ -2137,6 +2139,10 @@ int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8
> port)
>
> tmr_idx = ice_get_ptp_src_clock_index(hw);
>
> + err = ice_ptp_phy_soft_reset_eth56g(hw, port);
> + if (err)
> + return err;
> +
> err = ice_stop_phy_timer_eth56g(hw, port, false);
> if (err)
> return err;
>
Sashiko points out a couple of pre-existing issues that are unrelated to
this change:
> This is a pre-existing issue, but does ice_ptp_update_cached_phctime() use
> the ICE_CFG_BUSY bit flag as an ad-hoc lock?
> In drivers/net/ethernet/intel/ice/ice_ptp.c:ice_ptp_update_cached_phctime(),
> the flag guards iteration over the vsi array:
> if (test_and_set_bit(ICE_CFG_BUSY, pf->state))
> return -EAGAIN;
> and then clears it on exit:
> clear_bit(ICE_CFG_BUSY, pf->state);
> Does using a bit flag instead of a proper lock bypass lockdep and deadlock
> detection?
This likely could and should be improved, but I am not certain if it
makes sense to try and do so in this series, as this affects much larger
portions of the driver and is a long standing issue with the ice driver
(and Intel drivers in general) :\
> This is a pre-existing issue, but does the lockless read of
> tx->tstamps[idx].start in ice_ptp_process_tx_tstamp() race with its
> initialization?
> In drivers/net/ethernet/intel/ice/ice_ptp.c:ice_ptp_request_ts(), the in_use
> bit is set before the start timestamp is initialized:
> set_bit(idx, tx->in_use);
> clear_bit(idx, tx->stale);
> tx->tstamps[idx].start = jiffies;
> If a concurrent reader in ice_ptp_process_tx_tstamp() iterates over the
> in_use bits locklessly:
> for_each_set_bit(idx, tx->in_use, tx->len) {
> ...
> if (time_is_before_jiffies(tx->tstamps[idx].start + 2 * HZ)) {
> drop_ts = true;
> can it observe the newly set in_use bit and read a stale start value,
> causing it to erroneously drop a valid SKB?
This should be fixed, and we can fix it by correctly ordering so that
the start time is updated before setting the bit, (possibly with a
barrier?). I will add the fix to the series.
