[PATCH iwl-net 2/4] ice: clear unexpected Tx timestamp ready bits to prevent stuck PHY

[Przemyslaw Korba <przemyslaw.korba@intel.com>]

From: Jacob Keller <jacob.e.keller@intel.com>

Clear orphaned timestamp ready bits left in the PHY when a packet is
timestamped just as the link goes down. The driver clears its software
in_use bits during link-down cleanup, but the PHY has already latched
the timestamp, so on the next link-up a stale ready bit from the
previous link cycle remains set with no matching in_use entry.

The PHY timestamp interrupt logic will not generate a new interrupt
until ALL outstanding ready bits have been read. ice_ptp_process_tx_
tstamp() only iterates slots set in the software in_use bitmap, so it
never reads these orphaned slots. The result is a permanent interrupt
deadlock:

1. The PHY has ready bits set for slots with no in_use tracker entry
2. The driver never reads those slots because in_use is clear
3. The PHY refuses to generate new timestamp interrupts
4. All future Tx timestamps permanently fail
5. Only a power-on reset can recover the device

Clear these stale ready bits on link-up, in ice_ptp_link_change(),
before any new timestamp requests arrive, scoped to the affected port
only so timestamps still pending on other ports are left untouched. For
every affected MAC the clearing iterates the not-in_use slots with
for_each_clear_bit(), so a pending software request is never discarded
and tx->lock need not be held across the PHY access:

  - E810 is skipped (guarded by tx->has_ready_bitmap) because
    ice_get_phy_tx_tstamp_ready_e810() returns an all-ones bitmap rather
    than a real ready status.
  - E830 and E825/eth56g read the real Tx timestamp ready bitmap and
    clear only the slots the PHY actually latched, via
    ice_clear_phy_tstamp(). If the ready bitmap cannot be read, the
    clear is skipped and a rate-limited warning is logged.

On every MAC the ready bit is cleared by reading the slot's timestamp
memory; the orphaned bits linger only because the normal processing path
never reads those not-in_use slots. The link-up recovery clears them by
reading the timestamp memory via ice_clear_phy_tstamp():

  - E830: add ice_clear_phy_tstamp_e830(), which reads the
    PRTTSYN_TXTIME_H/L registers to clear the entry, and add an
    ICE_MAC_E830 case to the ice_clear_phy_tstamp() dispatch.
  - eth56g: ice_clear_ptp_tstamp_eth56g() reads the timestamp memory
    location, which per the PHY spec is the operation that clears the
    entry's valid bit and its corresponding (read-only) ts_memory_status
    bit. The ts_memory_status registers cannot be written to clear a
    bit, so only reading the timestamp memory has any effect.

The new for_each_clear_bit() loop runs from the service task via link
events. During device removal, tx->in_use is freed before the service
task is stopped. Fix by stopping the service task at the start of
ice_unload(), before ice_ptp_release() frees tx->in_use.

Reviewed-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Przemyslaw Korba <przemyslaw.korba@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c   |  1 +
 drivers/net/ethernet/intel/ice/ice_ptp.c    | 72 +++++++++++++++--
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 89 +++++++++++++++++----
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h |  1 +
 drivers/net/ethernet/intel/ice/ice_type.h   |  7 ++
 5 files changed, 149 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index e3d3810c791f..231d533309cb 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5169,6 +5169,7 @@ void ice_unload(struct ice_pf *pf)
 
 	devl_assert_locked(priv_to_devlink(pf));
 
+	ice_service_task_stop(pf);
 	ice_unplug_aux_dev(pf);
 	ice_deinit_rdma(pf);
 	ice_deinit_features(pf);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 1aa440b0639f..9d9d9958fe5c 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1372,15 +1372,73 @@ void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
 	switch (hw->mac_type) {
 	case ICE_MAC_E810:
 	case ICE_MAC_E830:
-		/* Do not reconfigure E810 or E830 PHY */
+	case ICE_MAC_GENERIC_3K_E825:
+		/* Do not reconfigure E810 or E830 PHY, but on link-up clear
+		 * any stale timestamp ready bits left over from a previous
+		 * link cycle. The PHY may have latched timestamps for packets
+		 * in flight when the link went down; these must be cleared
+		 * before new timestamp requests arrive.
+		 *
+		 * E810 does not have a real ready bitmap
+		 * (ice_get_phy_tx_tstamp_ready_e810 returns all-ones), so
+		 * skip clearing on E810 to avoid unnecessary sideband queue
+		 * operations for every not-in-use slot on each link-up.
+		 */
+		if (linkup && ptp_port->tx.has_ready_bitmap) {
+			struct ice_ptp_tx *tx = &ptp_port->tx;
+			u64 tstamp_ready;
+			int i;
+
+			if (ice_get_phy_tx_tstamp_ready(hw, tx->block,
+							&tstamp_ready)) {
+				dev_warn_ratelimited(ice_pf_to_dev(pf),
+						     "PTP failed to read Tx timestamp ready bitmap on link-up; stale PHY timestamps may remain and stall Tx timestamping\n");
+			} else {
+				/* Only clear stale ready bits for slots that
+				 * have no in-flight software request. Iterating
+				 * the not-in-use slots skips any concurrent
+				 * ice_ptp_request_ts() allocation without
+				 * holding tx->lock across the PHY access. E830
+				 * and E825 reach this clear; E810 is filtered
+				 * out above by has_ready_bitmap.
+				 */
+				for_each_clear_bit(i, tx->in_use, tx->len) {
+					u8 phy_idx = i + tx->offset;
+
+					if (tstamp_ready & BIT_ULL(phy_idx))
+						ice_clear_phy_tstamp(hw,
+								     tx->block,
+								     phy_idx);
+				}
+			}
+		}
+
+		/* E810 and E830 need no further PHY reconfiguration */
+		if (hw->mac_type != ICE_MAC_GENERIC_3K_E825)
+			return;
+
+		/* E825 recovers its Tx path by soft resetting the PHY
+		 * timestamp block and restarting the port, but only on
+		 * link-up. The reset is a three-step register toggle; if it
+		 * fails partway through, the port can be left held in reset,
+		 * and programming a PHY that is stuck in reset via
+		 * ice_ptp_port_phy_restart() would leave Tx timestamping
+		 * permanently broken. So warn and skip the restart on
+		 * failure; the sequence is retried on the next link-up event.
+		 */
+		if (!linkup)
+			return;
+
+		if (ice_ptp_phy_soft_reset_eth56g(hw, ptp_port->port_num))
+			dev_warn(ice_pf_to_dev(pf),
+				 "PTP failed to soft reset PHY port %u on link-up; skipping restart, Tx timestamping may be stuck, try toggle a link to recover\n",
+				 ptp_port->port_num);
+		else
+			ice_ptp_port_phy_restart(ptp_port);
 		return;
 	case ICE_MAC_GENERIC:
 		ice_ptp_port_phy_restart(ptp_port);
 		return;
-	case ICE_MAC_GENERIC_3K_E825:
-		if (linkup)
-			ice_ptp_port_phy_restart(ptp_port);
-		return;
 	default:
 		dev_warn(ice_pf_to_dev(pf), "%s: Unknown PHY type\n", __func__);
 	}
@@ -3380,6 +3438,7 @@ void ice_ptp_init(struct ice_pf *pf)
 
 	ptp->state = ICE_PTP_INITIALIZING;
 
+	mutex_init(&hw->ptp.tx_tstamp_lock);
 	if (hw->lane_num < 0) {
 		err = hw->lane_num;
 		goto err_exit;
@@ -3443,6 +3502,7 @@ void ice_ptp_init(struct ice_pf *pf)
 
 	ice_ptp_cleanup_adapter(pf);
 err_exit:
+	mutex_destroy(&hw->ptp.tx_tstamp_lock);
 	/* If we registered a PTP clock, release it */
 	if (pf->ptp.clock) {
 		ptp_clock_unregister(ptp->clock);
@@ -3469,6 +3529,7 @@ void ice_ptp_release(struct ice_pf *pf)
 
 	if (pf->ptp.state != ICE_PTP_READY) {
 		mutex_destroy(&pf->ptp.port.ps_lock);
+		mutex_destroy(&pf->hw.ptp.tx_tstamp_lock);
 		ice_ptp_cleanup_pf(pf);
 		ice_ptp_cleanup_adapter(pf);
 		if (pf->ptp.clock) {
@@ -3495,6 +3556,7 @@ void ice_ptp_release(struct ice_pf *pf)
 
 	ice_ptp_port_phy_stop(&pf->ptp.port);
 	mutex_destroy(&pf->ptp.port.ps_lock);
+	mutex_destroy(&pf->hw.ptp.tx_tstamp_lock);
 	if (pf->ptp.kworker) {
 		kthread_destroy_worker(pf->ptp.kworker);
 		pf->ptp.kworker = NULL;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index a1970a887c38..b23b68cd18ee 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -1160,11 +1160,15 @@ static int ice_read_ptp_tstamp_eth56g(struct ice_hw *hw, u8 port, u8 idx,
  *
  * 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().
+ * ice_ptp_reset_ts_memory_eth56g().
  *
  * This function should only be called on an idx whose bit is set according to
  * ice_get_phy_tx_tstamp_ready().
  *
+ * Serialized against ice_ptp_clear_tx_memory_status_eth56g() via
+ * tx_tstamp_lock so the two paths do not interleave their reads of the same
+ * port's Tx timestamp memory.
+ *
  * Return:
  * * %0     - success
  * * %other - failed to write to PHY
@@ -1175,25 +1179,62 @@ static int ice_clear_ptp_tstamp_eth56g(struct ice_hw *hw, u8 port, u8 idx)
 	u16 lo_addr;
 	int err;
 
-	/* Read the timestamp register to ensure the timestamp status bit is
-	 * cleared.
+	lo_addr = (u16)PHY_TSTAMP_L(idx);
+
+	mutex_lock(&hw->ptp.tx_tstamp_lock);
+
+	/* 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. This clears only this index, leaving any
+	 * other in-flight timestamps on the port untouched.
 	 */
 	err = ice_read_ptp_tstamp_eth56g(hw, port, idx, &unused_tstamp);
-	if (err) {
+	if (err)
 		ice_debug(hw, ICE_DBG_PTP, "Failed to read the PHY timestamp register for port %u, idx %u, err %d\n",
 			  port, idx, err);
-	}
-
-	lo_addr = (u16)PHY_TSTAMP_L(idx);
 
 	err = ice_write_port_mem_eth56g(hw, port, lo_addr, 0);
-	if (err) {
+	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;
+
+	mutex_unlock(&hw->ptp.tx_tstamp_lock);
+
+	return err;
+}
+
+/**
+ * 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().
+ *
+ * Holds tx_tstamp_lock so this full-port sweep does not interleave with the
+ * per-index reads in ice_clear_ptp_tstamp_eth56g() on the same port.
+ *
+ * Return: 0 on success, negative error code on failure to read the PHY.
+ */
+int ice_ptp_clear_tx_memory_status_eth56g(struct ice_hw *hw, u8 port)
+{
+	u64 unused_tstamp;
+	int err = 0;
+	u8 idx;
+
+	mutex_lock(&hw->ptp.tx_tstamp_lock);
+	for (idx = 0; idx < INDEX_PER_PORT; idx++) {
+		err = ice_read_ptp_tstamp_eth56g(hw, port, idx, &unused_tstamp);
+		if (err)
+			break;
 	}
+	mutex_unlock(&hw->ptp.tx_tstamp_lock);
 
-	return 0;
+	return err;
 }
 
 /**
@@ -1204,12 +1245,8 @@ static void ice_ptp_reset_ts_memory_eth56g(struct ice_hw *hw)
 {
 	unsigned int port;
 
-	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 (port = 0; port < hw->ptp.num_lports; port++)
+		ice_ptp_clear_tx_memory_status_eth56g(hw, port);
 }
 
 /**
@@ -5277,6 +5314,23 @@ static void ice_read_phy_tstamp_e830(const struct ice_hw *hw, u8 idx,
 		  FIELD_PREP(PHY_EXT_40B_LOW_M, lo);
 }
 
+/**
+ * ice_clear_phy_tstamp_e830 - Clear a timestamp from the E830 PHY
+ * @hw: pointer to the HW struct
+ * @idx: the timestamp index to clear
+ *
+ * Clear the valid bit for the given timestamp index in the Tx memory (TS_MEM).
+ * On E830 devices the PRTMAC_TS_TX_MEM_VALID_L/H registers are read-only
+ * mirrors of the per-entry TX_VALID bits and cannot be written. The actual
+ * TS_MEM entry's TX_VALID bit is cleared by reading the corresponding
+ * PRTTSYN_TXTIME_L/H registers (read-to-clear).
+ */
+static void ice_clear_phy_tstamp_e830(const struct ice_hw *hw, u8 idx)
+{
+	rd32(hw, E830_PRTTSYN_TXTIME_H(idx));
+	rd32(hw, E830_PRTTSYN_TXTIME_L(idx));
+}
+
 /**
  * ice_get_phy_tx_tstamp_ready_e830 - Read Tx memory status register
  * @hw: pointer to the HW struct
@@ -5772,6 +5826,9 @@ int ice_clear_phy_tstamp(struct ice_hw *hw, u8 block, u8 idx)
 	switch (hw->mac_type) {
 	case ICE_MAC_E810:
 		return ice_clear_phy_tstamp_e810(hw, block, idx);
+	case ICE_MAC_E830:
+		ice_clear_phy_tstamp_e830(hw, idx);
+		return 0;
 	case ICE_MAC_GENERIC:
 		return ice_clear_phy_tstamp_e82x(hw, block, idx);
 	case ICE_MAC_GENERIC_3K_E825:
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 16b1988e993d..b003e0aae8b1 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -304,6 +304,7 @@ int ice_ptp_clear_phy_offset_ready_e82x(struct ice_hw *hw);
 int ice_read_phy_tstamp(struct ice_hw *hw, u8 block, u8 idx, u64 *tstamp);
 int ice_clear_phy_tstamp(struct ice_hw *hw, u8 block, u8 idx);
 void ice_ptp_reset_ts_memory(struct ice_hw *hw);
+int ice_ptp_clear_tx_memory_status_eth56g(struct ice_hw *hw, u8 port);
 int ice_ptp_init_phc(struct ice_hw *hw);
 void ice_ptp_init_hw(struct ice_hw *hw);
 int ice_get_phy_tx_tstamp_ready(struct ice_hw *hw, u8 block, u64 *tstamp_ready);
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 710c519d670d..d8c73fcafc25 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -888,6 +888,13 @@ enum ice_global_link_topo {
 
 struct ice_ptp_hw {
 	union ice_phy_params phy;
+	/* Serializes eth56g Tx timestamp-memory reads (the per-index
+	 * timestamp entries, not the TX_MEMORY_STATUS registers). Reading an
+	 * entry auto-clears its ts_memory_status bit as a side effect, so the
+	 * per-index and full-port clear paths must not interleave on the same
+	 * port or a bit could re-latch the interrupt.
+	 */
+	struct mutex tx_tstamp_lock;
 	u8 num_lports;
 	u8 ports_per_phy;
 };
-- 
2.43.0