> -----Original Message-----
> From: Jacob Keller <[email protected]>
> Sent: Saturday, August 22, 2026 2:13 AM
> 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]>; Machnikowski, Maciej
> <[email protected]>
> Subject: [PATCH iwl-net 06/12] ice: E825: clear
> PHY_REG_TX_MEMORY_STATUS prior to soft reset
>
> The drivers current implementation of ice_ptp_reset_ts_memory_eth56g()
Probably 'The drivers current implementation' -> 'The driver's current
implementation' ?
> is flawed. It tries to clear the timestamp memory by writing to the
> PHY_REG_TX_MEMORY_STATUS region. This does not work properly, as it
> does not trigger appropriate PHY actions.
>
> To clear outstanding timestamp memory, the driver must read the
> timestamps.
> However, naively doing this as part of ice_ptp_reset_ts_memory() is
> problematic. When reading the timestamp index, hardware kicks off a
> chain of actions including clearing the ready bitmap index, and
> decrementing an internal counter if the timestamp index was marked as
> valid.
>
> This can potentially leave the internal hardware counter out of sync
> with the actual number of timestamps. This occurs because the
> PHY_REG_TX_MEMORY_STATUS region is not zero-initialized when the
> device boots up. Instead, it is filled with garbage. On a cold power
> on, attempts to read the stale data result in the hardware triggering
> a counter decrement for a timestamp that never happened. This
> underflows the counter, and prevents new timestamp interrupts from
> being triggered for real timestamp requests.
>
> We must read the PHY_REG_TX_MEMORY_STATUS in order to clear stale
> timestamps. But doing so may cause a desync with the counter. To
> prevent issues, perform this clearing always and only right before
> initiating a PHY soft reset.
>
> The soft reset will clear and reset the internal counter and the ready
> bitmap. The reads to PHY_REG_TX_MEMORY_STATUS will reset the region
> valid bits ensuring that no stale data is left behind. This
> combination ensures that we always have a clean slate with no stale
> data and with the counter properly reset to zero.
>
> Fixes: 3ec46e157c7f ("ice: perform PHY soft reset for E825C ports at
> initialization")
> Signed-off-by: Jacob Keller <[email protected]>
> Reviewed-by: Maciek Machnikowski <[email protected]>
> ---
> drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 89 +++++++++++++++-----
> ---------
> 1 file changed, 47 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> index d48eb3c61823..b7d217ac31f3 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
> @@ -737,24 +737,6 @@ static int ice_read_port_mem_eth56g(struct ice_hw
> *hw, u8 port, u16 offset,
> return ice_read_port_eth56g(hw, port, offset, val,
> ETH56G_PHY_MEM_PTP); }
>
> -/**
> - * ice_write_port_mem_eth56g - Write a PHY port memory location
> - * @hw: pointer to the HW struct
> - * @port: Port number to be read
> - * @offset: Offset from PHY port register base
> - * @val: Pointer to the value to read (out param)
> - *
> - * Return:
> - * * %0 - success
> - * * %EINVAL - invalid port number or resource type
> - * * %other - failed to write to PHY
> - */
> -static int ice_write_port_mem_eth56g(struct ice_hw *hw, u8 port, u16
> offset,
> - u32 val)
> -{
> - return ice_write_port_eth56g(hw, port, offset, val,
> ETH56G_PHY_MEM_PTP);
> -}
> -
> /**
> * ice_write_quad_ptp_reg_eth56g - Write a PHY quad register
> * @hw: pointer to the HW struct
> @@ -1139,8 +1121,8 @@ static int ice_read_ptp_tstamp_eth56g(struct
> ice_hw *hw, u8 port, u8 idx,
> * internal PHYs of the 56G devices.
> *
> * To directly clear the contents of the timestamp block entirely,
> discarding
> - * all timestamp data at once, software should instead use
> - * ice_ptp_reset_ts_memory_quad_eth56g().
> + * all timestamp data at once, software should instead perform a PHY
> + soft
> + * reset via ice_ptp_phy_soft_reset_eth56g().
> *
> * This function should only be called on an idx whose bit is set
> according to
> * ice_get_phy_tx_tstamp_ready().
> @@ -1152,11 +1134,11 @@ static int ice_read_ptp_tstamp_eth56g(struct
> ice_hw *hw, u8 port, u8 idx, static int
> ice_clear_ptp_tstamp_eth56g(struct ice_hw *hw, u8 port, u8 idx) {
> u64 unused_tstamp;
> - u16 lo_addr;
> int err;
>
> - /* Read the timestamp register to ensure the timestamp status
> bit is
> - * cleared.
> + /* Per the PHY spec, reading the timestamp memory location is
> what
> + * clears the entry's valid bit and its corresponding (read-
> only)
> + * ts_memory_status bit.
> */
> err = ice_read_ptp_tstamp_eth56g(hw, port, idx,
> &unused_tstamp);
> if (err) {
> @@ -1164,32 +1146,40 @@ static int ice_clear_ptp_tstamp_eth56g(struct
> ice_hw *hw, u8 port, u8 idx)
> port, idx, err);
> }
>
> - lo_addr = (u16)PHY_TSTAMP_L(idx);
> -
> - err = ice_write_port_mem_eth56g(hw, port, lo_addr, 0);
> - if (err) {
> - ice_debug(hw, ICE_DBG_PTP, "Failed to clear low PTP
> timestamp register for port %u, idx %u, err %d\n",
> - port, idx, err);
> - return err;
> - }
> -
> return 0;
> }
>
> /**
> - * ice_ptp_reset_ts_memory_eth56g - Clear all timestamps from the
> port block
> + * ice_ptp_clear_tx_memory_status_eth56g - Reset one port's Tx
> + timestamp memory
> * @hw: pointer to the HW struct
> + * @port: port number to clear
> + *
> + * Fully reset a single PHY port's Tx timestamp memory. Per the PHY
> + spec, the
> + * only way to clear a timestamp valid bit (and its read-only
> + ts_memory_status
> + * bit) is to read the timestamp memory location, so read every entry
> + for the
> + * port (two 32-bit reads each). This discards all timestamp data on
> + the port,
> + * so it must only be used for a full reset; callers that must
> preserve
> + * in-flight timestamps clear individual indices via
> ice_clear_phy_tstamp().
> + *
> + * Due to interactions with an internal HW counter for the number of
> + * outstanding Tx timestamps, this *must* only be called as part of
> the
> + * ice_ptp_phy_soft_reset_eth56g() procedure. Otherwise, the internal
> + counter
> + * may become out of sync and prevent new timestamp interrupts.
> + *
> + * Return: 0 on success, negative error code on failure to read the
> PHY.
> */
> -static void ice_ptp_reset_ts_memory_eth56g(struct ice_hw *hw)
> +static int ice_ptp_clear_tx_memory_status_eth56g(struct ice_hw *hw,
> u8
> +port)
> {
> - unsigned int port;
> + int err = 0;
> + u8 idx;
>
> - for (port = 0; port < hw->ptp.num_lports; port++) {
> - ice_write_ptp_reg_eth56g(hw, port,
> PHY_REG_TX_MEMORY_STATUS_L,
> - 0);
> - ice_write_ptp_reg_eth56g(hw, port,
> PHY_REG_TX_MEMORY_STATUS_U,
> - 0);
> + for (idx = 0; idx < INDEX_PER_PORT; idx++) {
> + err = ice_clear_ptp_tstamp_eth56g(hw, port, idx);
> + if (err)
> + return err;
> }
> +
> + return 0;
> }
>
> /**
> @@ -2290,6 +2280,7 @@ int
> ice_ptp_read_tx_hwtstamp_status_eth56g(struct ice_hw *hw, u32
> *ts_status)
> *
> * Trigger a soft reset of the ETH56G PHY by toggling the soft reset
> * bit in the PHY global register. The reset sequence consists of:
> + * 0. Reading every timestamp memory register to clear its valid
> bit
> * 1. Clearing the soft reset bit
> * 2. Asserting the soft reset bit
> * 3. Clearing the soft reset bit again
> @@ -2298,6 +2289,12 @@ int
> ice_ptp_read_tx_hwtstamp_status_eth56g(struct ice_hw *hw, u32
> *ts_status)
> * to settle. This provides a controlled way to reinitialize the PHY
> * without requiring a full device reset.
> *
> + * To ensure that the internal counter matches the contents of the
> + * PHY_REG_TX_MEMORY_STATUS, read every timestamp index prior to
> + performing
> + * the soft reset. The PHY_REG_TX_MEMORY_STATUS reads ensure that the
> + region
> + * is cleared, while the soft reset procedure ensures that the
> + timestamp
timestamp -> timestamp
> + * counter is reset to zero.
> + *
> * Return: 0 on success, or a negative error code on failure when
> * reading or writing the PHY register.
> */
> @@ -2306,6 +2303,13 @@ int ice_ptp_phy_soft_reset_eth56g(struct ice_hw
> *hw, u8 port)
> u32 global_val;
> int err;
>
> + err = ice_ptp_clear_tx_memory_status_eth56g(hw, port);
> + if (err) {
> + ice_debug(hw, ICE_DBG_PTP, "Failed to clear
> PHY_REG_TX_MEMORY_STATUS for port %d, err %d\n",
> + port, err);
> + return err;
> + }
> +
> err = ice_read_ptp_reg_eth56g(hw, port, PHY_REG_GLOBAL,
> &global_val);
> if (err) {
> ice_debug(hw, ICE_DBG_PTP, "Failed to read
> PHY_REG_GLOBAL for port %d, err %d\n", @@ -5797,8 +5801,9 @@ void
> ice_ptp_reset_ts_memory(struct ice_hw *hw)
> ice_ptp_reset_ts_memory_e82x(hw);
> break;
> case ICE_MAC_GENERIC_3K_E825:
> - ice_ptp_reset_ts_memory_eth56g(hw);
> - break;
> + /* E825 hardware must only reset timestamp memory as
> part of
> + * the soft reset procedure.
> + */
> case ICE_MAC_E810:
> default:
> return;
>
> --
> 2.55.0.814.gc42f45431d0f
Reviewed-by: Aleksandr Loktionov <[email protected]>
